diff --git a/app/controllers/doorkeeper/authorizations_controller.rb b/app/controllers/doorkeeper/authorizations_controller.rb index 7bb5518dd..35abb6b47 100644 --- a/app/controllers/doorkeeper/authorizations_controller.rb +++ b/app/controllers/doorkeeper/authorizations_controller.rb @@ -69,7 +69,8 @@ def pre_auth end def pre_auth_params - params.slice(*pre_auth_param_fields).permit(*pre_auth_param_fields) + params.slice(*pre_auth_param_fields).permit(*pre_auth_param_fields). + merge({current_resource_owner: current_resource_owner}) end def pre_auth_param_fields diff --git a/lib/doorkeeper/config.rb b/lib/doorkeeper/config.rb index 63204304c..2bb6db395 100644 --- a/lib/doorkeeper/config.rb +++ b/lib/doorkeeper/config.rb @@ -269,6 +269,9 @@ def configure_secrets_for(type, using:, fallback:) option :grant_flows, default: %w[authorization_code client_credentials] option :handle_auth_errors, default: :render + # Hook to allow arbitrary user-application access enforcement + option :application_access, default: ->(_application,_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 diff --git a/lib/doorkeeper/oauth/pre_authorization.rb b/lib/doorkeeper/oauth/pre_authorization.rb index 5d0e4599d..9805071e7 100644 --- a/lib/doorkeeper/oauth/pre_authorization.rb +++ b/lib/doorkeeper/oauth/pre_authorization.rb @@ -7,6 +7,7 @@ class PreAuthorization validate :client_id, error: :invalid_request validate :client, error: :invalid_client + validate :resource_can_access_client, error: :invalid_client validate :redirect_uri, error: :invalid_redirect_uri validate :params, error: :invalid_request validate :response_type, error: :unsupported_response_type @@ -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 = attrs[:current_resource_owner] end def authorizable? @@ -139,6 +141,10 @@ def pre_auth_hash status: I18n.t("doorkeeper.pre_authorization.status"), } end + + def validate_resource_can_access_client + client.application.viewable_to_resource_owner?(@resource_owner) + end end end end diff --git a/lib/doorkeeper/orm/active_record/mixins/application.rb b/lib/doorkeeper/orm/active_record/mixins/application.rb index cf34134c2..4a3fc649d 100644 --- a/lib/doorkeeper/orm/active_record/mixins/application.rb +++ b/lib/doorkeeper/orm/active_record/mixins/application.rb @@ -72,6 +72,10 @@ def as_json(options = {}) hash end + def viewable_to_resource_owner?(resource_owner) + Doorkeeper.configuration.application_access.call(self,resource_owner) + end + private def generate_uid diff --git a/spec/controllers/authorizations_controller_spec.rb b/spec/controllers/authorizations_controller_spec.rb index d3939c232..134f457f1 100644 --- a/spec/controllers/authorizations_controller_spec.rb +++ b/spec/controllers/authorizations_controller_spec.rb @@ -136,6 +136,37 @@ def query_params end end + context "when user cannot access application" do + before do + allow(Doorkeeper.configuration).to receive(:application_access).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 @@ -207,6 +238,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(:application_access).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) @@ -482,46 +546,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(:application_access).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(:application_access).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 diff --git a/spec/lib/oauth/pre_authorization_spec.rb b/spec/lib/oauth/pre_authorization_spec.rb index 37aefb81c..562eb229f 100644 --- a/spec/lib/oauth/pre_authorization_spec.rb +++ b/spec/lib/oauth/pre_authorization_spec.rb @@ -19,6 +19,7 @@ response_type: "code", redirect_uri: "https://app.com/callback", state: "save-this", + current_resource_owner: Object.new } end @@ -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(:application_access).and_return(->(*_){false})} + + it "is not authorizable" do + expect(subject).not_to be_authorizable + end + end + describe "as_json" do before { subject.authorizable? }