Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding client id to strong parameters #1296

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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.
Expand Down
6 changes: 2 additions & 4 deletions app/controllers/doorkeeper/authorizations_controller.rb
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion lib/doorkeeper/helpers/controller.rb
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions lib/doorkeeper/oauth/pre_authorization.rb
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 0 additions & 4 deletions lib/doorkeeper/server.rb
Expand Up @@ -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
Expand Down
35 changes: 11 additions & 24 deletions spec/lib/oauth/pre_authorization_spec.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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
Expand Down