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

Pass validated client credential to AuthorizationHandler #114

Merged
merged 1 commit into from
Jan 20, 2017

Conversation

tsuyoshizawa
Copy link
Member

fix #113

We removed AuthorizationHandler#clientCredential by #110

But AuthorizationHandler#validateClient and AuthorizationHandler#findUser may use it.
We needed to pass validated client credential to it.

@rmmeans
Copy link
Contributor

rmmeans commented Jan 20, 2017

This looks good to me 👍 and makes it easier. I understand the problem in #113, but you could work-around it just like before and ask parse the client credential (what we are doing right now) - but then they have to deal with the Either[InvalidClient, ClientCredential] type which is not as nice. This will make it simpler to deal with it at the data handler layer.

@tsuyoshizawa
Copy link
Member Author

Thank you for review.

but they have to deal with the Either[InvalidClient, ClientCredential] type which is not as nice

Yes, I agree with you. InvalidClient is already validated before executing validateClient and findUser in AuthorizationHandler. We need just Option[ClientCredential].

I will merge and release.
Thank you again.

@tsuyoshizawa tsuyoshizawa merged commit c1e9a4f into master Jan 20, 2017
@tsuyoshizawa tsuyoshizawa deleted the pass-credential-auth-req branch January 22, 2017 17:23
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.

value clientCredential is not a member
2 participants