houseassassin/twenty
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(server): encrypt token post refresh (#20819)

Browse Source 
# Introduction
Jobs were refreshing token and returning them as plain text, resulting
to underlying code flow failure as expecting encrypted tokens

## Next
We should define a strong typescript signature to avoid such things to
happen again, or least have an explicit naming
This commit is contained in:
Paul Rastoin
2026-05-21 18:53:03 +02:00
committed by GitHub
parent 0bfcd9a701
commit 3c91f3f276
4 changed files with 84 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/twenty-server/src/modules/connected-account/refresh-tokens-manager/services/connected-account-refresh-tokens.service.spec.ts
+54 -55
View File
@@ -12,7 +12,6 @@ import {
ConnectedAccountRefreshAccessTokenExceptionCode,
} from 'src/engine/metadata-modules/connected-account/exceptions/connected-account-refresh-tokens.exception';
import { ConnectedAccountTokenEncryptionService } from 'src/engine/metadata-modules/connected-account/services/connected-account-token-encryption.service';
import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager';
import { GoogleAPIRefreshAccessTokenService } from 'src/modules/connected-account/refresh-tokens-manager/drivers/google/services/google-api-refresh-tokens.service';
import { MicrosoftAPIRefreshAccessTokenService } from 'src/modules/connected-account/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service';
@@ -106,15 +105,6 @@ describe('ConnectedAccountRefreshTokensService', () => {
refreshTokens: jest.fn(),
},
},
{
provide: GlobalWorkspaceOrmManager,
useValue: {
executeInWorkspaceContext: jest
.fn()
.mockImplementation((fn: () => any, _authContext?: any) => fn()),
},
},
{
provide: getRepositoryToken(ConnectedAccountEntity),
useValue: {
@@ -149,7 +139,7 @@ describe('ConnectedAccountRefreshTokensService', () => {
});
describe('resolveTokens', () => {
it('should reuse the cached token, decrypt before returning to the caller, and skip the refresh call entirely', async () => {
it('should reuse the cached encrypted tokens as-is when valid, skipping decrypt and the refresh call entirely', async () => {
const connectedAccount = {
id: mockConnectedAccountId,
provider: ConnectedAccountProvider.MICROSOFT,
@@ -164,28 +154,19 @@ describe('ConnectedAccountRefreshTokensService', () => {
);
expect(result).toEqual({
accessToken: mockAccessTokenPlaintext,
refreshToken: mockRefreshTokenPlaintext,
accessToken: mockEncryptedAccessToken,
refreshToken: mockEncryptedRefreshToken,
});
expect(
connectedAccountTokenEncryptionService.decrypt,
).toHaveBeenCalledWith({
ciphertext: mockEncryptedAccessToken,
workspaceId: mockWorkspaceId,
});
expect(
connectedAccountTokenEncryptionService.decrypt,
).toHaveBeenCalledWith({
ciphertext: mockEncryptedRefreshToken,
workspaceId: mockWorkspaceId,
});
).not.toHaveBeenCalled();
expect(
microsoftAPIRefreshAccessTokenService.refreshTokens,
).not.toHaveBeenCalled();
expect(connectedAccountRepository.update).not.toHaveBeenCalled();
});
it('should decrypt the stored refresh token before sending to Microsoft, then re-encrypt the rotated tokens before persisting', async () => {
it('should decrypt the stored refresh token before sending to Microsoft, persist the re-encrypted tokens, and return them encrypted', async () => {
const connectedAccount = {
id: mockConnectedAccountId,
provider: ConnectedAccountProvider.MICROSOFT,
@@ -194,35 +175,41 @@ describe('ConnectedAccountRefreshTokensService', () => {
lastCredentialsRefreshedAt: new Date(Date.now() - 2 * 60 * 60 * 1000), // 2 hours ago
} as ConnectedAccountEntity;
const newTokens = {
const newPlaintextTokens = {
accessToken: mockNewAccessTokenPlaintext,
refreshToken: mockRefreshTokenPlaintext,
};
jest
.spyOn(microsoftAPIRefreshAccessTokenService, 'refreshTokens')
.mockResolvedValue(newTokens);
.mockResolvedValue(newPlaintextTokens);
const result = await service.resolveTokens(
connectedAccount,
mockWorkspaceId,
);
expect(result).toEqual(newTokens);
const expectedEncryptedNewAccessToken = `${FAKE_CIPHER_PREFIX}CIPHER(${mockNewAccessTokenPlaintext})`;
const expectedEncryptedNewRefreshToken = `${FAKE_CIPHER_PREFIX}CIPHER(${mockRefreshTokenPlaintext})`;
expect(result).toEqual({
accessToken: expectedEncryptedNewAccessToken,
refreshToken: expectedEncryptedNewRefreshToken,
});
expect(
microsoftAPIRefreshAccessTokenService.refreshTokens,
).toHaveBeenCalledWith(mockRefreshTokenPlaintext);
expect(connectedAccountRepository.update).toHaveBeenCalledWith(
{ id: mockConnectedAccountId, workspaceId: mockWorkspaceId },
expect.objectContaining({
accessToken: `${FAKE_CIPHER_PREFIX}CIPHER(${mockNewAccessTokenPlaintext})`,
refreshToken: `${FAKE_CIPHER_PREFIX}CIPHER(${mockRefreshTokenPlaintext})`,
accessToken: expectedEncryptedNewAccessToken,
refreshToken: expectedEncryptedNewRefreshToken,
lastCredentialsRefreshedAt: expect.any(Date),
}),
);
});
it('should decrypt the stored refresh token before sending to Google, then re-encrypt the rotated tokens before persisting', async () => {
it('should decrypt the stored refresh token before sending to Google, persist the re-encrypted tokens, and return them encrypted', async () => {
const connectedAccount = {
id: mockConnectedAccountId,
provider: ConnectedAccountProvider.GOOGLE,
@@ -231,29 +218,35 @@ describe('ConnectedAccountRefreshTokensService', () => {
lastCredentialsRefreshedAt: new Date(Date.now() - 2 * 60 * 60 * 1000), // 2 hours ago
} as ConnectedAccountEntity;
const newTokens = {
const newPlaintextTokens = {
accessToken: mockNewAccessTokenPlaintext,
refreshToken: mockRefreshTokenPlaintext,
};
jest
.spyOn(googleAPIRefreshAccessTokenService, 'refreshTokens')
.mockResolvedValue(newTokens);
.mockResolvedValue(newPlaintextTokens);
const result = await service.resolveTokens(
connectedAccount,
mockWorkspaceId,
);
expect(result).toEqual(newTokens);
const expectedEncryptedNewAccessToken = `${FAKE_CIPHER_PREFIX}CIPHER(${mockNewAccessTokenPlaintext})`;
const expectedEncryptedNewRefreshToken = `${FAKE_CIPHER_PREFIX}CIPHER(${mockRefreshTokenPlaintext})`;
expect(result).toEqual({
accessToken: expectedEncryptedNewAccessToken,
refreshToken: expectedEncryptedNewRefreshToken,
});
expect(
googleAPIRefreshAccessTokenService.refreshTokens,
).toHaveBeenCalledWith(mockRefreshTokenPlaintext);
expect(connectedAccountRepository.update).toHaveBeenCalledWith(
{ id: mockConnectedAccountId, workspaceId: mockWorkspaceId },
expect.objectContaining({
accessToken: `${FAKE_CIPHER_PREFIX}CIPHER(${mockNewAccessTokenPlaintext})`,
refreshToken: `${FAKE_CIPHER_PREFIX}CIPHER(${mockRefreshTokenPlaintext})`,
accessToken: expectedEncryptedNewAccessToken,
refreshToken: expectedEncryptedNewRefreshToken,
lastCredentialsRefreshedAt: expect.any(Date),
}),
);
@@ -268,35 +261,41 @@ describe('ConnectedAccountRefreshTokensService', () => {
lastCredentialsRefreshedAt: null,
} as ConnectedAccountEntity;
const newTokens = {
const newPlaintextTokens = {
accessToken: mockNewAccessTokenPlaintext,
refreshToken: mockRefreshTokenPlaintext,
};
jest
.spyOn(microsoftAPIRefreshAccessTokenService, 'refreshTokens')
.mockResolvedValue(newTokens);
.mockResolvedValue(newPlaintextTokens);
const result = await service.resolveTokens(
connectedAccount,
mockWorkspaceId,
);
expect(result).toEqual(newTokens);
const expectedEncryptedNewAccessToken = `${FAKE_CIPHER_PREFIX}CIPHER(${mockNewAccessTokenPlaintext})`;
const expectedEncryptedNewRefreshToken = `${FAKE_CIPHER_PREFIX}CIPHER(${mockRefreshTokenPlaintext})`;
expect(result).toEqual({
accessToken: expectedEncryptedNewAccessToken,
refreshToken: expectedEncryptedNewRefreshToken,
});
expect(
microsoftAPIRefreshAccessTokenService.refreshTokens,
).toHaveBeenCalledWith(mockRefreshTokenPlaintext);
expect(connectedAccountRepository.update).toHaveBeenCalledWith(
{ id: mockConnectedAccountId, workspaceId: mockWorkspaceId },
expect.objectContaining({
accessToken: `${FAKE_CIPHER_PREFIX}CIPHER(${mockNewAccessTokenPlaintext})`,
refreshToken: `${FAKE_CIPHER_PREFIX}CIPHER(${mockRefreshTokenPlaintext})`,
accessToken: expectedEncryptedNewAccessToken,
refreshToken: expectedEncryptedNewRefreshToken,
lastCredentialsRefreshedAt: expect.any(Date),
}),
);
});
it('should return decrypted access token and null refresh token when access token is valid but no refresh token exists', async () => {
it('should return the encrypted access token and null refresh token when access token is valid but no refresh token exists', async () => {
const connectedAccount = {
id: mockConnectedAccountId,
provider: ConnectedAccountProvider.APP,
@@ -311,18 +310,12 @@ describe('ConnectedAccountRefreshTokensService', () => {
);
expect(result).toEqual({
accessToken: mockAccessTokenPlaintext,
accessToken: mockEncryptedAccessToken,
refreshToken: null,
});
expect(
connectedAccountTokenEncryptionService.decrypt,
).toHaveBeenCalledWith({
ciphertext: mockEncryptedAccessToken,
workspaceId: mockWorkspaceId,
});
expect(
connectedAccountTokenEncryptionService.decrypt,
).toHaveBeenCalledTimes(1);
).not.toHaveBeenCalled();
expect(connectedAccountRepository.update).not.toHaveBeenCalled();
});
@@ -474,7 +467,7 @@ describe('ConnectedAccountRefreshTokensService', () => {
});
describe('resolveTokens - OIDC/SAML', () => {
it('should decrypt and return existing tokens for OIDC without attempting a refresh', async () => {
it('should return existing encrypted tokens for OIDC as-is without attempting a refresh', async () => {
const connectedAccount = {
id: mockConnectedAccountId,
provider: ConnectedAccountProvider.OIDC,
@@ -489,9 +482,12 @@ describe('ConnectedAccountRefreshTokensService', () => {
);
expect(result).toEqual({
accessToken: mockAccessTokenPlaintext,
refreshToken: mockRefreshTokenPlaintext,
accessToken: mockEncryptedAccessToken,
refreshToken: mockEncryptedRefreshToken,
});
expect(
connectedAccountTokenEncryptionService.decrypt,
).not.toHaveBeenCalled();
expect(
googleAPIRefreshAccessTokenService.refreshTokens,
).not.toHaveBeenCalled();
@@ -501,7 +497,7 @@ describe('ConnectedAccountRefreshTokensService', () => {
expect(connectedAccountRepository.update).not.toHaveBeenCalled();
});
it('should decrypt and return existing tokens for SAML without attempting a refresh', async () => {
it('should return existing encrypted tokens for SAML as-is without attempting a refresh', async () => {
const connectedAccount = {
id: mockConnectedAccountId,
provider: ConnectedAccountProvider.SAML,
@@ -516,9 +512,12 @@ describe('ConnectedAccountRefreshTokensService', () => {
);
expect(result).toEqual({
accessToken: mockAccessTokenPlaintext,
refreshToken: mockRefreshTokenPlaintext,
accessToken: mockEncryptedAccessToken,
refreshToken: mockEncryptedRefreshToken,
});
expect(
connectedAccountTokenEncryptionService.decrypt,
).not.toHaveBeenCalled();
expect(
googleAPIRefreshAccessTokenService.refreshTokens,
).not.toHaveBeenCalled();
packages/twenty-server/src/modules/connected-account/refresh-tokens-manager/services/connected-account-refresh-tokens.service.ts
+21 -38
View File
@@ -12,8 +12,6 @@ import {
ConnectedAccountRefreshAccessTokenExceptionCode,
} from 'src/engine/metadata-modules/connected-account/exceptions/connected-account-refresh-tokens.exception';
import { ConnectedAccountTokenEncryptionService } from 'src/engine/metadata-modules/connected-account/services/connected-account-token-encryption.service';
import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager';
import { buildSystemAuthContext } from 'src/engine/twenty-orm/utils/build-system-auth-context.util';
import { GoogleAPIRefreshAccessTokenService } from 'src/modules/connected-account/refresh-tokens-manager/drivers/google/services/google-api-refresh-tokens.service';
import { MicrosoftAPIRefreshAccessTokenService } from 'src/modules/connected-account/refresh-tokens-manager/drivers/microsoft/services/microsoft-api-refresh-tokens.service';
@@ -34,7 +32,6 @@ export class ConnectedAccountRefreshTokensService {
private readonly googleAPIRefreshAccessTokenService: GoogleAPIRefreshAccessTokenService,
private readonly microsoftAPIRefreshAccessTokenService: MicrosoftAPIRefreshAccessTokenService,
private readonly appOAuthRefreshAccessTokenService: AppOAuthRefreshAccessTokenService,
private readonly globalWorkspaceOrmManager: GlobalWorkspaceOrmManager,
private readonly connectedAccountTokenEncryptionService: ConnectedAccountTokenEncryptionService,
@InjectRepository(ConnectedAccountEntity)
private readonly connectedAccountRepository: Repository<ConnectedAccountEntity>,
@@ -52,7 +49,7 @@ export class ConnectedAccountRefreshTokensService {
`Reusing valid access token for connected account ${connectedAccount.id.slice(0, 7)} in workspace ${workspaceId.slice(0, 7)}`,
);
return this.decryptExistingTokens(connectedAccount, workspaceId);
return this.getExistingEncryptedTokens(connectedAccount, workspaceId);
}
const encryptedRefreshToken = connectedAccount.refreshToken;
@@ -75,16 +72,11 @@ export class ConnectedAccountRefreshTokensService {
);
}
private decryptExistingTokens(
private getExistingEncryptedTokens(
connectedAccount: ConnectedAccountEntity,
workspaceId: string,
): ConnectedAccountTokens {
const {
accessToken: encryptedAccessToken,
refreshToken: encryptedRefreshToken,
} = connectedAccount;
if (!isDefined(encryptedAccessToken)) {
if (!isDefined(connectedAccount.accessToken)) {
throw new ConnectedAccountRefreshAccessTokenException(
`Access token is required for connected account ${connectedAccount.id} in workspace ${workspaceId}`,
ConnectedAccountRefreshAccessTokenExceptionCode.ACCESS_TOKEN_NOT_FOUND,
@@ -92,16 +84,8 @@ export class ConnectedAccountRefreshTokensService {
}
return {
accessToken: this.connectedAccountTokenEncryptionService.decrypt({
ciphertext: encryptedAccessToken,
workspaceId,
}),
refreshToken: isDefined(encryptedRefreshToken)
? this.connectedAccountTokenEncryptionService.decrypt({
ciphertext: encryptedRefreshToken,
workspaceId,
})
: null,
accessToken: connectedAccount.accessToken,
refreshToken: connectedAccount.refreshToken,
};
}
@@ -116,35 +100,34 @@ export class ConnectedAccountRefreshTokensService {
workspaceId,
});
const connectedAccountTokens = await this.refreshTokens(
const plaintextTokens = await this.refreshTokens(
connectedAccount,
decryptedRefreshToken,
workspaceId,
);
const {
encryptedAccessToken: reEncryptedAccessToken,
encryptedAccessToken,
encryptedRefreshToken: reEncryptedRefreshToken,
} = this.connectedAccountTokenEncryptionService.encryptTokenPair({
accessToken: connectedAccountTokens.accessToken,
refreshToken: connectedAccountTokens.refreshToken,
accessToken: plaintextTokens.accessToken,
refreshToken: plaintextTokens.refreshToken,
workspaceId,
});
const authContext = buildSystemAuthContext(workspaceId);
await this.connectedAccountRepository.update(
{ id: connectedAccount.id, workspaceId },
{
accessToken: encryptedAccessToken,
refreshToken: reEncryptedRefreshToken,
lastCredentialsRefreshedAt: new Date(),
},
);
await this.globalWorkspaceOrmManager.executeInWorkspaceContext(async () => {
await this.connectedAccountRepository.update(
{ id: connectedAccount.id, workspaceId },
{
accessToken: reEncryptedAccessToken,
refreshToken: reEncryptedRefreshToken,
lastCredentialsRefreshedAt: new Date(),
},
);
}, authContext);
return connectedAccountTokens;
return {
accessToken: encryptedAccessToken,
refreshToken: reEncryptedRefreshToken,
};
}
async isAccessTokenStillValid(