Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

KNOX-2396 - TokenResource Renew and Revoke Should Respond With Error Codes That Identify Specific Errors #480

Merged
merged 1 commit into from Aug 25, 2021
Merged

KNOX-2396 - TokenResource Renew and Revoke Should Respond With Error Codes That Identify Specific Errors #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 40 additions & 3 deletions ...vice-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java
View file
Open in desktop
Expand Up @@ -155,6 +155,27 @@ public class TokenResource {
@Context
ServletContext context;

public enum ErrorCode {
UNKNOWN(0),
CONFIGURATION_ERROR(10),
UNAUTHORIZED(20),
INTERNAL_ERROR(30),
INVALID_TOKEN(40),
UNKNOWN_TOKEN(50),
ALREADY_DISABLED(60),
ALREADY_ENABLED(70);

private final int code;

ErrorCode(int code) {
this.code = code;
}

public int toInt() {
return code;
}
}

@PostConstruct
public void init() throws AliasServiceException, ServiceLifecycleException, KeyLengthException {

Expand Down Expand Up @@ -393,6 +414,7 @@ public Response renew(String token) {
long expiration = 0;

String error = "";
ErrorCode errorCode = ErrorCode.UNKNOWN;
Response.Status errorStatus = Response.Status.BAD_REQUEST;

if (tokenStateService == null) {
Expand All @@ -406,8 +428,10 @@ public Response renew(String token) {
} catch (ParseException e) {
log.invalidToken(getTopologyName(), Tokens.getTokenDisplayText(token), e);
error = safeGetMessage(e);
errorCode = ErrorCode.INVALID_TOKEN;
} catch (Exception e) {
error = safeGetMessage(e);
errorCode = ErrorCode.INTERNAL_ERROR;
}
} else {
String renewer = SubjectUtils.getCurrentEffectivePrincipalName();
Expand All @@ -423,13 +447,16 @@ public Response renew(String token) {
renewer);
} catch (ParseException e) {
log.invalidToken(getTopologyName(), Tokens.getTokenDisplayText(token), e);
errorCode = ErrorCode.INVALID_TOKEN;
error = safeGetMessage(e);
} catch (Exception e) {
error = safeGetMessage(e);
errorCode = ErrorCode.INTERNAL_ERROR;
}
} else {
errorStatus = Response.Status.FORBIDDEN;
error = "Caller (" + renewer + ") not authorized to renew tokens.";
errorCode = ErrorCode.UNAUTHORIZED;
}
}

Expand All @@ -440,7 +467,7 @@ public Response renew(String token) {
} else {
log.badRenewalRequest(getTopologyName(), Tokens.getTokenDisplayText(token), error);
resp = Response.status(errorStatus)
.entity("{\n \"renewed\": \"false\",\n \"error\": \"" + error + "\"\n}\n")
.entity("{\n \"renewed\": \"false\",\n \"error\": \"" + error + "\",\n \"code\": " + errorCode.toInt() + "\n}\n")
.build();
}

Expand All @@ -454,10 +481,12 @@ public Response revoke(String token) {
Response resp;

String error = "";
ErrorCode errorCode = ErrorCode.UNKNOWN;
Response.Status errorStatus = Response.Status.BAD_REQUEST;

if (tokenStateService == null) {
error = "Token revocation support is not configured";
errorCode = ErrorCode.CONFIGURATION_ERROR;
} else {
String renewer = SubjectUtils.getCurrentEffectivePrincipalName();
if (allowedRenewers.contains(renewer)) {
Expand All @@ -471,12 +500,15 @@ public Response revoke(String token) {
} catch (ParseException e) {
log.invalidToken(getTopologyName(), Tokens.getTokenDisplayText(token), e);
error = safeGetMessage(e);
errorCode = ErrorCode.INVALID_TOKEN;
} catch (UnknownTokenException e) {
error = safeGetMessage(e);
errorCode = ErrorCode.UNKNOWN_TOKEN;
}
} else {
errorStatus = Response.Status.FORBIDDEN;
error = "Caller (" + renewer + ") not authorized to revoke tokens.";
errorCode = ErrorCode.UNAUTHORIZED;
}
}

Expand All @@ -487,7 +519,7 @@ public Response revoke(String token) {
} else {
log.badRevocationRequest(getTopologyName(), Tokens.getTokenDisplayText(token), error);
resp = Response.status(errorStatus)
.entity("{\n \"revoked\": \"false\",\n \"error\": \"" + error + "\"\n}\n")
.entity("{\n \"revoked\": \"false\",\n \"error\": \"" + error + "\",\n \"code\": " + errorCode.toInt() + "\n}\n")
.build();
}

Expand Down Expand Up @@ -526,28 +558,33 @@ public Response disable(String tokenId) {

private Response setTokenEnabledFlag(String tokenId, boolean enabled) {
String error = "";
ErrorCode errorCode = ErrorCode.UNKNOWN;
if (tokenStateService == null) {
error = "Unable to " + (enabled ? "enable" : "disable") + " tokens because token management is not configured";
errorCode = ErrorCode.CONFIGURATION_ERROR;
} else {
try {
final TokenMetadata tokenMetadata = tokenStateService.getTokenMetadata(tokenId);
if (enabled && tokenMetadata.isEnabled()) {
error = "Token is already enabled";
errorCode = ErrorCode.ALREADY_ENABLED;
} else if (!enabled && !tokenMetadata.isEnabled()) {
error = "Token is already disabled";
errorCode = ErrorCode.ALREADY_DISABLED;
} else {
tokenMetadata.setEnabled(enabled);
tokenStateService.addMetadata(tokenId, tokenMetadata);
}
} catch (UnknownTokenException e) {
error = safeGetMessage(e);
errorCode = ErrorCode.UNKNOWN_TOKEN;
}
}
if (error.isEmpty()) {
return Response.status(Response.Status.OK).entity("{\n \"setEnabledFlag\": \"true\",\n \"isEnabled\": \"" + enabled + "\"\n}\n").build();
} else {
log.badSetEnabledFlagRequest(getTopologyName(), Tokens.getTokenIDDisplayText(tokenId), error);
return Response.status(Response.Status.BAD_REQUEST).entity("{\n \"setEnabledFlag\": \"false\",\n \"error\": \"" + error + "\"\n}\n").build();
return Response.status(Response.Status.BAD_REQUEST).entity("{\n \"setEnabledFlag\": \"false\",\n \"error\": \"" + error + "\",\n \"code\": " + errorCode.toInt() + "\n}\n").build();
}
}

Expand Down
36 changes: 22 additions & 14 deletions ...ken/src/test/java/org/apache/knox/gateway/service/knoxtoken/TokenServiceResourceTest.java
View file
Open in desktop
Expand Up @@ -684,7 +684,7 @@ public void testTokenRenewal_Disabled() throws Exception {
@Test
public void testTokenRenewal_Enabled_NoRenewersNoSubject() throws Exception {
Response renewalResponse = doTestTokenRenewal(true, null, null);
validateRenewalResponse(renewalResponse, 403, false, "Caller (null) not authorized to renew tokens.");
validateRenewalResponse(renewalResponse, 403, false, "Caller (null) not authorized to renew tokens.", TokenResource.ErrorCode.UNAUTHORIZED);
}

@Test
Expand All @@ -694,7 +694,7 @@ public void testTokenRenewal_Enabled_NoRenewersWithSubject() throws Exception {
validateRenewalResponse(renewalResponse,
403,
false,
"Caller (" + caller + ") not authorized to renew tokens.");
"Caller (" + caller + ") not authorized to renew tokens.", TokenResource.ErrorCode.UNAUTHORIZED);
}

@Test
Expand All @@ -703,7 +703,7 @@ public void testTokenRenewal_Enabled_WithRenewersNoSubject() throws Exception {
validateRenewalResponse(renewalResponse,
403,
false,
"Caller (null) not authorized to renew tokens.");
"Caller (null) not authorized to renew tokens.", TokenResource.ErrorCode.UNAUTHORIZED);
}

@Test
Expand All @@ -713,7 +713,7 @@ public void testTokenRenewal_Enabled_WithRenewersWithInvalidSubject() throws Exc
validateRenewalResponse(renewalResponse,
403,
false,
"Caller (" + caller + ") not authorized to renew tokens.");
"Caller (" + caller + ") not authorized to renew tokens.", TokenResource.ErrorCode.UNAUTHORIZED);
}

@Test
Expand Down Expand Up @@ -764,7 +764,7 @@ public void testTokenRevocation_ServerManagedStateNotConfigured() throws Excepti
validateRevocationResponse(renewalResponse,
400,
false,
"Token revocation support is not configured");
"Token revocation support is not configured", TokenResource.ErrorCode.CONFIGURATION_ERROR);
}

@Test
Expand All @@ -773,7 +773,7 @@ public void testTokenRevocation_Disabled() throws Exception {
validateRevocationResponse(renewalResponse,
400,
false,
"Token revocation support is not configured");
"Token revocation support is not configured", TokenResource.ErrorCode.CONFIGURATION_ERROR);
}

@Test
Expand All @@ -782,7 +782,7 @@ public void testTokenRevocation_Enabled_NoRenewersNoSubject() throws Exception {
validateRevocationResponse(renewalResponse,
403,
false,
"Caller (null) not authorized to revoke tokens.");
"Caller (null) not authorized to revoke tokens.", TokenResource.ErrorCode.UNAUTHORIZED);
}

@Test
Expand All @@ -792,7 +792,7 @@ public void testTokenRevocation_Enabled_NoRenewersWithSubject() throws Exception
validateRevocationResponse(renewalResponse,
403,
false,
"Caller (" + caller + ") not authorized to revoke tokens.");
"Caller (" + caller + ") not authorized to revoke tokens.", TokenResource.ErrorCode.UNAUTHORIZED);
}

@Test
Expand All @@ -801,7 +801,7 @@ public void testTokenRevocation_Enabled_WithRenewersNoSubject() throws Exception
validateRevocationResponse(renewalResponse,
403,
false,
"Caller (null) not authorized to revoke tokens.");
"Caller (null) not authorized to revoke tokens.", TokenResource.ErrorCode.UNAUTHORIZED);
}

@Test
Expand All @@ -811,7 +811,7 @@ public void testTokenRevocation_Enabled_WithRenewersWithInvalidSubject() throws
validateRevocationResponse(renewalResponse,
403,
false,
"Caller (" + caller + ") not authorized to revoke tokens.");
"Caller (" + caller + ") not authorized to revoke tokens.", TokenResource.ErrorCode.UNAUTHORIZED);
}

@Test
Expand Down Expand Up @@ -1220,13 +1220,14 @@ private static Response requestTokenRevocation(final TokenResource tr, final Str
}

private static void validateSuccessfulRenewalResponse(final Response response) throws IOException {
validateRenewalResponse(response, 200, true, null);
validateRenewalResponse(response, 200, true, null, null);
}

private static void validateRenewalResponse(final Response response,
final int expectedStatusCode,
final boolean expectedResult,
final String expectedMessage) throws IOException {
final String expectedMessage,
final TokenResource.ErrorCode expectedCode) throws IOException {
assertEquals(expectedStatusCode, response.getStatus());
assertTrue(response.hasEntity());
String responseContent = (String) response.getEntity();
Expand All @@ -1236,16 +1237,20 @@ private static void validateRenewalResponse(final Response response,
boolean result = Boolean.valueOf(json.get("renewed"));
assertEquals(expectedResult, result);
assertEquals(expectedMessage, json.get("error"));
if (expectedCode != null) {
assertEquals(expectedCode.toInt(), Integer.parseInt(json.get("code")));
}
}

private static void validateSuccessfulRevocationResponse(final Response response) throws IOException {
validateRevocationResponse(response, 200, true, null);
validateRevocationResponse(response, 200, true, null, null);
}

private static void validateRevocationResponse(final Response response,
final int expectedStatusCode,
final boolean expectedResult,
final String expectedMessage) throws IOException {
final String expectedMessage,
final TokenResource.ErrorCode expectedCode) throws IOException {
assertEquals(expectedStatusCode, response.getStatus());
assertTrue(response.hasEntity());
String responseContent = (String) response.getEntity();
Expand All @@ -1255,6 +1260,9 @@ private static void validateRevocationResponse(final Response response,
boolean result = Boolean.valueOf(json.get("revoked"));
assertEquals(expectedResult, result);
assertEquals(expectedMessage, json.get("error"));
if (expectedCode != null) {
assertEquals(expectedCode.toInt(), Integer.parseInt(json.get("code")));
}
}


Expand Down