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

Improve frontend OIDC callback #2871

Merged
merged 4 commits into from
Aug 8, 2024
Merged

Conversation

arbulu89
Copy link
Contributor

@arbulu89 arbulu89 commented Aug 7, 2024

Description

Refactor and style OIDC callback flow adding loading and failure states.
This is how it looks like:

oidc_flow

The failed login looks like this:
image

How was this tested?

Tested with jest

@arbulu89 arbulu89 added the enhancement New feature or request label Aug 7, 2024
@arbulu89 arbulu89 changed the base branch from main to oidc August 7, 2024 09:50
@arbulu89 arbulu89 force-pushed the improve-frontend-oidc-callback branch from 4e35cfa to b0b6b39 Compare August 7, 2024 10:31
@arbulu89 arbulu89 marked this pull request as ready for review August 7, 2024 11:36
@@ -9,14 +9,18 @@ defmodule TrentoWeb.PageController do
admin_username = Application.fetch_env!(:trento, :admin_user)
oidc_enabled = Application.fetch_env!(:trento, :oidc)[:enabled]

%URI{path: oidc_callback_url} =
URI.parse(Application.fetch_env!(:trento, :oidc)[:callback_url])
Copy link
Member

Choose a reason for hiding this comment

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

😍

Copy link
Member

@EMaksy EMaksy left a comment

Choose a reason for hiding this comment

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

LGTM!
Just left one suggestion.

@@ -12,3 +13,11 @@ export const getSingleSignOnLoginUrl = () => {

return '';
};

export const getSingleSignOnCallbackUrl = () => {
Copy link
Member

@EMaksy EMaksy Aug 8, 2024

Choose a reason for hiding this comment

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

What about using the ternary operator here?
export const getSingleSignOnCallbackUrl = () => OIDC_ENABLED ? OIDC_CALLBACK_URL : '';

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I preferred the if/else because in next implementations we are going to add new SSO integrations

Copy link
Member

@CDimonaco CDimonaco left a comment

Choose a reason for hiding this comment

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

🚀 perfect

@arbulu89 arbulu89 merged commit d160d3e into oidc Aug 8, 2024
26 checks passed
@arbulu89 arbulu89 deleted the improve-frontend-oidc-callback branch August 8, 2024 08:25
CDimonaco pushed a commit that referenced this pull request Aug 12, 2024
* Send oidc callback url to frontend

* Get oidc callback url in frontend functions

* Refactor and test oidc enrollment saga

* Refactor and test oidc callback component
EMaksy pushed a commit that referenced this pull request Aug 19, 2024
* Send oidc callback url to frontend

* Get oidc callback url in frontend functions

* Refactor and test oidc enrollment saga

* Refactor and test oidc callback component
arbulu89 added a commit that referenced this pull request Aug 20, 2024
* Configured local keycloak with realm provisioning

* Installation and first configuration of pow-assent

* Keycloak provisioned realm confidential

* frontend wip

* oidc callback as conf parameter

* locked users could not login through IDP

* Delete user deletes user identities

* User identity changeset to map oidc standard to user schema

* Add session controller callback test

* Disable and update user forms if OIDC is enabled (#2859)

* Send oicd is enabled value to frontend

* Create users function to check sso is enabled

* Add sso differences in the users views

* Add sso differences in profile views

* Disable password update requested notification

* Correct typo and remove test debug

* Move sso auth logic to auth folder

* Format mix.exs file

* Disable oidc by default in dev

* Disable actions when external idp is enabled (#2863)

* Add external idp guard plug

* Prevent write profile operations when external idp is configured

* Prevent traditional login operation when external idp is configured

* Disable create user endpoint when external idp is configured

* fix controller tests env

* mix credo

* Addressing review feedback

* dialyzer fix

* View field for idp users (#2865)

* Listing and getting users returns also the user identities

* User controller returns idp_user field in response

* profile controller returning idp_user field

* Create user adds empty user_identities

* User identities context assigns global abilities to a oidc user (#2868)

* Single sign on login view (#2866)

* Load oidc url in frontend

* Create SSO login view

* Improve frontend OIDC callback (#2871)

* Send oidc callback url to frontend

* Get oidc callback url in frontend functions

* Refactor and test oidc enrollment saga

* Refactor and test oidc callback component

* Allow only abilities update with OIDC (#2879)

* User update endpoint only updates abilities when oidc enabled

* User update skips password changeset if the user comes from idp

* Allow enable field in users update when oidc is enabled

* mix credo

* Add option to load oidc variables in runtime (#2874)

* Existing user is recovered when login with oidc (#2880)

* Existing user is recovered when login with oidc

* Addressing review feedbacks

* Fix fullname entry in trento console for a new created user by idp provider (#2883)

* fix typo

* Use name to display the full name in trento not only the first name

* fix oidc callback url parameter in oidc runtime (#2906)

* OIDC integration E2E  tests (#2908)

* Add admin user to keycloak trento realm

* Add OIDC integration e2e tests

* Run integration test conditionally

* Add github action to run integration test

---------

Co-authored-by: Carmine Di Monaco <[email protected]>
Co-authored-by: Carmine Di Monaco <[email protected]>
Co-authored-by: Eugen Maksymenko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

3 participants