Skip to content
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

Django integration is not integrated with Django's authentication framework #19

Open
miceg opened this issue Apr 2, 2024 · 4 comments
Labels
question Further information is requested

Comments

@miceg
Copy link

miceg commented Apr 2, 2024

The Django integration in this library appears to stash an authentication token from MS Identity API into the Django session:

identity/identity/web.py

Lines 217 to 220 in 3d705f0

self._save_user_into_session(result["id_token_claims"])
self._save_cache(cache)
flow = self._session.pop(self._AUTH_FLOW, {})
return {"next_link": flow.get(self.__NEXT_LINK)}

And then an app is expected to pull that using the context parameter, for example:

https://github.com/Azure-Samples/ms-identity-python-webapp-django/blob/9a277ede91f68293e1d95a45bfc50b1fb191d06d/mysite/views.py#L12-L19

In that snippet, settings.AUTH is identity.django.Auth.

I don't think this library implements Django authentication correctly, which leads to significant limitations:

  • MS Identity sessions are completely separate to normal Django user sessions – so there's no way to use these sessions in Django Admin, or apply permissions of any kind.

  • There's no way to reference an MS Identity user as a foreign key (a limitation noted in Microsoft's other samples)

Using Django's "writing an authentication backend" doc and Django's REMOTE_USER how-to as a similarly shaped example, I'd expect:

After spending a lot of time trying to research Entra authentication integration, I also note this is not even the complete solution – you'd ideally want a SCIM implementation running in Django to allow Entra to provision users and groups, so that you're not waiting on users to log in to update things, and can deactivate accounts. 😄

@rayluo rayluo added the question Further information is requested label Apr 3, 2024
@rayluo
Copy link
Owner

rayluo commented Apr 3, 2024

Thanks for your in-depth analysis, @miceg .

Actually I'm well aware that the current implementation does not wire up with Django's default authentication system. That kind of deep-integration with Django would require much more thoughts and work. We are not ready to implement SCIM inside this library. Currently, this library focuses on user login and obtaining a token. All the user management is expected to happen inside the Identity Provider (IdP)'s own portal, not in Django's admin page. Besides, this library supports at least 4 different IdP and theoretically supports all OIDC-compliant IdP. I am not sure Django's admin would be flexible enough to support all that.

In that sense, I wouldn't say the current implementation is incorrect. I would say "the current implementation is independent to Django's authentication framework". Let me see where I shall mention it in the docs.

@miceg miceg changed the title Django integration is implemented incorrectly Django integration is not integrated with Django's authentication framework Apr 4, 2024
@miceg
Copy link
Author

miceg commented Apr 4, 2024

On reflection, I recognise my phrasing of "implementation is incorrect" could be read as a touch too inflammatory, so I've changed the title of the issue to be more specific about what the actual issue is. I've left the initial post as-is, so that your commentary still makes sense in context. 😄

I also recognise that these limitations are products from Django's design choices – that it really needs a user to be a local database record, even if that only has a UID field (ie: UUID, SID, username, email address, phone number, etc.) which is managed by an external identity provider.

A lot of the solutions in this space are designed around "social authentication":

  • they allow arbitrary connection / disconnection of a user account to external authentication providers
  • they allow connecting to multiple authentication providers, or even the same authentication provider multiple times
  • external authentication is treated as an optional alternative to local authentication
  • they incorrectly assume usernames and/or email addresses are a permanent and unchanging identifier for a user
  • they assume email addresses can be used to link across identity providers

While a "social" strategy suits many web apps, I was really looking for something which is more "enterprise authentication":

  • there's a strong 1:1 mapping between that identity provider's users and Django users
  • mapping across multiple IdPs is for an external identity provider to do (eg: Auth0)
  • there's only one way to log in – through the IdP
  • you provision access to an app through the IdP
  • users are keyed on an permanent identifier from the IdP, like a UUID or SID
  • Django's user table effectively becomes a read-only replica of users which have been provisioned to the app by the IdP (enforcing this in Django Admin as well would be nice)

That way anything which has a ForeignKey to User can point at that external identity provider's users, and that normal Django functionality like request.user still works properly with that external auth provider.

It'd also be nice if managed identities/service accounts were also Django users when calling APIs (eg: via django-rest-framework); but I recognise that's another layer of complexity 😉

@rayluo
Copy link
Owner

rayluo commented Apr 4, 2024

Thank you for conducting this constructive conversation, @miceg !

I agree with you that there can be many ways to implement "social authentication". Personally, when I social-login to a new website (such as an old-school forum), I would feel frustrated if it still assigns me a random "user235465" account and sometimes even forces me to setup a local password. IMHO, that defeats the purpose of choosing social login.

The approach chosen by this identity library happens to be a "remote-only" approach, to the point that no local user record is required.

The app developer may still manually create a local entry (using the sub claim - and optionally the iss claim - from the context). This library just does not (yet?) facilitate that local entry creation. Either way, the outcome naturally achieves many of the "enterprise authentication" characteristic that you mentioned, especially the 1:1 mapping.

@rayluo
Copy link
Owner

rayluo commented Oct 20, 2024

Adding a link to another conversation for reference, when/if this feature request will be worked on.

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

No branches or pull requests

2 participants