From 6f3106831cbd658670441794f60388bddcf7f07c Mon Sep 17 00:00:00 2001 From: Alisa Wallace Date: Wed, 11 Dec 2024 16:44:00 -0800 Subject: [PATCH 1/5] Add scope and aud inputs for authorization redirect request --- .../authorization_code_redirect_test.rb | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/udap_security_test_kit/authorization_code_redirect_test.rb b/lib/udap_security_test_kit/authorization_code_redirect_test.rb index c6541f5..9ec2519 100644 --- a/lib/udap_security_test_kit/authorization_code_redirect_test.rb +++ b/lib/udap_security_test_kit/authorization_code_redirect_test.rb @@ -17,6 +17,23 @@ class AuthorizationCodeRedirectTest < Inferno::Test title: 'Client ID', description: 'Client ID as registered with the authorization server.' + input :udap_authorization_code_request_scopes, + title: 'Scope Parameter for Authorization Request', + description: %( + A list of space-separated scopes to include in the authorization request. If included, these may be equal + to or a subset of the scopes requested during registration. + If empty, scope will be omitted as a parameter to the authorization endpoint. + ), + optional: true + + input :udap_authorization_code_request_aud, + title: "Audience ('aud') Parameter for Authorization Request", + description: %( + If included, should be the same as the base FHIR URL. If empty, 'aud' parameter will be omitted as + a parameter to the authorization endpoint. + ), + optional: true + output :udap_authorization_code_state receives_request :redirect @@ -59,7 +76,9 @@ def authorization_url_builder(url, params) 'response_type' => 'code', 'client_id' => udap_client_id, 'redirect_uri' => config.options[:redirect_uri], - 'state' => udap_authorization_code_state + 'state' => udap_authorization_code_state, + 'scope' => udap_authorization_request_requested_scopes, + 'aud' => udap_authorization_code_request_aud }.compact authorization_url = authorization_url_builder( From 538b62692574f9d67c6f98b4c66218732b6833a3 Mon Sep 17 00:00:00 2001 From: Alisa Wallace Date: Thu, 12 Dec 2024 15:05:44 -0800 Subject: [PATCH 2/5] Add authorization URL as test output to support unit testing --- .../authorization_code_redirect_test.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/udap_security_test_kit/authorization_code_redirect_test.rb b/lib/udap_security_test_kit/authorization_code_redirect_test.rb index 9ec2519..cdd57dd 100644 --- a/lib/udap_security_test_kit/authorization_code_redirect_test.rb +++ b/lib/udap_security_test_kit/authorization_code_redirect_test.rb @@ -35,6 +35,7 @@ class AuthorizationCodeRedirectTest < Inferno::Test optional: true output :udap_authorization_code_state + output :udap_authorization_redirect_url receives_request :redirect @@ -77,7 +78,7 @@ def authorization_url_builder(url, params) 'client_id' => udap_client_id, 'redirect_uri' => config.options[:redirect_uri], 'state' => udap_authorization_code_state, - 'scope' => udap_authorization_request_requested_scopes, + 'scope' => udap_authorization_code_request_scopes, 'aud' => udap_authorization_code_request_aud }.compact @@ -88,6 +89,8 @@ def authorization_url_builder(url, params) info("Inferno redirecting browser to #{authorization_url}.") + output udap_authorization_redirect_url: authorization_url + wait( identifier: udap_authorization_code_state, message: wait_message(authorization_url) From 4ab8c60b20e933b678dc1a2cebf962169e0ccec7 Mon Sep 17 00:00:00 2001 From: Alisa Wallace Date: Thu, 12 Dec 2024 15:06:07 -0800 Subject: [PATCH 3/5] Update auth code redirect test spec to test for scope and aud param inputs --- .../authorization_code_redirect_test_spec.rb | 123 ++++++++++++------ 1 file changed, 86 insertions(+), 37 deletions(-) diff --git a/spec/udap_security_test_kit/authorization_code_redirect_test_spec.rb b/spec/udap_security_test_kit/authorization_code_redirect_test_spec.rb index 0ebf000..2ac49a0 100644 --- a/spec/udap_security_test_kit/authorization_code_redirect_test_spec.rb +++ b/spec/udap_security_test_kit/authorization_code_redirect_test_spec.rb @@ -15,6 +15,13 @@ } end + let(:inputs_scope_aud) do + { + udap_authorization_code_request_scopes: 'patient/Condition.rs', + udap_authorization_code_request_aud: 'http://example.com' + } + end + def run(runnable, inputs = {}) test_run_params = { test_session_id: test_session.id }.merge(runnable.reference_hash) test_run = Inferno::Repositories::TestRuns.new.create(test_run_params) @@ -31,54 +38,96 @@ def run(runnable, inputs = {}) Inferno::TestRunner.new(test_session:, test_run:).run(runnable) end - it 'waits and then passes when it receives a request with the correct state' do - allow(test).to receive(:parent).and_return(Inferno::TestGroup) - result = run(test, inputs) - expect(result.result).to eq('wait') + context "when optional 'scope' and 'aud' inputs are omitted" do + it 'waits and then passes when it receives a request with the correct state' do + allow(test).to receive(:parent).and_return(Inferno::TestGroup) + result = run(test, inputs) + expect(result.result).to eq('wait') - state = session_data_repo.load(test_session_id: test_session.id, name: 'udap_authorization_code_state') - get "/custom/udap_security/redirect?state=#{state}" + state = session_data_repo.load(test_session_id: test_session.id, name: 'udap_authorization_code_state') + get "/custom/udap_security/redirect?state=#{state}" - result = results_repo.find(result.id) - expect(result.result).to eq('pass') - end + result = results_repo.find(result.id) + expect(result.result).to eq('pass') + end - it 'continues to wait when it receives a request with the incorrect state' do - result = run(test, inputs) - expect(result.result).to eq('wait') + it 'continues to wait when it receives a request with the incorrect state' do + result = run(test, inputs) + expect(result.result).to eq('wait') - state = SecureRandom.uuid - get "/custom/smart/redirect?state=#{state}" + state = SecureRandom.uuid + get "/custom/smart/redirect?state=#{state}" - result = results_repo.find(result.id) - expect(result.result).to eq('wait') - end + result = results_repo.find(result.id) + expect(result.result).to eq('wait') + end - it 'fails if the authorization url is invalid' do - inputs[:udap_authorization_endpoint] = 'invalid' - result = run(test, inputs) - expect(result.result).to eq('fail') - expect(result.result_message).to match(/is not a valid URI/) - end + it 'fails if the authorization url is invalid' do + inputs[:udap_authorization_endpoint] = 'invalid' + result = run(test, inputs) + expect(result.result).to eq('fail') + expect(result.result_message).to match(/is not a valid URI/) + end + + it "persists the incoming 'redirect' request" do + allow(test).to receive(:parent).and_return(Inferno::TestGroup) + run(test, inputs) + state = session_data_repo.load(test_session_id: test_session.id, name: 'udap_authorization_code_state') + url = "/custom/udap_security/redirect?state=#{state}" + get url + + request = requests_repo.find_named_request(test_session.id, 'redirect') + expect(request.url).to end_with(url) + end + + it "persists the 'udap_authorization_code_state' output" do + result = run(test, inputs) + expect(result.result).to eq('wait') - it "persists the incoming 'redirect' request" do - allow(test).to receive(:parent).and_return(Inferno::TestGroup) - run(test, inputs) - state = session_data_repo.load(test_session_id: test_session.id, name: 'udap_authorization_code_state') - url = "/custom/udap_security/redirect?state=#{state}" - get url + state = result.result_message.match(/a state of `(.*)`/)[1] + persisted_state = session_data_repo.load(test_session_id: test_session.id, name: 'udap_authorization_code_state') - request = requests_repo.find_named_request(test_session.id, 'redirect') - expect(request.url).to end_with(url) + expect(persisted_state).to eq(state) + end + + it "omits 'scope' and 'aud' values from authorization redirect url" do + result = run(test, inputs) + expect(result.result).to eq('wait') + + authorization_url = session_data_repo.load( + test_session_id: test_session.id, + name: 'udap_authorization_redirect_url' + ) + + expect(authorization_url).to_not include('scope') + expect(authorization_url).to_not include('aud') + end end - it "persists the 'udap_authorization_code_state' output" do - result = run(test, inputs) - expect(result.result).to eq('wait') + context "when optional 'scope' and 'aud' inputs are provided" do + it 'waits and then passes when it receives a request with the correct state' do + allow(test).to receive(:parent).and_return(Inferno::TestGroup) + result = run(test, inputs.merge(inputs_scope_aud)) + expect(result.result).to eq('wait') - state = result.result_message.match(/a state of `(.*)`/)[1] - persisted_state = session_data_repo.load(test_session_id: test_session.id, name: 'udap_authorization_code_state') + state = session_data_repo.load(test_session_id: test_session.id, name: 'udap_authorization_code_state') + get "/custom/udap_security/redirect?state=#{state}" - expect(persisted_state).to eq(state) + result = results_repo.find(result.id) + expect(result.result).to eq('pass') + end + + it "includes 'scope' and 'aud' values in authorization redirect url" do + result = run(test, inputs.merge(inputs_scope_aud)) + expect(result.result).to eq('wait') + + authorization_url = session_data_repo.load( + test_session_id: test_session.id, + name: 'udap_authorization_redirect_url' + ) + + expect(authorization_url).to include('scope') + expect(authorization_url).to include('aud') + end end end From 9e1ca1e078b118f0bbd726983f582f2077a893ea Mon Sep 17 00:00:00 2001 From: Alisa Wallace Date: Wed, 18 Dec 2024 13:30:09 -0800 Subject: [PATCH 4/5] Update aud input to be base url + checkbox --- .../authorization_code_redirect_test.rb | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/udap_security_test_kit/authorization_code_redirect_test.rb b/lib/udap_security_test_kit/authorization_code_redirect_test.rb index cdd57dd..aea1a15 100644 --- a/lib/udap_security_test_kit/authorization_code_redirect_test.rb +++ b/lib/udap_security_test_kit/authorization_code_redirect_test.rb @@ -9,6 +9,10 @@ class AuthorizationCodeRedirectTest < Inferno::Test the provided client redirection URI using an HTTP redirection response. ) + input :udap_fhir_base_url, + title: 'FHIR Server Base URL', + description: 'Base FHIR URL of FHIR Server.' + input :udap_authorization_endpoint, title: 'Authorization Endpoint', description: 'The full URL from which Inferno will request an authorization code.' @@ -28,9 +32,18 @@ class AuthorizationCodeRedirectTest < Inferno::Test input :udap_authorization_code_request_aud, title: "Audience ('aud') Parameter for Authorization Request", + type: 'checkbox', + options: { + list_options: [ + { + label: "Include 'aud' parameter", + value: 'include' + } + ] + }, description: %( - If included, should be the same as the base FHIR URL. If empty, 'aud' parameter will be omitted as - a parameter to the authorization endpoint. + If selected, the Base FHIR URL will be used as the 'aud' parameter in the request to the authorization + endpoint. ), optional: true From 37bc399a18ea92e94500aab0caac672679f3b2c5 Mon Sep 17 00:00:00 2001 From: Alisa Wallace Date: Wed, 18 Dec 2024 13:43:50 -0800 Subject: [PATCH 5/5] Update test and spec implementations for checkbox input --- .../authorization_code_redirect_test.rb | 6 ++++-- .../authorization_code_redirect_test_spec.rb | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/udap_security_test_kit/authorization_code_redirect_test.rb b/lib/udap_security_test_kit/authorization_code_redirect_test.rb index aea1a15..72502d0 100644 --- a/lib/udap_security_test_kit/authorization_code_redirect_test.rb +++ b/lib/udap_security_test_kit/authorization_code_redirect_test.rb @@ -37,7 +37,7 @@ class AuthorizationCodeRedirectTest < Inferno::Test list_options: [ { label: "Include 'aud' parameter", - value: 'include' + value: 'include_aud' } ] }, @@ -86,13 +86,15 @@ def authorization_url_builder(url, params) output udap_authorization_code_state: SecureRandom.uuid + aud = udap_fhir_base_url if udap_authorization_code_request_aud.include? 'include_aud' + oauth2_params = { 'response_type' => 'code', 'client_id' => udap_client_id, 'redirect_uri' => config.options[:redirect_uri], 'state' => udap_authorization_code_state, 'scope' => udap_authorization_code_request_scopes, - 'aud' => udap_authorization_code_request_aud + 'aud' => aud }.compact authorization_url = authorization_url_builder( diff --git a/spec/udap_security_test_kit/authorization_code_redirect_test_spec.rb b/spec/udap_security_test_kit/authorization_code_redirect_test_spec.rb index 2ac49a0..305165a 100644 --- a/spec/udap_security_test_kit/authorization_code_redirect_test_spec.rb +++ b/spec/udap_security_test_kit/authorization_code_redirect_test_spec.rb @@ -10,6 +10,7 @@ let(:url) { 'http://example.com/fhir' } let(:inputs) do { + udap_fhir_base_url: url, udap_authorization_endpoint: 'http://example.com/authorize', udap_client_id: 'CLIENT_ID' } @@ -18,7 +19,7 @@ let(:inputs_scope_aud) do { udap_authorization_code_request_scopes: 'patient/Condition.rs', - udap_authorization_code_request_aud: 'http://example.com' + udap_authorization_code_request_aud: ['include_aud'] } end