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]

Reply via email to