Add option to authorize the calling user to access an Application #1354
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
Jan 29, 2020
JoshNorthrup
force-pushed
the
user-application-access-control
branch
from
b34d790
to
93d0857
Compare
houndci-bot
reviewed
Jan 29, 2020
JoshNorthrup
force-pushed
the
user-application-access-control
branch
2 times, most recently
from
e8d1b0d
to
d120c92
Compare
6 tasks
nbulaj
reviewed
Feb 5, 2020
JoshNorthrup
force-pushed
the
user-application-access-control
branch
from
d120c92
to
dcd7c1b
Compare
houndci-bot
reviewed
Feb 6, 2020
JoshNorthrup
force-pushed
the
user-application-access-control
branch
from
dcd7c1b
to
65934c2
Compare
nbulaj
reviewed
Feb 7, 2020
| @@ -7,6 +7,7 @@ class PreAuthorization | |||
|
|
|||
| validate :client_id, error: :invalid_request | |||
| validate :client, error: :invalid_client | |||
| validate :access_to_client, error: :invalid_client | |||
There was a problem hiding this comment.
And considering comment about the option name let rename it to resource_owner_authorized_for_client. Don't get angst about the long naming just because I think it must be explicit rather than implicit.
| @@ -17,7 +18,7 @@ class PreAuthorization | |||
| attr_reader :server, :client_id, :client, :redirect_uri, :response_type, :state, | |||
| :code_challenge, :code_challenge_method, :missing_param | |||
|
|
|||
| def initialize(server, attrs = {}) | |||
| def initialize(server, attrs = {}, resource_owner = nil) | |||
There was a problem hiding this comment.
It's OK for me just because attrs not options, so we can add one more argument
| @@ -26,6 +27,7 @@ def initialize(server, attrs = {}) | |||
| @state = attrs[:state] | |||
| @code_challenge = attrs[:code_challenge] | |||
| @code_challenge_method = attrs[:code_challenge_method] | |||
| @resource_owner = resource_owner | |||
There was a problem hiding this comment.
Let's add it also to attr_readers, maybe we wanna to access it in the future for some reason
…ion during pre-auth
JoshNorthrup
force-pushed
the
user-application-access-control
branch
from
65934c2
to
df87749
Compare
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.
Summary
I've added an option to define whether a user can authorize an application.
My specific use case is the following:
This is now achievable with the new option:
When this validation fails the
invalid_clienterror is used so it's indistinguishable from a nonexistent Application