Allow OmniAuth Setup Phase to be configured #76
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test coverage: 100.0% |
chrisroos
force-pushed
the
toggle-openid-connect-scope-at-runtime
branch
2 times, most recently
from
7adbcf8 to
a8286a8
Compare
chrisroos
changed the title
chrisroos
force-pushed
the
toggle-openid-connect-scope-at-runtime
branch
2 times, most recently
from
5d40807 to
e1ec493
Compare
chrisroos
force-pushed
the
toggle-openid-connect-scope-at-runtime
branch
from
e1ec493 to
92eff2d
Compare
Contributor
|
This looks good to me - I especially appreciate the tests that you've added which make things pretty clear! 🎉 |
Contributor
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. |
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.
floehopper
force-pushed
the
toggle-openid-connect-scope-at-runtime
branch
from
92eff2d to
dd0e884
Compare
Contributor
Addressed in 4d106e8. Marking as ready for review again. |
chrisroos
commented
Jun 4, 2025
Contributor
Author
chrisroos
left a comment
chrisroos
left a comment
There was a problem hiding this comment.
This looks good to me but I don't appear to have the relevant permission to Approve the PR unfortunately.
chrislo
approved these changes
Jun 4, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
studentscope to allow students to login.The Profile app uses the presence of the
studentscope 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 thescopeaccordingly. By default thescopeis fixed at Rails initialization time in theRpiAuth::Enginebut 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 thescopebased 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
studentscope if a specific value was set in the session but later decided that it should be up to the consuming apps to use thissetupphase as they see fit.