FI 3093: Transition to use auth info (WIP) #84

vanessuniq · 2024-12-03T15:47:08Z

Summary

This branch fully transitions all versions of the SMART App Launch (STU1, STU2, and STU2.2) to use the auth_info feature for collecting authentication credentials during tests, replacing the use of oauth_credentials.

Testing Instructions

Run the tests in the browser to verify the correct display of inputs and ensure appropriate test behavior.
Execute the unit tests to confirm all tests pass successfully.

Todo:

Point the inferno_core gem to its repository's main branch (required to customize the auth_info authentication type options) and test the auth_type customization.
Update the presets to align with this implementation.
When a new release is available, bump the inferno_core version.

…s mode Signed-off-by: Vanessa Fotso <[email protected]>

Signed-off-by: Vanessa Fotso <[email protected]>

Jammjammjamm

I really don't like all of the linting changes in this branch. The real changes in this PR are already large, and all the linting fixes make it harder to focus on what matters. This branch is also likely to be open for some time, so there's a lot of potential for conflicts due to these changes which are unrelated to the substance of this PR.

The token url is still defined as a separate input in a lot of places. Is this due to an http client relying on it? If so, I think it's worth just ditching those http clients which are doing very little and making requests using the full url.

Jammjammjamm · 2024-12-09T14:23:29Z

lib/smart_app_launch/app_redirect_test.rb

-          title: 'Proof Key for Code Exchange (PKCE)',
-          type: 'radio',
-          default: 'false',
+    input :url, :smart_authorization_url


The authorization url is one of the auth_info fields.

Jammjammjamm · 2024-12-09T14:28:43Z

lib/smart_app_launch/backend_services_authorization_group.rb

+              }
+            ]
+          }
+
    input :smart_token_url,


This is an auth_info field.

Jammjammjamm · 2024-12-09T14:29:12Z

lib/smart_app_launch/backend_services_authorization_request_success_test.rb

-          :smart_token_url
-    input :backend_services_jwks_kid,
-          optional: true
+    input :smart_token_url


This is an auth_info field.

Jammjammjamm · 2024-12-09T14:29:45Z

lib/smart_app_launch/backend_services_authorization_response_body_test.rb

@@ -30,11 +30,11 @@ class BackendServicesAuthorizationResponseBodyTest < Inferno::Test

      output bearer_token: access_token

-      required_keys = ['token_type', 'expires_in', 'scope']
+      required_keys = %w[token_type expires_in scope]


Let's not use the %whatever array forms.

Jammjammjamm · 2024-12-09T14:30:48Z

lib/smart_app_launch/backend_services_invalid_client_assertion_test.rb

-          :smart_token_url
-    input :backend_services_jwks_kid, 
-          optional: true
+    input :smart_token_url


This is an auth_info field.

Jammjammjamm · 2024-12-09T14:31:54Z

lib/smart_app_launch/backend_services_invalid_grant_type_test.rb

-          :smart_token_url
-    input :backend_services_jwks_kid,
-          optional: true
+    input :smart_token_url


This is an auth_info field.

Jammjammjamm · 2024-12-09T14:32:06Z

lib/smart_app_launch/backend_services_invalid_jwt_test.rb

-          :smart_token_url
-    input :backend_services_jwks_kid,
-          optional: true
+    input :smart_token_url


This is an auth_info field.

Jammjammjamm · 2024-12-09T14:33:07Z

lib/smart_app_launch/client_assertion_builder.rb

    end

    def jwt_payload
      { iss:, sub:, aud:, exp:, jti: }.compact
    end

    def signing_key
-      private_key()
+      private_key


Get rid of this and just make the next line if private_key.nil?.

Jammjammjamm · 2024-12-09T14:37:42Z

lib/smart_app_launch/token_exchange_test.rb

-              }
-            ]
-          }
+    input :code, :smart_token_url


Token url is an auth_info field.

Jammjammjamm · 2024-12-09T14:40:56Z

lib/smart_app_launch/token_refresh_test.rb

@@ -16,17 +16,18 @@ class TokenRefreshTest < Inferno::Test
      the Pragma response header field with a value of no-cache to be
      consistent with the requirements of the inital access token exchange.
    )
-    input :smart_token_url, :refresh_token, :client_id, :received_scopes
-    input :client_secret, optional: true
+    input :smart_token_url, :refresh_token, :received_scopes


Token url and refresh token are auth_info fields. Received scopes isn't, but maybe it should be?

vanessuniq · 2024-12-09T21:07:21Z

I really don't like all of the linting changes in this branch. The real changes in this PR are already large, and all the linting fixes make it harder to focus on what matters. This branch is also likely to be open for some time, so there's a lot of potential for conflicts due to these changes which are unrelated to the substance of this PR.

The token url is still defined as a separate input in a lot of places. Is this due to an http client relying on it? If so, I think it's worth just ditching those http clients which are doing very little and making requests using the full url.

I'll disable the auto-lint from my editor. Regarding the other auth_info fields (token url, authorization url, etc.), these are outputted in previous tests (discovery). I'm not sure how to pass their values to the subsequent tests that require them without defining them explicitly as inputs.

Jammjammjamm · 2024-12-10T12:51:27Z

Regarding the other auth_info fields (token url, authorization url, etc.), these are outputted in previous tests (discovery). I'm not sure how to pass their values to the subsequent tests that require them without defining them explicitly as inputs.

Aren't these tests also receiving an auth_info input, so the value could just be extracted from there?

Signed-off-by: Vanessa Fotso <[email protected]>

Jammjammjamm

Once the new core release is out, bump the core requirement in the gemspec to the new version, and update the version of this test kit to 0.5.0.dev.

Jammjammjamm · 2024-12-17T17:36:27Z

lib/smart_app_launch/token_introspection_request_group.rb

-      input :standalone_access_token,
-            title: 'Access Token',
-            description: 'The access token to be introspected. MUST be active.'
+      input :standalone_smart_auth_info, type: :auth_info, options: { mode: 'auth' }


This should be access mode.

Jammjammjamm · 2024-12-17T18:25:32Z

lib/smart_app_launch/backend_services_authorization_group.rb

+            components: [
+              {
+                name: :auth_type,
+                type: 'select',


You shouldn't have to specify the type for these, right?

Jammjammjamm · 2024-12-17T18:28:31Z

lib/smart_app_launch/openid_token_payload_test.rb

@@ -16,22 +16,20 @@ class OpenIDTokenPayloadTest < Inferno::Test
      - `sub` must be a non-blank string not exceeding 255 characters in length
    )

-    REQUIRED_CLAIMS = ['iss', 'sub', 'aud', 'exp', 'iat'].freeze
+    REQUIRED_CLAIMS = %w[iss sub aud exp iat].freeze


Please revert this change.

Signed-off-by: Vanessa Fotso <[email protected]>

vanessuniq added 3 commits November 13, 2024 08:29

update to use authinfo instead of oauth_credentials in stu1 for acces…

2dcd49b

…s mode Signed-off-by: Vanessa Fotso <[email protected]>

Add authinfo feature flag

7bd36d2

Signed-off-by: Vanessa Fotso <[email protected]>

Updated smart stu1 to use auth_info feature

bed180d

Signed-off-by: Vanessa Fotso <[email protected]>

vanessuniq requested a review from Jammjammjamm December 3, 2024 16:19

vanessuniq added 4 commits December 4, 2024 00:04

Updated spec

1554742

Signed-off-by: Vanessa Fotso <[email protected]>

removed auth_info feature flag and updated stu1 spec

3ae35d9

Signed-off-by: Vanessa Fotso <[email protected]>

Updated stu2 to use auth_info

5e898bf

Signed-off-by: Vanessa Fotso <[email protected]>

Updated stu2.2 to use auth_info

70e3a86

Signed-off-by: Vanessa Fotso <[email protected]>

vanessuniq marked this pull request as ready for review December 6, 2024 13:45

Jammjammjamm reviewed Dec 9, 2024

View reviewed changes

vanessuniq added 5 commits December 15, 2024 21:06

stu1 update, extracting additional auth_info fields

b3cbc8f

Signed-off-by: Vanessa Fotso <[email protected]>

stu2 update, extracting additional auth_info fields

d9b8e28

Signed-off-by: Vanessa Fotso <[email protected]>

stu2.2 update, extracting additional auth_info fields

86090ae

Signed-off-by: Vanessa Fotso <[email protected]>

STU1: moved auth_info config to well known enpoint test

29dab0d

Signed-off-by: Vanessa Fotso <[email protected]>

updated presets

a408ca2

Signed-off-by: Vanessa Fotso <[email protected]>

vanessuniq requested a review from Jammjammjamm December 17, 2024 16:09

refactored #signing_key

b4dc556

Signed-off-by: Vanessa Fotso <[email protected]>

Jammjammjamm reviewed Dec 17, 2024

View reviewed changes

vanessuniq added 4 commits December 18, 2024 13:45

minor refactor, addressing review comments

8fe6645

Signed-off-by: Vanessa Fotso <[email protected]>

removed type field from the auth_info auth_type component

91fae18

Signed-off-by: Vanessa Fotso <[email protected]>

bump kit and core version

eaecea1

Signed-off-by: Vanessa Fotso <[email protected]>

bump core version

a5de2fa

Signed-off-by: Vanessa Fotso <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FI 3093: Transition to use auth info (WIP) #84

FI 3093: Transition to use auth info (WIP) #84

vanessuniq commented Dec 3, 2024 •

edited

Loading

Jammjammjamm left a comment

Jammjammjamm Dec 9, 2024

Jammjammjamm Dec 9, 2024

Jammjammjamm Dec 9, 2024

Jammjammjamm Dec 9, 2024

Jammjammjamm Dec 9, 2024

Jammjammjamm Dec 9, 2024

Jammjammjamm Dec 9, 2024

Jammjammjamm Dec 9, 2024

Jammjammjamm Dec 9, 2024

Jammjammjamm Dec 9, 2024

vanessuniq commented Dec 9, 2024

Jammjammjamm commented Dec 10, 2024

Jammjammjamm left a comment

Jammjammjamm Dec 17, 2024

Jammjammjamm Dec 17, 2024

Jammjammjamm Dec 17, 2024

FI 3093: Transition to use auth info (WIP) #84

Are you sure you want to change the base?

FI 3093: Transition to use auth info (WIP) #84

Conversation

vanessuniq commented Dec 3, 2024 • edited Loading

Summary

Testing Instructions

Jammjammjamm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanessuniq commented Dec 9, 2024

Jammjammjamm commented Dec 10, 2024

Jammjammjamm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanessuniq commented Dec 3, 2024 •

edited

Loading