From b0f70626c88cfe53bc992940a73518af66ae8ab7 Mon Sep 17 00:00:00 2001 From: Ramon Date: Mon, 3 Mar 2025 09:24:55 +0100 Subject: [PATCH] fix: load metadata using user service (#9429) # Which Problems Are Solved - #9382 "When I log in and get to my user profile page, I get an empty error message at the top:" # How the Problems Are Solved load metadata using user service # Additional Changes - The roles observable returns an empty array instead of never emiting - Small refactorings in app.component.ts because at first I thought the errors stems from there. - Added withLatestFromSynchronousFix RXJS operator because withLatestFrom has confusing behavior when used in synchronous contexts. Why this operator is needed is described here: https://github.com/ReactiveX/rxjs/issues/7068 # Additional Context - Closes #9382 --- console/src/app/app.component.ts | 66 +++++++++---------- .../auth-user-detail.component.ts | 49 ++++---------- .../user-detail/user-detail.component.ts | 1 + console/src/app/services/grpc-auth.service.ts | 18 +---- console/src/app/services/new-auth.service.ts | 15 +++-- .../app/utils/withLatestFromSynchronousFix.ts | 19 ++++++ 6 files changed, 74 insertions(+), 94 deletions(-) create mode 100644 console/src/app/utils/withLatestFromSynchronousFix.ts diff --git a/console/src/app/app.component.ts b/console/src/app/app.component.ts index f201f704bd..bd46e30cee 100644 --- a/console/src/app/app.component.ts +++ b/console/src/app/app.component.ts @@ -1,14 +1,14 @@ import { BreakpointObserver } from '@angular/cdk/layout'; import { OverlayContainer } from '@angular/cdk/overlay'; import { DOCUMENT, ViewportScroller } from '@angular/common'; -import { Component, HostBinding, HostListener, Inject, OnDestroy, ViewChild } from '@angular/core'; +import { Component, DestroyRef, HostBinding, HostListener, Inject, OnDestroy, ViewChild } from '@angular/core'; import { MatIconRegistry } from '@angular/material/icon'; import { MatDrawer } from '@angular/material/sidenav'; import { DomSanitizer } from '@angular/platform-browser'; import { ActivatedRoute, Router, RouterOutlet } from '@angular/router'; import { LangChangeEvent, TranslateService } from '@ngx-translate/core'; -import { Observable, of, Subject } from 'rxjs'; -import { filter, map, startWith, takeUntil } from 'rxjs/operators'; +import { Observable, of, Subject, switchMap } from 'rxjs'; +import { filter, map, startWith, takeUntil, tap } from 'rxjs/operators'; import { accountCard, adminLineAnimation, navAnimations, routeAnimations, toolbarAnimation } from './animations'; import { Org } from './proto/generated/zitadel/org_pb'; @@ -21,6 +21,7 @@ import { ThemeService } from './services/theme.service'; import { UpdateService } from './services/update.service'; import { fallbackLanguage, supportedLanguages, supportedLanguagesRegexp } from './utils/language'; import { PosthogService } from './services/posthog.service'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; @Component({ selector: 'cnsl-root', @@ -28,7 +29,7 @@ import { PosthogService } from './services/posthog.service'; styleUrls: ['./app.component.scss'], animations: [toolbarAnimation, ...navAnimations, accountCard, routeAnimations, adminLineAnimation], }) -export class AppComponent implements OnDestroy { +export class AppComponent { @ViewChild('drawer') public drawer!: MatDrawer; public isHandset$: Observable = this.breakpointObserver.observe('(max-width: 599px)').pipe( map((result) => { @@ -48,8 +49,6 @@ export class AppComponent implements OnDestroy { public showProjectSection: boolean = false; - private destroy$: Subject = new Subject(); - public language: string = 'en'; public privacyPolicy!: PrivacyPolicy.AsObject; constructor( @@ -70,6 +69,7 @@ export class AppComponent implements OnDestroy { private activatedRoute: ActivatedRoute, @Inject(DOCUMENT) private document: Document, private posthog: PosthogService, + private readonly destroyRef: DestroyRef, ) { console.log( '%cWait!', @@ -199,42 +199,43 @@ export class AppComponent implements OnDestroy { this.getProjectCount(); - this.authService.activeOrgChanged.pipe(takeUntil(this.destroy$)).subscribe((org) => { + this.authService.activeOrgChanged.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((org) => { if (org) { this.org = org; this.getProjectCount(); } }); - this.activatedRoute.queryParams.pipe(filter((params) => !!params['org'])).subscribe((params) => { - const { org } = params; - this.authService.getActiveOrg(org); - }); + this.activatedRoute.queryParamMap + .pipe( + map((params) => params.get('org')), + filter(Boolean), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe((org) => this.authService.getActiveOrg(org)); - this.authenticationService.authenticationChanged.pipe(takeUntil(this.destroy$)).subscribe((authenticated) => { - if (authenticated) { - this.authService - .getActiveOrg() - .then(async (org) => { - this.org = org; - // TODO add when console storage is implemented - // this.startIntroWorkflow(); - }) - .catch((error) => { - console.error(error); - this.router.navigate(['/users/me']); - }); - } - }); + this.authenticationService.authenticationChanged + .pipe( + filter(Boolean), + switchMap(() => this.authService.getActiveOrg()), + takeUntilDestroyed(this.destroyRef), + ) + .subscribe({ + next: (org) => (this.org = org), + error: async (err) => { + console.error(err); + return this.router.navigate(['/users/me']); + }, + }); this.isDarkTheme = this.themeService.isDarkTheme; - this.isDarkTheme.pipe(takeUntil(this.destroy$)).subscribe((dark) => { + this.isDarkTheme.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((dark) => { const theme = dark ? 'dark-theme' : 'light-theme'; this.onSetTheme(theme); this.setFavicon(theme); }); - this.translate.onLangChange.pipe(takeUntil(this.destroy$)).subscribe((language: LangChangeEvent) => { + this.translate.onLangChange.pipe(takeUntilDestroyed(this.destroyRef)).subscribe((language: LangChangeEvent) => { this.document.documentElement.lang = language.lang; this.language = language.lang; }); @@ -254,11 +255,6 @@ export class AppComponent implements OnDestroy { // }, 1000); // } - public ngOnDestroy(): void { - this.destroy$.next(); - this.destroy$.complete(); - } - public prepareRoute(outlet: RouterOutlet): boolean { return outlet && outlet.activatedRouteData && outlet.activatedRouteData['animation']; } @@ -283,7 +279,7 @@ export class AppComponent implements OnDestroy { this.translate.addLangs(supportedLanguages); this.translate.setDefaultLang(fallbackLanguage); - this.authService.user.pipe(filter(Boolean), takeUntil(this.destroy$)).subscribe((userprofile) => { + this.authService.user.pipe(filter(Boolean), takeUntilDestroyed(this.destroyRef)).subscribe((userprofile) => { const cropped = navigator.language.split('-')[0] ?? fallbackLanguage; const fallbackLang = cropped.match(supportedLanguagesRegexp) ? cropped : fallbackLanguage; @@ -306,7 +302,7 @@ export class AppComponent implements OnDestroy { } private setFavicon(theme: string): void { - this.authService.labelpolicy$.pipe(startWith(undefined), takeUntil(this.destroy$)).subscribe((lP) => { + this.authService.labelpolicy$.pipe(startWith(undefined), takeUntilDestroyed(this.destroyRef)).subscribe((lP) => { if (theme === 'dark-theme' && lP?.iconUrlDark) { // Check if asset url is stable, maybe it was deleted but still wasn't applied fetch(lP.iconUrlDark).then((response) => { diff --git a/console/src/app/pages/users/user-detail/auth-user-detail/auth-user-detail.component.ts b/console/src/app/pages/users/user-detail/auth-user-detail/auth-user-detail.component.ts index 841af052da..2752dd9265 100644 --- a/console/src/app/pages/users/user-detail/auth-user-detail/auth-user-detail.component.ts +++ b/console/src/app/pages/users/user-detail/auth-user-detail/auth-user-detail.component.ts @@ -5,19 +5,7 @@ import { MatDialog } from '@angular/material/dialog'; import { ActivatedRoute, Router } from '@angular/router'; import { TranslateService } from '@ngx-translate/core'; import { Buffer } from 'buffer'; -import { - combineLatestWith, - defer, - EMPTY, - fromEvent, - mergeWith, - Observable, - of, - shareReplay, - Subject, - switchMap, - take, -} from 'rxjs'; +import { defer, EMPTY, fromEvent, mergeWith, Observable, of, shareReplay, Subject, switchMap, take } from 'rxjs'; import { ChangeType } from 'src/app/modules/changes/changes.component'; import { phoneValidator, requiredValidator } from 'src/app/modules/form-field/validators/validators'; import { InfoDialogComponent } from 'src/app/modules/info-dialog/info-dialog.component'; @@ -37,7 +25,7 @@ import { formatPhone } from 'src/app/utils/formatPhone'; import { EditDialogComponent, EditDialogData, EditDialogResult, EditDialogType } from './edit-dialog/edit-dialog.component'; import { LanguagesService } from 'src/app/services/languages.service'; import { Gender, HumanProfile, HumanUser, User, UserState } from '@zitadel/proto/zitadel/user/v2/user_pb'; -import { catchError, filter, map, startWith, tap, withLatestFrom } from 'rxjs/operators'; +import { catchError, filter, map, startWith, withLatestFrom } from 'rxjs/operators'; import { pairwiseStartWith } from 'src/app/utils/pairwiseStartWith'; import { NewAuthService } from 'src/app/services/new-auth.service'; import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; @@ -47,12 +35,12 @@ import { UserService } from 'src/app/services/user.service'; import { LoginPolicy } from '@zitadel/proto/zitadel/policy_pb'; import { query } from '@angular/animations'; -type UserQuery = { state: 'success'; value: User } | { state: 'error'; value: string } | { state: 'loading'; value?: User }; +type UserQuery = { state: 'success'; value: User } | { state: 'error'; error: any } | { state: 'loading'; value?: User }; type MetadataQuery = | { state: 'success'; value: Metadata[] } | { state: 'loading'; value: Metadata[] } - | { state: 'error'; value: string }; + | { state: 'error'; error: any }; type UserWithHumanType = Omit & { type: { case: 'human'; value: HumanUser } }; @@ -92,9 +80,9 @@ export class AuthUserDetailComponent implements OnInit { protected readonly userName$: Observable; constructor( - public translate: TranslateService, + private translate: TranslateService, private toast: ToastService, - public grpcAuthService: GrpcAuthService, + protected grpcAuthService: GrpcAuthService, private dialog: MatDialog, private auth: AuthenticationService, private breadcrumbService: BreadcrumbService, @@ -111,7 +99,7 @@ export class AuthUserDetailComponent implements OnInit { this.user$ = this.getUser$().pipe(shareReplay({ refCount: true, bufferSize: 1 })); this.userName$ = this.getUserName(this.user$); this.savedLanguage$ = this.getSavedLanguage$(this.user$); - this.metadata$ = this.getMetadata$(this.user$).pipe(shareReplay({ refCount: true, bufferSize: 1 })); + this.metadata$ = this.getMetadata$().pipe(shareReplay({ refCount: true, bufferSize: 1 })); this.loginPolicy$ = defer(() => this.newMgmtService.getLoginPolicy()).pipe( catchError(() => EMPTY), @@ -164,7 +152,7 @@ export class AuthUserDetailComponent implements OnInit { }); this.user$.pipe(mergeWith(this.metadata$), takeUntilDestroyed(this.destroyRef)).subscribe((query) => { if (query.state == 'error') { - this.toast.showError(query.value); + this.toast.showError(query.error); } }); @@ -206,24 +194,15 @@ export class AuthUserDetailComponent implements OnInit { private getMyUser(): Observable { return defer(() => this.userService.getMyUser()).pipe( map((user) => ({ state: 'success' as const, value: user })), - catchError((error) => of({ state: 'error', value: error.message ?? '' } as const)), + catchError((error) => of({ state: 'error', error } as const)), startWith({ state: 'loading' } as const), ); } - getMetadata$(user$: Observable): Observable { + getMetadata$(): Observable { return this.refreshMetadata$.pipe( startWith(true), - combineLatestWith(user$), - switchMap(([_, user]) => { - if (!(user.state === 'success' || user.state === 'loading')) { - return EMPTY; - } - if (!user.value) { - return EMPTY; - } - return this.getMetadataById(user.value.userId); - }), + switchMap(() => this.getMetadata()), pairwiseStartWith(undefined), map(([prev, curr]) => { if (prev?.state === 'success' && curr.state === 'loading') { @@ -234,11 +213,11 @@ export class AuthUserDetailComponent implements OnInit { ); } - private getMetadataById(userId: string): Observable { - return defer(() => this.newMgmtService.listUserMetadata(userId)).pipe( + private getMetadata(): Observable { + return defer(() => this.newAuthService.listMyMetadata()).pipe( map((metadata) => ({ state: 'success', value: metadata.result }) as const), startWith({ state: 'loading', value: [] as Metadata[] } as const), - catchError((err) => of({ state: 'error', value: err.message ?? '' } as const)), + catchError((error) => of({ state: 'error', error } as const)), ); } diff --git a/console/src/app/pages/users/user-detail/user-detail/user-detail.component.ts b/console/src/app/pages/users/user-detail/user-detail/user-detail.component.ts index b63c255487..5e2d5fd041 100644 --- a/console/src/app/pages/users/user-detail/user-detail/user-detail.component.ts +++ b/console/src/app/pages/users/user-detail/user-detail/user-detail.component.ts @@ -445,6 +445,7 @@ export class UserDetailComponent implements OnInit { }; const dialogRef = this.dialog.open(WarnDialogComponent, { + data, width: '400px', }); diff --git a/console/src/app/services/grpc-auth.service.ts b/console/src/app/services/grpc-auth.service.ts index bb522e7083..3967f1df06 100644 --- a/console/src/app/services/grpc-auth.service.ts +++ b/console/src/app/services/grpc-auth.service.ts @@ -1,21 +1,7 @@ import { Injectable } from '@angular/core'; import { SortDirection } from '@angular/material/sort'; import { OAuthService } from 'angular-oauth2-oidc'; -import { - BehaviorSubject, - combineLatestWith, - defer, - distinctUntilKeyChanged, - EMPTY, - forkJoin, - mergeWith, - NEVER, - Observable, - of, - shareReplay, - Subject, - TimeoutError, -} from 'rxjs'; +import { BehaviorSubject, combineLatestWith, EMPTY, mergeWith, NEVER, Observable, of, shareReplay, Subject } from 'rxjs'; import { catchError, distinctUntilChanged, filter, finalize, map, startWith, switchMap, tap, timeout } from 'rxjs/operators'; import { @@ -186,7 +172,6 @@ export class GrpcAuthService { .then((resp) => resp.resultList) .catch(() => []), ), - filter((roles) => !!roles.length), distinctUntilChanged((a, b) => { return JSON.stringify(a.sort()) === JSON.stringify(b.sort()); }), @@ -302,7 +287,6 @@ export class GrpcAuthService { } return this.zitadelPermissions.pipe( - filter((permissions) => !!permissions.length), map((permissions) => this.hasRoles(permissions, roles, requiresAll)), distinctUntilChanged(), ); diff --git a/console/src/app/services/new-auth.service.ts b/console/src/app/services/new-auth.service.ts index 3e09ea7ad4..34b12bb90f 100644 --- a/console/src/app/services/new-auth.service.ts +++ b/console/src/app/services/new-auth.service.ts @@ -1,12 +1,9 @@ import { Injectable } from '@angular/core'; import { GrpcService } from './grpc.service'; -import { create } from '@bufbuild/protobuf'; import { - AddMyAuthFactorOTPSMSRequestSchema, AddMyAuthFactorOTPSMSResponse, - GetMyUserRequestSchema, GetMyUserResponse, - VerifyMyPhoneRequestSchema, + ListMyMetadataResponse, VerifyMyPhoneResponse, } from '@zitadel/proto/zitadel/auth_pb'; @@ -17,14 +14,18 @@ export class NewAuthService { constructor(private readonly grpcService: GrpcService) {} public getMyUser(): Promise { - return this.grpcService.authNew.getMyUser(create(GetMyUserRequestSchema)); + return this.grpcService.authNew.getMyUser({}); } public verifyMyPhone(code: string): Promise { - return this.grpcService.authNew.verifyMyPhone(create(VerifyMyPhoneRequestSchema, { code })); + return this.grpcService.authNew.verifyMyPhone({}); } public addMyAuthFactorOTPSMS(): Promise { - return this.grpcService.authNew.addMyAuthFactorOTPSMS(create(AddMyAuthFactorOTPSMSRequestSchema)); + return this.grpcService.authNew.addMyAuthFactorOTPSMS({}); + } + + public listMyMetadata(): Promise { + return this.grpcService.authNew.listMyMetadata({}); } } diff --git a/console/src/app/utils/withLatestFromSynchronousFix.ts b/console/src/app/utils/withLatestFromSynchronousFix.ts new file mode 100644 index 0000000000..d2e362731e --- /dev/null +++ b/console/src/app/utils/withLatestFromSynchronousFix.ts @@ -0,0 +1,19 @@ +import { combineLatestWith, distinctUntilChanged, Observable, ObservableInput, ObservableInputTuple } from 'rxjs'; +import { map } from 'rxjs/operators'; + +// withLatestFrom does not work in this case, so we use +// combineLatestWith + distinctUntilChanged +// here the problem is described in more detail +// https://github.com/ReactiveX/rxjs/issues/7068 +export const withLatestFromSynchronousFix = + (...secondaries$: [...ObservableInputTuple]) => + (primary$: Observable) => + primary$.pipe( + // we add the index, so we can distinguish + // primary submissions from each other, + // and then we can only emit when primary changes + map((primary, i) => [primary, i]), + combineLatestWith(...secondaries$), + distinctUntilChanged(undefined!, ([[_, i]]) => i), + map(([[primary], ...secondaries]) => [primary, ...secondaries]), + );