davidjrh / dnn.azureadprovider

The DNN Azure Active Directory Provider is an Authentication provider for DNN Platform (formerly DotNetNuke) that uses Azure Active Directory OAuth2 authentication to authenticate users.
MIT License
35 stars 21 forks source link

Error "AADSTS90023 Request is malformed or invalid." #22

Closed andrewqit closed 4 years ago

andrewqit commented 5 years ago

The below issue from the previous version has returned with the latest version:

Fixed double exchange code for token calls, causing a login issue introduced on Azure AD after October 10th, 2018

I verified this by getting the error caused by the issue using the latest version. I then installed the previous version, and the issue was gone.

davidjrh commented 5 years ago

Hi @andrewqit for notifying this. Checking the code, I see that the code fix I made to solve the old issue is still on the master branch https://github.com/davidjrh/dnn.azureadprovider/blob/master/DotNetNuke.Authentication.Azure/Login.ascx.cs#L80

So after downloading the 3.1.0 package and decompiling the assembly, I see that the compiled code is effectively looking like the old version. I'm not sure of how that happened (did I upload the build from another branch?)

       private void loginButton_Click(object sender, EventArgs e)
        {
            if (string.IsNullOrEmpty(base.Request["error"]))
            {
                if ((!base.get_OAuthClient().IsCurrentService() ? true : !base.get_OAuthClient().HaveVerificationCode()) && base.get_OAuthClient().Authorize() == null)
                {
                    Skin.AddModuleMessage(this, Localization.GetString("PrivateConfirmationMessage", Localization.get_SharedResourceFile()), 1);
                }
                return;
            }
            string str = Localization.GetString("LoginError", base.get_LocalResourceFile());
            str = string.Format(str, base.Request["error"], base.Request["error_description"]);
            this._logger.Error(str);
            Skin.AddModuleMessage(this, str, 2);
        }

Going to check tomorrow and update the latest package.

davidjrh commented 5 years ago

After a second review before rebuild, there was no build error and that the code shown above correspondes with the optimized version that the compiler does. Going to check more deeply.

andrewqit commented 4 years ago

Hi David, checking to see if you had a chance to peek at this. I'm still getting the error in some circumstances.

AbsoluteURL:/Default.aspx

DefaultDataProvider:DotNetNuke.Data.SqlDataProvider, DotNetNuke

ExceptionGUID:70fd4af7-c761-4c67-9c8c-62ba6712c55f

AssemblyVersion:9.3.2

PortalId:0

UserId:-1

TabId:86

RawUrl:/login?code=

Referrer:

UserAgent:Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:69.0) Gecko/20100101 Firefox/69.0

ExceptionHash:jc8NEbeuodygo1K+G+gSe9OhNdI=

Message:Object reference not set to an instance of an object.

StackTrace:

InnerMessage:Object reference not set to an instance of an object.

InnerStackTrace:

at DotNetNuke.Security.Membership.AspNetMembershipProvider.UserLogin(Int32 portalId, String username, String password, String authType, String verificationCode, UserLoginStatus& loginStatus) at DotNetNuke.Entities.Users.UserController.ValidateUser(Int32 portalId, String username, String password, String authType, String verificationCode, String portalName, String ip, UserLoginStatus& loginStatus) at DotNetNuke.Services.Authentication.OAuth.OAuthClientBase.AuthenticateUser(UserData user, PortalSettings settings, String IPAddress, Action1 addCustomProperties, Action1 onAuthenticated) at DotNetNuke.Services.Authentication.OAuth.OAuthLoginBase.OnLoad(EventArgs e) at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Control.LoadRecursive() at System.Web.UI.Page.ProcessRequestMain(Boolean includeStagesBeforeAsyncPoint, Boolean includeStagesAfterAsyncPoint) Source:

FileName:

FileLineNumber:0

FileColumnNumber:0

Method:

Server Name:

davidjrh commented 4 years ago

Hi, last weekend I tried to reproduce the error with a fresh install and I couldn't, but I think I have a site with the same behavior. Working again on it this weekend.

davidjrh commented 4 years ago

@andrewqit can you check your log4net logs and see if there is any error like the one below?

2019-09-21 15:11:35,574 [DESKTOP-JQL0N5G][Thread:42][ERROR] DotNetNuke.Services.Authentication.OAuth.OAuthClientBase- WebResponse exception: {"error":"invalid_grant","error_description":"AADSTS9002313: Invalid request. Request is malformed or invalid.\r\nTrace ID: 9bc01980-c631-4d36-94cc-d36780e70d00\r\nCorrelation ID: b19a7264-f35c-4a3b-9337-057ff0763f6a\r\nTimestamp: 2019-09-21 14:11:10Z","error_codes":[9002313],"timestamp":"2019-09-21 14:11:10Z","trace_id":"9bc01980-c631-4d36-94cc-d36780e70d00","correlation_id":"b19a7264-f35c-4a3b-9337-057ff0763f6a","error_uri":"https://login.microsoftonline.com/error?code=9002313"}

davidjrh commented 4 years ago

If that is your case, I have found that if you enable the SEO Setting "Redirect Mixed Case URLs" to "True", this causes the Url (including parameters such as the auth code returned by AAD) to be converted to lowercase, so can't be exchanged correctly.

You can check the value of that setting by running this SQL query. The SettingValue should be False

select * from PortalSettings where SettingName='AUM_RedirectMixedCase'

davidjrh commented 4 years ago

IMHO, that setting should only affect the Host and Path of the URL, but never the QueryString part. I have created this issue on DNN Platform https://github.com/dnnsoftware/Dnn.Platform/issues/3011

davidjrh commented 4 years ago

PS: the error detail you posted above doesn't contain any value after the "?code=", and should have the exchange token there. Not sure if that's caused because you are using a previous version, so please, update again to 3.1.0 and check the log4net logs for more details.

andrewqit commented 4 years ago

Hi David, I’m looking into your comments now. As far as the code goes, I redacted the token.

From: David Rodríguez notifications@github.com Sent: September 21, 2019 9:06 AM To: davidjrh/dnn.azureadprovider dnn.azureadprovider@noreply.github.com Cc: Andrew Bryson Andrew@QuercusIT.com; Mention mention@noreply.github.com Subject: Re: [davidjrh/dnn.azureadprovider] V3.1.0 issue (#22)

PS: the error detail you posted above doesn't contain any value after the "?code=", and should have the exchange token there. Not sure if that's caused because you are using a previous version, so please, update again to 3.1.0 and check the log4net logs for more details.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/davidjrh/dnn.azureadprovider/issues/22?email_source=notifications&email_token=ALBO5TJCX5XMRYHJWSUOYLTQKYZ6VA5CNFSM4IUCVKP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7ITQFA#issuecomment-533805076, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALBO5TNKZW5VQ66KPIP2WZ3QKYZ6VANCNFSM4IUCVKPQ.

andrewqit commented 4 years ago

Indeed that settings is True. I had some other issues a while ago related to QueryStrings coming in with mixed case, causing issues with another module. I’ve set it back to False. Unfortunately, I can’t reproduce the error myself but I have a specific user that can and am waiting for him to give it a try.

Thank you for looking into this.

From: David Rodríguez notifications@github.com Sent: September 21, 2019 8:46 AM To: davidjrh/dnn.azureadprovider dnn.azureadprovider@noreply.github.com Cc: Andrew Bryson Andrew@QuercusIT.com; Mention mention@noreply.github.com Subject: Re: [davidjrh/dnn.azureadprovider] V3.1.0 issue (#22)

If that is your case, I have found that if you enable the SEO Setting "Redirect Mixed Case URLs" to "True", this causes the Url (including parameters such as the auth code returned by AAD) to be converted to lowercase, so can't be exchanged correctly.

You can check the value of that setting by running this SQL query. The SettingValue should be False

select * from PortalSettings where SettingName='AUM_RedirectMixedCase'

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/davidjrh/dnn.azureadprovider/issues/22?email_source=notifications&email_token=ALBO5TPH7MT3Z3J5TJW5YB3QKYXSPA5CNFSM4IUCVKP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7ITD5I#issuecomment-533803509, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ALBO5TMAXY3GMUQ4AG2PT5DQKYXSPANCNFSM4IUCVKPQ.

andrewqit commented 4 years ago

Hi David, I changed the setting but it's having no effect. I'm still getting the same problem, but it's intermittent. It's possible it's not the Auth provider, it may be DNN. However, it is the Auth provider that's throwing the error as shown in a previous post.

andrewqit commented 4 years ago

For anyone else that's exhausted all the obvious and perhaps seemingly obvious resolutions, I did manage to get my problem resolved. I deleted the application registration, re-created it and it worked. One would think it was a permissions issue, but it definitely was not. I'm not really sure what it was, perhaps a glitch with the code to create application registrations. Who knows...

davidjrh commented 4 years ago

Found another use case where the exchange code was bening exchanged twice. Added code to avoid this situation on v4.0. https://github.com/davidjrh/dnn.azureadprovider/releases/tag/v4.0.0-preview

davidjrh commented 4 years ago

Self note: