Weird-Sheep-Labs / django-azure-auth

A simple Django app for user authentication with Azure Active Directory/Entra ID.
MIT License
17 stars 10 forks source link

Mix of accounts and log out when a single account #17

Closed whikloj closed 6 months ago

whikloj commented 6 months ago

Hi,

First thanks for this middleware, it is working great but I have hit on a couple of things that I needed to change locally to use it.

1) When you have a mix of AD accounts and local accounts, they all get sent to Microsoft to logout. The local accounts of course don't have the necessary information and were having problems.

2) This is more of a nuisance, when an AD user goes to logout they always have to "choose" the account even if there is only one logged in. This is totally an Microsoft thing, but I found some simple instructions (https://blogs.aaddevsup.xyz/2022/03/how-to-logout-of-an-oauth2-application-without-getting-prompted-to-select-a-user/) to avoid this in some cases.

I'm avoiding both of these locally by skipping your logout view and doing:

class AzurePythonLogoutDecider(RedirectView):
    """ Local accounts get stuck when passed to Azure for logout, this avoids sending them there. Also passes
          the login_hint to Azure for logout. """
    permanent = False

    def get_redirect_url(self, *args, **kwargs):
        if AuthHandler(self.request).get_token_from_cache() is None:
            return reverse('logout')
        claims = self.request.session['id_token_claims']
        logout(self.request)
        logout_uri = AuthHandler.get_logout_uri()
        if claims and "login_hint" in claims:
            logout_uri += ("&" if '?' in logout_uri else "?") + "logout_hint=" + claims["login_hint"]
        return logout_uri

I'm not a strong python developer, so I'm sure there is a better way but I wanted to offer it up.

cheers

regoawt commented 6 months ago

Hi,

Thanks for this issue. Just to confirm, when you say 'local accounts' you mean Django-only user accounts that don't have a corresponding AD account?

Assuming that's what you mean, you have a good point, we're not handling that case at the moment. Will look into it soon!

whikloj commented 6 months ago

Hey @regoawt, you are correct I meant local Django only accounts. In our use case we have staff that could use Azure but external users that will have to get a Django account.

regoawt commented 6 months ago

Hey @whikloj, have given some thought to both your issues:

  1. If you have users logging in without Azure as the IdP, I would say the best course of action is to write a separate logout view with the required logic there - I don't think that functionality should be handled within this package.
  2. I like this feature, thanks! I'll create a separate issue for it as I'm closing this one.

Cheers

regoawt commented 5 months ago

@whikloj #22 is now available in v1.3.0.