Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow set state in case it exists on oauth2 provider #253

Merged
merged 3 commits into from
Feb 10, 2019

Conversation

epartipilo
Copy link
Contributor

Some times I need some values to be returned from the auth service and the state param of the Oauth2 scheme give us that option. Right now it sends a random string.

@farnabaz farnabaz merged commit 6420ddc into nuxt-community:dev Feb 10, 2019
@farnabaz
Copy link
Member

Thank you and sorry for the late response

@@ -18,7 +18,8 @@ auth: {
token_type: 'Bearer',
redirect_uri: undefined,
client_id: 'SET_ME',
token_key: 'access_token'
token_key: 'access_token',
state: 'UNIQUE_AND_NON_GUESSABLE'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be random per login not documented here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad
You're right, state must keep random and internal.
We could persist our custom values with auth storage in order to recover them on OAuth2 callback

@gijswijs
Copy link

gijswijs commented Apr 9, 2019

Wouldn't it make sense to also allow for state to be set through arguments of the loginWith and login methods?
e.g.: loginWith('strategyName',{ state: 'statevalue'})
Because now it seems to me state is always fixed to what you set it in the auth config. Or am I missing something here?

@pi0 pi0 mentioned this pull request May 23, 2019
@pi0
Copy link
Member

pi0 commented May 23, 2019

@gijswijs Yes, you are completely right. Fixed in e5579e9

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

Successfully merging this pull request may close these issues.

None yet

4 participants