lmccay commented on code in PR #900:
URL: https://github.com/apache/knox/pull/900#discussion_r1564247287
##########
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:
Yeah, I'm not thrilled with it either but can't exactly say it is wrong
given the content. How about TokenResponseContext? Seems more than a context to
me since it includes the builder too. It does have a build() so we could call
it a builder but it is also used to access the other data directly in extension
classes and the build method not called so it isn't always a builder.
--
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
