rayluo / identity

This is an authentication/authorization library optimized for web apps. It provides some higher level APIs built on top of Microsoft's MSAL Python. Read its documentation here: https://identity-library.readthedocs.io
https://identity-library.readthedocs.io
MIT License
44 stars 5 forks source link

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

Open miceg opened 5 months ago

miceg commented 5 months ago

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

https://github.com/rayluo/identity/blob/3d705f0aa92cdba7649cf1d616f35b162c7caf23/identity/web.py#L217-L220

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:

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 commented 4 months ago

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 commented 4 months ago

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":

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

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 commented 4 months ago

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.