refactor(auth/sso): rename GetAuthorizationUrl for clarity (#10173)
- Rename `GetAuthorizationUrl` to `GetAuthorizationUrlForSSO` - Move `GetAuthorizationUrlForSSO` from `sso.resolver.ts` to `auth.resolver.ts` to avoid the permission guard and let users use an SSO provider. - Fix an issue in OIDC guard that breaks the connection if you have multiple SSO providers + add tests for OIDC guard.
This commit is contained in:
@ -11,6 +11,7 @@ import { UserService } from 'src/engine/core-modules/user/services/user.service'
|
||||
import { User } from 'src/engine/core-modules/user/user.entity';
|
||||
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
|
||||
import { PermissionsService } from 'src/engine/metadata-modules/permissions/permissions.service';
|
||||
import { SSOService } from 'src/engine/core-modules/sso/services/sso.service';
|
||||
|
||||
import { AuthResolver } from './auth.resolver';
|
||||
|
||||
@ -95,6 +96,10 @@ describe('AuthResolver', () => {
|
||||
provide: FeatureFlagService,
|
||||
useValue: {},
|
||||
},
|
||||
{
|
||||
provide: SSOService,
|
||||
useValue: {},
|
||||
},
|
||||
// {
|
||||
// provide: OAuthService,
|
||||
// useValue: {},
|
||||
|
||||
@ -4,6 +4,7 @@ import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import { SettingsFeatures, SOURCE_LOCALE } from 'twenty-shared';
|
||||
import { Repository } from 'typeorm';
|
||||
import omit from 'lodash.omit';
|
||||
|
||||
import { ApiKeyTokenInput } from 'src/engine/core-modules/auth/dto/api-key-token.input';
|
||||
import { AppTokenInput } from 'src/engine/core-modules/auth/dto/app-token.input';
|
||||
@ -47,6 +48,9 @@ import { SettingsPermissionsGuard } from 'src/engine/guards/settings-permissions
|
||||
import { UserAuthGuard } from 'src/engine/guards/user-auth.guard';
|
||||
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
|
||||
import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter';
|
||||
import { SSOService } from 'src/engine/core-modules/sso/services/sso.service';
|
||||
import { GetAuthorizationUrlForSSOOutput } from 'src/engine/core-modules/auth/dto/get-authorization-url-for-sso.output';
|
||||
import { GetAuthorizationUrlForSSOInput } from 'src/engine/core-modules/auth/dto/get-authorization-url-for-sso.input';
|
||||
|
||||
import { GetAuthTokensFromLoginTokenInput } from './dto/get-auth-tokens-from-login-token.input';
|
||||
import { GetLoginTokenFromCredentialsInput } from './dto/get-login-token-from-credentials.input';
|
||||
@ -77,6 +81,7 @@ export class AuthResolver {
|
||||
private domainManagerService: DomainManagerService,
|
||||
private userWorkspaceService: UserWorkspaceService,
|
||||
private emailVerificationTokenService: EmailVerificationTokenService,
|
||||
private sSOService: SSOService,
|
||||
) {}
|
||||
|
||||
@UseGuards(CaptchaGuard)
|
||||
@ -87,6 +92,16 @@ export class AuthResolver {
|
||||
return await this.authService.checkUserExists(checkUserExistsInput.email);
|
||||
}
|
||||
|
||||
@Mutation(() => GetAuthorizationUrlForSSOOutput)
|
||||
async getAuthorizationUrlForSSO(
|
||||
@Args('input') params: GetAuthorizationUrlForSSOInput,
|
||||
) {
|
||||
return await this.sSOService.getAuthorizationUrlForSSO(
|
||||
params.identityProviderId,
|
||||
omit(params, ['identityProviderId']),
|
||||
);
|
||||
}
|
||||
|
||||
@Query(() => WorkspaceInviteHashValid)
|
||||
async checkWorkspaceInviteHashIsValid(
|
||||
@Args() workspaceInviteHashValidInput: WorkspaceInviteHashValidInput,
|
||||
|
||||
@ -5,7 +5,7 @@ import { Field, InputType } from '@nestjs/graphql';
|
||||
import { IsOptional, IsString } from 'class-validator';
|
||||
|
||||
@InputType()
|
||||
export class GetAuthorizationUrlInput {
|
||||
export class GetAuthorizationUrlForSSOInput {
|
||||
@Field(() => String)
|
||||
@IsString()
|
||||
identityProviderId: string;
|
||||
@ -5,7 +5,7 @@ import { Field, ObjectType } from '@nestjs/graphql';
|
||||
import { SSOConfiguration } from 'src/engine/core-modules/sso/types/SSOConfigurations.type';
|
||||
|
||||
@ObjectType()
|
||||
export class GetAuthorizationUrlOutput {
|
||||
export class GetAuthorizationUrlForSSOOutput {
|
||||
@Field(() => String)
|
||||
authorizationURL: string;
|
||||
|
||||
@ -28,7 +28,9 @@ export class OIDCAuthGuard extends AuthGuard('openidconnect') {
|
||||
identityProviderId: string;
|
||||
} {
|
||||
if (request.params.identityProviderId) {
|
||||
return request.params.identityProviderId;
|
||||
return {
|
||||
identityProviderId: request.params.identityProviderId,
|
||||
};
|
||||
}
|
||||
|
||||
if (
|
||||
@ -57,6 +59,13 @@ export class OIDCAuthGuard extends AuthGuard('openidconnect') {
|
||||
try {
|
||||
const state = this.getStateByRequest(request);
|
||||
|
||||
if (!state.identityProviderId) {
|
||||
throw new AuthException(
|
||||
'identityProviderId missing',
|
||||
AuthExceptionCode.INVALID_DATA,
|
||||
);
|
||||
}
|
||||
|
||||
identityProvider = await this.sSOService.findSSOIdentityProviderById(
|
||||
state.identityProviderId,
|
||||
);
|
||||
@ -67,7 +76,6 @@ export class OIDCAuthGuard extends AuthGuard('openidconnect') {
|
||||
AuthExceptionCode.INVALID_DATA,
|
||||
);
|
||||
}
|
||||
|
||||
const issuer = await Issuer.discover(identityProvider.issuer);
|
||||
|
||||
new OIDCAuthStrategy(
|
||||
|
||||
@ -0,0 +1,130 @@
|
||||
import { ExecutionContext } from '@nestjs/common';
|
||||
import { Test, TestingModule } from '@nestjs/testing';
|
||||
import { AuthGuard } from '@nestjs/passport';
|
||||
|
||||
import { Issuer } from 'openid-client';
|
||||
|
||||
import { SSOService } from 'src/engine/core-modules/sso/services/sso.service';
|
||||
import { GuardRedirectService } from 'src/engine/core-modules/guard-redirect/services/guard-redirect.service';
|
||||
import { OIDCAuthGuard } from 'src/engine/core-modules/auth/guards/oidc-auth.guard';
|
||||
import { SSOConfiguration } from 'src/engine/core-modules/sso/types/SSOConfigurations.type';
|
||||
import { WorkspaceSSOIdentityProvider } from 'src/engine/core-modules/sso/workspace-sso-identity-provider.entity';
|
||||
|
||||
const createMockExecutionContext = (mockedRequest: any): ExecutionContext => {
|
||||
return {
|
||||
switchToHttp: jest.fn().mockReturnValue({
|
||||
getRequest: jest.fn().mockReturnValue(mockedRequest),
|
||||
}),
|
||||
} as unknown as ExecutionContext;
|
||||
};
|
||||
|
||||
const createMockedRequest = (params = {}, query = {}) => ({
|
||||
params,
|
||||
query,
|
||||
});
|
||||
|
||||
jest
|
||||
.spyOn(AuthGuard('openidconnect').prototype, 'canActivate')
|
||||
.mockResolvedValue(true);
|
||||
|
||||
jest.mock('openid-client', () => ({
|
||||
Strategy: jest.fn(),
|
||||
Issuer: {
|
||||
discover: jest.fn().mockResolvedValue({} as Issuer),
|
||||
},
|
||||
}));
|
||||
|
||||
describe('OIDCAuthGuard', () => {
|
||||
let guard: OIDCAuthGuard;
|
||||
let sSOService: SSOService;
|
||||
let guardRedirectService: GuardRedirectService;
|
||||
let mockExecutionContext: ExecutionContext;
|
||||
|
||||
beforeEach(async () => {
|
||||
const module: TestingModule = await Test.createTestingModule({
|
||||
providers: [
|
||||
OIDCAuthGuard,
|
||||
{
|
||||
provide: SSOService,
|
||||
useValue: {
|
||||
findSSOIdentityProviderById: jest.fn(),
|
||||
getOIDCClient: jest.fn(),
|
||||
},
|
||||
},
|
||||
{
|
||||
provide: GuardRedirectService,
|
||||
useValue: {
|
||||
dispatchErrorFromGuard: jest.fn(),
|
||||
getSubdomainAndCustomDomainFromWorkspace: jest.fn(),
|
||||
},
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
guard = module.get<OIDCAuthGuard>(OIDCAuthGuard);
|
||||
sSOService = module.get<SSOService>(SSOService);
|
||||
guardRedirectService =
|
||||
module.get<GuardRedirectService>(GuardRedirectService);
|
||||
|
||||
mockExecutionContext = {
|
||||
switchToHttp: jest.fn().mockReturnValue({
|
||||
getRequest: jest.fn(),
|
||||
}),
|
||||
} as unknown as ExecutionContext;
|
||||
});
|
||||
|
||||
it('should activate when identity provider exists and is valid', async () => {
|
||||
const mockedRequest = createMockedRequest({
|
||||
identityProviderId: 'test-id',
|
||||
});
|
||||
|
||||
mockExecutionContext = createMockExecutionContext(mockedRequest);
|
||||
|
||||
jest.spyOn(sSOService, 'findSSOIdentityProviderById').mockResolvedValue({
|
||||
id: 'test-id',
|
||||
issuer: 'https://issuer.example.com',
|
||||
workspace: {},
|
||||
} as SSOConfiguration & WorkspaceSSOIdentityProvider);
|
||||
|
||||
const result = await guard.canActivate(mockExecutionContext);
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(guardRedirectService.dispatchErrorFromGuard).not.toHaveBeenCalled();
|
||||
expect(sSOService.findSSOIdentityProviderById).toHaveBeenCalledWith(
|
||||
'test-id',
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw error when identity provider is not found', async () => {
|
||||
const mockedRequest = createMockedRequest({
|
||||
identityProviderId: 'non-existent-id',
|
||||
});
|
||||
|
||||
mockExecutionContext = createMockExecutionContext(mockedRequest);
|
||||
|
||||
jest
|
||||
.spyOn(sSOService, 'findSSOIdentityProviderById')
|
||||
.mockResolvedValue(null);
|
||||
|
||||
await expect(guard.canActivate(mockExecutionContext)).resolves.toBe(false);
|
||||
expect(sSOService.findSSOIdentityProviderById).toHaveBeenCalledWith(
|
||||
'non-existent-id',
|
||||
);
|
||||
expect(guardRedirectService.dispatchErrorFromGuard).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should handle invalid OIDC identity provider params in request', async () => {
|
||||
const mockedRequest = createMockedRequest({
|
||||
identityProviderId: undefined,
|
||||
});
|
||||
|
||||
mockExecutionContext = createMockExecutionContext(mockedRequest);
|
||||
|
||||
jest
|
||||
.spyOn(sSOService, 'findSSOIdentityProviderById')
|
||||
.mockResolvedValue(null);
|
||||
|
||||
await expect(guard.canActivate(mockExecutionContext)).resolves.toBe(false);
|
||||
expect(guardRedirectService.dispatchErrorFromGuard).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@ -198,7 +198,7 @@ export class SSOService {
|
||||
});
|
||||
}
|
||||
|
||||
async getAuthorizationUrl(
|
||||
async getAuthorizationUrlForSSO(
|
||||
identityProviderId: string,
|
||||
searchParams: Record<string, string | boolean>,
|
||||
) {
|
||||
|
||||
@ -3,7 +3,6 @@
|
||||
import { UseFilters, UseGuards } from '@nestjs/common';
|
||||
import { Args, Mutation, Query, Resolver } from '@nestjs/graphql';
|
||||
|
||||
import omit from 'lodash.omit';
|
||||
import { SettingsFeatures } from 'twenty-shared';
|
||||
|
||||
import { EnterpriseFeaturesEnabledGuard } from 'src/engine/core-modules/auth/guards/enterprise-features-enabled.guard';
|
||||
@ -12,8 +11,6 @@ import { DeleteSsoOutput } from 'src/engine/core-modules/sso/dtos/delete-sso.out
|
||||
import { EditSsoInput } from 'src/engine/core-modules/sso/dtos/edit-sso.input';
|
||||
import { EditSsoOutput } from 'src/engine/core-modules/sso/dtos/edit-sso.output';
|
||||
import { FindAvailableSSOIDPOutput } from 'src/engine/core-modules/sso/dtos/find-available-SSO-IDP.output';
|
||||
import { GetAuthorizationUrlInput } from 'src/engine/core-modules/sso/dtos/get-authorization-url.input';
|
||||
import { GetAuthorizationUrlOutput } from 'src/engine/core-modules/sso/dtos/get-authorization-url.output';
|
||||
import {
|
||||
SetupOIDCSsoInput,
|
||||
SetupSAMLSsoInput,
|
||||
@ -53,14 +50,6 @@ export class SSOResolver {
|
||||
return this.sSOService.listSSOIdentityProvidersByWorkspaceId(workspaceId);
|
||||
}
|
||||
|
||||
@Mutation(() => GetAuthorizationUrlOutput)
|
||||
async getAuthorizationUrl(@Args('input') params: GetAuthorizationUrlInput) {
|
||||
return await this.sSOService.getAuthorizationUrl(
|
||||
params.identityProviderId,
|
||||
omit(params, ['identityProviderId']),
|
||||
);
|
||||
}
|
||||
|
||||
@UseGuards(WorkspaceAuthGuard, EnterpriseFeaturesEnabledGuard)
|
||||
@Mutation(() => SetupSsoOutput)
|
||||
async createSAMLIdentityProvider(
|
||||
|
||||
Reference in New Issue
Block a user