passport-oauth2 icon indicating copy to clipboard operation
passport-oauth2 copied to clipboard

state query parameter drops out in v1.4.0 (used to work in v1.3.0)

Open hwangbible opened this issue 9 years ago • 3 comments

The microservice that I am working on started failing during the oauth handshake because the state query parameter gets lost in the pipeline. I noticed some changes in the params handling in the version upgrade of passport-oauth2 package from v1.3.0 to v1.4.0.

Here are some debugging info I can provide for now. Please let me know if you need anymore information.

NodeJS version: v6.9.1 Express version: v4.14.0

This is the options sent to the request:

{ 
  authorizationURL: 'https://localhost:3000/login/oauth/authorize?state=%2Ftoken',
  scope: 'openid cloud_controller.write cloud_controller.read',
  response_type: 'code',
  redirect_uri: 'https://localhost:5000/auth/callback' 
}

The request used to result in the following redirect location in v1.3.0:

https://localhost:3000/login/oauth/authorize?state=%2Ftoken&scope=openid%20cloud_controller.write%20cloud_controller.read&response_type=code&redirect_uri=https%3A%2F%2Flocalhost%3A5000%2Fauth%2Fcallback&client_id=test

However, it is redirected to the following:

https://localhost:3000/login/oauth/authorize?authorizationURL=https%3A%2F%2Flocalhost:3000%2Flogin%2Foauth%2Fauthorize%3Fstate%3D%252Ftoken&scope=openid%20cloud_controller.write%20cloud_controller.read&response_type=code&redirect_uri=https%3A%2F%2Flocalhost%3A5000%2Fauth%2Fcallback&client_id=test

Formerly those extra query params were merged into the authorizationURL which would preserve state query param. Now it's attaching the whole url with state query param, as authorizationURL query param, to the parsed url href (https://localhost:3000/login/oauth/authorize in this case). As a result, the redirect location no longer has state query parameter in the url causing an issue whenever it is returned back to the callback url (redirect_uri) after the authentication.

Is this an expected behavior/change for v1.4.0? Please let me know if this change is something that I need to modify on my side to stay on track.

hwangbible avatar Jan 11 '17 21:01 hwangbible

When you say "this is the options sent to the request"

{ 
  authorizationURL: 'https://localhost:3000/login/oauth/authorize?state=%2Ftoken',
  scope: 'openid cloud_controller.write cloud_controller.read',
  response_type: 'code',
  redirect_uri: 'https://localhost:5000/auth/callback' 
}

How are you passing the options to passport? Via a new Strategy constructor, via passport.authenticate() or some other mechanism?

jaredhanson avatar Jan 14 '17 01:01 jaredhanson

@jaredhanson We do it via passport.autheticate('strategy-name', options).

hwangbible avatar Jan 20 '17 21:01 hwangbible

This issue is 5yrs old, but posting this in case anyone else lands here. I'm not certain there is actually any regression as implied.

You have to pass state yourself to passport.authenticate(). Here is the PR where the feature was added, and in the PR description it says you must pass state value to passport.authenticate() from your issue report, it looks like you are not passing it.

joshribakoff-sm avatar Aug 09 '22 19:08 joshribakoff-sm