grafana / django-saml2-auth

Django SAML2 Authentication Made Easy. Easily integrate with SAML2 SSO identity providers like Okta, Azure AD and others.
Other
189 stars 56 forks source link

Extract user identity hook #344

Closed mostafa closed 1 month ago

mostafa commented 1 month ago

Closes #343.

CC: @jeremylivingston Please test this and see if this solution works for you. Feel free to review it as well and provide feedback.

jeremylivingston commented 1 month ago

Wow, this is great. Thanks for putting this together!

Just tested it out and it appears to be working well for my use case. Just curious, is there a reason why the call to create_new_user in the library isn't supplied the user Dict as kwargs? This would simplify the creation of the new user and would prevent cases where the user is created in the create_user manager, but then fails to add other required data in the CREATE_USER trigger (something I encountered during testing).

The current solution still works well, but I was wondering if there was a reason that more data wasn't passed to the create_user call to accomplish something similar.

mostafa commented 1 month ago

@jeremylivingston The idea is that the library should create a bare minimum user and then let the user (i.e. developer) set it up later.

jeremylivingston commented 1 month ago

Makes sense. In my situation it can sometimes result in an orphaned user if there are problems with setup. The user will get created with no team assigned and the hook attempts to assign the team. If that step fails for some reason, it means the user is unable to login since they have no password and are not assigned to the correct team.

It's not a huge deal since I can set up the necessary logging to notify us when it happens, but it would be cleaner if I could access the enriched data in the create_user step.

Thanks again for all of your help with this!

jeremylivingston commented 1 month ago

Thanks again for this! Any chance you'd be willing to tag a new version containing this update?

mostafa commented 1 month ago

Will do tomorrow.