snok / django-auth-adfs

A Django authentication backend for Microsoft ADFS and AzureAD
http://django-auth-adfs.readthedocs.io/
BSD 2-Clause "Simplified" License
272 stars 99 forks source link

Upgrading from 0.1.2 --> 1.9.4 and Django 1.11 --> 2.2.25 - MSIS9605 related to OpenID? #214

Closed 777GE90 closed 2 years ago

777GE90 commented 2 years ago

Hello, I am working on upgrading Django to version 2.2.X alongside upgrading our Python to 3.8.

These changes mean I can no longer use version 0.1.2 of django-auth-adfs library.

I have implemented all the required changes to upgrade django-auth-adfs to the latest 1.9.4 version.

However, I have hit a bit of a stumbling block with the new open_id connect code that is introduced in the newer versions of this library.

Essentially, now that I have upgraded, I am getting the following error back from the Microsoft Server 2016: MSIS9605%3a+The+client+is+not+allowed+to+access+the+requested+resource.

I have estbalished that if I comment out the following code in the build_authorization_endpoint (config.py) method:

        # if self._mode == "openid_connect":
        #     query["scope"] = "openid"
        #     if (disable_sso is None and settings.DISABLE_SSO) or disable_sso is True:
        #         query["prompt"] = "login"
        #     if force_mfa:
        #         query["amr_values"] = "ngcmfa"

Then the generated authentication URL works fine and I am able to login no problem.

I'm still quite new to ADFS and don't fully understand how openid_connect works, but my organisation has two ADFS instances, one is the legacy instance which uses Active Directory and the other is the newer one using Azure. We still use the old one and it seems like this openid_connect is related to Azure, so is this plugin wrongly trying to use OpenID when it shouldn't in our instance?

On ADFS, I already have the permissions configured correctly as I understand and login works fine once I remove "openid" stuff from the URL.

Ideally, I need a way to force the plugin to stop using openid_connect, I understand it is trying to use openid_connect because it has found a valid config on our server and it uses it by default.

JonasKs commented 2 years ago

Oh wow, that's a big jump from 0.1.2, things are unfortunately bound to break. 😁

Ideally, I need a way to force the plugin to stop using openid_connect, I understand it is trying to use openid_connect because it has found a valid config on our server and it uses it by default.

The error says that client is not allowed to access the resource. The lingo can be a bit confusing, so I'd recommend reading here. So, the fix isn't really to mess with the code, but to set up ADFS correctly. This thread might help you.

In other words, I don't want to create a patch to force oauth2. It is legacy, and it would enable another way of configuring this package, which already have way too many paths for me to maintain. This package will most likely shift into only support Azure AD in the future, since I do not have any way of using ADFS anymore (nor want to). I strongly recommend you jump on that train too.

777GE90 commented 2 years ago

Oh wow, that's a big jump from 0.1.2, things are unfortunately bound to break. 😁

Tell me about it ... :sweat_smile:

The error says that client is not allowed to access the resource. The lingo can be a bit confusing, so I'd recommend reading here. So, the fix isn't really to mess with the code, but to set up ADFS correctly. This thread might help you.

I did have a look at that thread and I did get the team that handles ADFS in our organisation to try these but it was with no luck, I have seen that we appear to have permissions granted for our intranet users. The trouble I have is I don't have any direct access to our ADFS instance, that is a different team so my troubleshooting steps on our ADFS instance itself is very limited.

Looking at the lingo link you posted, I understand we are using Windows Server 2016, so I assume we are being treated as "ADFS 2016". If that is the case, then the comment about requiring application configuration may be why we are having issues, as my understanding is we have setup a Relying Party Trust currently. I don't think it will be easy for me to setup application configurations but I will check.

In other words, I don't want to create a patch to force oauth2. It is legacy, and it would enable another way of configuring this package, which already have way too many paths for me to maintain. This package will most likely shift into only support Azure AD in the future, since I do not have any way of using ADFS anymore (nor want to). I strongly recommend you jump on that train too.

Understood, I suppose my hope or suggestion was to have a flag to allow oauth2 to be used as a short term fix, but I guess I could patch it myself until we are able to upgrade to Azure, which is definitely what I want to do, but that is not such a simple task and I'm hesitant to be upgrading so many things at one time.

JonasKs commented 2 years ago

Hehe, same as me, I don’t have direct access to ADFS. Hence it’s very hard for me to support it anymore.

I can help you with a branch you can use until you can migrate til Azure AD. That won’t be pushed PyPi and will have to be installed from git though.

777GE90 commented 2 years ago

Hehe, same as me, I don’t have direct access to ADFS. Hence it’s very hard for me to support it anymore.

I can help you with a branch you can use until you can migrate til Azure AD. That won’t be pushed PyPi and will have to be installed from git though.

I was planning to do the same on our internal Git, but having one here would be neat actually, would possibly help others stuck in similar situations.

JonasKs commented 2 years ago

That’s up to you! If you want to fork and link me to it, I can help if needed. I’m not at home, so I won’t be able to help much until later this week.

777GE90 commented 2 years ago

That’s up to you! If you want to fork and link me to it, I can help if needed. I’m not at home, so I won’t be able to help much until later this week.

Sorry got side tracked on another project yesterday, I have already patched this locally now (using the above comment seems to work fine) so will leave it. Have been testing Azure ADFS and seems to work, just having trouble with getting the group membership to come over (they are showing some sort of UUID instead of the group name). But think I will look into this another time.

Thanks for your help.

JonasKs commented 2 years ago

Azure AD -> Application registration -> Token configuration -> sAMAccountName instead of Group ID on Access (or all of them).

bilde

JonasKs commented 2 years ago

Closing this issue, but I'll still be notified if you reply.

Have a good week!

777GE90 commented 2 years ago

Azure AD -> Application registration -> Token configuration -> sAMAccountName instead of Group ID on Access (or all of them).

bilde

Awesome, that works :).

You don't happen to know how to get the "winaccountname"? Am I right in understanding that it is not available anymore - seems like I only get the e-mail back now?

JonasKs commented 2 years ago

@777GE90 Hmm, no, I'm not sure. We're using upn, but not sure if that's 100% the same.

JonasKs commented 2 years ago

I have a FastAPI-Azure-Auth package too, and in the tests you can easily compare the tokens from v1 to v2.

In v2 tokens, there are prefferred_username which I guess could be used.

777GE90 commented 2 years ago

@777GE90 Hmm, no, I'm not sure. We're using upn, but not sure if that's 100% the same.

Ye it differs for us at least, makes the upgrade a bit more work but everything seems very doable now.

I have a FastAPI-Azure-Auth package too, and in the tests you can easily compare the tokens from v1 to v2.

In v2 tokens, there are prefferred_username which I guess could be used.

Ye I was passing the preferred_username but it doesn't seem to exist in the token, so I am thinking it may not be set in our Azure or it's hidden.

Anyway I'll stop bothering you now lol, thanks for your help :).

JonasKs commented 2 years ago

Ye I was passing the preferred_username but it doesn't seem to exist in the token

This will vary on which token version you configure. FastAPI-Azure-Auth is a much more up2date package, since it's been written for Azure AD. You can see how you change the token version here.