-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Security] Enhancing CAS authentication handling by extracting user attributes #59951
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 8.1
Are you sure you want to change the base?
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
This comment was marked as resolved.
This comment was marked as resolved.
|
@nacorp friendly ping 😉 |
src/Symfony/Component/Security/Http/AccessToken/Cas/Cas2Handler.php
Outdated
Show resolved
Hide resolved
|
That's a cool update of the CAS authenticator @erseco ! 👏 |
|
OIDC handlers create a |
|
@chalasr Thanks for your question! Regarding whether we need a specific user object like According to the [CAS protocol specification](https://apereo.github.io/cas/7.0.x/protocol/CAS-Protocol-Specification.html#257-example-response-with-custom-attributes), CAS attributes are included directly in the XML/JSON response: <cas:serviceResponse xmlns:cas="http://www.yale.edu/tp/cas">
<cas:authenticationSuccess>
<cas:user>username</cas:user>
<cas:attributes>
<cas:firstname>John</cas:firstname>
<cas:lastname>Doe</cas:lastname>
<cas:email>[email protected]</cas:email>
<cas:affiliation>staff</cas:affiliation>
<cas:affiliation>faculty</cas:affiliation>
</cas:attributes>
</cas:authenticationSuccess>
</cas:serviceResponse>The changes in this PR extract these attributes and pass them through the I've committed the necessary changes to align with the CAS documentation. All tests now pass after adding some additional cases. However, I'm still encountering issues in PHP 8.2 (low-deps). |
|
As far as I know, the current implementation of the CAS protocol in Symfony follows the version 2.0 ( However, the presence of attributes in the Therefore, what do you think about using this pull request as an opportunity to add, beforehand, a # config/packages/security.yaml
security:
firewalls:
main:
access_token:
token_handler:
cas:
validation_url: https://www.example.com/cas/validate
version: 2 # Or 3 |
|
All things considered, if there is not so much differences between the versions 2.0 and 3.0 of the CAS protocol specification, then a renaming could be enough (e.g. |
|
Thanks @nxtpge, IMHO your suggestion makes sense and will be better to simply rename it to |
|
Hi, Thanks for this great improvement! In my previous discussion #57390, I mentioned that I had applied a patch similar to @eresco’s to fix the issue with retrieving additional attributes from the CAS response, and later implemented a workaround in CasAuthenticatorListener.php to remove the CAS ticket from the URL after a successful authentication. Could you please confirm whether this workaround is still needed with the changes introduced in this PR, or if the new behavior already handles this case? Thanks! |
Description
This pull request enhances the CAS authentication process by extracting user attributes from the CAS response and passing them to the
UserBadge. This ensures that additional information, such as email or roles, can be accessed within authentication workflows.Changes
src/Symfony/Component/Security/Http/AccessToken/Cas/Cas2Handler.phpgetUserBadgeFromto extract attributes from the CAS response.UserBadgeconstructor, making them available in authentication logic.Test Updates
src/Symfony/Component/Security/Http/Tests/AccessToken/Cas/Cas2HandlerTest.phptestWithValidTicketto include attributes in the CAS response.UserBadgecorrectly includes and retains these attributes.Why?
✅ Tests updated and verified
✅ No backward compatibility breaks
Let me know if any adjustments are needed! 🚀