-
-
Notifications
You must be signed in to change notification settings - Fork 34
OIDC Improvement: Auto redirecting #47
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
base: main
Are you sure you want to change the base?
Changes from all commits
815e8a9
35b950e
492399c
9c685a8
9ccab18
bf482b6
2dbd5ef
7916c49
6b24007
fb1ed24
c9236e0
af191e8
17858ab
08445cf
d13ada2
97affed
dd037d3
c1ab9c3
cba6950
f58c761
4d4ffad
82f5502
b6ac209
9071cae
821c7e7
b115414
51fb470
5660bf9
4f3e530
2535709
292747d
4589413
51c8a9e
3a98f52
cc7bec8
8a5b084
9ef2bc3
a82795e
31d9e55
3cb101d
63f27c9
6d8af58
e6a39f4
85bdcc4
bd743eb
7a0387d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,4 +57,12 @@ export class AuthApiService extends BaseApiService<'/auth'> { | |
| isPasswordSet(): Observable<boolean> { | ||
| return this.httpClient.get<boolean>(`${this.basePath}/is_password_set`); | ||
| } | ||
|
|
||
| /** | ||
| * Check if OIDC auto redirect is enabled | ||
| * @returns | ||
| */ | ||
| isOidcAutoRedirectEnabled(): Observable<boolean> { | ||
| return this.httpClient.get<boolean>(`${this.basePath}/oidc_auto_redirect`); | ||
| } | ||
|
Comment on lines
+61
to
+67
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, it's need to be universal request for provider |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,11 @@ export class AuthPage { | |
| private readonly router = inject(Router); | ||
| private readonly toastService = inject(ToastService); | ||
|
|
||
| constructor() { | ||
| // Check for OIDC auto-redirect when component initializes | ||
| this.checkOidcAutoRedirect(); | ||
| } | ||
|
|
||
| public readonly isLoading = signal<boolean>(false); | ||
| public readonly isPasswordSet = resource({ | ||
| defaultValue: false, | ||
|
|
@@ -65,6 +70,20 @@ export class AuthPage { | |
| ), | ||
| }); | ||
|
|
||
| public readonly isOidcAutoRedirectEnabled = resource({ | ||
| defaultValue: false, | ||
| loader: () => | ||
| firstValueFrom( | ||
| this.authApiService.isOidcAutoRedirectEnabled().pipe( | ||
| retry(1), | ||
| catchError((error) => { | ||
| this.toastService.error(error); | ||
| return throwError(() => error); | ||
| }), | ||
| ), | ||
| ), | ||
| }); | ||
|
Comment on lines
+73
to
+85
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the resource pattern isn't justified sirce this value is not used in the template. |
||
|
|
||
| onSubmitNewPassword($event: ISetPasswordBody): void { | ||
| this.isLoading.set(true); | ||
| this.authApiService | ||
|
|
@@ -99,4 +118,18 @@ export class AuthPage { | |
| onOidcLogin(): void { | ||
| this.authApiService.initiateLogin('oidc'); | ||
| } | ||
|
|
||
| private checkOidcAutoRedirect(): void { | ||
| // Wait for resources to load, then check if auto-redirect should happen | ||
| setTimeout(() => { | ||
| const oidcEnabled = this.isOidcEnabled.value(); | ||
| const passwordEnabled = this.isPasswordEnabled.value(); | ||
| const autoRedirectEnabled = this.isOidcAutoRedirectEnabled.value(); | ||
|
|
||
| // Auto-redirect if OIDC auto-redirect is enabled, OIDC is enabled, and password auth is disabled | ||
| if (autoRedirectEnabled && oidcEnabled && !passwordEnabled) { | ||
| this.authApiService.initiateLogin('oidc'); | ||
| } | ||
| }, 100); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the right approach to implement it as separate exclusive method for the OIDC in auth_api.
I think should be in AuthProvider and child classes - something like existing /{provider}/login and /{provider}/callback
For password provider it is always false obviously, but other providers may be added in the future.