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

exchanges: add scope type validation #213

Merged

Conversation

ziluvatar
Copy link
Contributor

@ziluvatar ziluvatar commented Aug 9, 2017

It assures scope parameter is a string before we start manipulating it, otherwise you find this type error: scope.split is not a function

There is older PR about this: #170 (but mine solves this for all exchanges + testing)

Notes:

  • I used invalid_scope instead of invalid_request since by spec:

    The requested scope is invalid, unknown, or malformed.

    I guess wrong format can be in the malformed aspect, however, the library already does this check in code grant and it returns invalid_request. I've added another commit in this PR to "fix" that and to unify them to invalid_scope, let me know if you prefer the other way around: all of them returning invalid_request.

  • The code is repeated in all exchanges, not sure if you have in mind some refactor before or after this PR, let me know if you prefer another validation approach.

@coveralls
Copy link

coveralls commented Aug 9, 2017

Coverage Status

Coverage increased (+0.001%) to 99.787% when pulling abc35a2 on ziluvatar:validate-scope-on-exchanges into 0516025 on jaredhanson:master.

@coveralls
Copy link

coveralls commented Aug 9, 2017

Coverage Status

Coverage increased (+0.001%) to 99.787% when pulling 48a4ea7 on ziluvatar:validate-scope-on-exchanges into 0516025 on jaredhanson:master.

@coveralls
Copy link

coveralls commented Aug 9, 2017

Coverage Status

Coverage increased (+0.001%) to 99.787% when pulling fdca1cd on ziluvatar:validate-scope-on-exchanges into 0516025 on jaredhanson:master.

@jaredhanson jaredhanson merged commit 1c91488 into jaredhanson:master Aug 9, 2017
@jaredhanson
Copy link
Owner

Thanks. I switched the errors to invalid_request in this commit: d0f0fde

My rationale is this. invalid_request is used when the payload is malformed (which it is in this case, since scope must be a string). invalid_scope would be used if one of the scopes (carried in the string) were malformed in some way. It would be up to the application to check this (for example, in the issue callback), since this library doesn't interpret scope values.

Released oauth2orize@1.8.1 to npm.

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