From f35d0302334b7f8ecc599074c86f2880ad8ef010 Mon Sep 17 00:00:00 2001 From: Matteo Centenaro Date: Fri, 23 Mar 2012 16:43:09 +0100 Subject: [PATCH 1/3] Let client_secret parameter to be optional when requesting an assertion grant. OAuth2's draft#10 state that client credentials must be validated only if present. See: http://tools.ietf.org/html/draft-ietf-oauth-v2-10#section-4.1.3 --- lib/oauth2/provider/exchange.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/oauth2/provider/exchange.rb b/lib/oauth2/provider/exchange.rb index e39a240c..ee048ea3 100644 --- a/lib/oauth2/provider/exchange.rb +++ b/lib/oauth2/provider/exchange.rb @@ -101,6 +101,7 @@ def validate! def validate_required_params REQUIRED_PARAMS.each do |param| next if @params.has_key?(param) + next if param == CLIENT_SECRET && @grant_type == ASSERTION @error = INVALID_REQUEST @error_description = "Missing required parameter #{param}" end @@ -113,7 +114,7 @@ def validate_client @error_description = "Unknown client ID #{@params[CLIENT_ID]}" end - if @client and not @client.valid_client_secret? @params[CLIENT_SECRET] + if @client and @params[CLIENT_SECRET] and not @client.valid_client_secret? @params[CLIENT_SECRET] @error = INVALID_CLIENT @error_description = 'Parameter client_secret does not match' end From f6c69416a165265c1ca4d2b548f541e6e6d1aad5 Mon Sep 17 00:00:00 2001 From: Matteo Centenaro Date: Fri, 23 Mar 2012 16:46:15 +0100 Subject: [PATCH 2/3] client_secret is optional for an assertion grant --- spec/oauth2/provider/exchange_spec.rb | 63 ++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/spec/oauth2/provider/exchange_spec.rb b/spec/oauth2/provider/exchange_spec.rb index 0891ba0e..b8c461b2 100644 --- a/spec/oauth2/provider/exchange_spec.rb +++ b/spec/oauth2/provider/exchange_spec.rb @@ -243,7 +243,68 @@ OAuth2::Provider.clear_assertion_handlers! end - it_should_behave_like "validates required parameters" + describe "missing grant_type" do + before { params.delete('client_id') } + + it "is invalid" do + exchange.error.should == "invalid_request" + exchange.error_description.should == "Missing required parameter client_id" + end + end + + describe "with an unknown grant type" do + before { params['grant_type'] = 'unknown' } + + it "is invalid" do + exchange.error.should == "unsupported_grant_type" + exchange.error_description.should == "The grant type unknown is not recognized" + end + end + + describe "missing client_id" do + before { params.delete('client_id') } + + it "is invalid" do + exchange.error.should == "invalid_request" + exchange.error_description.should == "Missing required parameter client_id" + end + end + + describe "with an unknown client_id" do + before { params['client_id'] = "unknown" } + + it "is invalid" do + exchange.error.should == "invalid_client" + exchange.error_description.should == "Unknown client ID unknown" + end + end + + describe "with a mismatched client_secret" do + before { params['client_secret'] = "nosoupforyou" } + + it "is invalid" do + exchange.error.should == "invalid_client" + exchange.error_description.should == "Parameter client_secret does not match" + end + end + + describe "with lesser scope than the authorization code represents" do + before { params['scope'] = 'bar' } + + it "is valid" do + exchange.error.should be_nil + end + end + + describe "with scopes not covered by the authorization code" do + before { params['scope'] = 'qux' } + + it "is invalid" do + exchange.error.should == 'invalid_scope' + exchange.error_description.should == 'The request scope was never granted by the user' + end + end + it_should_behave_like "valid token request" describe "missing assertion_type" do From 9595292ea0439e079bd934bb99d5a890bc857d71 Mon Sep 17 00:00:00 2001 From: Matteo Centenaro Date: Mon, 25 Feb 2013 16:23:54 +0100 Subject: [PATCH 3/3] client_secret is optional for a password grant_type --- lib/oauth2/provider/exchange.rb | 2 +- spec/oauth2/provider/exchange_spec.rb | 63 ++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/lib/oauth2/provider/exchange.rb b/lib/oauth2/provider/exchange.rb index ee048ea3..60a96ed8 100644 --- a/lib/oauth2/provider/exchange.rb +++ b/lib/oauth2/provider/exchange.rb @@ -101,7 +101,7 @@ def validate! def validate_required_params REQUIRED_PARAMS.each do |param| next if @params.has_key?(param) - next if param == CLIENT_SECRET && @grant_type == ASSERTION + next if param == CLIENT_SECRET && ([ASSERTION, PASSWORD].include?(@grant_type)) @error = INVALID_REQUEST @error_description = "Missing required parameter #{param}" end diff --git a/spec/oauth2/provider/exchange_spec.rb b/spec/oauth2/provider/exchange_spec.rb index b8c461b2..b7b10f44 100644 --- a/spec/oauth2/provider/exchange_spec.rb +++ b/spec/oauth2/provider/exchange_spec.rb @@ -189,7 +189,68 @@ end end - it_should_behave_like "validates required parameters" + describe "missing grant_type" do + before { params.delete('client_id') } + + it "is invalid" do + exchange.error.should == "invalid_request" + exchange.error_description.should == "Missing required parameter client_id" + end + end + + describe "with an unknown grant type" do + before { params['grant_type'] = 'unknown' } + + it "is invalid" do + exchange.error.should == "unsupported_grant_type" + exchange.error_description.should == "The grant type unknown is not recognized" + end + end + + describe "missing client_id" do + before { params.delete('client_id') } + + it "is invalid" do + exchange.error.should == "invalid_request" + exchange.error_description.should == "Missing required parameter client_id" + end + end + + describe "with an unknown client_id" do + before { params['client_id'] = "unknown" } + + it "is invalid" do + exchange.error.should == "invalid_client" + exchange.error_description.should == "Unknown client ID unknown" + end + end + + describe "with a mismatched client_secret" do + before { params['client_secret'] = "nosoupforyou" } + + it "is invalid" do + exchange.error.should == "invalid_client" + exchange.error_description.should == "Parameter client_secret does not match" + end + end + + describe "with lesser scope than the authorization code represents" do + before { params['scope'] = 'bar' } + + it "is valid" do + exchange.error.should be_nil + end + end + + describe "with scopes not covered by the authorization code" do + before { params['scope'] = 'qux' } + + it "is invalid" do + exchange.error.should == 'invalid_scope' + exchange.error_description.should == 'The request scope was never granted by the user' + end + end + it_should_behave_like "valid token request" describe "missing username" do