Migrate `MicrosoftAPIRefreshAccessTokenService` to `@azure/msal-node` by neo773 · Pull Request #16954 · twentyhq/twenty

neo773 · 2026-01-06T13:39:45Z

No description provided.

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

socket-security · 2026-01-06T13:40:44Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Quality
	@graphql-codegen/typescript-operations@3.0.4
	@blocknote/server-util@0.31.1
	@esbuild-plugins/node-modules-polyfill@0.2.2
	@graphql-codegen/typescript-react-apollo@3.3.7
	@graphql-codegen/client-preset@4.3.3
	@blocknote/mantine@0.31.1
	@graphql-codegen/typescript@3.0.4
	@babel/preset-react@7.26.3
	@blocknote/xl-docx-exporter@0.31.1
	@babel/preset-typescript@7.24.7
	@ai-sdk/react@2.0.52
	@emotion/is-prop-valid@1.3.0
	@blocknote/xl-pdf-exporter@0.31.1
	@ai-sdk/provider-utils@3.0.9
	@calcom/embed-react@1.5.3
	@floating-ui/react@0.24.8
	@graphql-codegen/cli@3.3.1
	@envelop/core@4.0.3
	@dagrejs/dagre@1.1.3
	@babel/preset-env@7.26.9
	@cyntler/react-doc-viewer@1.17.0
	@codesandbox/sandpack-react@2.18.2
	@babel/core@7.28.5 ⏵ 7.28.0	⁺¹
	@blocknote/react@0.31.1
	@emotion/styled@11.13.0
	@graphiql/plugin-explorer@1.0.4
	@docsearch/react@3.6.2
	@hello-pangea/dnd@16.6.0
	@ai-sdk/xai@2.0.43
	@emotion/react@11.13.0
	@ai-sdk/anthropic@2.0.17
	@ai-sdk/openai@2.0.30
	@e2b/code-interpreter@1.5.1
See 14 more rows in the dashboard

View full report

...nt/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts

cubic-dev-ai

1 issue found across 4 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/twenty-server/src/modules/connected-account/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts">

<violation number="1" location="packages/twenty-server/src/modules/connected-account/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts:5">
P1: Using `import type` for `TwentyConfigService` may break NestJS dependency injection. Type-only imports are erased at compile time, which can prevent TypeScript from emitting the design-time type metadata that NestJS needs for constructor injection. Use a regular import to match the pattern in the Google service.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

cubic-dev-ai · 2026-01-06T13:48:44Z

...nt/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts

+import { ConfidentialClientApplication } from '@azure/msal-node';

-import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
+import type { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';


P1: Using import type for TwentyConfigService may break NestJS dependency injection. Type-only imports are erased at compile time, which can prevent TypeScript from emitting the design-time type metadata that NestJS needs for constructor injection. Use a regular import to match the pattern in the Google service.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-server/src/modules/connected-account/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts, line 5: <comment>Using `import type` for `TwentyConfigService` may break NestJS dependency injection. Type-only imports are erased at compile time, which can prevent TypeScript from emitting the design-time type metadata that NestJS needs for constructor injection. Use a regular import to match the pattern in the Google service.</comment> <file context> @@ -1,67 +1,53 @@ +import { ConfidentialClientApplication } from '@azure/msal-node'; -import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; +import type { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { ConnectedAccountRefreshAccessTokenException, </file context>

Suggested change

import type { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';

import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';

✅ Addressed in d4ecb69

...nt/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts

neo773 · 2026-01-06T14:37:11Z

...nt/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts

+   * Extracts the refresh token from the MSAL token cache.
+   * @see https://github.com/duolingo/metasearch/blob/3d782bba8c0068461acb442d89e7d555df5d0025/src/oauth.microsoft.ts#L42-L44
+   */
+  private extractMicrosoftRefreshTokenFromCache(): string {


While this is not ideal, it should do for now as the serialized token cache is an complex object, and it's not backwards compatible with our current approach which only stores the raw refresh_token, this would involve creating migration strategies and the other alternative, our old approach (raw HTTP request) wasn't that good either

...nt/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts

github-actions · 2026-01-06T15:54:32Z

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:37734

This environment will automatically shut down when the PR is closed or after 5 hours.

FelixMalfait · 2026-01-07T11:52:42Z

...nt/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts

+      const response = await msalClient.acquireTokenByRefreshToken({
+        refreshToken,
+        scopes: ['https://graph.microsoft.com/.default'],
+        forceCache: true,


Why do we force cache?

During my testing it didn't store the token in TokenCache if this was off

@neo773
This does not explain the why here:
Per documentation: Force MSAL to cache a refresh token flow response when there is no account in the cache. Used for migration scenarios.

Why do we need it?

FelixMalfait · 2026-01-07T11:57:19Z

...nt/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts

-        },
-      );
-      const responseData = response.data as MicrosoftRefreshTokenResponse;
+      const response = await msalClient.acquireTokenByRefreshToken({


Since this is risky maybe give a bit more details in your PR description, thanks!

I did add the info here
#16954 (comment)

TLDR;
Microsoft wants you to offload the token lifecycle to their new SDK with a cache storage layer for persisting data.
This data is a proprietary JSON blob and not compatible with our existing schema as it involves heavy changes.

I'm sorry, I don't understand the rational behind this PR, what are we migrating? why? could you add more details?

FelixMalfait

LGTM but please add details before merging to make sure everything is well considered thanks!

charlesBochet · 2026-01-08T11:53:52Z

Codewise it works but I don't get this change, please add more details why we need this

twenty-eng-sync · 2026-01-08T17:31:38Z

Hey @neo773! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

twenty-eng-sync · 2026-01-08T17:31:40Z

Hey @neo773! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

Migrate to azure/msal-node

0bb8cc9

greptile-apps bot reviewed Jan 6, 2026

View reviewed changes

sentry bot reviewed Jan 6, 2026

View reviewed changes

...nt/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts Show resolved Hide resolved

cubic-dev-ai bot reviewed Jan 6, 2026

View reviewed changes

fix refreshToken

26a5fdf

sentry bot reviewed Jan 6, 2026

View reviewed changes

...nt/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts Show resolved Hide resolved

Update microsoft-api-refresh-tokens.service.ts

0164ba5

neo773 commented Jan 6, 2026

View reviewed changes

fix nest

d4ecb69

sentry bot reviewed Jan 6, 2026

View reviewed changes

...nt/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service.ts Show resolved Hide resolved

FelixMalfait reviewed Jan 7, 2026

View reviewed changes

FelixMalfait approved these changes Jan 7, 2026

View reviewed changes

charlesBochet approved these changes Jan 8, 2026

View reviewed changes

charlesBochet added the -PR: awaiting author label Jan 8, 2026

charlesBochet assigned neo773 Jan 8, 2026

charlesBochet approved these changes Jan 8, 2026

View reviewed changes

charlesBochet added this pull request to the merge queue Jan 8, 2026

Merged via the queue into main with commit a5eae50 Jan 8, 2026
76 checks passed

charlesBochet deleted the fix-ms-oauth2-token-refres branch January 8, 2026 17:31

	import type { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
	import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';

Conversation

neo773 commented Jan 6, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

socket-security bot commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

neo773 Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jan 6, 2026

Uh oh!

FelixMalfait Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

neo773 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

charlesBochet Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

FelixMalfait Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neo773 Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

charlesBochet Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

FelixMalfait left a comment

Choose a reason for hiding this comment

Uh oh!

charlesBochet commented Jan 8, 2026

Uh oh!

Uh oh!

twenty-eng-sync bot commented Jan 8, 2026

Uh oh!

twenty-eng-sync bot commented Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

socket-security bot commented Jan 6, 2026 •

edited

Loading

cubic-dev-ai bot Jan 6, 2026 •

edited

Loading

FelixMalfait Jan 7, 2026 •

edited

Loading