nscendoni commented on code in PR #15:
URL:
https://github.com/apache/sling-org-apache-sling-auth-oauth-client/pull/15#discussion_r2104625354
##########
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:
In [rfc6265](https://datatracker.ietf.org/doc/html/rfc6265?#section-6.1)
read that these are the limits that browsers SHOULD support:
```
o At least 4096 bytes per cookie (as measured by the sum of the
length of the cookie's name, value, and attributes).
o At least 50 cookies per domain.
o At least 3000 cookies total.
```
If we want to be in a safer side and limit the number of cookies as well we
may:
- keep the current setup for OAuth
- Modify the current code to have only one new cookie for nonce and code
verifier
--
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]