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

Document response_type in verify_client callback #7

Merged
merged 1 commit into from Feb 23, 2017

Conversation

mrenvoize
Copy link
Collaborator

This clarifies the documentation for the verify_client callback
subroutine signature and also updates the example too.

This clarifies the documentation for the verify_client callback
subroutine signature and also updates the example too.
@mrenvoize
Copy link
Collaborator Author

Pretty sure this resolves issue #4. The default _verify_client code found within ImplicitGrant appears to take account response type inherently without the need to response_type being passed (::Plugin::OAuth2::Server traps that we're in an ImplicitGrant situation before the verify_callback is called so directs us to the right callback (so long as the callbacks aren't overridden and therefore shared between grant types).

I've therefore tried to resolve this documentation clarification.

@coveralls
Copy link

coveralls commented Feb 23, 2017

Coverage Status

Coverage remained the same at 97.802% when pulling da2131a on mrenvoize:response_type into a335511 on Humanstate:master.

@mrenvoize
Copy link
Collaborator Author

We could further enforce this by requiring response_type be passed to, and equal to 'token', _verify_client for this grant type which may add further code clarity.. But I'm not entirely sure it's required with these documentation updates in place?

@leejo leejo merged commit 2489dec into payprop:master Feb 23, 2017
@leejo
Copy link
Contributor

leejo commented Feb 23, 2017

Merged, thanks! I'll hold back on building a release for CPAN until the other issues/PRs are done/tweaked.

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

3 participants