Merge pull request #2790 from ORCID/aromanovv/PD-5144-improve-disable-authorize-access-button-logic

PD-5144 improve disable authorize access button logic

Author: cryptalith

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;
+}

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 {

src/app/core/oauth/oauth.service.ts

@@ -99,7 +99,7 @@ export class OauthService {
       )
   }
 
-  authorize(approved: boolean): Observable<RequestInfoForm> {
+  authorize(approved: boolean): Observable<string> {
     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<any>) => {
+        switchMap((res: HttpResponse<any>) => {
           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) =>