Fix of broken API Auth (#8338)

Fix done this morning with @FelixMalfait  from #8295

---------

Co-authored-by: guillim <guillaume@twenty.com>
Co-authored-by: Félix Malfait <felix@twenty.com>
This commit is contained in:
Guillim 2024-11-06 14:45:33 +01:00 committed by GitHub
parent 24656e777e
commit 4b5d096441
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 309 additions and 67 deletions

View File

@ -7,6 +7,7 @@ FRONT_BASE_URL=http://localhost:3001
APP_SECRET=replace_me_with_a_random_string APP_SECRET=replace_me_with_a_random_string
SIGN_IN_PREFILLED=true SIGN_IN_PREFILLED=true
ACCESS_TOKEN_SECRET=replace_me_with_a_random_string_access
# ———————— Optional ———————— # ———————— Optional ————————
# PORT=3000 # PORT=3000
@ -14,7 +15,6 @@ SIGN_IN_PREFILLED=true
# DEBUG_PORT=9000 # DEBUG_PORT=9000
# ACCESS_TOKEN_EXPIRES_IN=30m # ACCESS_TOKEN_EXPIRES_IN=30m
# LOGIN_TOKEN_EXPIRES_IN=15m # LOGIN_TOKEN_EXPIRES_IN=15m
# API_TOKEN_EXPIRES_IN=1000y
# REFRESH_TOKEN_EXPIRES_IN=90d # REFRESH_TOKEN_EXPIRES_IN=90d
# FILE_TOKEN_EXPIRES_IN=1d # FILE_TOKEN_EXPIRES_IN=1d
# FRONT_AUTH_CALLBACK_URL=http://localhost:3001/verify # FRONT_AUTH_CALLBACK_URL=http://localhost:3001/verify

View File

@ -61,10 +61,14 @@ describe('ApiKeyService', () => {
expect(result).toEqual({ token: mockToken }); expect(result).toEqual({ token: mockToken });
expect(jwtWrapperService.sign).toHaveBeenCalledWith( expect(jwtWrapperService.sign).toHaveBeenCalledWith(
{ sub: workspaceId }, {
sub: workspaceId,
type: 'API_KEY',
workspaceId: workspaceId,
},
expect.objectContaining({ expect.objectContaining({
secret: 'mocked-secret', secret: 'mocked-secret',
expiresIn: '1h', expiresIn: '100y',
jwtid: apiKeyId, jwtid: apiKeyId,
}), }),
); );
@ -84,7 +88,11 @@ describe('ApiKeyService', () => {
await service.generateApiKeyToken(workspaceId, apiKeyId, expiresAt); await service.generateApiKeyToken(workspaceId, apiKeyId, expiresAt);
expect(jwtWrapperService.sign).toHaveBeenCalledWith( expect(jwtWrapperService.sign).toHaveBeenCalledWith(
{ sub: workspaceId }, {
sub: workspaceId,
type: 'API_KEY',
workspaceId: workspaceId,
},
expect.objectContaining({ expect.objectContaining({
secret: 'mocked-secret', secret: 'mocked-secret',
expiresIn: expect.any(Number), expiresIn: expect.any(Number),

View File

@ -21,6 +21,8 @@ export class ApiKeyService {
} }
const jwtPayload = { const jwtPayload = {
sub: workspaceId, sub: workspaceId,
type: 'API_KEY',
workspaceId,
}; };
const secret = this.jwtWrapperService.generateAppSecret( const secret = this.jwtWrapperService.generateAppSecret(
'ACCESS', 'ACCESS',
@ -33,7 +35,7 @@ export class ApiKeyService {
(new Date(expiresAt).getTime() - new Date().getTime()) / 1000, (new Date(expiresAt).getTime() - new Date().getTime()) / 1000,
); );
} else { } else {
expiresIn = this.environmentService.get('API_TOKEN_EXPIRES_IN'); expiresIn = '100y';
} }
const token = this.jwtWrapperService.sign(jwtPayload, { const token = this.jwtWrapperService.sign(jwtPayload, {
secret, secret,

View File

@ -0,0 +1,185 @@
import {
AuthException,
AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception';
import { JwtPayload } from 'src/engine/core-modules/auth/types/auth-context.type';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { JwtAuthStrategy } from './jwt.auth.strategy';
xdescribe('JwtAuthStrategy', () => {
let strategy: JwtAuthStrategy;
let workspaceRepository: any;
let userRepository: any;
let dataSourceService: any;
let typeORMService: any;
const jwt = {
sub: 'sub-default',
jti: 'jti-default',
};
workspaceRepository = {
findOneBy: jest.fn(async () => new Workspace()),
};
userRepository = {
findOne: jest.fn(async () => null),
};
// first we test the API_KEY case
it('should throw AuthException if type is API_KEY and workspace is not found', async () => {
const payload = {
...jwt,
type: 'API_KEY',
};
workspaceRepository = {
findOneBy: jest.fn(async () => null),
};
strategy = new JwtAuthStrategy(
{} as any,
{} as any,
typeORMService,
dataSourceService,
workspaceRepository,
{} as any,
);
await expect(strategy.validate(payload as JwtPayload)).rejects.toThrow(
new AuthException(
'Workspace not found',
AuthExceptionCode.WORKSPACE_NOT_FOUND,
),
);
});
it('should throw AuthExceptionCode if type is API_KEY not found', async () => {
const payload = {
...jwt,
type: 'API_KEY',
};
workspaceRepository = {
findOneBy: jest.fn(async () => new Workspace()),
};
dataSourceService = {
getLastDataSourceMetadataFromWorkspaceIdOrFail: jest.fn(async () => ({})),
};
typeORMService = {
connectToDataSource: jest.fn(async () => {}),
};
strategy = new JwtAuthStrategy(
{} as any,
{} as any,
typeORMService,
dataSourceService,
workspaceRepository,
{} as any,
);
await expect(strategy.validate(payload as JwtPayload)).rejects.toThrow(
new AuthException(
'This API Key is revoked',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
),
);
});
it('should be truthy if type is API_KEY and API_KEY is not revoked', async () => {
const payload = {
...jwt,
type: 'API_KEY',
};
workspaceRepository = {
findOneBy: jest.fn(async () => new Workspace()),
};
const mockDataSource = {
query: jest
.fn()
.mockResolvedValue([{ id: 'api-key-id', revokedAt: null }]),
};
jest
.spyOn(typeORMService, 'connectToDataSource')
.mockResolvedValue(mockDataSource as any);
strategy = new JwtAuthStrategy(
{} as any,
{} as any,
typeORMService,
dataSourceService,
workspaceRepository,
{} as any,
);
const result = await strategy.validate(payload as JwtPayload);
expect(result).toBeTruthy();
expect(result.apiKey?.id).toBe('api-key-id');
});
// second we test the ACCESS cases
it('should throw AuthExceptionCode if type is ACCESS, no jti, and user not found', async () => {
const payload = {
sub: 'sub-default',
type: 'ACCESS',
};
workspaceRepository = {
findOneBy: jest.fn(async () => new Workspace()),
};
userRepository = {
findOne: jest.fn(async () => null),
};
strategy = new JwtAuthStrategy(
{} as any,
{} as any,
typeORMService,
dataSourceService,
workspaceRepository,
userRepository,
);
await expect(strategy.validate(payload as JwtPayload)).rejects.toThrow(
new AuthException('User not found', AuthExceptionCode.INVALID_INPUT),
);
});
it('should be truthy if type is ACCESS, no jti, and user exist', async () => {
const payload = {
sub: 'sub-default',
type: 'ACCESS',
};
workspaceRepository = {
findOneBy: jest.fn(async () => new Workspace()),
};
userRepository = {
findOne: jest.fn(async () => ({ lastName: 'lastNameDefault' })),
};
strategy = new JwtAuthStrategy(
{} as any,
{} as any,
typeORMService,
dataSourceService,
workspaceRepository,
userRepository,
);
const user = await strategy.validate(payload as JwtPayload);
expect(user.user?.lastName).toBe('lastNameDefault');
});
});

View File

@ -10,7 +10,10 @@ import {
AuthException, AuthException,
AuthExceptionCode, AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception'; } from 'src/engine/core-modules/auth/auth.exception';
import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; import {
AuthContext,
JwtPayload,
} from 'src/engine/core-modules/auth/types/auth-context.type';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service'; import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service'; import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';
import { User } from 'src/engine/core-modules/user/user.entity'; import { User } from 'src/engine/core-modules/user/user.entity';
@ -18,13 +21,6 @@ import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service'; import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service';
import { ApiKeyWorkspaceEntity } from 'src/modules/api-key/standard-objects/api-key.workspace-entity'; import { ApiKeyWorkspaceEntity } from 'src/modules/api-key/standard-objects/api-key.workspace-entity';
export type JwtPayload = {
sub: string;
workspaceId: string;
workspaceMemberId?: string;
jti?: string;
};
@Injectable() @Injectable()
export class JwtAuthStrategy extends PassportStrategy(Strategy, 'jwt') { export class JwtAuthStrategy extends PassportStrategy(Strategy, 'jwt') {
constructor( constructor(
@ -59,13 +55,13 @@ export class JwtAuthStrategy extends PassportStrategy(Strategy, 'jwt') {
}); });
} }
async validate(payload: JwtPayload): Promise<AuthContext> { private async validateAPIKey(payload: JwtPayload): Promise<AuthContext> {
const workspace = await this.workspaceRepository.findOneBy({
id: payload.workspaceId ?? payload.sub,
});
let user: User | null = null;
let apiKey: ApiKeyWorkspaceEntity | null = null; let apiKey: ApiKeyWorkspaceEntity | null = null;
const workspace = await this.workspaceRepository.findOneBy({
id: payload['sub'],
});
if (!workspace) { if (!workspace) {
throw new AuthException( throw new AuthException(
'Workspace not found', 'Workspace not found',
@ -73,14 +69,6 @@ export class JwtAuthStrategy extends PassportStrategy(Strategy, 'jwt') {
); );
} }
if (payload.jti) {
// TODO: Check why it's not working
// const apiKeyRepository =
// await this.twentyORMGlobalManager.getRepositoryForWorkspace<ApiKeyWorkspaceEntity>(
// workspace.id,
// 'apiKey',
// );
const dataSourceMetadata = const dataSourceMetadata =
await this.dataSourceService.getLastDataSourceMetadataFromWorkspaceIdOrFail( await this.dataSourceService.getLastDataSourceMetadataFromWorkspaceIdOrFail(
workspace.id, workspace.id,
@ -102,9 +90,23 @@ export class JwtAuthStrategy extends PassportStrategy(Strategy, 'jwt') {
AuthExceptionCode.FORBIDDEN_EXCEPTION, AuthExceptionCode.FORBIDDEN_EXCEPTION,
); );
} }
return { apiKey, workspace };
}
private async validateAccessToken(payload: JwtPayload): Promise<AuthContext> {
let user: User | null = null;
const workspace = await this.workspaceRepository.findOneBy({
id: payload['workspaceId'],
});
if (!workspace) {
throw new AuthException(
'Workspace not found',
AuthExceptionCode.WORKSPACE_NOT_FOUND,
);
} }
if (payload.workspaceId) {
user = await this.userRepository.findOne({ user = await this.userRepository.findOne({
where: { id: payload.sub }, where: { id: payload.sub },
}); });
@ -114,11 +116,21 @@ export class JwtAuthStrategy extends PassportStrategy(Strategy, 'jwt') {
AuthExceptionCode.INVALID_INPUT, AuthExceptionCode.INVALID_INPUT,
); );
} }
return { user, workspace };
} }
// We don't check if the user is a member of the workspace yet async validate(payload: JwtPayload): Promise<AuthContext> {
const workspaceMemberId = payload.workspaceMemberId; const workspaceMemberId = payload.workspaceMemberId;
return { user, apiKey, workspace, workspaceMemberId }; if (!payload.type && !payload.workspaceId) {
return { ...(await this.validateAPIKey(payload)), workspaceMemberId };
}
if (payload.type === 'API_KEY') {
return { ...(await this.validateAPIKey(payload)), workspaceMemberId };
}
return { ...(await this.validateAccessToken(payload)), workspaceMemberId };
} }
} }

View File

@ -12,11 +12,11 @@ import {
AuthExceptionCode, AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception'; } from 'src/engine/core-modules/auth/auth.exception';
import { AuthToken } from 'src/engine/core-modules/auth/dto/token.entity'; import { AuthToken } from 'src/engine/core-modules/auth/dto/token.entity';
import { JwtAuthStrategy } from 'src/engine/core-modules/auth/strategies/jwt.auth.strategy';
import { import {
JwtAuthStrategy, AuthContext,
JwtPayload, JwtPayload,
} from 'src/engine/core-modules/auth/strategies/jwt.auth.strategy'; } from 'src/engine/core-modules/auth/types/auth-context.type';
import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service'; import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service'; import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';
import { User } from 'src/engine/core-modules/user/user.entity'; import { User } from 'src/engine/core-modules/user/user.entity';

View File

@ -1,3 +1,4 @@
import { WorkspaceTokenType } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';
import { User } from 'src/engine/core-modules/user/user.entity'; import { User } from 'src/engine/core-modules/user/user.entity';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { ApiKeyWorkspaceEntity } from 'src/modules/api-key/standard-objects/api-key.workspace-entity'; import { ApiKeyWorkspaceEntity } from 'src/modules/api-key/standard-objects/api-key.workspace-entity';
@ -8,3 +9,11 @@ export type AuthContext = {
workspaceMemberId?: string; workspaceMemberId?: string;
workspace: Workspace; workspace: Workspace;
}; };
export type JwtPayload = {
sub: string;
workspaceId: string;
workspaceMemberId?: string;
jti?: string;
type?: WorkspaceTokenType;
};

View File

@ -137,6 +137,10 @@ export class EnvironmentVariables {
@IsString() @IsString()
APP_SECRET: string; APP_SECRET: string;
@IsOptional()
@IsString()
ACCESS_TOKEN_SECRET: string;
@IsDuration() @IsDuration()
@IsOptional() @IsOptional()
ACCESS_TOKEN_EXPIRES_IN = '30m'; ACCESS_TOKEN_EXPIRES_IN = '30m';
@ -394,8 +398,6 @@ export class EnvironmentVariables {
}) })
REDIS_URL: string; REDIS_URL: string;
API_TOKEN_EXPIRES_IN = '100y';
SHORT_TERM_TOKEN_EXPIRES_IN = '5m'; SHORT_TERM_TOKEN_EXPIRES_IN = '5m';
@CastToBoolean() @CastToBoolean()

View File

@ -11,13 +11,14 @@ import {
} from 'src/engine/core-modules/auth/auth.exception'; } from 'src/engine/core-modules/auth/auth.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service'; import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
type WorkspaceTokenType = export type WorkspaceTokenType =
| 'ACCESS' | 'ACCESS'
| 'LOGIN' | 'LOGIN'
| 'REFRESH' | 'REFRESH'
| 'FILE' | 'FILE'
| 'POSTGRES_PROXY' | 'POSTGRES_PROXY'
| 'REMOTE_SERVER'; | 'REMOTE_SERVER'
| 'API_KEY';
@Injectable() @Injectable()
export class JwtWrapperService { export class JwtWrapperService {
@ -58,6 +59,13 @@ export class JwtWrapperService {
} }
try { try {
if (!type && !payload.workspaceId) {
return this.jwtService.verify(token, {
...options,
secret: this.generateAppSecretLegacy(type, payload.workspaceId),
});
}
return this.jwtService.verify(token, { return this.jwtService.verify(token, {
...options, ...options,
secret: this.generateAppSecret(type, payload.workspaceId), secret: this.generateAppSecret(type, payload.workspaceId),
@ -93,4 +101,21 @@ export class JwtWrapperService {
.update(`${appSecret}${workspaceId}${type}`) .update(`${appSecret}${workspaceId}${type}`)
.digest('hex'); .digest('hex');
} }
generateAppSecretLegacy(
type: WorkspaceTokenType,
workspaceId?: string,
): string {
const accessTokenSecret = this.environmentService.get(
'ACCESS_TOKEN_SECRET',
);
if (!accessTokenSecret) {
throw new Error('ACCESS_TOKEN_SECRET is not set');
}
return createHash('sha256')
.update(`${accessTokenSecret}${workspaceId}${type}`)
.digest('hex');
}
} }

View File

@ -1,14 +1,14 @@
import { import {
HostComponentInfo,
ContextId, ContextId,
ContextIdFactory, ContextIdFactory,
ContextIdStrategy, ContextIdStrategy,
HostComponentInfo,
} from '@nestjs/core'; } from '@nestjs/core';
import { jwtDecode } from 'jwt-decode';
import { Request } from 'express'; import { Request } from 'express';
import { jwtDecode } from 'jwt-decode';
import { JwtPayload } from 'src/engine/core-modules/auth/strategies/jwt.auth.strategy'; import { JwtPayload } from 'src/engine/core-modules/auth/types/auth-context.type';
const workspaces = new Map<string, ContextId>(); const workspaces = new Map<string, ContextId>();

View File

@ -57,7 +57,6 @@ yarn command:prod cron:calendar:calendar-event-list-fetch
['REFRESH_TOKEN_EXPIRES_IN', '90d', 'Refresh token expiration time'], ['REFRESH_TOKEN_EXPIRES_IN', '90d', 'Refresh token expiration time'],
['REFRESH_TOKEN_COOL_DOWN', '1m', 'Refresh token cooldown'], ['REFRESH_TOKEN_COOL_DOWN', '1m', 'Refresh token cooldown'],
['FILE_TOKEN_EXPIRES_IN', '1d', 'File token expiration time'], ['FILE_TOKEN_EXPIRES_IN', '1d', 'File token expiration time'],
['API_TOKEN_EXPIRES_IN', '1000y', 'API token expiration time'],
]}></ArticleTable> ]}></ArticleTable>
### Auth ### Auth