Skip to content

added user email upn data in request session #364 #365

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

Closed
wants to merge 6 commits into from

Conversation

ImamAbdullahKhan
Copy link

The OAuth2CallbackView has user as None if user is not registered in Django, so it can't provide user details but the AdfsAuthCodeBackend class authenticate function is called from the Django Backend authenticate function, so instead of extending OAuth2CallbackView, I have extended AdfsAuthCodeBackend class authenticate function to store username_claim in request session.
It can be used in login_failed as "user_email = request.session.get("username_claim")".

Please let me know if you need any further clarifications

@ImamAbdullahKhan
Copy link
Author

I think coverage / codecov (pull request) test failure can resolved by updating the token:
image

Copy link
Member

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't something I'm willing to merge. I don't see the utility in the library capturing the user's email in the session. If there's a hook that can be added to make this easier for you to implement, that is more likely to be accepted.

@ImamAbdullahKhan
Copy link
Author

Thanks for the feedback! Instead of modifying the backend to store claims in the session, I’ve implemented a Django signal hook. This allows developers to listen for authentication events and handle claims as needed, also added sample code in signals.rst file. Please review it and let me know if still needs any changes.

@@ -420,6 +420,11 @@ def authenticate(self, request=None, authorization_code=None, **kwargs):

adfs_response = self.exchange_auth_code(authorization_code, request)
access_token = adfs_response["access_token"]

# Extract claims before user lookup
claims = self.validate_access_token(access_token)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason you're not overriding validate_access_token in your code to achieve the logic you want?

Copy link
Member

@tim-schilling tim-schilling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm against modifying the project for this purpose at this point. It should be possible to override validate_access_token in your own project to achieve what you're going for. That means the project is flexible enough as is and no changes are required.

Additionally, I suspect you may be feeding AI directly responses to us which isn't the most polite thing to do.

@ImamAbdullahKhan
Copy link
Author

ImamAbdullahKhan commented Apr 9, 2025

Ok Thanks for your Response, I thought Django unregistered email address display was a good feature to be add to the Login Failed Page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants