[ 
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)

Reply via email to