-
Notifications
You must be signed in to change notification settings - Fork 628
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JWT Authentication for API Gateway #6769
Conversation
JSONObject payload = null; | ||
boolean isVerified = false; | ||
|
||
String tokenSignature = splitToken[2]; |
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.
possible array index out of bounds if someone sends a invalid jwt?
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.
Invalid token split length is handled in the OAuthAuthenticator before hitting this code
String signatureAlgorithm; | ||
JSONObject header; | ||
try { | ||
header = new JSONObject(new String(Base64.getUrlDecoder().decode(splitToken[0]))); |
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.
seems we do url decode? Can there be any other decoding methods?
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.
In the Key manager, we do URL encode. That's why I used URL decode here.
header = new JSONObject(new String(Base64.getUrlDecoder().decode(splitToken[0]))); | ||
} catch (JSONException | IllegalArgumentException e) { | ||
log.debug("Token decryption failure when retrieving header.", e); | ||
throw new APISecurityException(APISecurityConstants.API_AUTH_INVALID_CREDENTIALS, |
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.
saying token invalid. Can't we trim and log atkeast last few letters
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.
Added
@@ -345,6 +349,7 @@ | |||
org.apache.synapse.endpoints.*, | |||
org.apache.synapse.mediators.base, | |||
org.apache.axis2.transport.base, | |||
io.swagger.parser.*; version="${openapitools.swagger.parser.version}", |
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.
better to import version range
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.
done
if (!isVerified) { | ||
log.debug("Token not found in the cache."); | ||
try { | ||
payload = new JSONObject(new String(Base64.getUrlDecoder().decode(splitToken[1]))); |
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.
posiible array index out of bounds/
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.
this split length validation is happening at the OAuthAuthenticator so when we hit this method, it should definitely have an array of length 3
d92610f
to
30dcba2
Compare
30dcba2
to
0254f10
Compare
0254f10
to
380ec16
Compare
if (StringUtils.countMatches(apiKey, APIConstants.DOT) != 2) { | ||
log.debug("Invalid JWT token. The expected token format is <header.payload.signature>"); | ||
throw new APISecurityException(APISecurityConstants.API_AUTH_INVALID_CREDENTIALS, | ||
"Invalid JWT token"); |
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.
Cant we state the same debug message here as well?
It provides a hint.
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.
Fixed in #9250
try { | ||
AuthenticationContext authenticationContext = jwtValidator.authenticate(apiKey, synCtx, openAPI); | ||
APISecurityUtils.setAuthenticationContext(synCtx, authenticationContext, securityContextHeader); | ||
log.debug("User is authorized using JWT token to access the resource."); |
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.
improve the debug log by appending specific information such as user etc
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.
Fixed in #9250
Enable API authentication support using JWT tokens.
Issue - wso2/product-apim#5115