rombert commented on code in PR #15:
URL:
https://github.com/apache/sling-org-apache-sling-auth-oauth-client/pull/15#discussion_r2104546978
##########
src/main/java/org/apache/sling/auth/oauth_client/impl/OidcAuthenticationHandler.java:
##########
@@ -187,32 +198,40 @@ public AuthenticationInfo extractCredentials(@NotNull
HttpServletRequest request
return authInfo;
}
- //The request is not authenticated.
- // 1. Check if the State cookie match with the state in the request
received from the idp
+ //The request is not authenticated.
+ // 1. Extract nonce cookie and state cookie from the request
StringBuffer requestURL = request.getRequestURL();
if ( request.getQueryString() != null )
requestURL.append('?').append(request.getQueryString());
Optional<OAuthState> clientState; //state returned by the idp in the
redirect request
String authCode; //authorization code returned by the idp in the
redirect request
Cookie stateCookie;
+ Cookie nonceCookie;
Review Comment:
So your proposal is:
- state parameter: perRequestKey
- encrypted cookie: perRequestKey, connectionName, redirect, nonce, code
verifier
?
I don't see a problem with it but also I can't say I can figure out all the
security implications on my own. One issue I would raise is that we make sure
we don't exceed some sort of limits related to Cookie/HTTP Header size. That is
of course true for the state parameter in the redirect as well.
--
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]
