pzampino commented on code in PR #900: URL: https://github.com/apache/knox/pull/900#discussion_r1563146549
########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java: ########## @@ -853,105 +917,148 @@ private Response getAuthenticationToken() { if (userTokens.size() >= tokenLimitPerUser) { log.tokenLimitExceeded(userName); if (UserLimitExceededAction.RETURN_ERROR == userLimitExceededAction) { - return Response.status(Response.Status.FORBIDDEN).entity("{ \"Unable to get token - token limit exceeded.\" }").build(); + response = Response.status(Response.Status.FORBIDDEN).entity("{ \"Unable to get token - token limit exceeded.\" }").build(); } else { // userTokens is an ordered collection (by issue time) -> the first element is the oldest one final String oldestTokenId = userTokens.iterator().next().getTokenId(); log.generalInfoMessage(String.format(Locale.getDefault(), "Revoking %s's oldest token %s ...", userName, Tokens.getTokenIDDisplayText(oldestTokenId))); final Response revocationResponse = revoke(oldestTokenId); if (Response.Status.OK.getStatusCode() != revocationResponse.getStatus()) { - return Response.status(Response.Status.fromStatusCode(revocationResponse.getStatus())) + response = Response.status(Response.Status.fromStatusCode(revocationResponse.getStatus())) .entity("{\n \"error\": \"An error occurred during the oldest token revocation of " + userName + " \"\n}\n").build(); } } } } } + return response; + } - try { - final boolean managedToken = tokenStateService != null; - JWT token; - JWTokenAttributes jwtAttributes; - final JWTokenAttributesBuilder jwtAttributesBuilder = new JWTokenAttributesBuilder(); - jwtAttributesBuilder - .setIssuer(tokenIssuer) - .setUserName(userName) - .setAlgorithm(signatureAlgorithm) - .setExpires(expires) - .setManaged(managedToken) - .setJku(jku) - .setType(tokenType); - if (!targetAudiences.isEmpty()) { - jwtAttributesBuilder.setAudiences(targetAudiences); + protected void setupPublicCertPEM() { + GatewayServices services = getGatewayServices(); + if (endpointPublicCert == null) { + // acquire PEM for gateway identity of this gateway instance + KeystoreService ks = services.getService(ServiceType.KEYSTORE_SERVICE); + if (ks != null) { + try { + Certificate cert = ks.getCertificateForGateway(); + byte[] bytes = cert.getEncoded(); + endpointPublicCert = Base64.encodeBase64String(bytes); + } catch (KeyStoreException | KeystoreServiceException | CertificateEncodingException e) { + // assuming that certs will be properly provisioned across all clients + log.unableToAcquireCertForEndpointClients(e); + } } - if (shouldIncludeGroups()) { - if (includeGroupsInTokenAllowed) { - jwtAttributesBuilder.setGroups(groups()); - } else { - return Response - .status(Response.Status.BAD_REQUEST) - .entity("{\n \"error\": \"Including group information in tokens is disabled\"\n}\n") - .build(); + } + } + + protected Response enforceClientCertIfRequired() { + Response response = null; + if (clientCertRequired) { + X509Certificate cert = extractCertificate(request); + if (cert != null) { + if (!allowedDNs.contains(cert.getSubjectDN().getName().replaceAll("\\s+", ""))) { + response = Response.status(Response.Status.FORBIDDEN) + .entity("{ \"Unable to get token - untrusted client cert.\" }") + .build(); } + } else { + response = Response.status(Response.Status.FORBIDDEN) + .entity("{ \"Unable to get token - client cert required.\" }") + .build(); } + } + return response; + } - jwtAttributes = jwtAttributesBuilder.build(); - token = ts.issueToken(jwtAttributes); + protected void persistTokenDetails(ResponseMap result, long expires, String userName, String createdBy) { + // Optional token store service persistence + if (tokenStateService != null) { + final long issueTime = System.currentTimeMillis(); + tokenStateService.addToken(result.tokenId, + issueTime, + expires, + maxTokenLifetime.orElse(tokenStateService.getDefaultMaxLifetimeDuration())); + final String comment = request.getParameter(COMMENT); + final TokenMetadata tokenMetadata = new TokenMetadata(userName, StringUtils.isBlank(comment) ? null : comment); + tokenMetadata.setPasscode(tokenMAC.hash(result.tokenId, issueTime, userName, result.passcode)); + addArbitraryTokenMetadata(tokenMetadata); + if (createdBy != null) { + tokenMetadata.setCreatedBy(createdBy); + } + tokenStateService.addMetadata(result.tokenId, tokenMetadata); + log.storedToken(getTopologyName(), Tokens.getTokenDisplayText(result.accessToken), Tokens.getTokenIDDisplayText(result.tokenId)); + } + } - if (token != null) { - String accessToken = token.toString(); - String tokenId = TokenUtils.getTokenId(token); - log.issuedToken(getTopologyName(), Tokens.getTokenDisplayText(accessToken), Tokens.getTokenIDDisplayText(tokenId)); - - final HashMap<String, Object> map = new HashMap<>(); - map.put(ACCESS_TOKEN, accessToken); - map.put(TOKEN_ID, tokenId); - map.put(MANAGED_TOKEN, String.valueOf(managedToken)); - map.put(TOKEN_TYPE, BEARER); - map.put(EXPIRES_IN, expires); - if (tokenTargetUrl != null) { - map.put(TARGET_URL, tokenTargetUrl); - } - if (tokenClientDataMap != null) { - map.putAll(tokenClientDataMap); - } - if (endpointPublicCert != null) { - map.put(ENDPOINT_PUBLIC_CERT, endpointPublicCert); - } + protected ResponseMap buildResponseMap(JWT token, long expires) { + String accessToken = token.toString(); + String tokenId = TokenUtils.getTokenId(token); + final boolean managedToken = tokenStateService != null; + + log.issuedToken(getTopologyName(), Tokens.getTokenDisplayText(accessToken), Tokens.getTokenIDDisplayText(tokenId)); + + final HashMap<String, Object> map = new HashMap<>(); Review Comment: Why is the LHS specifically HashMap as opposed to Map? ########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java: ########## @@ -650,14 +652,14 @@ public Response revoke(String token) { } if (error.isEmpty()) { - resp = Response.status(Response.Status.OK) - .entity("{\n \"revoked\": \"true\"\n}\n") - .build(); + resp = Response.status(Response.Status.OK) + .entity("{\n \"revoked\": \"true\"\n}\n") Review Comment: formatting change ########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java: ########## @@ -650,14 +652,14 @@ public Response revoke(String token) { } if (error.isEmpty()) { - resp = Response.status(Response.Status.OK) - .entity("{\n \"revoked\": \"true\"\n}\n") - .build(); + resp = Response.status(Response.Status.OK) + .entity("{\n \"revoked\": \"true\"\n}\n") + .build(); } else { log.badRevocationRequest(getTopologyName(), Tokens.getTokenDisplayText(token), error); resp = Response.status(errorStatus) - .entity("{\n \"revoked\": \"false\",\n \"error\": \"" + error + "\",\n \"code\": " + errorCode.toInt() + "\n}\n") - .build(); + .entity("{\n \"revoked\": \"false\",\n \"error\": \"" + error + "\",\n \"code\": " + errorCode.toInt() + "\n}\n") Review Comment: formatting change ########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java: ########## @@ -780,26 +782,98 @@ private X509Certificate extractCertificate(HttpServletRequest req) { return null; } - private Response getAuthenticationToken() { - if (clientCertRequired) { - X509Certificate cert = extractCertificate(request); - if (cert != null) { - if (!allowedDNs.contains(cert.getSubjectDN().getName().replaceAll("\\s+", ""))) { - return Response.status(Response.Status.FORBIDDEN) - .entity("{ \"Unable to get token - untrusted client cert.\" }") - .build(); - } + protected Response getAuthenticationToken() { + Response response = enforceClientCertIfRequired(); + if (response != null) { return response; } + + response = onlyAllowGroupsToBeAddedWhenEnabled(); + if (response != null) { return response; } + + UserContext context = buildUserContext(request); + + response = enforceTokenLimitsAsRequired(context.userName); + if (response != null) { return response; } + + TokenResponse resp = getTokenResponse(context); + return resp.build(); + } + + protected TokenResponse getTokenResponse(UserContext context) { + TokenResponse response = null; + long expires = getExpiry(); + setupPublicCertPEM(); + String jku = getJku(); + try + { + JWT token = getJWT(context.userName, expires, jku); + if (token != null) { + ResponseMap result = buildResponseMap(token, expires); + String jsonResponse = JsonUtils.renderAsJsonString(result.map); + persistTokenDetails(result, expires, context.userName, context.createdBy); + + response = new TokenResponse(result, jsonResponse, Response.ok()); } else { - return Response.status(Response.Status.FORBIDDEN) - .entity("{ \"Unable to get token - client cert required.\" }") - .build(); + response = new TokenResponse(null, null, Response.serverError()); + } + } catch (TokenServiceException e) { + log.unableToIssueToken(e); + response = new TokenResponse(null + , "{ \"Unable to acquire token.\" }" + , Response.serverError()); + } + return response; + } + + protected static class TokenResponse { Review Comment: I don't love the name of this class, but I can't think of something more appropriate atm ########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/OAuthResource.java: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.knox.gateway.service.knoxtoken; + +import org.apache.knox.gateway.i18n.messages.MessagesFactory; +import org.apache.knox.gateway.util.JsonUtils; + +import javax.inject.Singleton; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.Response; + +import java.time.Duration; +import java.time.format.DateTimeParseException; +import java.util.HashMap; + +import static javax.ws.rs.core.MediaType.APPLICATION_JSON; +import static javax.ws.rs.core.MediaType.APPLICATION_XML; + +@Singleton +@Path(OAuthResource.RESOURCE_PATH) +public class OAuthResource extends TokenResource { + private static TokenServiceMessages log = MessagesFactory.get(TokenServiceMessages.class); + static final String RESOURCE_PATH = "/{serviceName:.*}/v1/{oauthSegment:(oauth|token)}{path:(/tokens)?}"; + public static final String ISSUED_TOKEN_TYPE = "issued_token_type"; + public static final String REFRESH_TOKEN = "refresh_token"; + public static final String ISSUED_TOKEN_TYPE_ACCESS_TOKEN_VALUE = "urn:ietf:params:oauth:token-type:access_token"; + + @Override + @GET + @Produces({ APPLICATION_JSON, APPLICATION_XML }) + public Response doGet() { + return super.doGet(); + } + + @Override + @POST + @Produces({ APPLICATION_JSON, APPLICATION_XML }) + public Response doPost() { + return super.doPost(); + } + + @Override + public Response getAuthenticationToken() { + + Response response = enforceClientCertIfRequired(); + if (response != null) { return response; } + + response = onlyAllowGroupsToBeAddedWhenEnabled(); + if (response != null) { return response; } + + UserContext context = buildUserContext(request); + + response = enforceTokenLimitsAsRequired(context.userName); + if (response != null) { return response; } + + TokenResponse resp = getTokenResponse(context); + // if the responseMap isn't null then the knoxtoken request was successful + // if not then there may have been an error and the underlying response + // builder will communicate those details + if (resp.responseMap != null) { + // let's get the subset of the KnoxToken Response needed for OAuth + String accessToken = resp.responseMap.accessToken; + String passcode = resp.responseMap.passcode; + long expires = (long) resp.responseMap.map.get(EXPIRES_IN); + String tokenType = (String) resp.responseMap.map.get(TOKEN_TYPE); + + // build and return the expected OAuth response + final HashMap<String, Object> map = new HashMap<>(); + map.put(ACCESS_TOKEN, accessToken); + map.put(TOKEN_TYPE, tokenType); + map.put(EXPIRES_IN, expires); + map.put(ISSUED_TOKEN_TYPE, ISSUED_TOKEN_TYPE_ACCESS_TOKEN_VALUE); + // let's use the passcode as the refresh token + map.put(REFRESH_TOKEN, passcode); + String jsonResponse = JsonUtils.renderAsJsonString(map); + return resp.responseBuilder.entity(jsonResponse).build(); + } + // there was an error if we got here - let's surface it appropriately + // TODO: LJM we may need to translate certain errors into OAuth error messages Review Comment: Is this TODO left intentionally? ########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/OAuthResource.java: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.knox.gateway.service.knoxtoken; + +import org.apache.knox.gateway.i18n.messages.MessagesFactory; +import org.apache.knox.gateway.util.JsonUtils; + +import javax.inject.Singleton; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.Response; + +import java.time.Duration; +import java.time.format.DateTimeParseException; +import java.util.HashMap; + +import static javax.ws.rs.core.MediaType.APPLICATION_JSON; +import static javax.ws.rs.core.MediaType.APPLICATION_XML; + +@Singleton +@Path(OAuthResource.RESOURCE_PATH) +public class OAuthResource extends TokenResource { + private static TokenServiceMessages log = MessagesFactory.get(TokenServiceMessages.class); + static final String RESOURCE_PATH = "/{serviceName:.*}/v1/{oauthSegment:(oauth|token)}{path:(/tokens)?}"; + public static final String ISSUED_TOKEN_TYPE = "issued_token_type"; + public static final String REFRESH_TOKEN = "refresh_token"; + public static final String ISSUED_TOKEN_TYPE_ACCESS_TOKEN_VALUE = "urn:ietf:params:oauth:token-type:access_token"; + + @Override + @GET + @Produces({ APPLICATION_JSON, APPLICATION_XML }) + public Response doGet() { + return super.doGet(); + } + + @Override + @POST + @Produces({ APPLICATION_JSON, APPLICATION_XML }) + public Response doPost() { + return super.doPost(); + } + + @Override + public Response getAuthenticationToken() { + + Response response = enforceClientCertIfRequired(); + if (response != null) { return response; } + + response = onlyAllowGroupsToBeAddedWhenEnabled(); + if (response != null) { return response; } + + UserContext context = buildUserContext(request); + + response = enforceTokenLimitsAsRequired(context.userName); + if (response != null) { return response; } + + TokenResponse resp = getTokenResponse(context); + // if the responseMap isn't null then the knoxtoken request was successful + // if not then there may have been an error and the underlying response + // builder will communicate those details + if (resp.responseMap != null) { Review Comment: Why are TokenResponse and ResponseMap treated like structs instead of classes? Just curious if there is a reason. ########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java: ########## @@ -523,17 +525,17 @@ public Response renew(String token) { long expiration = 0; - String error = ""; - ErrorCode errorCode = ErrorCode.UNKNOWN; + String error = ""; + ErrorCode errorCode = ErrorCode.UNKNOWN; Response.Status errorStatus = Response.Status.BAD_REQUEST; if (tokenStateService == null) { // If the token state service is disabled, then return the expiration from the specified token try { JWTToken jwt = new JWTToken(token); log.renewalDisabled(getTopologyName(), - Tokens.getTokenDisplayText(token), - Tokens.getTokenIDDisplayText(TokenUtils.getTokenId(jwt))); + Tokens.getTokenDisplayText(token), Review Comment: formatting change ########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java: ########## @@ -184,7 +184,9 @@ public class TokenResource { private boolean includeGroupsInTokenAllowed; private String tokenIssuer; - enum UserLimitExceededAction {REMOVE_OLDEST, RETURN_ERROR}; + enum UserLimitExceededAction {REMOVE_OLDEST, RETURN_ERROR} + + ; Review Comment: semi-colon bumped down two lines? ########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java: ########## @@ -780,26 +782,98 @@ private X509Certificate extractCertificate(HttpServletRequest req) { return null; } - private Response getAuthenticationToken() { - if (clientCertRequired) { - X509Certificate cert = extractCertificate(request); - if (cert != null) { - if (!allowedDNs.contains(cert.getSubjectDN().getName().replaceAll("\\s+", ""))) { - return Response.status(Response.Status.FORBIDDEN) - .entity("{ \"Unable to get token - untrusted client cert.\" }") - .build(); - } + protected Response getAuthenticationToken() { + Response response = enforceClientCertIfRequired(); + if (response != null) { return response; } + + response = onlyAllowGroupsToBeAddedWhenEnabled(); + if (response != null) { return response; } + + UserContext context = buildUserContext(request); + + response = enforceTokenLimitsAsRequired(context.userName); + if (response != null) { return response; } + + TokenResponse resp = getTokenResponse(context); + return resp.build(); + } + + protected TokenResponse getTokenResponse(UserContext context) { + TokenResponse response = null; + long expires = getExpiry(); + setupPublicCertPEM(); + String jku = getJku(); + try + { + JWT token = getJWT(context.userName, expires, jku); + if (token != null) { + ResponseMap result = buildResponseMap(token, expires); + String jsonResponse = JsonUtils.renderAsJsonString(result.map); + persistTokenDetails(result, expires, context.userName, context.createdBy); + + response = new TokenResponse(result, jsonResponse, Response.ok()); } else { - return Response.status(Response.Status.FORBIDDEN) - .entity("{ \"Unable to get token - client cert required.\" }") - .build(); + response = new TokenResponse(null, null, Response.serverError()); + } + } catch (TokenServiceException e) { + log.unableToIssueToken(e); + response = new TokenResponse(null + , "{ \"Unable to acquire token.\" }" + , Response.serverError()); + } + return response; + } + + protected static class TokenResponse { + public ResponseMap responseMap; + public String responseStr; + public Response.ResponseBuilder responseBuilder; + + public TokenResponse(ResponseMap respMap, String resp, Response.ResponseBuilder builder) { + responseMap = respMap; + responseStr = resp; + responseBuilder = builder; + } + + public Response build() { + Response response = null; + if (responseStr != null) { + response = responseBuilder.entity(responseStr).build(); } + else { Review Comment: else on its own line? ########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/OAuthResource.java: ########## @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.knox.gateway.service.knoxtoken; + +import org.apache.knox.gateway.i18n.messages.MessagesFactory; +import org.apache.knox.gateway.util.JsonUtils; + +import javax.inject.Singleton; +import javax.ws.rs.GET; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.Response; + +import java.time.Duration; +import java.time.format.DateTimeParseException; +import java.util.HashMap; + +import static javax.ws.rs.core.MediaType.APPLICATION_JSON; +import static javax.ws.rs.core.MediaType.APPLICATION_XML; + +@Singleton +@Path(OAuthResource.RESOURCE_PATH) +public class OAuthResource extends TokenResource { + private static TokenServiceMessages log = MessagesFactory.get(TokenServiceMessages.class); + static final String RESOURCE_PATH = "/{serviceName:.*}/v1/{oauthSegment:(oauth|token)}{path:(/tokens)?}"; + public static final String ISSUED_TOKEN_TYPE = "issued_token_type"; + public static final String REFRESH_TOKEN = "refresh_token"; + public static final String ISSUED_TOKEN_TYPE_ACCESS_TOKEN_VALUE = "urn:ietf:params:oauth:token-type:access_token"; + + @Override + @GET + @Produces({ APPLICATION_JSON, APPLICATION_XML }) + public Response doGet() { + return super.doGet(); + } + + @Override + @POST + @Produces({ APPLICATION_JSON, APPLICATION_XML }) + public Response doPost() { + return super.doPost(); + } + + @Override + public Response getAuthenticationToken() { + + Response response = enforceClientCertIfRequired(); + if (response != null) { return response; } + + response = onlyAllowGroupsToBeAddedWhenEnabled(); + if (response != null) { return response; } + + UserContext context = buildUserContext(request); + + response = enforceTokenLimitsAsRequired(context.userName); + if (response != null) { return response; } + + TokenResponse resp = getTokenResponse(context); + // if the responseMap isn't null then the knoxtoken request was successful + // if not then there may have been an error and the underlying response + // builder will communicate those details + if (resp.responseMap != null) { + // let's get the subset of the KnoxToken Response needed for OAuth + String accessToken = resp.responseMap.accessToken; + String passcode = resp.responseMap.passcode; + long expires = (long) resp.responseMap.map.get(EXPIRES_IN); + String tokenType = (String) resp.responseMap.map.get(TOKEN_TYPE); + + // build and return the expected OAuth response + final HashMap<String, Object> map = new HashMap<>(); + map.put(ACCESS_TOKEN, accessToken); + map.put(TOKEN_TYPE, tokenType); + map.put(EXPIRES_IN, expires); + map.put(ISSUED_TOKEN_TYPE, ISSUED_TOKEN_TYPE_ACCESS_TOKEN_VALUE); + // let's use the passcode as the refresh token + map.put(REFRESH_TOKEN, passcode); + String jsonResponse = JsonUtils.renderAsJsonString(map); + return resp.responseBuilder.entity(jsonResponse).build(); + } + // there was an error if we got here - let's surface it appropriately + // TODO: LJM we may need to translate certain errors into OAuth error messages + if (resp.responseStr != null) { + return resp.responseBuilder.entity(resp.responseStr).build(); + } + else { + return resp.responseBuilder.build(); + } + } + + @Override + protected long getExpiry() { + long secs = tokenTTL/1000; + + String lifetimeStr = request.getParameter(LIFESPAN); + if (lifetimeStr == null || lifetimeStr.isEmpty()) { + if (tokenTTL == -1) { + return -1; + } + } + else { Review Comment: else on its own line? ########## gateway-service-knoxtoken/src/main/java/org/apache/knox/gateway/service/knoxtoken/TokenResource.java: ########## @@ -626,14 +628,14 @@ public Response revoke(String token) { final String tokenId = getTokenId(token); if (isKnoxSsoCookie(tokenId)) { errorStatus = Response.Status.FORBIDDEN; - error = "SSO cookie (" + Tokens.getTokenIDDisplayText(tokenId) + ") cannot not be revoked." ; + error = "SSO cookie (" + Tokens.getTokenIDDisplayText(tokenId) + ") cannot not be revoked."; errorCode = ErrorCode.UNAUTHORIZED; } else if (triesToRevokeOwnToken(tokenId, revoker) || allowedRenewers.contains(revoker)) { tokenStateService.revokeToken(tokenId); log.revokedToken(getTopologyName(), - Tokens.getTokenDisplayText(token), - Tokens.getTokenIDDisplayText(tokenId), - revoker); + Tokens.getTokenDisplayText(token), Review Comment: formatting change -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org