Skip to content
This repository has been archived by the owner on Aug 28, 2023. It is now read-only.

"State" gets encoded and causes "collectInfoFromReq: invalid state received" #309

Closed
AskYous opened this issue Jun 1, 2017 · 9 comments
Closed
Assignees
Labels
Milestone

Comments

@AskYous
Copy link

AskYous commented Jun 1, 2017

When I try to authenticate, I received the following error:

authentication failed due to: In collectInfoFromReq: invalid state received in the request

When the user is forwarded to the authorization endpoint, the state I have is this:

CUSTOMdujKnvj%2BJ1ezw5qrAUW6RaHqAUtiU1jxmy_state

When they're forwarded back to the redirectUri, the state is this:

CUSTOMdujKnvj+J1ezw5qrAUW6RaHqAUtiU1jxmy_state

Notice the difference:
image

Looks like the '+' is being encoded where it wasn't before.

@AskYous
Copy link
Author

AskYous commented Jun 1, 2017

I used a custom state too:

passport.authenticate("office365", { "tenantIdOrName": <myTenantId>, customState: "my_state" });

@lovemaths lovemaths self-assigned this Jun 2, 2017
@lovemaths lovemaths added the bug label Jun 2, 2017
@lovemaths
Copy link
Contributor

@AskYous Thank you for the bug. I will fix later.

@AskYous
Copy link
Author

AskYous commented Jun 2, 2017

@lovemaths good to hear. Do you know any temporary workaround I can use for now? A lot of my users aren't getting authenticated. Can I have the state not have any special characters?

@lovemaths
Copy link
Contributor

@AskYous Yes, there is a quick fix to it. In lib/aadutils.js, line 107.

change

exports.uid = (len) => {
  return crypto.randomBytes(Math.ceil(len * 3 / 4))
    .toString('base64')
    .slice(0, len);
};

to

exports.uid = (len) => {
  var bytes = crypto.randomBytes(Math.ceil(len * 3 / 4));
  return base64url.encode(bytes).slice(0,len);
};

@AskYous
Copy link
Author

AskYous commented Jun 2, 2017

I went to /lib/passport-azure-ad/aadutils.js and made the following changes but it didn't work (more details below) :

image

I tested it, and the state sent was the exact same as the state received since it had no special characters, and it still gave the same error message... now I'm confused. Maybe the plus sign being encoded was just Chrome's way of displaying it to me.

Here's the network tab of Chrome. When the user was forwarded, this was the url with the state (highlighted in yellow):
image

Then in the callback, this was the state:
image

They're the same state, but the server gave me this:

authentication failed due to: In collectInfoFromReq: invalid state received in the request

@lovemaths
Copy link
Contributor

lovemaths commented Jun 2, 2017

@AskYous Before we send the state, we save it in session (or cookie, based on your config). After we receive the state back from Azure AD, we will try to retrieve it from session (or cookie, based on your config). If we don't find the exact state, we throw the error that you saw. It seems something went wrong where the state was saved.

(1) What is the config you used for passport-azure-ad?

(2) Are you running the app in one server or multiple servers?

@AskYous
Copy link
Author

AskYous commented Jun 3, 2017

Hey @lovemaths,

It's run on my localhost server and in a production server. On localhost, it seems to work 100% of the time. On production, it fails most of the time. I even used an incognito tab for the production server and it fails.

This is the config file we have:

passport.use("office365", new OAuth2Strategy({
    "identityMetadata": "https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration",
    "clientID": clientID,
    "clientSecret": clientSecret,
    "responseType": "code",
    "responseMode": "form_post",
    "redirectUrl": `https://www.<mywebsite>.com/login/callback`,
    "allowHttpForRedirectUrl": true,
    "scope": ["email", "profile", "offline_access", "User.ReadBasic.All"],
    "loggingLevel": "info",
}, (iss, sub, profile, access_token, refresh_token, done) => {
    agent.get("https://graph.microsoft.com/v1.0/me").set("Authorization", "Bearer " + access_token).end((err, resp) => {
        // ... Then the verification function

and I have the following for authenticate:

 passport.authenticate("office365", { "tenantIdOrName": azureFlexTenantId, customState: "my_state" });

I have something that may be causing this issue. Because I don't know a way to set the redirectUrl dynamically (either to localhost/callback or to <mywebsite>.com/callback), I run the same code above again when the user tries to login, except the second time I set the redirectUrl depending on where they called it from (they either called it from <mywebsite>.com or localhost).

@lovemaths
Copy link
Contributor

@AskYous Not sure why you need different redirectUrl. My understanding is that you need to run two apps, one runs locally (for localhost) and the other one deployed (for <mywebsite>.com). You can set up environment specific configuration, so the two apps can use the same code, but pick up different config depending on the environment.

Per your config, state is saved in session. When we validate state, we will try to find it in session. Most likely your dynamic redirectUrl messed up the session, because express finds session via cookie, and cookie is bounded to a specific domain. I would suggest use the environment specific config method I mentioned above.

We can add some logging to session, see if something is wrong there. state is handled by lib/sessionContentHandler.js. Add some logging in add and findAndDeleteTupleByState function in that file, for example, add console.log(array) after line 50 and 77. Check the log, see if the state is added to session before sending login request to Azure AD; and also do so in findAndDeleteTupleByState function, see if that state was saved in session.

@brentschmaltz brentschmaltz added this to the vNext milestone Jun 6, 2017
lovemaths added a commit that referenced this issue Jun 15, 2017
lovemaths added a commit that referenced this issue Jun 15, 2017
lovemaths added a commit that referenced this issue Jun 16, 2017
Issue #309 correct uid encoding problem
lovemaths added a commit that referenced this issue Jun 16, 2017
@lovemaths
Copy link
Contributor

@AskYous The encoding problem is fixed in the new release.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants