flyrain commented on code in PR #1001:
URL: https://github.com/apache/polaris/pull/1001#discussion_r1961017860
##########
service/common/src/main/java/org/apache/polaris/service/auth/JWTBroker.java:
##########
@@ -86,10 +88,13 @@ public String getSub() {
@Override
public String getScope() {
- return decodedJWT.getClaim("scope").asString();
+ return decodedJWT.getClaim(CLAIM_KEY_SCOPE).asString();
}
};
+ } catch (TokenExpiredException e) {
+ LOGGER.error("Credentials have timed out with error", e);
+ throw new AuthenticationTimeoutException("Credentials have timed out");
Review Comment:
Thanks for chiming in. I think this does deserve a discussion in the dev
list.
I generally agree that expose whether a token is invalid isn't a good
practice. This PR doesn't make Polaris worse in terms of security concern per
my following analysis.
1. If third‐party IdPs don’t reveal token expiration details, then Polaris
won’t either.
2. If third‐party IdPs do reveal that a token is expired, then a 419
response from Polaris would mirror that behavior, not introduce any new risk.
3. If users interact with the `/token` endpoint (which is for testing/POC),
then yes, it may reveal expiration details—but that endpoint is not meant for
production use. I believe this is the case described in #791. @sungwy can
correct me if I'm wrong.
So the only potential issue is scenario 2, but since the IdP is already
leaking that info, Polaris doesn't make the situation worse.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
