Frachtwerk / essencium-frontend

Next.js based starter monorepo consisting of a component library, types, docs and a boilerplate application.
https://staging.essencium.dev
GNU Lesser General Public License v3.0
17 stars 1 forks source link

490 create sso UI #528

Closed qhantom closed 6 months ago

qhantom commented 6 months ago

DESCRIPTION

This PR adds support for single-sign-on (SSO) via any provider. This not only includes the ability to login but also to hide UI elements which are not relevant for the user:

Beside those changes, I made further relevant changes for the SSO support:

How to test SSO

Since SSO is depended on how the backend is configured, here is a step-by-step guide to test it locally with an experimental backend instance:

  1. inside the .env file, replace the value at NEXT_PUBLIC_API_BASE_URL with the following: https://essencium-experimental.staging.dev.frachtwerk.de and the value at NEXT_PUBLIC_API_URL with this: ${NEXT_PUBLIC_API_BASE_URL}/${NEXT_PUBLIC_API_VERSION}
  2. spin up your Next.js dev server
  3. you should see the GitLab SSO option on the login screen, choose this
  4. authorise via GitLab
  5. see redirect to home page of Essencium (since you have the USER role you cannot do much)
  6. go to your profile and verify if you cannot change mail and password

TO-DO

CLOSES

Closes #490

vercel[bot] commented 6 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
app ✅ Ready (Inspect) Visit Preview Mar 14, 2024 0:59am
docs ✅ Ready (Inspect) Visit Preview Mar 14, 2024 0:59am
nx-cloud[bot] commented 6 months ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit becafccb5a282d2182a2de4f257fe16100391286. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets - [`nx build @frachtwerk/essencium-app`](https://cloud.nx.app/runs/RJnwS6NQYG?utm_source=pull-request&utm_medium=comment) - [`nx affected --target=lint --parallel=10`](https://cloud.nx.app/runs/ehYHdYxpO7?utm_source=pull-request&utm_medium=comment)

Sent with 💌 from NxCloud.

qhantom commented 6 months ago

I guess there is no other user registered at https://essencium-experimental.staging.dev.frachtwerk.de, but I think we should test the combination of both login ways. Like log in with SSO, test all restrictions, log out - log in with normal admin user and check for any bugs. Should work in general but its safer to test. What do you think?

The loom and the migration guide was very helpful for the review! Such a good new feature and also good refactoring in the profile/roles! Thanks for the work.

Thanks! 🙏 There should be the default user, could you please check it again? I tested it with both login strategies and it worked fine.

qhantom commented 6 months ago

Looks good with the admin user! I just ran into an edgecase, I think we need a few small additional changes: https://www.loom.com/share/60e6ae389b144d52a01d2455ab49bd5d?sid=c84915b6-cf1e-4efc-abff-a5009a78618e

Very important take from you, thanks @PhilKsr, I'm on it 🚀

qhantom commented 6 months ago

@PhilKsr I pushed new commits to this PR:

I recorded another Loom for better understanding

qhantom commented 6 months ago

Thanks for the work again! There are some small questions on my side.

In addition I think the Sso label in the header is a bit too much for the small box on the right. Maybe it would be an option to add it to the user profile instead (left side box for example). What do you think? Screenshot 2024-03-12 at 09 46 53

The intention was to clearly see with one blink with which user I'm currently logged in. When it's inside the profile card, it's one click away you need to know. For me, I think the spacing is fine. What do you think @CathrinTruchan?

UPDATE: I agree with @PhilKsr that the information for the user isn't that necessary and it's better placed inside the profile card. But lets wait for @CathrinTruchan opinion 🙂

CathrinTruchan commented 6 months ago

Thanks for the work again! There are some small questions on my side. In addition I think the Sso label in the header is a bit too much for the small box on the right. Maybe it would be an option to add it to the user profile instead (left side box for example). What do you think? Screenshot 2024-03-12 at 09 46 53

The intention was to clearly see with one blink with which user I'm currently logged in. When it's inside the profile card, it's one click away you need to know. For me, I think the spacing is fine. What do you think @CathrinTruchan?

UPDATE: I agree with @PhilKsr that the information for the user isn't that necessary and it's better placed inside the profile card. But lets wait for @CathrinTruchan opinion 🙂

I agree, the profile card is a good place for this information, since it is not super important for the user 🙂

qhantom commented 6 months ago

@PhilKsr I removed the SSO badge and put it into the profile. I realised, that the badge looks like the active-badge of the user so I refactored it into an indicator and put it on the avatar. Please let me know what you think. I also added further tests for the profile components. You can give it another review 🚀

image