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

Duplication of 'db.clients.findByClientId()' in example #6

Open
bloadvenro opened this issue Feb 27, 2017 · 2 comments
Open

Duplication of 'db.clients.findByClientId()' in example #6

bloadvenro opened this issue Feb 27, 2017 · 2 comments

Comments

@bloadvenro
Copy link

bloadvenro commented Feb 27, 2017

Hi.

My question is:

Why do we need to check client twice? Isn't it enough to fetch client in passport strategy and then simply use client object which was passed down to oauth2orize clientCredentials exchange handler? Or this duplication is just for demo purposes?

Thank you!

@gerges-beshay
Copy link
Contributor

Can you clarify where are the two locations in the code that you believe to be duplicated calls?

@ksmithut
Copy link

ksmithut commented Sep 6, 2018

It's been a while, but just for sake of clarification, this is how it is duplicated:

In the token endpoint middleware stack here: https://github.com/gerges-beshay/oauth2orize-examples/blob/master/routes/oauth2.js#L204-L205, it declares a passport usage and the oauth2orize token() middleware. The passport strategy fetches the client here: https://github.com/gerges-beshay/oauth2orize-examples/blob/master/auth/index.js#L46, then that fetched client is passed as an argument to the token exchange here: https://github.com/gerges-beshay/oauth2orize-examples/blob/master/routes/oauth2.js#L123, but rather than just use the fetched client, it fetches it again in the oauth2orize client_credential exchange.

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

No branches or pull requests

3 participants