Skip to content
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

fix: Added retry logic to OneAuth login flows when an interactive sign in is required #6467

Merged
merged 10 commits into from
Mar 23, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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');
Expand Down
82 changes: 62 additions & 20 deletions Composer/packages/electron-server/src/auth/oneAuthService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,29 @@ type GetTenantsResult = {
value: AzureTenant[];
};

// Pulled from ./oneAuth.d.ts
enum Status {
tonyanziano marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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.');
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down