fix: Added retry logic to OneAuth login flows when an interactive sig…

…n in is required (#6467) * Added interactive retry logic to ARM login flow * Added more verbose logging * Fixed retry to use a constant instead of undefined enum * Fixed oneauth retry logic * Reverted custom version * Added comments specifying the source of some OneAuth types Co-authored-by: Andy Brown <[email protected]>
microsoft · Mar 23, 2021 · 30f6132 · 30f6132
1 parent 294514f
commit 30f6132
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 21 deletions.
diff --git a/Composer/packages/electron-server/__tests__/auth/oneAuthService.test.ts b/Composer/packages/electron-server/__tests__/auth/oneAuthService.test.ts
@@ -103,7 +103,7 @@ describe('OneAuth Serivce', () => {
   });
 
   it('should try to acquire a token interactively if interaction is required', async () => {
-    mockOneAuth.acquireCredentialSilently.mockReturnValueOnce({ error: { status: INTERACTION_REQUIRED } });
+    mockOneAuth.acquireCredentialSilently.mockRejectedValueOnce({ error: { status: 2 /* Interaction Required */ } });
     const result = await oneAuthService.getAccessToken({ targetResource: 'someProtectedResource' });
 
     expect(mockOneAuth.acquireCredentialInteractively).toHaveBeenCalled();
@@ -217,6 +217,16 @@ describe('OneAuth Serivce', () => {
     expect(result).toBe('someARMToken');
   });
 
+  it('should get an ARM token for a tenant interactively if interaction is required', async () => {
+    mockOneAuth.acquireCredentialInteractively.mockReturnValueOnce({ credential: { value: 'someARMToken' } });
+    mockOneAuth.acquireCredentialSilently.mockRejectedValueOnce({ error: { status: 2 /* Interaction Required */ } });
+    (oneAuthService as any).signedInARMAccount = {};
+    const result = await oneAuthService.getARMTokenForTenant('someTenant');
+
+    expect(mockOneAuth.acquireCredentialInteractively).toHaveBeenCalled();
+    expect(result).toBe('someARMToken');
+  });
+
   it('should login first if signedInARMAccount is undefined', async () => {
     (oneAuthService as any).signedInARMAccount = undefined;
     await oneAuthService.getARMTokenForTenant('tenantId');

diff --git a/Composer/packages/electron-server/src/auth/oneAuthService.ts b/Composer/packages/electron-server/src/auth/oneAuthService.ts
@@ -34,6 +34,29 @@ type GetTenantsResult = {
   value: AzureTenant[];
 };
 
+// Pulled from ./oneAuth.d.ts
+enum Status {
+  Unexpected = 0,
+  Reserved = 1,
+  InteractionRequired = 2,
+  NoNetwork = 3,
+  NetworkTemporarilyUnavailable = 4,
+  ServerTemporarilyUnavailable = 5,
+  ApiContractViolation = 6,
+  UserCanceled = 7,
+  ApplicationCanceled = 8,
+  IncorrectConfiguration = 9,
+  InsufficientBuffer = 10,
+  AuthorityUntrusted = 11,
+}
+
+// Pulled from ./oneAuth.d.ts
+enum Flight {
+  UseMsalforMsa = 2,
+  UseWamforMSA = 1002,
+  UseWamforAAD = 1003,
+}
+
 export class OneAuthInstance extends OneAuthBase {
   private initialized: boolean;
   private _oneAuth: typeof OneAuth | null = null; //eslint-disable-line
@@ -77,9 +100,7 @@ export class OneAuthInstance extends OneAuthBase {
         GRAPH_RESOURCE,
         false // prefer broker
       );
-      this.oneAuth.setFlights([
-        2, // UseMsalforMsa
-      ]);
+      this.oneAuth.setFlights([Flight.UseMsalforMsa]);
       this.oneAuth.initialize(appConfig, msaConfig, aadConfig, undefined);
       this.initialized = true;
       log('Service initialized.');
@@ -126,7 +147,7 @@ export class OneAuthInstance extends OneAuthBase {
         this.signedInAccount.realm,
         ''
       );
-      let result = await this.oneAuth.acquireCredentialSilently(this.signedInAccount?.id, reqParams, '');
+      const result = await this.oneAuth.acquireCredentialSilently(this.signedInAccount?.id, reqParams, '');
       if (result.credential && result.credential.value) {
         log('Acquired access token. %s', result.credential.value);
         return {
@@ -135,24 +156,29 @@ export class OneAuthInstance extends OneAuthBase {
           expiryTime: result.credential.expiresOn,
         };
       }
-      if (result.error) {
-        if (result.error.status === this.oneAuth.Status.InteractionRequired) {
-          // try again but interactively
-          log('Interaction required. Trying again interactively to get access token.');
-          result = await this.oneAuth.acquireCredentialInteractively(this.signedInAccount?.id, reqParams, '');
-          if (result.credential && result.credential.value) {
-            log('Acquired access token interactively. %s', result.credential.value);
-            return {
-              accessToken: result.credential.value,
-              acquiredAt: Date.now(),
-              expiryTime: result.credential.expiresOn,
-            };
-          }
-        }
-        throw result.error;
-      }
       throw 'Could not acquire an access token.';
     } catch (e) {
+      if (e.error?.status === Status.InteractionRequired && this.signedInAccount) {
+        // try again but interactively
+        log('Interaction required. Trying again interactively to get access token.');
+        // use the signed in account to acquire a token
+        const reqParams = new this.oneAuth.AuthParameters(
+          DEFAULT_AUTH_SCHEME,
+          DEFAULT_AUTH_AUTHORITY,
+          params.targetResource,
+          this.signedInAccount.realm,
+          ''
+        );
+        const result = await this.oneAuth.acquireCredentialInteractively(this.signedInAccount?.id, reqParams, '');
+        if (result.credential && result.credential.value) {
+          log('Acquired access token interactively. %s', result.credential.value);
+          return {
+            accessToken: result.credential.value,
+            acquiredAt: Date.now(),
+            expiryTime: result.credential.expiresOn,
+          };
+        }
+      }
       log('Error while trying to get an access token: %O', e);
       throw e;
     }
@@ -208,6 +234,22 @@ export class OneAuthInstance extends OneAuthBase {
         log('Acquired ARM token for tenant: %s', result.credential.value);
         return result.credential.value;
       } catch (e) {
+        if (e.error?.status === Status.InteractionRequired) {
+          // try again but interactively
+          log('Acquiring ARM token failed: Interaction required. Trying again interactively to get access token.');
+          const tokenParams = new this.oneAuth.AuthParameters(
+            DEFAULT_AUTH_SCHEME,
+            `https://login.microsoftonline.com/${tenantId}`,
+            ARM_RESOURCE,
+            '',
+            ''
+          );
+          const result = await this.oneAuth.acquireCredentialInteractively(this.signedInARMAccount.id, tokenParams, '');
+          if (result.credential && result.credential.value) {
+            log('Acquired access token interactively. %s', result.credential.value);
+            return result.credential.value;
+          }
+        }
         log('There was an error trying to get an ARM token for tenant %s: %O', tenantId, e);
         throw e;
       }