Draft: Support OAuth2 token introspection #157 #158
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| package com.tngtech.keycloakmock.impl.handler; | ||
|
|
||
| import com.tngtech.keycloakmock.impl.helper.TokenHelper; | ||
| import io.jsonwebtoken.Claims; | ||
| import io.netty.handler.codec.http.HttpResponseStatus; | ||
| import io.vertx.core.Handler; | ||
| import io.vertx.core.json.Json; | ||
| import io.vertx.ext.web.RoutingContext; | ||
| import java.time.Instant; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import javax.annotation.Nonnull; | ||
| import javax.inject.Inject; | ||
| import javax.inject.Singleton; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| @Singleton | ||
| public class TokenIntrospectionRoute implements Handler<RoutingContext> { | ||
|
There was a problem hiding this comment. Please add at least one happy test case to verify that passing in a valid token will return its claims plus the "active" field set to true. Also, please add some tests for the error cases (expired token, missing token, premature token, malformed token). |
||
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(TokenIntrospectionRoute.class); | ||
|
|
||
| private static final String TOKEN_PREFIX = "token="; | ||
| private static final String ACTIVE_CLAIM = "active"; | ||
|
|
||
| @Nonnull private final TokenHelper tokenHelper; | ||
|
|
||
| @Inject | ||
| public TokenIntrospectionRoute(@Nonnull TokenHelper tokenHelper) { | ||
| this.tokenHelper = tokenHelper; | ||
| } | ||
|
|
||
| @Override | ||
| public void handle(RoutingContext routingContext) { | ||
| LOG.info( | ||
| "Inside TokenIntrospectionRoute. Request body is : {}", routingContext.body().asString()); | ||
|
There was a problem hiding this comment. I'd like to keep the logging limited to the bare minimum. We already log every response in the CommonHandler, I see no need to log this here. |
||
|
|
||
| String body = routingContext.body().asString(); | ||
|
There was a problem hiding this comment. The RFC states that |
||
|
|
||
| if (!body.startsWith(TOKEN_PREFIX)) { | ||
| routingContext | ||
| .response() | ||
| .putHeader("content-type", "application/json") | ||
| .setStatusCode(HttpResponseStatus.BAD_REQUEST.code()) | ||
|
There was a problem hiding this comment. While netty is a transient dependency of vert.x at the moment, I'd rather not rely on that. I also don't see the point of adding an explicit dependency to a 0.5 MB library just for some constants. So please just explicitly use the number 400 here. |
||
| .end("Invalid request body"); | ||
|
There was a problem hiding this comment. a) I don't see the need to have a body in the response, so you could just use b) You should add a return statement, because if the body is malformed, running the code down below is useless, and will fail anyway because you will end up calling |
||
| } | ||
|
|
||
| String token = body.replaceFirst("^" + TOKEN_PREFIX, ""); | ||
|
|
||
| LOG.debug("Received a request to introspect token : {}", token); | ||
|
There was a problem hiding this comment. Same here, we don't need to log the token here. The tool runs locally in a testing scope, so the user should know which token was passed in. |
||
|
|
||
| Map<String, Object> claims; | ||
| try { | ||
| claims = tokenHelper.parseToken(token); | ||
| } catch (Exception e) { | ||
|
There was a problem hiding this comment. I'd actually differentiate here. If the exception is a ClaimJwtException, we still have access to the claims of the token, even if the token itself is expired or otherwise invalid. In this case, I'd still add all claims to the response, only with "active": "false". |
||
| // If the token is invalid, initialize an empty claims map | ||
|
There was a problem hiding this comment. This would be a good point to actually add logging. |
||
| claims = new HashMap<>(); | ||
| } | ||
|
|
||
| // To support various use cases, we are returning the same claims as the input token | ||
| Map<String, Object> responseClaims = new HashMap<>(claims); | ||
|
|
||
| if (responseClaims.get(Claims.EXPIRATION) != null | ||
|
There was a problem hiding this comment. This check is unnecessary, as the JWT library will already throw an exception if the token is expired. Instead, you should add the active claim as true within the try block above, and as false in the catch block. |
||
| && isExpiryTimeInFuture(responseClaims.get(Claims.EXPIRATION).toString())) { | ||
| LOG.debug("Introspected token is valid"); | ||
| responseClaims.put(ACTIVE_CLAIM, true); | ||
| } else { | ||
| LOG.debug("Introspected token is invalid"); | ||
| responseClaims.put(ACTIVE_CLAIM, false); | ||
| } | ||
|
|
||
| routingContext | ||
| .response() | ||
| .putHeader("content-type", "application/json") | ||
| .end(Json.encode(responseClaims)); | ||
| } | ||
|
|
||
| private boolean isExpiryTimeInFuture(String expiryTime) { | ||
|
There was a problem hiding this comment. This is unnecessary (see comment above). |
||
| long currentTimeInSec = Instant.now().getEpochSecond(); | ||
| long expiryTimeInSec = Long.parseLong(expiryTime); | ||
| return currentTimeInSec < expiryTimeInSec; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this empty line.