Skip to content

Commit

Permalink
add option to define a resource owner's ability to access an applicat…
Browse files Browse the repository at this point in the history
…ion during pre-auth
  • Loading branch information
JoshNorthrup committed Feb 7, 2020
1 parent 9f0af2c commit df87749
Show file tree
Hide file tree
Showing 7 changed files with 181 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ User-visible changes worth mentioning.

## master

- [#1354] Add option to authorize the calling user to access an application.
- [#1355] Allow to enable polymorphic Resource Owner association for Access Token & Grant
models (`use_polymorphic_resource_owner` configuration option).

Expand Down
6 changes: 5 additions & 1 deletion app/controllers/doorkeeper/authorizations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ def redirect_or_render(auth)
end

def pre_auth
@pre_auth ||= OAuth::PreAuthorization.new(Doorkeeper.configuration, pre_auth_params)
@pre_auth ||= OAuth::PreAuthorization.new(
Doorkeeper.configuration,
pre_auth_params,
current_resource_owner,
)
end

def pre_auth_params
Expand Down
4 changes: 4 additions & 0 deletions lib/doorkeeper/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,10 @@ def configure_secrets_for(type, using:, fallback:)
default: {},
deprecated: { message: "Customize Doorkeeper models instead" }

# Hook to allow arbitrary user-client authorization
option :authorize_resource_owner_for_client,
default: ->(_client, _resource_owner) { true }

# Allows to customize OAuth grant flows that +each+ application support.
# You can configure a custom block (or use a class respond to `#call`) that must
# return `true` in case Application instance supports requested OAuth grant flow
Expand Down
11 changes: 9 additions & 2 deletions lib/doorkeeper/oauth/pre_authorization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ class PreAuthorization

validate :client_id, error: :invalid_request
validate :client, error: :invalid_client
# The authorize_resource_owner_for_client config option is used for this validation
validate :access_to_client, error: :invalid_client
validate :redirect_uri, error: :invalid_redirect_uri
validate :params, error: :invalid_request
validate :response_type, error: :unsupported_response_type
Expand All @@ -15,9 +17,9 @@ class PreAuthorization
validate :client_supports_grant_flow, error: :unauthorized_client

attr_reader :server, :client_id, :client, :redirect_uri, :response_type, :state,
:code_challenge, :code_challenge_method, :missing_param
:code_challenge, :code_challenge_method, :missing_param, :resource_owner

def initialize(server, attrs = {})
def initialize(server, attrs = {}, resource_owner = nil)
@server = server
@client_id = attrs[:client_id]
@response_type = attrs[:response_type]
Expand All @@ -26,6 +28,7 @@ def initialize(server, attrs = {})
@state = attrs[:state]
@code_challenge = attrs[:code_challenge]
@code_challenge_method = attrs[:code_challenge_method]
@resource_owner = resource_owner
end

def authorizable?
Expand Down Expand Up @@ -137,6 +140,10 @@ def pre_auth_hash
status: I18n.t("doorkeeper.pre_authorization.status"),
}
end

def validate_access_to_client
client.application.authorized_for_resource_owner?(@resource_owner)
end
end
end
end
4 changes: 4 additions & 0 deletions lib/doorkeeper/orm/active_record/mixins/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ def as_json(options = {})
hash
end

def authorized_for_resource_owner?(resource_owner)
Doorkeeper.configuration.authorize_resource_owner_for_client.call(self, resource_owner)
end

private

def generate_uid
Expand Down
174 changes: 149 additions & 25 deletions spec/controllers/authorizations_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,37 @@ def query_params
end
end

context "when user cannot access application" do
before do
allow(Doorkeeper.configuration).to receive(:authorize_resource_owner_for_client).and_return(->(*_) { false })
post :create, params: {
client_id: client.uid,
response_type: "token",
redirect_uri: client.redirect_uri,
}
end

let(:response_json_body) { JSON.parse(response.body) }

it "renders 400 error" do
expect(response.status).to eq 401
end

it "includes error name" do
expect(response_json_body["error"]).to eq("invalid_client")
end

it "includes error description" do
expect(response_json_body["error_description"]).to eq(
translated_error_message(:invalid_client),
)
end

it "does not issue any access token" do
expect(Doorkeeper::AccessToken.all).to be_empty
end
end

context "when other error happens" do
before do
default_scopes_exist :public
Expand Down Expand Up @@ -214,6 +245,39 @@ def query_params
end
end

context "when user cannot access application" do
before do
allow(Doorkeeper.configuration).to receive(:api_only).and_return(true)
allow(Doorkeeper.configuration).to receive(:authorize_resource_owner_for_client).and_return(->(*_) { false })

post :create, params: {
client_id: client.uid,
response_type: "token",
redirect_uri: client.redirect_uri,
}
end

let(:response_json_body) { JSON.parse(response.body) }

it "renders 400 error" do
expect(response.status).to eq 401
end

it "includes error name" do
expect(response_json_body["error"]).to eq("invalid_client")
end

it "includes error description" do
expect(response_json_body["error_description"]).to eq(
translated_error_message(:invalid_client),
)
end

it "does not issue any access token" do
expect(Doorkeeper::AccessToken.all).to be_empty
end
end

context "when other error happens" do
before do
allow(Doorkeeper.config).to receive(:api_only).and_return(true)
Expand Down Expand Up @@ -489,46 +553,106 @@ def query_params
end

describe "GET #new with errors" do
before do
default_scopes_exist :public
get :new, params: { an_invalid: "request" }
end
context "without valid params" do
before do
default_scopes_exist :public
get :new, params: { an_invalid: "request" }
end

it "does not redirect" do
expect(response).to_not be_redirect
end

it "does not redirect" do
expect(response).to_not be_redirect
it "does not issue any token" do
expect(Doorkeeper::AccessGrant.count).to eq 0
expect(Doorkeeper::AccessToken.count).to eq 0
end
end

it "does not issue any token" do
expect(Doorkeeper::AccessGrant.count).to eq 0
expect(Doorkeeper::AccessToken.count).to eq 0
context "when user cannot access application" do
before do
allow(Doorkeeper.configuration).to receive(:authorize_resource_owner_for_client).and_return(->(*_) { false })

get :new, params: {
client_id: client.uid,
response_type: "token",
redirect_uri: client.redirect_uri,
}
end

it "does not redirect" do
expect(response).to_not be_redirect
end

it "does not issue any token" do
expect(Doorkeeper::AccessGrant.count).to eq 0
expect(Doorkeeper::AccessToken.count).to eq 0
end
end
end

describe "GET #new in API mode with errors" do
let(:response_json_body) { JSON.parse(response.body) }

before do
default_scopes_exist :public
allow(Doorkeeper.configuration).to receive(:api_only).and_return(true)
get :new, params: { an_invalid: "request" }
default_scopes_exist :public
end

it "should render bad request" do
expect(response).to have_http_status(:bad_request)
end
context "without valid params" do
before do
get :new, params: { an_invalid: "request" }
end

it "includes error in body" do
expect(response_json_body["error"]).to eq("invalid_request")
end
let(:response_json_body) { JSON.parse(response.body) }

it "should render bad request" do
expect(response).to have_http_status(:bad_request)
end

it "includes error in body" do
expect(response_json_body["error"]).to eq("invalid_request")
end

it "includes error description in body" do
expect(response_json_body["error_description"])
.to eq(translated_invalid_request_error_message(:missing_param, :client_id))
it "includes error description in body" do
expect(response_json_body["error_description"])
.to eq(translated_invalid_request_error_message(:missing_param, :client_id))
end

it "does not issue any token" do
expect(Doorkeeper::AccessGrant.count).to eq 0
expect(Doorkeeper::AccessToken.count).to eq 0
end
end

it "does not issue any token" do
expect(Doorkeeper::AccessGrant.count).to eq 0
expect(Doorkeeper::AccessToken.count).to eq 0
context "when user cannot access application" do
before do
allow(Doorkeeper.configuration).to receive(:authorize_resource_owner_for_client).and_return(->(*_) { false })

get :new, params: {
client_id: client.uid,
response_type: "token",
redirect_uri: client.redirect_uri,
}
end

let(:response_json_body) { JSON.parse(response.body) }

it "should render bad request" do
expect(response).to have_http_status(:bad_request)
end

it "includes error in body" do
expect(response_json_body["error"]).to eq("invalid_client")
end

it "includes error description in body" do
expect(response_json_body["error_description"])
.to eq(translated_error_message(:invalid_client))
end

it "does not issue any token" do
expect(Doorkeeper::AccessGrant.count).to eq 0
expect(Doorkeeper::AccessToken.count).to eq 0
end
end
end

Expand Down
9 changes: 9 additions & 0 deletions spec/lib/oauth/pre_authorization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
response_type: "code",
redirect_uri: "https://app.com/callback",
state: "save-this",
current_resource_owner: Object.new,
}
end

Expand Down Expand Up @@ -177,6 +178,14 @@
expect(subject).not_to be_authorizable
end

context "when resource_owner cannot access client application" do
before { allow(Doorkeeper.configuration).to receive(:authorize_resource_owner_for_client).and_return(->(*_) { false }) }

it "is not authorizable" do
expect(subject).not_to be_authorizable
end
end

describe "as_json" do
before { subject.authorizable? }

Expand Down

0 comments on commit df87749

Please sign in to comment.