You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by "hanicz (via GitHub)" <gi...@apache.org> on 2025/05/08 11:30:07 UTC

[PR] KNOX-3148: Make SameSite configurable for pac4j session cookies [knox]

hanicz opened a new pull request, #1042:
URL: https://github.com/apache/knox/pull/1042

   ## What changes were proposed in this pull request?
   
   SameSite attribute for the pac4j session cookies are configurable now. The default value is `Strict`.
   The pac4j Cookie class doesn't support the SameSite attribute so I had to change the implementation to Set-Cookie headers.
   Modified MockHttpServletResponse class to accommodate the testing of multiple Set-Cookie headers.
   
   ## How was this patch tested?
   
   Manually tested, unit tests
   ![image](https://github.com/user-attachments/assets/d3dc44e2-9ab4-449e-ae3f-04ec58156754)
   
   


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KNOX-3148: Make SameSite configurable for pac4j session cookies [knox]

Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 commented on PR #1042:
URL: https://github.com/apache/knox/pull/1042#issuecomment-2865777411

   Thanks, @hanicz, for your contribution. It was a good call to build the Set-Cookie header and add all values. Since you are doing that, I think there is an opportunity to improve the code.
   
   See the same (similar) code fragment in `WebSSOResource`:
   ![image](https://github.com/user-attachments/assets/ebd46c93-81df-4f9e-ab67-ba913588a878)
   I think it'd be beneficial to create a utility class that builds the `Set-Cookie` header, where all the params can be constants, the utility class can be tested, etc... Then resource, filter classes can use that class and its method(s).
   What do you think?


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KNOX-3148: Make SameSite configurable for pac4j session cookies [knox]

Posted by "smolnar82 (via GitHub)" <gi...@apache.org>.
smolnar82 merged PR #1042:
URL: https://github.com/apache/knox/pull/1042


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KNOX-3148: Make SameSite configurable for pac4j session cookies [knox]

Posted by "hanicz (via GitHub)" <gi...@apache.org>.
hanicz commented on PR #1042:
URL: https://github.com/apache/knox/pull/1042#issuecomment-2866261228

   Thanks @smolnar82 for you input. Yes it makes sense to unify these two solutions. I'll work on it.


-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org