django-auth-adfs icon indicating copy to clipboard operation
django-auth-adfs copied to clipboard

could we have USERNAME_CLAIM non-mandatory?

Open cusco opened this issue 3 years ago • 6 comments

Hello, thanks for the great work on this plugin.

I'm attempting to integrate with AzureAD, and our django backend has some custom pattern for usernames, that won't match anything in Azure. We'd like to use the email field to match Azure's UPN only, being that email is unique in our backend.

Do you think this makes sense and should be changed in the plugin?

Like a mandatory setting like UPN_CLAIM_MAPPING = 'email_field'

cusco avatar Nov 17 '22 12:11 cusco

Am I understanding you correctly that you'd like to key authentication based on the email rather than the username because the username in Django can't match what Azure would pass back?

If that's the case, would this new configuration only be usable if the setting to create users is disabled because a username can't be determined?

tim-schilling avatar Nov 17 '22 14:11 tim-schilling

Ah! I see your point. In our case we don’t want to create users, only to match existing. But I’m guessing upon creation it would either use the UPN..?

On Thu, 17 Nov 2022 at 14:00, Tim Schilling @.***> wrote:

Am I understanding you correctly that you'd like to key authentication based on the email rather than the username because the username in Django can't match what Azure would pass back.

If that's the case, would this new configuration only be usable if the setting to create users is disabled because a username can't be determined?

— Reply to this email directly, view it on GitHub https://github.com/snok/django-auth-adfs/issues/263#issuecomment-1318681477, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAW3L73GS4F77XOBCXVYNKDWIY3ANANCNFSM6AAAAAASDJRF5E . You are receiving this because you authored the thread.Message ID: @.***>

cusco avatar Nov 17 '22 14:11 cusco

I think I'd need to be convinced that this is an issue for more folks before agreeing that it's something that should be supported out of the box. Let's see what other maintainers think.

I'm happy to help with PRs that refactor the application to make it easier for you to implement this on your end. It's clearly a real possibility, but I think I'd prefer that you maintain it in your own project (with the information and understanding I have currently).

tim-schilling avatar Nov 17 '22 14:11 tim-schilling

Hmm, I'm not entirely sure I understand, so please clarify:

  • you already have the users
  • you don't want creation of new users
  • you want the user to log in through azure, but not really do anything with it, except match it towards an already existing user

?

JonasKs avatar Nov 17 '22 19:11 JonasKs

Yes! We onboard users manually in advance. Then organisations are able to use their own Azure AD as a SSO option.

On Thu, 17 Nov 2022 at 19:37, Jonas Krüger Svensson < @.***> wrote:

Hmm, I'm not entirely sure I understand, so please clarify:

  • you already have the users
  • you don't want creation of new users
  • you want the user to log in through azure, but not really do anything with it, except match it towards an already existing user

?

— Reply to this email directly, view it on GitHub https://github.com/snok/django-auth-adfs/issues/263#issuecomment-1319110521, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAW3L77OP2F7CDJWIPH6SSLWI2CNRANCNFSM6AAAAAASDJRF5E . You are receiving this because you authored the thread.Message ID: @.***>

cusco avatar Nov 17 '22 23:11 cusco

I think I'd need to be convinced that this is an issue for more folks before agreeing that it's something that should be supported out of the box. Let's see what other maintainers think.

I'm happy to help with PRs that refactor the application to make it easier for you to implement this on your end. It's clearly a real possibility, but I think I'd prefer that you maintain it in your own project (with the information and understanding I have currently).

Thank you for your support. Tho this might not be needed by anyone else, I did create a PR with changes suiting my needs: https://github.com/snok/django-auth-adfs/pull/264

I guess that if it were to be approved, docs would need to reflect the new setting

cusco avatar Nov 18 '22 17:11 cusco