Make native redirect code a query param #1003
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
houndci-bot
reviewed
Nov 30, 2017
| @@ -154,7 +154,7 @@ def translated_error_message(key) | |||
|
|
|||
| it 'should redirect immediately' do | |||
| expect(response).to be_redirect | |||
| expect(response.location).to match(/oauth\/authorize\//) | |||
| expect(response.location).to match(/oauth\/authorize\/native?code=/) | |||
| @@ -49,7 +49,7 @@ def authorization_routes(mapping) | |||
| as: mapping[:as], | |||
| controller: mapping[:controllers] | |||
| ) do | |||
| routes.get '/:code', action: :show, on: :member | |||
| routes.get '/native', action: :show, on: :member | |||
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
dv
force-pushed
the
make-native-auth-code-a-param
branch
from
267a544
to
185c9f5
Compare
houndci-bot
reviewed
Jan 29, 2018
| @@ -154,7 +154,7 @@ def translated_error_message(key) | |||
|
|
|||
| it 'should redirect immediately' do | |||
| expect(response).to be_redirect | |||
| expect(response.location).to match(/oauth\/authorize\//) | |||
| expect(response.location).to match(/oauth\/authorize\/native\?code=#{Doorkeeper::AccessGrant.first.token}/) | |||
There was a problem hiding this comment.
Use %r around regular expression.
Line is too long. [113/80]
| @@ -29,6 +29,7 @@ | |||
|
|
|||
| access_grant_should_exist_for(@client, @resource_owner) | |||
|
|
|||
| url_should_have_param('code', Doorkeeper::AccessGrant.first.token) | |||
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
dv
force-pushed
the
make-native-auth-code-a-param
branch
from
90c5ca0
to
7dea128
Compare
This allows for automated flows to detect that an Authorization code was granted in much the same way as a normal redirect. This is used by e.g. Mac Paw.
dv
force-pushed
the
make-native-auth-code-a-param
branch
from
7dea128
to
2236363
Compare
|
Hey @nbulaj, thanks! Did as requested, is this ok? |
|
Great, thanks @dv! |
pakwfoley
pushed a commit
to pakwfoley/doorkeeper
that referenced
this pull request
Apr 5, 2018
pakwfoley
pushed a commit
to pakwfoley/doorkeeper
that referenced
this pull request
Apr 5, 2018
Remove changes from doorkeeper PR doorkeeper-gem#1003
pakwfoley
pushed a commit
to pakwfoley/doorkeeper
that referenced
this pull request
Apr 5, 2018
pakwfoley
pushed a commit
to pakwfoley/doorkeeper
that referenced
this pull request
Apr 5, 2018
Revert "Remove changes from doorkeeper PR doorkeeper-gem#1003"
pakwfoley
pushed a commit
to pakwfoley/doorkeeper
that referenced
this pull request
Apr 5, 2018
pakwfoley
pushed a commit
to pakwfoley/doorkeeper
that referenced
this pull request
Apr 5, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using native redirect, instead of redirecting to
oauth/authorize/abc123abc123it redirects to
oauth/authorize/native?code=abc123abc123This allows for automated flows to detect that an Authorization code was granted in much the same way as a normal redirect. This is used by e.g. Mac Paw (and possibly Postman) to detect whether a code was given or not. I'm not sure if this was intentional or not, but adding the code as an actual param would make sense to mimic the non-oob flow as much as possible, gaining support by any software that works with non-oob flow.
Before:
After:
Related