mitodl / ol-keycloak

Custom theme and extensions for Keycloak SSO
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

SSO UI MVP #41

Closed rhysyngsun closed 6 months ago

rhysyngsun commented 6 months ago

What are the relevant tickets?

Part of https://github.com/mitodl/hq/issues/3473

Description (What does it do?)

This updates our theme to align to the current designs.

Screenshots (if appropriate):

Screenshot 2024-03-20 at 14-19-14 MIT Open Screenshot 2024-03-20 at 14-50-29 MIT Open Screenshot 2024-03-20 at 17-11-50 MIT Open Screenshot 2024-03-20 at 14-58-58 MIT Open

How can this be tested?

Install the code changes:
Test functionality:

Through this testing, double check that everything matches the designs.

Additional Context

Note that this is a fairly large amount of changes which is primarily due to the fact that our original theme was based on the base keycloak theme in 23.0. Keycloak 24.0 came out about 2 weeks ago removing that and replacing it with a more modern theme (v2). This required me to gut quite a bit of our theme and reapply the customizations on top of the new v2 theme.

Checklist:

gitguardian[bot] commented 6 months ago

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | | | -------------- | ------------------ | ------------------------------ | ---------------- | --------------- | -------------------- | | [10049559](https://dashboard.gitguardian.com/incidents/10049559?occurrence=127664550) | Triggered | Generic Password | a9463d88bfb34782552e258e6c97ad24f187ab92 | ol-keycloak/oltheme/src/main/resources/theme/ol/login/theme.properties | [View secret](https://github.com/mitodl/ol-keycloak/commit/a9463d88bfb34782552e258e6c97ad24f187ab92#diff-d7edf8b20007c6760456c9042512ad8ba1a30370909d29684411a7d665819077R13) |
🛠 Guidelines to remediate hardcoded secrets
1. Understand the implications of revoking this secret by investigating where it is used in your code. 2. Replace and store your secret safely. [Learn here](https://blog.gitguardian.com/secrets-api-management?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment) the best practices. 3. Revoke and [rotate this secret](https://docs.gitguardian.com/secrets-detection/secrets-detection-engine/detectors/generics/generic_password#revoke-the-secret?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment). 4. If possible, [rewrite git history](https://blog.gitguardian.com/rewriting-git-history-cheatsheet?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment). Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data. To avoid such incidents in the future consider - following these [best practices](https://blog.gitguardian.com/secrets-api-management/?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment) for managing and storing secrets including API keys and other credentials - install [secret detection on pre-commit](https://docs.gitguardian.com/ggshield-docs/integrations/git-hooks/pre-commit?utm_source=product&utm_medium=GitHub_checks&utm_campaign=check_run_comment) to catch secret before it leaves your machine and ease remediation.

🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

rhysyngsun commented 6 months ago

@collinpreston I addressed most of your feedback, there are some things from the mockups that aren't feasible with theme changes or I left for other reasons, more details below:

6. When an email address is entered into the username-password form that does not match an existing Keycloak account, the error message displayed does not match the mock-ups.

I updated this to be as close as I can to the mock up, but I can't put a link in that message or dynamic information like the email address in there so I adapted the wording as best I can. I made Steve aware there are issues like this.

9. Should the text "©2024 Massachusetts Institute of Technology" be at the bottom of the pages?  I don't see it in the mock-ups.

Asked on Slack, @pdpinch said we some kind of minimal footer, including copyright, at some point despite it not being in the mocks, so I'll leave this in.

10. The email-verification-sent page does not match the mock-up.

I looked into this, it can't be accomplished by templates, I didn't look into the feasibility but it would require a new Java customization if it can be done, that's out of scope for what I'm doing here. It's been asked a few times in various community forums and received no response as to how to even attempt it.

11. The formatting of the incorrect password error message on the username-password form does not match the mock-ups.

I updated this to be as close as I can to the mock up, but same as the email fields I can't put a link in that message so I carried over the "select Forgot Password below" wording. There's likely going to be more adjustments to the UX after I merge anyway, so this is probably fine for now.

12. The _show password_ button is shown on the username-password form.

This was intentional, there was a separate figma file from steve that noted this as a nice to have (#3 on second row): https://www.figma.com/file/M7raxNajixAyAB2MNfAB6l/MIT-Canvas?type=design&node-id=456-11513&mode=design

pdpinch commented 6 months ago

Thanks for enumerating those. I think they are fine for now (and maybe fine forever), so we can move ahead. I'm eager to test this on RC.

rhysyngsun commented 6 months ago

@collinpreston all set