[ https://issues.apache.org/jira/browse/KNOX-3028?focusedWorklogId=914477&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-914477 ]
ASF GitHub Bot logged work on KNOX-3028: ---------------------------------------- Author: ASF GitHub Bot Created on: 12/Apr/24 21:23 Start Date: 12/Apr/24 21:23 Worklog Time Spent: 10m Work Description: 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 Issue Time Tracking ------------------- Worklog Id: (was: 914477) Time Spent: 20m (was: 10m) > KnoxToken extension for OAuth Token Flows > ----------------------------------------- > > Key: KNOX-3028 > URL: https://issues.apache.org/jira/browse/KNOX-3028 > Project: Apache Knox > Issue Type: Bug > Components: JWT > Reporter: Larry McCay > Assignee: Larry McCay > Priority: Major > Fix For: 2.1.0 > > Time Spent: 20m > Remaining Estimate: 0h > > This change will extend the existing TokenResource for KNOXTOKEN service to > include OAuth specifics such as expected URL, error messages and flows to > support Token Exchange Flow and Token Refresh. > This is being driven by a specific need to proxy access to the Iceberg REST > Catalog API. In this specific usecase, we need to intercept the use of the > following endpoint URLs and serve the token exchange flow for the > authenticating user. > {code} > /v1/oauth/tokens > {code} > Details for these requirements can be found in the openapi description for > the catalog API [1]. > In addition to this usecase, we should add generic support for the token > exchange flow with more generic URL that better aligns with what others use. > {code} > /oauth/v1/token > {code} > We will support the use of the "oauth" service name within the existing > KNOXTOKEN service with an extension of the TokenResource which adapts the > existing KNOXTOKEN behavior to the expectations of clients on OAuth responses. > In order to support both URLs, the deployment contributor will need to > register a url pattern for each usecase and the resource path within the > jersey service will need to accommodate the dynamic nature of the Iceberg > REST Catalog API which will add the catalog API service name as well. > {code} > /icecli/v1/oauth/tokens/ > {code} > Where "icecli" may be some configurable service name and need to match to the > incoming URL. > We will wildcard that by making it a regex matched path param. > We will also need to accommodate a first-class Knox pattern and service name > of "oauth" and only allow "token" or "oauth" after the v1 with the remaining > path fragment being optional for the iceberg specific "tokens". > Not pretty but it will work. > 1. > https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml -- This message was sent by Atlassian Jira (v8.20.10#820010)