fix: auto-retry Microsoft OAuth on AADSTS650051 race condition (#18405)
## Summary Fixes #13008 Azure AD has a [known bug](https://learn.microsoft.com/en-us/answers/questions/5570899/our-adminconsent-url-always-fails-with-aadsts65005) where the first consent attempt for a multi-tenant app fails with `AADSTS650051` because the service principal is mid-provisioning in the target tenant. Microsoft's own engineers have acknowledged the issue with no fix ETA. This PR adds a transparent retry to the `MicrosoftOAuthGuard`: 1. When the OAuth redirect returns `AADSTS650051`, the guard parses the `state` parameter to recover the original query params (workspaceId, invite hash, locale, etc.) 2. Redirects the user back to `/auth/microsoft` with a `msRetry=1` counter 3. The full OAuth flow restarts — by this point the service principal has finished provisioning, so consent succeeds 4. Capped at 1 retry (`MAX_MICROSOFT_AUTH_RETRIES`) to prevent infinite loops. If it still fails, falls through to the normal error page From the user's perspective, they just see a brief extra redirect instead of a cryptic error page. ### Context - `AADSTS650051` is a race condition in Azure AD's service principal provisioning during multi-tenant app consent - It's distinct from missing consent (`AADSTS65001`) or admin consent required (`AADSTS90094`) - The error is transient — retrying the same flow immediately succeeds - Microsoft Q&A threads confirm this affects many multi-tenant apps, not just Twenty ### References - [Microsoft Q&A: adminconsent always fails with AADSTS650051 on first attempt](https://learn.microsoft.com/en-us/answers/questions/5570899/our-adminconsent-url-always-fails-with-aadsts65005) - [Microsoft Q&A: AADSTS650051 for multiple applications](https://learn.microsoft.com/en-us/answers/questions/5571098/when-customers-attempt-to-sign-in-and-grant-consen) ## Test plan - [ ] Deploy to a staging environment with Microsoft OAuth enabled - [ ] Have a user from a new Azure AD tenant attempt Microsoft sign-in for the first time - [ ] If AADSTS650051 occurs, verify the user is transparently retried and signs in successfully - [ ] Verify `msRetry` counter prevents infinite redirect loops (check server logs for the warn message) - [ ] Verify normal Microsoft sign-in flow (no error) is unaffected 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: neo773 <62795688+neo773@users.noreply.github.com> Co-authored-by: neo773 <neo773@protonmail.com>
This commit is contained in:
+1
@@ -0,0 +1 @@
|
||||
export const MICROSOFT_OAUTH_MAX_RETRY_ATTEMPTS = 1;
|
||||
+103
@@ -0,0 +1,103 @@
|
||||
import { MicrosoftOAuthGuard } from 'src/engine/core-modules/auth/guards/microsoft-oauth.guard';
|
||||
|
||||
const AADSTS650051_ERROR = new Error(
|
||||
'AADSTS650051: The application needs to be provisioned for this tenant.',
|
||||
);
|
||||
|
||||
const createMockContext = () => {
|
||||
const redirect = jest.fn();
|
||||
|
||||
return {
|
||||
redirect,
|
||||
context: {
|
||||
switchToHttp: () => ({
|
||||
getResponse: () => ({ redirect }),
|
||||
}),
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
const createMockRequest = (query: Record<string, string | undefined> = {}) =>
|
||||
({ query }) as any;
|
||||
|
||||
const callHandler = (
|
||||
guard: MicrosoftOAuthGuard,
|
||||
context: any,
|
||||
request: any,
|
||||
error: unknown,
|
||||
): boolean => {
|
||||
return (guard as any).handleTransientMicrosoftOAuthError(
|
||||
context,
|
||||
request,
|
||||
error,
|
||||
);
|
||||
};
|
||||
|
||||
describe('MicrosoftOAuthGuard', () => {
|
||||
let guard: MicrosoftOAuthGuard;
|
||||
|
||||
beforeEach(() => {
|
||||
guard = new (MicrosoftOAuthGuard as any)(null, null, null);
|
||||
});
|
||||
|
||||
describe('handleTransientMicrosoftOAuthError', () => {
|
||||
it('should redirect to /auth/microsoft on first AADSTS650051', () => {
|
||||
const { context, redirect } = createMockContext();
|
||||
const request = createMockRequest({
|
||||
state: JSON.stringify({ workspaceId: 'ws-123', locale: 'en' }),
|
||||
});
|
||||
|
||||
const result = callHandler(guard, context, request, AADSTS650051_ERROR);
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(redirect).toHaveBeenCalledWith(
|
||||
expect.stringContaining('/auth/microsoft?'),
|
||||
);
|
||||
|
||||
const redirectUrl = redirect.mock.calls[0][0];
|
||||
|
||||
expect(redirectUrl).toContain('oauthRetryCount=1');
|
||||
expect(redirectUrl).toContain('workspaceId=ws-123');
|
||||
expect(redirectUrl).toContain('locale=en');
|
||||
});
|
||||
|
||||
it('should not redirect when retry count is exhausted', () => {
|
||||
const { context, redirect } = createMockContext();
|
||||
const request = createMockRequest({
|
||||
state: JSON.stringify({ oauthRetryCount: 1 }),
|
||||
});
|
||||
|
||||
const result = callHandler(guard, context, request, AADSTS650051_ERROR);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(redirect).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not redirect for non-transient errors', () => {
|
||||
const { context, redirect } = createMockContext();
|
||||
const request = createMockRequest();
|
||||
|
||||
const result = callHandler(
|
||||
guard,
|
||||
context,
|
||||
request,
|
||||
new Error('invalid_client'),
|
||||
);
|
||||
|
||||
expect(result).toBe(false);
|
||||
expect(redirect).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should handle missing state without crashing', () => {
|
||||
const { context, redirect } = createMockContext();
|
||||
const request = createMockRequest();
|
||||
|
||||
const result = callHandler(guard, context, request, AADSTS650051_ERROR);
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(redirect).toHaveBeenCalledWith(
|
||||
'/auth/microsoft?oauthRetryCount=1',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -2,12 +2,21 @@ import { type ExecutionContext, Injectable } from '@nestjs/common';
|
||||
import { AuthGuard } from '@nestjs/passport';
|
||||
import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import { type Request } from 'express';
|
||||
import { parseJson } from 'twenty-shared/utils';
|
||||
import { Repository } from 'typeorm';
|
||||
|
||||
import { MICROSOFT_OAUTH_MAX_RETRY_ATTEMPTS } from 'src/engine/core-modules/auth/constants/microsoft-oauth-max-retry-attempts.constants';
|
||||
import { type SocialSSOState } from 'src/engine/core-modules/auth/types/social-sso-state.type';
|
||||
import { isMicrosoftOAuthTransientError } from 'src/engine/core-modules/auth/utils/is-microsoft-oauth-transient-error.util';
|
||||
import { WorkspaceDomainsService } from 'src/engine/core-modules/domain/workspace-domains/services/workspace-domains.service';
|
||||
import { GuardRedirectService } from 'src/engine/core-modules/guard-redirect/services/guard-redirect.service';
|
||||
import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity';
|
||||
|
||||
type SocialSSOStateWithRetry = SocialSSOState & {
|
||||
oauthRetryCount?: number;
|
||||
};
|
||||
|
||||
@Injectable()
|
||||
export class MicrosoftOAuthGuard extends AuthGuard('microsoft') {
|
||||
constructor(
|
||||
@@ -38,6 +47,10 @@ export class MicrosoftOAuthGuard extends AuthGuard('microsoft') {
|
||||
|
||||
return (await super.canActivate(context)) as boolean;
|
||||
} catch (err) {
|
||||
if (this.handleTransientMicrosoftOAuthError(context, request, err)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
this.guardRedirectService.dispatchErrorFromGuard(
|
||||
context,
|
||||
err,
|
||||
@@ -49,4 +62,43 @@ export class MicrosoftOAuthGuard extends AuthGuard('microsoft') {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private handleTransientMicrosoftOAuthError(
|
||||
context: ExecutionContext,
|
||||
request: Request,
|
||||
error: unknown,
|
||||
): boolean {
|
||||
if (!isMicrosoftOAuthTransientError(error)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const state = parseJson<SocialSSOStateWithRetry>(
|
||||
request.query.state as string,
|
||||
);
|
||||
|
||||
const oauthRetryCount = Math.max(0, Number(state?.oauthRetryCount) || 0);
|
||||
|
||||
if (oauthRetryCount >= MICROSOFT_OAUTH_MAX_RETRY_ATTEMPTS) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const url = new URL('/auth/microsoft', 'http://localhost');
|
||||
|
||||
url.searchParams.set('oauthRetryCount', String(oauthRetryCount + 1));
|
||||
|
||||
if (state) {
|
||||
for (const [key, value] of Object.entries(state)) {
|
||||
if (key !== 'oauthRetryCount' && value != null) {
|
||||
url.searchParams.set(key, String(value));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
context
|
||||
.switchToHttp()
|
||||
.getResponse()
|
||||
.redirect(url.pathname + url.search);
|
||||
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
+3
@@ -58,6 +58,9 @@ export class MicrosoftStrategy extends PassportStrategy(Strategy, 'microsoft') {
|
||||
billingCheckoutSessionState: req.query.billingCheckoutSessionState,
|
||||
workspacePersonalInviteToken: req.query.workspacePersonalInviteToken,
|
||||
action: req.query.action,
|
||||
oauthRetryCount: req.query.oauthRetryCount
|
||||
? Number(req.query.oauthRetryCount)
|
||||
: undefined,
|
||||
}),
|
||||
};
|
||||
|
||||
|
||||
+19
@@ -0,0 +1,19 @@
|
||||
import { isMicrosoftOAuthTransientError } from 'src/engine/core-modules/auth/utils/is-microsoft-oauth-transient-error.util';
|
||||
|
||||
describe('isMicrosoftOAuthTransientError', () => {
|
||||
it('should detect AADSTS650051 in the error message', () => {
|
||||
const error = new Error(
|
||||
'AADSTS650051: The application needs to be provisioned for this tenant.',
|
||||
);
|
||||
|
||||
expect(isMicrosoftOAuthTransientError(error)).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject other errors and non-Error values', () => {
|
||||
expect(isMicrosoftOAuthTransientError(new Error('invalid_client'))).toBe(
|
||||
false,
|
||||
);
|
||||
expect(isMicrosoftOAuthTransientError('AADSTS650051')).toBe(false);
|
||||
expect(isMicrosoftOAuthTransientError(null)).toBe(false);
|
||||
});
|
||||
});
|
||||
+7
@@ -0,0 +1,7 @@
|
||||
export const isMicrosoftOAuthTransientError = (error: unknown): boolean => {
|
||||
if (!(error instanceof Error)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return error.message.includes('AADSTS650051');
|
||||
};
|
||||
Reference in New Issue
Block a user