From ef997be2196ed0eedb41d52e9d847c95ebe597ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20S=C3=B3jko?= Date: Wed, 25 Jan 2023 13:23:56 +0100 Subject: [PATCH] refactor: generating authentication options --- .../Controller/v1/AuthenticatorsController.ts | 17 +++---- packages/auth/src/Bootstrap/Container.ts | 3 +- .../Controller/AuthenticatorsController.ts | 31 +----------- ...AuthenticatorAuthenticationOptions.spec.ts | 50 +++++++++++++++++-- ...erateAuthenticatorAuthenticationOptions.ts | 35 ++++++++++++- ...teAuthenticatorAuthenticationOptionsDTO.ts | 2 +- ...catorAuthenticationOptionsRequestParams.ts | 2 +- ...nversifyExpressAuthenticatorsController.ts | 26 +++------- 8 files changed, 97 insertions(+), 69 deletions(-) diff --git a/packages/api-gateway/src/Controller/v1/AuthenticatorsController.ts b/packages/api-gateway/src/Controller/v1/AuthenticatorsController.ts index 4478a96b6..f56749da5 100644 --- a/packages/api-gateway/src/Controller/v1/AuthenticatorsController.ts +++ b/packages/api-gateway/src/Controller/v1/AuthenticatorsController.ts @@ -5,13 +5,13 @@ import { controller, BaseHttpController, httpPost, httpGet, httpDelete } from 'i import TYPES from '../../Bootstrap/Types' import { HttpServiceInterface } from '../../Service/Http/HttpServiceInterface' -@controller('/v1/authenticators', TYPES.AuthMiddleware) +@controller('/v1/authenticators') export class AuthenticatorsController extends BaseHttpController { constructor(@inject(TYPES.HTTPService) private httpService: HttpServiceInterface) { super() } - @httpDelete('/:authenticatorId') + @httpDelete('/:authenticatorId', TYPES.AuthMiddleware) async delete(request: Request, response: Response): Promise { await this.httpService.callAuthServer( request, @@ -21,12 +21,12 @@ export class AuthenticatorsController extends BaseHttpController { ) } - @httpGet('/') + @httpGet('/', TYPES.AuthMiddleware) async list(request: Request, response: Response): Promise { await this.httpService.callAuthServer(request, response, 'authenticators/', request.body) } - @httpGet('/generate-registration-options') + @httpGet('/generate-registration-options', TYPES.AuthMiddleware) async generateRegistrationOptions(request: Request, response: Response): Promise { await this.httpService.callAuthServer( request, @@ -36,7 +36,7 @@ export class AuthenticatorsController extends BaseHttpController { ) } - @httpGet('/generate-authentication-options') + @httpPost('/generate-authentication-options') async generateAuthenticationOptions(request: Request, response: Response): Promise { await this.httpService.callAuthServer( request, @@ -46,13 +46,8 @@ export class AuthenticatorsController extends BaseHttpController { ) } - @httpPost('/verify-registration') + @httpPost('/verify-registration', TYPES.AuthMiddleware) async verifyRegistration(request: Request, response: Response): Promise { await this.httpService.callAuthServer(request, response, 'authenticators/verify-registration', request.body) } - - @httpPost('/verify-authentication') - async verifyAuthentication(request: Request, response: Response): Promise { - await this.httpService.callAuthServer(request, response, 'authenticators/verify-authentication', request.body) - } } diff --git a/packages/auth/src/Bootstrap/Container.ts b/packages/auth/src/Bootstrap/Container.ts index ada21f1ee..47474228e 100644 --- a/packages/auth/src/Bootstrap/Container.ts +++ b/packages/auth/src/Bootstrap/Container.ts @@ -589,8 +589,10 @@ export class ContainerConfigLoader { .bind(TYPES.GenerateAuthenticatorAuthenticationOptions) .toConstantValue( new GenerateAuthenticatorAuthenticationOptions( + container.get(TYPES.UserRepository), container.get(TYPES.AuthenticatorRepository), container.get(TYPES.AuthenticatorChallengeRepository), + container.get(TYPES.PSEUDO_KEY_PARAMS_KEY), ), ) container @@ -708,7 +710,6 @@ export class ContainerConfigLoader { container.get(TYPES.GenerateAuthenticatorRegistrationOptions), container.get(TYPES.VerifyAuthenticatorRegistrationResponse), container.get(TYPES.GenerateAuthenticatorAuthenticationOptions), - container.get(TYPES.VerifyAuthenticatorAuthenticationResponse), container.get(TYPES.ListAuthenticators), container.get(TYPES.DeleteAuthenticator), container.get(TYPES.AuthenticatorHttpMapper), diff --git a/packages/auth/src/Controller/AuthenticatorsController.ts b/packages/auth/src/Controller/AuthenticatorsController.ts index 059a25e56..16fe1f9ec 100644 --- a/packages/auth/src/Controller/AuthenticatorsController.ts +++ b/packages/auth/src/Controller/AuthenticatorsController.ts @@ -6,20 +6,17 @@ import { DeleteAuthenticator } from '../Domain/UseCase/DeleteAuthenticator/Delet import { GenerateAuthenticatorAuthenticationOptions } from '../Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptions' import { GenerateAuthenticatorRegistrationOptions } from '../Domain/UseCase/GenerateAuthenticatorRegistrationOptions/GenerateAuthenticatorRegistrationOptions' import { ListAuthenticators } from '../Domain/UseCase/ListAuthenticators/ListAuthenticators' -import { VerifyAuthenticatorAuthenticationResponse } from '../Domain/UseCase/VerifyAuthenticatorAuthenticationResponse/VerifyAuthenticatorAuthenticationResponse' import { VerifyAuthenticatorRegistrationResponse } from '../Domain/UseCase/VerifyAuthenticatorRegistrationResponse/VerifyAuthenticatorRegistrationResponse' import { AuthenticatorHttpProjection } from '../Infra/Http/Projection/AuthenticatorHttpProjection' import { DeleteAuthenticatorRequestParams } from '../Infra/Http/Request/DeleteAuthenticatorRequestParams' import { GenerateAuthenticatorAuthenticationOptionsRequestParams } from '../Infra/Http/Request/GenerateAuthenticatorAuthenticationOptionsRequestParams' import { GenerateAuthenticatorRegistrationOptionsRequestParams } from '../Infra/Http/Request/GenerateAuthenticatorRegistrationOptionsRequestParams' import { ListAuthenticatorsRequestParams } from '../Infra/Http/Request/ListAuthenticatorsRequestParams' -import { VerifyAuthenticatorAuthenticationResponseRequestParams } from '../Infra/Http/Request/VerifyAuthenticatorAuthenticationResponseRequestParams' import { VerifyAuthenticatorRegistrationResponseRequestParams } from '../Infra/Http/Request/VerifyAuthenticatorRegistrationResponseRequestParams' import { DeleteAuthenticatorResponse } from '../Infra/Http/Response/DeleteAuthenticatorResponse' import { GenerateAuthenticatorAuthenticationOptionsResponse } from '../Infra/Http/Response/GenerateAuthenticatorAuthenticationOptionsResponse' import { GenerateAuthenticatorRegistrationOptionsResponse } from '../Infra/Http/Response/GenerateAuthenticatorRegistrationOptionsResponse' import { ListAuthenticatorsResponse } from '../Infra/Http/Response/ListAuthenticatorsResponse' -import { VerifyAuthenticatorAuthenticationResponseResponse } from '../Infra/Http/Response/VerifyAuthenticatorAuthenticationResponseResponse' import { VerifyAuthenticatorRegistrationResponseResponse } from '../Infra/Http/Response/VerifyAuthenticatorRegistrationResponseResponse' export class AuthenticatorsController { @@ -27,7 +24,6 @@ export class AuthenticatorsController { private generateAuthenticatorRegistrationOptions: GenerateAuthenticatorRegistrationOptions, private verifyAuthenticatorRegistrationResponse: VerifyAuthenticatorRegistrationResponse, private generateAuthenticatorAuthenticationOptions: GenerateAuthenticatorAuthenticationOptions, - private verifyAuthenticatorAuthenticationResponse: VerifyAuthenticatorAuthenticationResponse, private listAuthenticators: ListAuthenticators, private deleteAuthenticator: DeleteAuthenticator, private authenticatorHttpMapper: MapperInterface, @@ -117,7 +113,7 @@ export class AuthenticatorsController { params: GenerateAuthenticatorAuthenticationOptionsRequestParams, ): Promise { const result = await this.generateAuthenticatorAuthenticationOptions.execute({ - userUuid: params.userUuid, + username: params.username, }) if (result.isFailed()) { @@ -136,29 +132,4 @@ export class AuthenticatorsController { data: { options: result.getValue() }, } } - - async verifyAuthenticationResponse( - params: VerifyAuthenticatorAuthenticationResponseRequestParams, - ): Promise { - const result = await this.verifyAuthenticatorAuthenticationResponse.execute({ - userUuid: params.userUuid, - authenticatorResponse: params.authenticatorResponse, - }) - - if (result.isFailed()) { - return { - status: HttpStatusCode.Unauthorized, - data: { - error: { - message: result.getError(), - }, - }, - } - } - - return { - status: HttpStatusCode.Success, - data: { success: result.getValue() }, - } - } } diff --git a/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptions.spec.ts b/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptions.spec.ts index f0c28f12d..2b9644cef 100644 --- a/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptions.spec.ts +++ b/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptions.spec.ts @@ -4,14 +4,22 @@ import { Authenticator } from '../../Authenticator/Authenticator' import { AuthenticatorChallenge } from '../../Authenticator/AuthenticatorChallenge' import { AuthenticatorChallengeRepositoryInterface } from '../../Authenticator/AuthenticatorChallengeRepositoryInterface' import { AuthenticatorRepositoryInterface } from '../../Authenticator/AuthenticatorRepositoryInterface' +import { User } from '../../User/User' +import { UserRepositoryInterface } from '../../User/UserRepositoryInterface' import { GenerateAuthenticatorAuthenticationOptions } from './GenerateAuthenticatorAuthenticationOptions' describe('GenerateAuthenticatorAuthenticationOptions', () => { + let userRepository: UserRepositoryInterface let authenticatorRepository: AuthenticatorRepositoryInterface let authenticatorChallengeRepository: AuthenticatorChallengeRepositoryInterface const createUseCase = () => - new GenerateAuthenticatorAuthenticationOptions(authenticatorRepository, authenticatorChallengeRepository) + new GenerateAuthenticatorAuthenticationOptions( + userRepository, + authenticatorRepository, + authenticatorChallengeRepository, + 'pseudo-key-params-key', + ) beforeEach(() => { const authenticator = Authenticator.create({ @@ -31,13 +39,33 @@ describe('GenerateAuthenticatorAuthenticationOptions', () => { authenticatorChallengeRepository = {} as jest.Mocked authenticatorChallengeRepository.save = jest.fn() + + userRepository = {} as jest.Mocked + userRepository.findOneByEmail = jest.fn().mockReturnValue({ + uuid: '00000000-0000-0000-0000-000000000000', + } as jest.Mocked) }) - it('should return error if userUuid is invalid', async () => { + it('should return error if username is invalid', async () => { const useCase = createUseCase() const result = await useCase.execute({ - userUuid: 'invalid', + username: '', + }) + + expect(result.isFailed()).toBe(true) + expect(result.getError()).toBe('Could not generate authenticator registration options: Username cannot be empty') + }) + + it('should return error if user uuid is not valid', async () => { + userRepository.findOneByEmail = jest.fn().mockReturnValue({ + uuid: 'invalid', + } as jest.Mocked) + + const useCase = createUseCase() + + const result = await useCase.execute({ + username: 'test@test.te', }) expect(result.isFailed()).toBe(true) @@ -46,6 +74,18 @@ describe('GenerateAuthenticatorAuthenticationOptions', () => { ) }) + it('should return pseudo options if user is not found', async () => { + userRepository.findOneByEmail = jest.fn().mockReturnValue(null) + + const useCase = createUseCase() + + const result = await useCase.execute({ + username: 'test@test.te', + }) + + expect(result.isFailed()).toBe(false) + }) + it('should return error if authenticator challenge is invalid', async () => { const mock = jest.spyOn(AuthenticatorChallenge, 'create') mock.mockReturnValue(Result.fail('Oops')) @@ -53,7 +93,7 @@ describe('GenerateAuthenticatorAuthenticationOptions', () => { const useCase = createUseCase() const result = await useCase.execute({ - userUuid: '00000000-0000-0000-0000-000000000000', + username: 'test@test.te', }) expect(result.isFailed()).toBe(true) @@ -66,7 +106,7 @@ describe('GenerateAuthenticatorAuthenticationOptions', () => { const useCase = createUseCase() const result = await useCase.execute({ - userUuid: '00000000-0000-0000-0000-000000000000', + username: 'test@test.te', }) expect(result.isFailed()).toBe(false) diff --git a/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptions.ts b/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptions.ts index 699608d18..21da7bd95 100644 --- a/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptions.ts +++ b/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptions.ts @@ -1,19 +1,50 @@ -import { Result, UseCaseInterface, Uuid } from '@standardnotes/domain-core' +import * as crypto from 'crypto' +import { Result, UseCaseInterface, Username, Uuid } from '@standardnotes/domain-core' import { generateAuthenticationOptions } from '@simplewebauthn/server' import { GenerateAuthenticatorAuthenticationOptionsDTO } from './GenerateAuthenticatorAuthenticationOptionsDTO' import { AuthenticatorRepositoryInterface } from '../../Authenticator/AuthenticatorRepositoryInterface' import { AuthenticatorChallengeRepositoryInterface } from '../../Authenticator/AuthenticatorChallengeRepositoryInterface' import { AuthenticatorChallenge } from '../../Authenticator/AuthenticatorChallenge' +import { UserRepositoryInterface } from '../../User/UserRepositoryInterface' export class GenerateAuthenticatorAuthenticationOptions implements UseCaseInterface> { constructor( + private userRepository: UserRepositoryInterface, private authenticatorRepository: AuthenticatorRepositoryInterface, private authenticatorChallengeRepository: AuthenticatorChallengeRepositoryInterface, + private pseudoKeyParamsKey: string, ) {} async execute(dto: GenerateAuthenticatorAuthenticationOptionsDTO): Promise>> { - const userUuidOrError = Uuid.create(dto.userUuid) + const usernameOrError = Username.create(dto.username) + if (usernameOrError.isFailed()) { + return Result.fail(`Could not generate authenticator registration options: ${usernameOrError.getError()}`) + } + const username = usernameOrError.getValue() + + const user = await this.userRepository.findOneByEmail(username.value) + if (user === null) { + const credentialIdHash = crypto + .createHash('sha256') + .update(`u2f-selector-${dto.username}${this.pseudoKeyParamsKey}`) + .digest('base64url') + + const options = generateAuthenticationOptions({ + allowCredentials: [ + { + id: Buffer.from(credentialIdHash), + type: 'public-key', + transports: [], + }, + ], + userVerification: 'preferred', + }) + + return Result.ok(options) + } + + const userUuidOrError = Uuid.create(user.uuid) if (userUuidOrError.isFailed()) { return Result.fail(`Could not generate authenticator registration options: ${userUuidOrError.getError()}`) } diff --git a/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptionsDTO.ts b/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptionsDTO.ts index c4d348502..cb6a100ba 100644 --- a/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptionsDTO.ts +++ b/packages/auth/src/Domain/UseCase/GenerateAuthenticatorAuthenticationOptions/GenerateAuthenticatorAuthenticationOptionsDTO.ts @@ -1,3 +1,3 @@ export interface GenerateAuthenticatorAuthenticationOptionsDTO { - userUuid: string + username: string } diff --git a/packages/auth/src/Infra/Http/Request/GenerateAuthenticatorAuthenticationOptionsRequestParams.ts b/packages/auth/src/Infra/Http/Request/GenerateAuthenticatorAuthenticationOptionsRequestParams.ts index ab31192a7..7ba7e4fb3 100644 --- a/packages/auth/src/Infra/Http/Request/GenerateAuthenticatorAuthenticationOptionsRequestParams.ts +++ b/packages/auth/src/Infra/Http/Request/GenerateAuthenticatorAuthenticationOptionsRequestParams.ts @@ -1,3 +1,3 @@ export interface GenerateAuthenticatorAuthenticationOptionsRequestParams { - userUuid: string + username: string } diff --git a/packages/auth/src/Infra/InversifyExpressUtils/InversifyExpressAuthenticatorsController.ts b/packages/auth/src/Infra/InversifyExpressUtils/InversifyExpressAuthenticatorsController.ts index 9c14bf89a..3a1ebb19b 100644 --- a/packages/auth/src/Infra/InversifyExpressUtils/InversifyExpressAuthenticatorsController.ts +++ b/packages/auth/src/Infra/InversifyExpressUtils/InversifyExpressAuthenticatorsController.ts @@ -12,13 +12,13 @@ import { import TYPES from '../../Bootstrap/Types' import { AuthenticatorsController } from '../../Controller/AuthenticatorsController' -@controller('/authenticators', TYPES.ApiGatewayAuthMiddleware) +@controller('/authenticators') export class InversifyExpressAuthenticatorsController extends BaseHttpController { constructor(@inject(TYPES.AuthenticatorsController) private authenticatorsController: AuthenticatorsController) { super() } - @httpGet('/') + @httpGet('/', TYPES.ApiGatewayAuthMiddleware) async list(_request: Request, response: Response): Promise { const result = await this.authenticatorsController.list({ userUuid: response.locals.user.uuid, @@ -27,7 +27,7 @@ export class InversifyExpressAuthenticatorsController extends BaseHttpController return this.json(result.data, result.status) } - @httpDelete('/:authenticatorId') + @httpDelete('/:authenticatorId', TYPES.ApiGatewayAuthMiddleware) async delete(request: Request, response: Response): Promise { const result = await this.authenticatorsController.delete({ userUuid: response.locals.user.uuid, @@ -37,7 +37,7 @@ export class InversifyExpressAuthenticatorsController extends BaseHttpController return this.json(result.data, result.status) } - @httpGet('/generate-registration-options') + @httpGet('/generate-registration-options', TYPES.ApiGatewayAuthMiddleware) async generateRegistrationOptions(_request: Request, response: Response): Promise { const result = await this.authenticatorsController.generateRegistrationOptions({ username: response.locals.user.email, @@ -47,7 +47,7 @@ export class InversifyExpressAuthenticatorsController extends BaseHttpController return this.json(result.data, result.status) } - @httpPost('/verify-registration') + @httpPost('/verify-registration', TYPES.ApiGatewayAuthMiddleware) async verifyRegistration(request: Request, response: Response): Promise { const result = await this.authenticatorsController.verifyRegistrationResponse({ userUuid: response.locals.user.uuid, @@ -58,20 +58,10 @@ export class InversifyExpressAuthenticatorsController extends BaseHttpController return this.json(result.data, result.status) } - @httpGet('/generate-authentication-options') - async generateAuthenticationOptions(_request: Request, response: Response): Promise { + @httpPost('/generate-authentication-options') + async generateAuthenticationOptions(request: Request): Promise { const result = await this.authenticatorsController.generateAuthenticationOptions({ - userUuid: response.locals.user.uuid, - }) - - return this.json(result.data, result.status) - } - - @httpPost('/verify-authentication') - async verifyAuthentication(request: Request, response: Response): Promise { - const result = await this.authenticatorsController.verifyAuthenticationResponse({ - userUuid: response.locals.user.uuid, - authenticatorResponse: request.body, + username: request.body.username, }) return this.json(result.data, result.status)