Skip to content

Test OIDC login controller#9923

Merged
dawei-nava merged 13 commits intomainfrom
dwang/test_oidc
Jan 18, 2024
Merged

Test OIDC login controller#9923
dawei-nava merged 13 commits intomainfrom
dwang/test_oidc

Conversation

@dawei-nava
Copy link
Contributor

@dawei-nava dawei-nava commented Jan 16, 2024

🛠 Summary of changes

Test OIDC controller to facilitate testing selfie features(enable/disable) on desktop or mobile platform.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Step 1: Run rake db:seed to update SP database entries, enable selfie feature
doc_auth_selfie_capture_enabled: true

And start https

  • Step 2: Visit https://root_url/test/oidc/login from mobile device, choose biometric link
  • Step 3: Verify doc capture screen selfie section is shown

- 'sp_sinatra_demo'
friendly_name: 'Example Test OIDC SP'
in_person_proofing_enabled: true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I add this or updating an existing one?

end

def openid_configuration
@openid_configuration ||= OpenidConnectConfigurationPresenter.new.configuration
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is can be fetched at {root_url}/.well-known/openid-configuration, but due to Puma dev mode and RAILS_MAX_THREAD configuration(with value of 1), making a loop back request to the same server will cause deadlock.

if @service_provider&.redirect_uris && !@service_provider.redirect_uris.include?(url)
@service_provider.redirect_uris.append(url)
changed = true
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Developer machines can have various ips, add it dynamically so no manual intervention is needed.

@night-jellyfish
Copy link
Contributor

night-jellyfish commented Jan 17, 2024

I tested this out locally on both mobile and desktop and was able to choose the selfie flow, and upload an image (desktop) or use the SDK (mobile)!

For transparency, I did not test the other links, I only tried the biometric option.

I did not finish looking through all the code changes but plan to take another look tomorrow.

@dawei-nava dawei-nava marked this pull request as ready for review January 17, 2024 15:31
Copy link
Contributor

@amirbey amirbey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍🏿

Ideally, I would prefer to add add https/ssl support to the oidc-sinatra app to avoid duplicating oidc-sinatra-app code in the IdP but from briefly poking around i didn't see a quick solution 🤔 . Ultimately, this will definitely make local development on mobile easier. Thanks Dawei 🙌🏿

@dawei-nava dawei-nava merged commit 3f2f1c1 into main Jan 18, 2024
@dawei-nava dawei-nava deleted the dwang/test_oidc branch January 18, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants