diff --git a/src/app/authorize/components/form-authorize/form-authorize.component.scss b/src/app/authorize/components/form-authorize/form-authorize.component.scss index f604b5971..f4a3aedf0 100644 --- a/src/app/authorize/components/form-authorize/form-authorize.component.scss +++ b/src/app/authorize/components/form-authorize/form-authorize.component.scss @@ -145,3 +145,7 @@ mat-card-content.authorize-content { color: #fff; background: #085c77; } + +.authorize-button:disabled { + opacity: 0.3; +} diff --git a/src/app/authorize/components/form-authorize/form-authorize.component.ts b/src/app/authorize/components/form-authorize/form-authorize.component.ts index a4ae67ecc..b8d489eae 100644 --- a/src/app/authorize/components/form-authorize/form-authorize.component.ts +++ b/src/app/authorize/components/form-authorize/form-authorize.component.ts @@ -190,90 +190,88 @@ export class FormAuthorizeComponent implements OnInit, OnDestroy { const signinUrl = queryParams ? `/signin?${queryParams}` : '/signin' ;(this.window as any).outOfRouterNavigation(signinUrl) } - + authorize(value = true) { - this.loadingAuthorizeEndpoint = true + this.loadingAuthorizeEndpoint = true; this._togglz .getStateOf(TogglzFlag.OAUTH_AUTHORIZATION) .pipe( take(1), tap((useAuthServerFlag) => { + // 1. New logic: Update journey context right away this._observability.updateJourneyContext('oauth_authorization', { OAUTH_AUTHORIZATION: useAuthServerFlag, - }) + }); + }), + switchMap((useAuthServerFlag) => { if (useAuthServerFlag === true) { - this._oauth - .authorizeOnAuthServer(this.oauthRequest, value) - .pipe( - tap(() => { - this._observability.recordEvent( - 'oauth_authorization', - value - ? AppEventName.OauthAuthorizationSuccess - : AppEventName.OauthAuthorizationDenied, - { - OAUTH_AUTHORIZATION: true, - } - ) - }), - catchError((error) => { - this._observability.recordEvent( - 'oauth_authorization', - AppEventName.OauthAuthorizationError, - oauthAuthorizeHttpFailureEventAttrs( - error, - 'auth_server', - value, - true - ) + return this._oauth.authorizeOnAuthServer(this.oauthRequest, value).pipe( + // 2. Auth Server Telemetry + tap(() => { + this._observability.recordEvent( + 'oauth_authorization', + value + ? AppEventName.OauthAuthorizationSuccess + : AppEventName.OauthAuthorizationDenied, + { OAUTH_AUTHORIZATION: true } + ); + }), + catchError((error) => { + this._observability.recordEvent( + 'oauth_authorization', + AppEventName.OauthAuthorizationError, + oauthAuthorizeHttpFailureEventAttrs( + error, + 'auth_server', + value, + true ) - return throwError(() => error) - }), - take(1), - finalize(() => (this.loadingAuthorizeEndpoint = false)) - ) - .subscribe((redirectUrl) => { - this.redirectUrl.next(redirectUrl) + ); + // Throw the error down to the final subscribe block + return throwError(() => error); }) + ); } else { - this._oauth - .authorize(value) - .pipe( - tap(() => { - this._observability.recordEvent( - 'oauth_authorization', - value - ? AppEventName.OauthAuthorizationSuccess - : AppEventName.OauthAuthorizationDenied, - { - OAUTH_AUTHORIZATION: false, - } + return this._oauth.authorize(value).pipe( + // 3. Legacy Telemetry + tap(() => { + this._observability.recordEvent( + 'oauth_authorization', + value + ? AppEventName.OauthAuthorizationSuccess + : AppEventName.OauthAuthorizationDenied, + { OAUTH_AUTHORIZATION: false } + ); + }), + catchError((error) => { + this._observability.recordEvent( + 'oauth_authorization', + AppEventName.OauthAuthorizationError, + oauthAuthorizeHttpFailureEventAttrs( + error, + 'legacy', + value, + false ) - }), - catchError((error) => { - this._observability.recordEvent( - 'oauth_authorization', - AppEventName.OauthAuthorizationError, - oauthAuthorizeHttpFailureEventAttrs( - error, - 'legacy', - value, - false - ) - ) - return throwError(() => error) - }), - take(1), - finalize(() => (this.loadingAuthorizeEndpoint = false)) - ) - .subscribe((data) => { - this.redirectUrl.next(data.redirectUrl) + ); + // Throw the error down to the final subscribe block + return throwError(() => error); }) + ); } }) ) - .subscribe() + .subscribe({ + next: (redirectUrl: string) => { + console.log('Redirecting to:', redirectUrl) + this.redirectUrl.next(redirectUrl) + }, + error: (err) => { + console.error('Authorization error:', err) + this.loadingAuthorizeEndpoint = false + }, + }) } getIconName(ScopeObject: Scope): string { diff --git a/src/app/core/oauth/oauth.service.ts b/src/app/core/oauth/oauth.service.ts index 100e7cce3..215ef6be2 100644 --- a/src/app/core/oauth/oauth.service.ts +++ b/src/app/core/oauth/oauth.service.ts @@ -99,7 +99,7 @@ export class OauthService { ) } - authorize(approved: boolean): Observable { + authorize(approved: boolean): Observable { const value: OauthAuthorize = { // tslint:disable-next-line: max-line-length // TODO @angel please confirm that persistentTokenEnabled is always true https://github.com/ORCID/ORCID-Source/blob/master/orcid-web/src/main/webapp/static/javascript/ng1Orcid/app/modules/oauthAuthorization/oauthAuthorization.component.ts#L161 @@ -123,7 +123,8 @@ export class OauthService { ), tap((requestInfo) => { this.requestInfoSubject.next(requestInfo) - }) + }), + map((requestInfo) => requestInfo.redirectUrl) ) } @@ -157,12 +158,11 @@ export class OauthService { observe: 'response', }) .pipe( - map((res: HttpResponse) => { + switchMap((res: HttpResponse) => { if (res.body && res.body['error']) { - if (res.body['error'] == 'access_denied') { + if (res.body['error'] === 'access_denied') { // Not a server error per see, just a user denied the authorization - - return res.body['uri'] + return of(res.body['uri']) } else { this._observability.recordSimpleEvent( AppEventName.OauthAuthorizeAuthServerErrorBody, @@ -175,13 +175,20 @@ export class OauthService { client_id: data?.clientId, } ) - this._errorHandler.handleError( - res.body, + const errorMessage = + res.body['error_description'] || + 'Authorization failed on server' + const authError = new Error(errorMessage) + + authError.name = res.body['error'] || 'AuthError' + + return this._errorHandler.handleError( + authError, ERROR_REPORT.STANDARD_VERBOSE ) } } else { - return res.headers.get('location') + return of(res.headers.get('location')) } }), catchError((error) =>