Skip to content

Allow OmniAuth Setup Phase to be configured #76

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chrisroos
Copy link

@chrisroos chrisroos commented Apr 10, 2025

The OmniAuth Setup Phase allows for "request-time modification of an OmniAuth strategy". We're intending to use this in Experience CS so that we can optionally include the student scope to allow students to login.

The Profile app uses the presence of the student scope in the login request to decide whether to display the student login form (i.e. the form containing the school code textbox). We want to allow students (as well as non-students) to login to Experience CS and so need a way of changing the scope accordingly. By default the scope is fixed at Rails initialization time in the RpiAuth::Engine but we need to be able to toggle it at runtime depending on whether user is a student or not. By utilising OmniAuth's setup phase we can toggle the scope based on something we set in the app (e.g. we might set something in the session to indicate that we want the student login flow).

I initially added logic in this Gem to add the student scope if a specific value was set in the session but later decided that it should be up to the consuming apps to use this setup phase as they see fit.

Copy link

github-actions bot commented Apr 10, 2025

Test coverage: 100.0%

@chrisroos chrisroos force-pushed the toggle-openid-connect-scope-at-runtime branch 2 times, most recently from 7adbcf8 to a8286a8 Compare May 21, 2025 15:53
@chrisroos chrisroos changed the title WIP: Toggle openid connect scope at runtime Allow OmniAuth Setup Phase to be configured May 21, 2025
@chrisroos chrisroos force-pushed the toggle-openid-connect-scope-at-runtime branch 2 times, most recently from 5d40807 to e1ec493 Compare May 21, 2025 16:01
@chrisroos chrisroos marked this pull request as ready for review May 21, 2025 16:02
chrisroos added 2 commits May 21, 2025 17:04
The OmniAuth Setup Phase[1] allows for "request-time modification of an
OmniAuth strategy". We're intending to use this in Experience CS so that
we can optionally include the `student` scope[2] to allow students to
login.

The Profile app uses the presence of the `student` scope in the login
request to decide whether to display the student login form (i.e. the
form containing the school code textbox). We want to allow students (as
well as non-students) to login to Experience CS and so need a way of
changing the `scope` accordingly. By default the `scope` is fixed at
Rails initialization time in the `RpiAuth::Engine`[3] but we need to be
able to toggle it at runtime depending on whether user is a student or
not. By utilising OmniAuth's setup phase we can toggle the `scope` based
on something we set in the app (e.g. we might set something in the
session to indicate that we want the student login flow).

I initially added logic in this Gem to add the `student` scope if a
specific value was set in the session but later decided that it should
be up to the consuming apps to use this `setup` phase as they see fit.

[1]: https://github.com/omniauth/omniauth/wiki/Setup-Phase
[2]: https://github.com/RaspberryPiFoundation/documentation/blob/a92f03446a347d2d1acea48c50ea37f15293a9b6/docs/technology/codebases-and-products/accounts/profile-app/custom-oidc-scopes.md#available-custom-scopes
[3]: https://github.com/RaspberryPiFoundation/rpi-auth/blob/b7771ee120254e4c6f2d90c5025be5714daeb542/lib/rpi_auth/engine.rb#L31
So that I can avoid disabling the Metrics/BlockLength RuboCop rule.
@chrisroos chrisroos force-pushed the toggle-openid-connect-scope-at-runtime branch from e1ec493 to 92eff2d Compare May 21, 2025 16:05
@floehopper
Copy link
Contributor

This looks good to me - I especially appreciate the tests that you've added which make things pretty clear! 🎉

@chrisroos chrisroos marked this pull request as draft May 29, 2025 13:08
@chrisroos
Copy link
Author

I've marked this as draft again so that I can fix the same problem that @sebjacobs fixed in https://github.com/RaspberryPiFoundation/experience-cs/pull/612.

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.

2 participants