Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Check user access to service on sign in #333

Merged
merged 5 commits into from
Sep 13, 2023
Merged

Check user access to service on sign in #333

merged 5 commits into from
Sep 13, 2023

Conversation

steventux
Copy link
Contributor

@steventux steventux commented Aug 31, 2023 •
edited
Loading

Context

We would like to check user roles as an authorisation step on the check-records part of the service.
The DfE Signin API exposes an endpoint which replies with user roles for a given user, service and organisation.

Changes

  • Call the DfE Sign in API get user access to service endpoint as part of the DSI login journey.
  • Authorise user based on presence of valid role for service, valid role codes are stored in ENV["DFE_SIGN_IN_API_ROLE_CODES"]
  • Create a DsiUserSession record on each successful login (except when bypass is active). This record contains the role and org info the user authenticated with.

Guidance to review

Local overrides are necessary to make this PR work. Specifically the env vars

DFE_SIGN_IN_API_BASE_URL
DFE_SIGN_IN_API_SECRET
DFE_SIGN_IN_API_AUDIENCE
DFE_SIGN_IN_API_ROLE_CODES

I have edited the review app keyvault with working values.

Link to Trello card

https://trello.com/c/mTFXWXo7/166-dfe-signin-role-spike
https://trello.com/c/7vJeIbez/185-store-more-things-about-the-user

Checklist

  • Attach to Trello card
  • Rebased main
  • Cleaned commit history
  • Tested by running locally

@steventux steventux force-pushed the 166-dfe-signin-role-spike branch 3 times, most recently from 1a7ec5c to 4ad691f Compare September 4, 2023 10:35
@steventux
Copy link
Contributor Author

DfE Signin 'role' spike

@steventux steventux temporarily deployed to review September 5, 2023 08:40 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Sep 5, 2023

Review app deployed to https://s165d01-aytq-review-pr-333-app.azurewebsites.net

@steventux steventux force-pushed the 166-dfe-signin-role-spike branch from 4ad691f to 53432b4 Compare September 5, 2023 09:52
@steventux steventux temporarily deployed to review September 5, 2023 09:54 — with GitHub Actions Inactive
@steventux steventux force-pushed the 166-dfe-signin-role-spike branch from 53432b4 to c44a262 Compare September 5, 2023 10:16
@steventux steventux temporarily deployed to review September 5, 2023 10:18 — with GitHub Actions Inactive
@steventux steventux force-pushed the 166-dfe-signin-role-spike branch from c44a262 to 2b4dc65 Compare September 5, 2023 12:06
@steventux steventux temporarily deployed to review September 5, 2023 12:08 — with GitHub Actions Inactive
@steventux steventux changed the title Check user access to service on sign in callback Check user access to service on sign in Sep 7, 2023
@steventux steventux force-pushed the 166-dfe-signin-role-spike branch from 2b4dc65 to 6a31cbb Compare September 7, 2023 15:11
@steventux steventux temporarily deployed to review September 7, 2023 15:13 — with GitHub Actions Inactive
@steventux steventux temporarily deployed to review September 7, 2023 15:17 — with GitHub Actions Inactive
@steventux steventux marked this pull request as ready for review September 7, 2023 15:31
@steventux
Copy link
Contributor Author

Store more things about the user

@steventux steventux temporarily deployed to review September 11, 2023 10:51 — with GitHub Actions Inactive
@steventux
Add jwt gem needed for DSI API calls
6680790
@steventux
Add dsi_user_sessions
48b6cca 
This provides a way to record which role and organisation a user has authenticated with.
@steventux
Add a service to call the DSI API for user roles
f5a352f 
This DSI API endpoint will respond with roles belonging to the user.
@steventux
Create a DsiUserSession record when creating or updating the user
a5a1795 
We only do this if supplied an optional role. Bypass mechanisisms will still function without a role.
@steventux
Check the role info from DSI API when user logs in
aa4b2b7 
We don't do this if bypassing DSI.
The first role to match the list of authorised roles will be recorded in the DsiUserSession along with org info.
The absence of a valid role takes the user to the 'Not authorised' page.
@steventux steventux force-pushed the 166-dfe-signin-role-spike branch from 10556a6 to aa4b2b7 Compare September 11, 2023 14:02
@steventux steventux temporarily deployed to review September 11, 2023 14:04 — with GitHub Actions Inactive
malcolmbaig
malcolmbaig approved these changes Sep 12, 2023
View reviewed changes
Copy link
Contributor

@malcolmbaig malcolmbaig left a comment

Choose a reason for hiding this comment

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

LGTM

@steventux steventux temporarily deployed to preprod September 13, 2023 10:32 — with GitHub Actions Inactive
@steventux steventux merged commit edcf6bc into main Sep 13, 2023
18 checks passed
@steventux steventux deleted the 166-dfe-signin-role-spike branch September 13, 2023 14:51
@steventux steventux temporarily deployed to review September 13, 2023 14:52 — with GitHub Actions Inactive
@malcolmbaig malcolmbaig mentioned this pull request Sep 19, 2023
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@malcolmbaig malcolmbaig malcolmbaig approved these changes

@gpeng gpeng gpeng approved these changes

@felixclack felixclack Awaiting requested review from felixclack

@AbigailMcP AbigailMcP Awaiting requested review from AbigailMcP

Assignees
No one assigned
Labels
deploy
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@steventux @gpeng @malcolmbaig