From a3458b00d89fb79e0f0a4efc733124c3bfe56db6 Mon Sep 17 00:00:00 2001 From: Duy Linh Dang Date: Sun, 11 Aug 2019 11:25:02 +0900 Subject: [PATCH] add client_id to permitted strong parameters --- CHANGELOG.md | 1 + .../doorkeeper/authorizations_controller.rb | 6 ++-- lib/doorkeeper/helpers/controller.rb | 2 +- lib/doorkeeper/oauth/pre_authorization.rb | 11 +++--- lib/doorkeeper/server.rb | 4 --- spec/lib/oauth/pre_authorization_spec.rb | 35 ++++++------------- 6 files changed, 21 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2a3db3ee..eb05427f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ User-visible changes worth mentioning. ## master +- [#1296] Adding client_id to strong parameters. - [#1293] Move ar specific redirect uri validator to ar orm directory. - [#1288] Allow to pass attributes to the `Doorkeeper::OAuth::PreAuthorization#as_json` method to customize the PreAuthorization response. diff --git a/app/controllers/doorkeeper/authorizations_controller.rb b/app/controllers/doorkeeper/authorizations_controller.rb index db59acb84..3154d64c8 100644 --- a/app/controllers/doorkeeper/authorizations_controller.rb +++ b/app/controllers/doorkeeper/authorizations_controller.rb @@ -66,13 +66,11 @@ def redirect_or_render(auth) end def pre_auth - @pre_auth ||= OAuth::PreAuthorization.new(Doorkeeper.configuration, - server.client_via_uid, - pre_auth_params) + @pre_auth ||= OAuth::PreAuthorization.new(Doorkeeper.configuration, pre_auth_params) end def pre_auth_params - params.permit(:response_type, :redirect_uri, :scope, :state, + params.permit(:client_id, :response_type, :redirect_uri, :scope, :state, :code_challenge, :code_challenge_method) end diff --git a/lib/doorkeeper/helpers/controller.rb b/lib/doorkeeper/helpers/controller.rb index 9bd597090..fdb614101 100644 --- a/lib/doorkeeper/helpers/controller.rb +++ b/lib/doorkeeper/helpers/controller.rb @@ -58,7 +58,7 @@ def handle_token_exception(exception) def skip_authorization? !!instance_exec( - [@server.current_resource_owner, @pre_auth.client], + [server.current_resource_owner, @pre_auth.client], &Doorkeeper.configuration.skip_authorization ) end diff --git a/lib/doorkeeper/oauth/pre_authorization.rb b/lib/doorkeeper/oauth/pre_authorization.rb index 1488ec47d..10c94d986 100644 --- a/lib/doorkeeper/oauth/pre_authorization.rb +++ b/lib/doorkeeper/oauth/pre_authorization.rb @@ -16,9 +16,9 @@ class PreAuthorization :code_challenge, :code_challenge_method attr_writer :scope - def initialize(server, client, attrs = {}) + def initialize(server, attrs = {}) @server = server - @client = client + @client_id = attrs[:client_id] @response_type = attrs[:response_type] @redirect_uri = attrs[:redirect_uri] @scope = attrs[:scope] @@ -56,7 +56,7 @@ def as_json(attributes = {}) private def build_scopes - client_scopes = client.application.scopes + client_scopes = client.scopes if client_scopes.blank? server.default_scopes.to_s else @@ -69,7 +69,8 @@ def validate_response_type end def validate_client - client.present? + @client = OAuth::Client.find(@client_id) + @client.present? end def validate_scopes @@ -78,7 +79,7 @@ def validate_scopes Helpers::ScopeChecker.valid?( scope_str: scope, server_scopes: server.scopes, - app_scopes: client.application.scopes, + app_scopes: client.scopes, grant_type: grant_type ) end diff --git a/lib/doorkeeper/server.rb b/lib/doorkeeper/server.rb index c295b7fa2..68eb1b2ee 100644 --- a/lib/doorkeeper/server.rb +++ b/lib/doorkeeper/server.rb @@ -27,10 +27,6 @@ def client @client ||= OAuth::Client.authenticate(credentials) end - def client_via_uid - @client_via_uid ||= OAuth::Client.find(parameters[:client_id]) - end - def current_resource_owner context.send :current_resource_owner end diff --git a/spec/lib/oauth/pre_authorization_spec.rb b/spec/lib/oauth/pre_authorization_spec.rb index 13a150688..e20205388 100644 --- a/spec/lib/oauth/pre_authorization_spec.rb +++ b/spec/lib/oauth/pre_authorization_spec.rb @@ -11,26 +11,20 @@ module Doorkeeper::OAuth server end - let(:application) do - application = double :application - allow(application).to receive(:scopes).and_return(Scopes.from_string("")) - application - end - - let(:client) do - double :client, redirect_uri: "http://tst.com/auth", application: application - end + let(:application) { FactoryBot.create(:application, redirect_uri: "https://app.com/callback") } + let(:client) { Client.find(application.uid) } let :attributes do { + client_id: client.uid, response_type: "code", - redirect_uri: "http://tst.com/auth", + redirect_uri: "https://app.com/callback", state: "save-this", } end subject do - PreAuthorization.new(server, client, attributes) + PreAuthorization.new(server, attributes) end it "is authorizable when request is valid" do @@ -109,9 +103,7 @@ module Doorkeeper::OAuth context "client application restricts valid scopes" do let(:application) do - application = double :application - allow(application).to receive(:scopes).and_return(Scopes.from_string("public nonsense")) - application + FactoryBot.create(:application, scopes: Scopes.from_string("public nonsense")) end it "accepts valid scopes" do @@ -145,6 +137,7 @@ module Doorkeeper::OAuth it "uses default scopes when none is required" do allow(server).to receive(:default_scopes).and_return(Scopes.from_string("default")) subject.scope = nil + subject.client = client expect(subject.scope).to eq("default") expect(subject.scopes).to eq(Scopes.from_string("default")) end @@ -164,7 +157,7 @@ module Doorkeeper::OAuth end it "requires an existing client" do - subject.client = nil + attributes[:client_id] = nil expect(subject).not_to be_authorizable end @@ -174,24 +167,18 @@ module Doorkeeper::OAuth end describe "as_json" do - let(:client_id) { "client_uid_123" } - let(:client_name) { "Acme Co." } - - before do - allow(client).to receive(:uid).and_return client_id - allow(client).to receive(:name).and_return client_name - end + before { subject.client = client } it { is_expected.to respond_to :as_json } shared_examples "returns the pre authorization" do it "returns the pre authorization" do - expect(json[:client_id]).to eq client_id + expect(json[:client_id]).to eq client.uid expect(json[:redirect_uri]).to eq subject.redirect_uri expect(json[:state]).to eq subject.state expect(json[:response_type]).to eq subject.response_type expect(json[:scope]).to eq subject.scope - expect(json[:client_name]).to eq client_name + expect(json[:client_name]).to eq client.name expect(json[:status]).to eq I18n.t("doorkeeper.pre_authorization.status") end end