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.

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

Update create user to expect users from Entra #1429

Merged
merged 8 commits into from
Jan 13, 2025
Merged

Conversation

michaeljcollinsuk
Copy link
Contributor

@michaeljcollinsuk michaeljcollinsuk commented Jan 9, 2025

This PR resolves relates to ministryofjustice/analytical-platform#6237

To enable CICA users to access the CP, we will enable a new connection in Auth0 with EntraID.
This updates the code we use to create a user from the ID token to normalize their name, and store their email as the justice_email, to avoid them having to reauth to capture it.

Tooling and all AP functionality other than QuickSight have been disabled for users that are not created via GitHub.
In future, to enable tooling for users coming via Entra, I think we would need have to update the helm charts so that we do not rely on their username, as those created from Entra are not compatible.

Another side effect is that we have a unique restriction on the justice_email field on the user model. This means if you auth via entra and your justice email is already stored against your normal "alpha" user, upon login a 500 error is raised as a user will already exist with that justice email address. To get around this in testing we will need to remove the justice_email from your alpha user before authing with entra.

🧑‍💻 How should the reviewer test these changes?

  • In Auth0 requires the connection with entra (justicedigital-panda-awsidentitycenter) to be set up and enabled for the Control Panel EKS app in Auth0. This is currently set up in dev.
  • If you have a justice_email set against your current alpha user created by logging in with GitHub, remove it
  • Log out and go to the login screen
  • Type in your justice email in field below the "or" on the login page
  • You should then be logged in and created as a new user and redirected to the "help" page
  • A superuser will then need to enable the user as a QuickSight user in order to allow them to access QS

📚 Documentation status

  • No changes to the documentation are required
  • This PR includes all relevant documentation
  • Documentation will be added in the future because ... (unclear what and how we want to document these changes as they are so specific to CICA currently)

To enable CICA users to access the CP, we will
enable a new connection in Auth0 with EntraID.
This updates the code we use to create a user from
the ID token to normalize their name, and store
their email as the justice_email, to avoid them
having to reauth to capture it.
controlpanel/oidc.py Fixed Show fixed Hide fixed
Previously the username would be used when installing user helm charts.
This was fine when all users came from github, as their usernames were
guaranteed to be valid with helm. However this is not the case with
usernames from Entra, as they can include invalid characters such as '.'
which results in an error after the user logs in trying to provision
the user. This changes uses the slug, which is valid for helm, however
accessing tools will still not be compatible so tooling will not be
available for these users.
@michaeljcollinsuk michaeljcollinsuk marked this pull request as ready for review January 10, 2025 11:46
This reverts commit 93cb6cd.

We will resolve the issue these changes were meant to fix in a different
way when we fully implement access with EntraID
Copy link
Contributor

@jamesstottmoj jamesstottmoj left a comment

Choose a reason for hiding this comment

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

Tested locally with my justice user.

Changed my existing user's email so user could log into control panel.

Could see limited toolbar options. Switched to superuser and granted new user access to quicksight.

Switched back to new user and could access quicksight successfully.

@jamesstottmoj jamesstottmoj merged commit a584722 into main Jan 13, 2025
11 checks passed
@jamesstottmoj jamesstottmoj deleted the feature/cica-access branch January 13, 2025 10:24
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