diff --git a/apps/server/src/core/error/loggable/axios-error.loggable.spec.ts b/apps/server/src/core/error/loggable/axios-error.loggable.spec.ts new file mode 100644 index 00000000000..f2b480a4bf7 --- /dev/null +++ b/apps/server/src/core/error/loggable/axios-error.loggable.spec.ts @@ -0,0 +1,32 @@ +import { axiosErrorFactory } from '@shared/testing/factory'; +import { AxiosError } from 'axios'; +import { AxiosErrorLoggable } from './axios-error.loggable'; + +describe(AxiosErrorLoggable.name, () => { + describe('getLogMessage', () => { + const setup = () => { + const error = { + error: 'invalid_request', + }; + const type = 'mockType'; + const axiosError: AxiosError = axiosErrorFactory.withError(error).build(); + + const axiosErrorLoggable = new AxiosErrorLoggable(axiosError, type); + + return { axiosErrorLoggable, error, axiosError }; + }; + + it('should return error log message', () => { + const { axiosErrorLoggable, error, axiosError } = setup(); + + const result = axiosErrorLoggable.getLogMessage(); + + expect(result).toEqual({ + type: 'mockType', + message: axiosError.message, + data: JSON.stringify(error), + stack: 'mockStack', + }); + }); + }); +}); diff --git a/apps/server/src/core/error/loggable/axios-error.loggable.ts b/apps/server/src/core/error/loggable/axios-error.loggable.ts new file mode 100644 index 00000000000..29e6ad32dad --- /dev/null +++ b/apps/server/src/core/error/loggable/axios-error.loggable.ts @@ -0,0 +1,20 @@ +import { HttpException, HttpStatus } from '@nestjs/common'; +import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger'; +import { AxiosError } from 'axios'; + +export class AxiosErrorLoggable extends HttpException implements Loggable { + constructor(private readonly axiosError: AxiosError, protected readonly type: string) { + super(JSON.stringify(axiosError.response?.data), axiosError.status ?? HttpStatus.INTERNAL_SERVER_ERROR, { + cause: axiosError.cause, + }); + } + + getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage { + return { + message: this.axiosError.message, + type: this.type, + data: JSON.stringify(this.axiosError.response?.data), + stack: this.axiosError.stack, + }; + } +} diff --git a/apps/server/src/core/error/loggable/index.ts b/apps/server/src/core/error/loggable/index.ts new file mode 100644 index 00000000000..0470cbee690 --- /dev/null +++ b/apps/server/src/core/error/loggable/index.ts @@ -0,0 +1,2 @@ +export * from './error.loggable'; +export * from './axios-error.loggable'; diff --git a/apps/server/src/infra/oauth-provider/hydra/hydra.adapter.spec.ts b/apps/server/src/infra/oauth-provider/hydra/hydra.adapter.spec.ts index 7e30c5668c7..2a373195bc6 100644 --- a/apps/server/src/infra/oauth-provider/hydra/hydra.adapter.spec.ts +++ b/apps/server/src/infra/oauth-provider/hydra/hydra.adapter.spec.ts @@ -1,7 +1,5 @@ -import { DeepMocked, createMock } from '@golevelup/ts-jest'; +import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Configuration } from '@hpi-schul-cloud/commons/lib'; -import { HttpService } from '@nestjs/axios'; -import { Test, TestingModule } from '@nestjs/testing'; import { AcceptConsentRequestBody, AcceptLoginRequestBody, @@ -12,11 +10,15 @@ import { ProviderRedirectResponse, RejectRequestBody, } from '@infra/oauth-provider/dto'; +import { HttpService } from '@nestjs/axios'; +import { Test, TestingModule } from '@nestjs/testing'; import { axiosResponseFactory } from '@shared/testing'; -import { AxiosRequestConfig, Method, RawAxiosRequestHeaders } from 'axios'; -import { of } from 'rxjs'; +import { axiosErrorFactory } from '@shared/testing/factory'; +import { AxiosError, AxiosRequestConfig, Method, RawAxiosRequestHeaders } from 'axios'; +import { of, throwError } from 'rxjs'; +import { ProviderConsentSessionResponse } from '../dto'; +import { HydraOauthFailedLoggableException } from '../loggable'; import { HydraAdapter } from './hydra.adapter'; -import { ProviderConsentSessionResponse } from '../dto/response/consent-session.response'; import resetAllMocks = jest.resetAllMocks; class HydraAdapterSpec extends HydraAdapter { @@ -66,100 +68,66 @@ describe('HydraService', () => { }); describe('request', () => { - it('should return data when called with all parameters', async () => { - const data: { test: string } = { - test: 'data', - }; - - httpService.request.mockReturnValue(of(createAxiosResponse(data))); - - const result: { test: string } = await service.requestSpec( - 'GET', - 'testUrl', - { dataKey: 'dataValue' }, - { headerKey: 'headerValue' } - ); + describe('when called with all parameters', () => { + const setup = () => { + const data: { test: string } = { + test: 'data', + }; - expect(result).toEqual(data); - expect(httpService.request).toHaveBeenCalledWith( - expect.objectContaining({ - url: 'testUrl', - method: 'GET', - headers: { - 'X-Forwarded-Proto': 'https', - headerKey: 'headerValue', - }, - data: { dataKey: 'dataValue' }, - }) - ); - }); + httpService.request.mockReturnValue(of(createAxiosResponse(data))); - it('should return data when called with only necessary parameters', async () => { - const data: { test: string } = { - test: 'data', + return { + data, + }; }; - httpService.request.mockReturnValue(of(createAxiosResponse(data))); - - const result: { test: string } = await service.requestSpec('GET', 'testUrl'); - - expect(result).toEqual(data); - expect(httpService.request).toHaveBeenCalledWith( - expect.objectContaining({ - url: 'testUrl', - method: 'GET', - headers: { - 'X-Forwarded-Proto': 'https', - }, - }) - ); - }); - }); - - describe('Client Flow', () => { - describe('listOAuth2Clients', () => { - it('should list all oauth2 clients', async () => { - const data: ProviderOauthClient[] = [ - { - client_id: 'client1', - }, - { - client_id: 'client2', - }, - ]; - - httpService.request.mockReturnValue(of(createAxiosResponse(data))); + it('should return data', async () => { + const { data } = setup(); - const result: ProviderOauthClient[] = await service.listOAuth2Clients(); + const result: { test: string } = await service.requestSpec( + 'GET', + 'testUrl', + { dataKey: 'dataValue' }, + { headerKey: 'headerValue' } + ); expect(result).toEqual(data); expect(httpService.request).toHaveBeenCalledWith( expect.objectContaining({ - url: `${hydraUri}/clients`, + url: 'testUrl', method: 'GET', headers: { 'X-Forwarded-Proto': 'https', + headerKey: 'headerValue', }, + data: { dataKey: 'dataValue' }, }) ); }); + }); - it('should list all oauth2 clients within parameters', async () => { - const data: ProviderOauthClient[] = [ - { - client_id: 'client1', - owner: 'clientOwner', - }, - ]; + describe('when called with only necessary parameters', () => { + const setup = () => { + const data: { test: string } = { + test: 'data', + }; httpService.request.mockReturnValue(of(createAxiosResponse(data))); - const result: ProviderOauthClient[] = await service.listOAuth2Clients(1, 0, 'client1', 'clientOwner'); + return { + data, + }; + }; + + it('should return data', async () => { + const { data } = setup(); + + const result: { test: string } = await service.requestSpec('GET', 'testUrl'); expect(result).toEqual(data); expect(httpService.request).toHaveBeenCalledWith( expect.objectContaining({ - url: `${hydraUri}/clients?limit=1&offset=0&client_name=client1&owner=clientOwner`, + url: 'testUrl', method: 'GET', headers: { 'X-Forwarded-Proto': 'https', @@ -169,13 +137,130 @@ describe('HydraService', () => { }); }); + describe('when error occurs', () => { + describe('when error is an axios error', () => { + const setup = () => { + const error = { + error: 'invalid_request', + }; + const axiosError: AxiosError = axiosErrorFactory.withError(error).build({}); + + httpService.request.mockReturnValueOnce(throwError(() => axiosError)); + + return { + axiosError, + }; + }; + + it('should throw hydra oauth loggable exception', async () => { + const { axiosError } = setup(); + + await expect(service.listOAuth2Clients()).rejects.toThrow(new HydraOauthFailedLoggableException(axiosError)); + }); + }); + + describe('when error is any other error', () => { + const setup = () => { + httpService.request.mockReturnValueOnce(throwError(() => new Error('unknown error'))); + }; + + it('should throw the error', async () => { + setup(); + + await expect(service.listOAuth2Clients()).rejects.toThrow(new Error('unknown error')); + }); + }); + }); + }); + + describe('Client Flow', () => { + describe('listOAuth2Clients', () => { + describe('when only clientIds are given', () => { + const setup = () => { + const data: ProviderOauthClient[] = [ + { + client_id: 'client1', + }, + { + client_id: 'client2', + }, + ]; + + httpService.request.mockReturnValue(of(createAxiosResponse(data))); + + return { + data, + }; + }; + + it('should list all oauth2 clients', async () => { + const { data } = setup(); + + const result: ProviderOauthClient[] = await service.listOAuth2Clients(); + + expect(result).toEqual(data); + expect(httpService.request).toHaveBeenCalledWith( + expect.objectContaining({ + url: `${hydraUri}/clients`, + method: 'GET', + headers: { + 'X-Forwarded-Proto': 'https', + }, + }) + ); + }); + }); + + describe('when clientId and other parameters are given', () => { + const setup = () => { + const data: ProviderOauthClient[] = [ + { + client_id: 'client1', + owner: 'clientOwner', + }, + ]; + + httpService.request.mockReturnValue(of(createAxiosResponse(data))); + + return { + data, + }; + }; + + it('should list all oauth2 clients within parameters', async () => { + const { data } = setup(); + + const result: ProviderOauthClient[] = await service.listOAuth2Clients(1, 0, 'client1', 'clientOwner'); + + expect(result).toEqual(data); + expect(httpService.request).toHaveBeenCalledWith( + expect.objectContaining({ + url: `${hydraUri}/clients?limit=1&offset=0&client_name=client1&owner=clientOwner`, + method: 'GET', + headers: { + 'X-Forwarded-Proto': 'https', + }, + }) + ); + }); + }); + }); + describe('getOAuth2Client', () => { - it('should get oauth2 client', async () => { + const setup = () => { const data: ProviderOauthClient = { client_id: 'client', }; httpService.request.mockReturnValue(of(createAxiosResponse(data))); + return { + data, + }; + }; + + it('should get oauth2 client', async () => { + const { data } = setup(); + const result: ProviderOauthClient = await service.getOAuth2Client('clientId'); expect(result).toEqual(data); @@ -192,12 +277,20 @@ describe('HydraService', () => { }); describe('createOAuth2Client', () => { - it('should create oauth2 client', async () => { + const setup = () => { const data: ProviderOauthClient = { client_id: 'client', }; httpService.request.mockReturnValue(of(createAxiosResponse(data))); + return { + data, + }; + }; + + it('should create oauth2 client', async () => { + const { data } = setup(); + const result: ProviderOauthClient = await service.createOAuth2Client(data); expect(result).toEqual(data); @@ -215,12 +308,20 @@ describe('HydraService', () => { }); describe('updateOAuth2Client', () => { - it('should update oauth2 client', async () => { + const setup = () => { const data: ProviderOauthClient = { client_id: 'client', }; httpService.request.mockReturnValue(of(createAxiosResponse(data))); + return { + data, + }; + }; + + it('should update oauth2 client', async () => { + const { data } = setup(); + const result: ProviderOauthClient = await service.updateOAuth2Client('clientId', data); expect(result).toEqual(data); @@ -238,8 +339,12 @@ describe('HydraService', () => { }); describe('deleteOAuth2Client', () => { - it('should delete oauth2 client', async () => { + const setup = () => { httpService.request.mockReturnValue(of(createAxiosResponse({}))); + }; + + it('should delete oauth2 client', async () => { + setup(); await service.deleteOAuth2Client('clientId'); @@ -268,26 +373,30 @@ describe('HydraService', () => { }); describe('getConsentRequest', () => { - it('should make http request', async () => { - // Arrange + const setup = () => { const config: AxiosRequestConfig = { method: 'GET', url: `${hydraUri}/oauth2/auth/requests/consent?consent_challenge=${challenge}`, }; httpService.request.mockReturnValue(of(createAxiosResponse({ challenge }))); - // Act + return { + config, + }; + }; + + it('should make http request', async () => { + const { config } = setup(); + const result: ProviderConsentResponse = await service.getConsentRequest(challenge); - // Assert expect(result.challenge).toEqual(challenge); expect(httpService.request).toHaveBeenCalledWith(expect.objectContaining(config)); }); }); describe('acceptConsentRequest', () => { - it('should make http request', async () => { - // Arrange + const setup = () => { const body: AcceptConsentRequestBody = { grant_scope: ['offline', 'openid'], }; @@ -301,18 +410,25 @@ describe('HydraService', () => { of(createAxiosResponse({ redirect_to: expectedRedirectTo })) ); - // Act + return { + body, + config, + expectedRedirectTo, + }; + }; + + it('should make http request', async () => { + const { body, config, expectedRedirectTo } = setup(); + const result: ProviderRedirectResponse = await service.acceptConsentRequest(challenge, body); - // Assert expect(result.redirect_to).toEqual(expectedRedirectTo); expect(httpService.request).toHaveBeenCalledWith(expect.objectContaining(config)); }); }); describe('rejectConsentRequest', () => { - it('should make http request', async () => { - // Arrange + const setup = () => { const body: RejectRequestBody = { error: 'error', }; @@ -326,20 +442,36 @@ describe('HydraService', () => { of(createAxiosResponse({ redirect_to: expectedRedirectTo })) ); - // Act + return { + body, + config, + expectedRedirectTo, + }; + }; + + it('should make http request', async () => { + const { body, config, expectedRedirectTo } = setup(); + const result: ProviderRedirectResponse = await service.rejectConsentRequest(challenge, body); - // Assert expect(result.redirect_to).toEqual(expectedRedirectTo); expect(httpService.request).toHaveBeenCalledWith(expect.objectContaining(config)); }); }); describe('listConsentSessions', () => { - it('should list all consent sessions', async () => { + const setup = () => { const response: ProviderConsentSessionResponse[] = [{ consent_request: { challenge: 'challenge' } }]; httpService.request.mockReturnValue(of(createAxiosResponse(response))); + return { + response, + }; + }; + + it('should list all consent sessions', async () => { + const { response } = setup(); + const result: ProviderConsentSessionResponse[] = await service.listConsentSessions('userId'); expect(result).toEqual(response); @@ -356,8 +488,12 @@ describe('HydraService', () => { }); describe('revokeConsentSession', () => { - it('should revoke all consent sessions', async () => { + const setup = () => { httpService.request.mockReturnValue(of(createAxiosResponse({}))); + }; + + it('should revoke all consent sessions', async () => { + setup(); await service.revokeConsentSession('userId', 'clientId'); @@ -375,7 +511,7 @@ describe('HydraService', () => { describe('Logout Flow', () => { describe('acceptLogoutRequest', () => { - it('should make http request', async () => { + const setup = () => { const responseMock: ProviderRedirectResponse = { redirect_to: 'redirect_mock' }; httpService.request.mockReturnValue(of(createAxiosResponse(responseMock))); const config: AxiosRequestConfig = { @@ -384,6 +520,15 @@ describe('HydraService', () => { headers: { 'X-Forwarded-Proto': 'https' }, }; + return { + responseMock, + config, + }; + }; + + it('should make http request', async () => { + const { responseMock, config } = setup(); + const response: ProviderRedirectResponse = await service.acceptLogoutRequest('challenge_mock'); expect(httpService.request).toHaveBeenCalledWith(expect.objectContaining(config)); @@ -394,12 +539,20 @@ describe('HydraService', () => { describe('Miscellaneous', () => { describe('introspectOAuth2Token', () => { - it('should return introspect', async () => { + const setup = () => { const response: IntrospectResponse = { active: true, }; httpService.request.mockReturnValue(of(createAxiosResponse(response))); + return { + response, + }; + }; + + it('should return introspect', async () => { + const { response } = setup(); + const result: IntrospectResponse = await service.introspectOAuth2Token('token', 'scope'); expect(result).toEqual(response); @@ -418,8 +571,12 @@ describe('HydraService', () => { }); describe('isInstanceAlive', () => { - it('should check if hydra is alive', async () => { + const setup = () => { httpService.request.mockReturnValue(of(createAxiosResponse(true))); + }; + + it('should check if hydra is alive', async () => { + setup(); const result: boolean = await service.isInstanceAlive(); @@ -459,25 +616,30 @@ describe('HydraService', () => { }); describe('getLoginRequest', () => { - it('should send login request', async () => { - // Arrange + const setup = () => { const requestConfig: AxiosRequestConfig = { method: 'GET', url: `${hydraUri}/oauth2/auth/requests/login?login_challenge=${challenge}`, }; httpService.request.mockReturnValue(of(createAxiosResponse(providerLoginResponse))); - // Act + return { + requestConfig, + }; + }; + + it('should send login request', async () => { + const { requestConfig } = setup(); + const response: ProviderLoginResponse = await service.getLoginRequest(challenge); - // Assert expect(response).toEqual(providerLoginResponse); expect(httpService.request).toHaveBeenCalledWith(expect.objectContaining(requestConfig)); }); }); describe('acceptLoginRequest', () => { - it('should send accept login request', async () => { + const setup = () => { const body: AcceptLoginRequestBody = { subject: '', force_subject_identifier: '', @@ -494,6 +656,16 @@ describe('HydraService', () => { of(createAxiosResponse({ redirect_to: expectedRedirectTo })) ); + return { + body, + config, + expectedRedirectTo, + }; + }; + + it('should send accept login request', async () => { + const { body, config, expectedRedirectTo } = setup(); + const result: ProviderRedirectResponse = await service.acceptLoginRequest(challenge, body); expect(result.redirect_to).toEqual(expectedRedirectTo); @@ -502,8 +674,7 @@ describe('HydraService', () => { }); describe('rejectLoginRequest', () => { - it('should send reject login request', async () => { - // Arrange + const setup = () => { const body: RejectRequestBody = { error: 'error', }; @@ -517,10 +688,18 @@ describe('HydraService', () => { of(createAxiosResponse({ redirect_to: expectedRedirectTo })) ); - // Act + return { + body, + config, + expectedRedirectTo, + }; + }; + + it('should send reject login request', async () => { + const { body, config, expectedRedirectTo } = setup(); + const result: ProviderRedirectResponse = await service.rejectLoginRequest(challenge, body); - // Assert expect(result.redirect_to).toEqual(expectedRedirectTo); expect(httpService.request).toHaveBeenCalledWith(expect.objectContaining(config)); }); diff --git a/apps/server/src/infra/oauth-provider/hydra/hydra.adapter.ts b/apps/server/src/infra/oauth-provider/hydra/hydra.adapter.ts index f554a15abd3..3e2d389d643 100644 --- a/apps/server/src/infra/oauth-provider/hydra/hydra.adapter.ts +++ b/apps/server/src/infra/oauth-provider/hydra/hydra.adapter.ts @@ -1,21 +1,23 @@ import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { HttpService } from '@nestjs/axios'; import { Injectable } from '@nestjs/common'; -import { AxiosResponse, Method, RawAxiosRequestHeaders } from 'axios'; +import { AxiosResponse, isAxiosError, Method, RawAxiosRequestHeaders } from 'axios'; import QueryString from 'qs'; -import { Observable, firstValueFrom } from 'rxjs'; +import { firstValueFrom, Observable } from 'rxjs'; +import { catchError } from 'rxjs/operators'; import { URL } from 'url'; import { AcceptConsentRequestBody, AcceptLoginRequestBody, IntrospectResponse, ProviderConsentResponse, + ProviderConsentSessionResponse, ProviderLoginResponse, ProviderOauthClient, ProviderRedirectResponse, RejectRequestBody, } from '../dto'; -import { ProviderConsentSessionResponse } from '../dto/response/consent-session.response'; +import { HydraOauthFailedLoggableException } from '../loggable'; import { OauthProviderService } from '../oauth-provider.service'; @Injectable() @@ -160,15 +162,26 @@ export class HydraAdapter extends OauthProviderService { data?: unknown, additionalHeaders: RawAxiosRequestHeaders = {} ): Promise { - const observable: Observable> = this.httpService.request({ - url, - method, - headers: { - 'X-Forwarded-Proto': 'https', - ...additionalHeaders, - }, - data, - }); + const observable: Observable> = this.httpService + .request({ + url, + method, + headers: { + 'X-Forwarded-Proto': 'https', + ...additionalHeaders, + }, + data, + }) + .pipe( + catchError((error: unknown) => { + if (isAxiosError(error)) { + throw new HydraOauthFailedLoggableException(error); + } else { + throw error; + } + }) + ); + const response: AxiosResponse = await firstValueFrom(observable); return response.data; } diff --git a/apps/server/src/infra/oauth-provider/loggable/hydra-oauth-failed-loggable-exception.spec.ts b/apps/server/src/infra/oauth-provider/loggable/hydra-oauth-failed-loggable-exception.spec.ts new file mode 100644 index 00000000000..a78b365d126 --- /dev/null +++ b/apps/server/src/infra/oauth-provider/loggable/hydra-oauth-failed-loggable-exception.spec.ts @@ -0,0 +1,35 @@ +import { axiosErrorFactory } from '@shared/testing/factory'; +import { AxiosError } from 'axios'; +import { HydraOauthFailedLoggableException } from './hydra-oauth-failed-loggable-exception'; + +describe(HydraOauthFailedLoggableException.name, () => { + describe('getLogMessage', () => { + const setup = () => { + const error = { + error: 'invalid_request', + }; + const axiosError: AxiosError = axiosErrorFactory.withError(error).build({ stack: 'someStack' }); + + const exception = new HydraOauthFailedLoggableException(axiosError); + + return { + exception, + axiosError, + error, + }; + }; + + it('should return the correct log message', () => { + const { exception, axiosError, error } = setup(); + + const message = exception.getLogMessage(); + + expect(message).toEqual({ + type: 'HYDRA_OAUTH_FAILED', + message: axiosError.message, + stack: axiosError.stack, + data: JSON.stringify(error), + }); + }); + }); +}); diff --git a/apps/server/src/infra/oauth-provider/loggable/hydra-oauth-failed-loggable-exception.ts b/apps/server/src/infra/oauth-provider/loggable/hydra-oauth-failed-loggable-exception.ts new file mode 100644 index 00000000000..c92dd3c7fff --- /dev/null +++ b/apps/server/src/infra/oauth-provider/loggable/hydra-oauth-failed-loggable-exception.ts @@ -0,0 +1,8 @@ +import { AxiosErrorLoggable } from '@src/core/error/loggable'; +import { AxiosError } from 'axios'; + +export class HydraOauthFailedLoggableException extends AxiosErrorLoggable { + constructor(error: AxiosError) { + super(error, 'HYDRA_OAUTH_FAILED'); + } +} diff --git a/apps/server/src/infra/oauth-provider/loggable/index.ts b/apps/server/src/infra/oauth-provider/loggable/index.ts new file mode 100644 index 00000000000..677fe4f84e6 --- /dev/null +++ b/apps/server/src/infra/oauth-provider/loggable/index.ts @@ -0,0 +1 @@ +export * from './hydra-oauth-failed-loggable-exception'; diff --git a/apps/server/src/modules/oauth/loggable/index.ts b/apps/server/src/modules/oauth/loggable/index.ts index b4e63107161..4c35983a4ca 100644 --- a/apps/server/src/modules/oauth/loggable/index.ts +++ b/apps/server/src/modules/oauth/loggable/index.ts @@ -1,3 +1,4 @@ export * from './oauth-sso.error'; export * from './sso-error-code.enum'; export * from './user-not-found-after-provisioning.loggable-exception'; +export * from './token-request-loggable-exception'; diff --git a/apps/server/src/modules/oauth/loggable/oauth-sso.error.ts b/apps/server/src/modules/oauth/loggable/oauth-sso.error.ts index 35659b2778f..cc1486adcb7 100644 --- a/apps/server/src/modules/oauth/loggable/oauth-sso.error.ts +++ b/apps/server/src/modules/oauth/loggable/oauth-sso.error.ts @@ -1,6 +1,10 @@ import { InternalServerErrorException } from '@nestjs/common'; import { SSOErrorCode } from './sso-error-code.enum'; +/** + * @deprecated Please create a loggable instead. + * This will be removed with: https://ticketsystem.dbildungscloud.de/browse/N21-1483 + */ export class OAuthSSOError extends InternalServerErrorException { readonly message: string; diff --git a/apps/server/src/modules/oauth/loggable/token-request-loggable-exception.spec.ts b/apps/server/src/modules/oauth/loggable/token-request-loggable-exception.spec.ts new file mode 100644 index 00000000000..6716175bdbe --- /dev/null +++ b/apps/server/src/modules/oauth/loggable/token-request-loggable-exception.spec.ts @@ -0,0 +1,34 @@ +import { axiosErrorFactory } from '@shared/testing/factory'; +import { AxiosError } from 'axios'; +import { TokenRequestLoggableException } from './token-request-loggable-exception'; + +describe(TokenRequestLoggableException.name, () => { + describe('getLogMessage', () => { + const setup = () => { + const error = { + error: 'invalid_request', + }; + const axiosError: AxiosError = axiosErrorFactory.withError(error).build(); + const exception = new TokenRequestLoggableException(axiosError); + + return { + axiosError, + exception, + error, + }; + }; + + it('should return the correct log message', () => { + const { axiosError, exception, error } = setup(); + + const logMessage = exception.getLogMessage(); + + expect(logMessage).toStrictEqual({ + type: 'OAUTH_TOKEN_REQUEST_ERROR', + message: axiosError.message, + data: JSON.stringify(error), + stack: axiosError.stack, + }); + }); + }); +}); diff --git a/apps/server/src/modules/oauth/loggable/token-request-loggable-exception.ts b/apps/server/src/modules/oauth/loggable/token-request-loggable-exception.ts new file mode 100644 index 00000000000..fd852186829 --- /dev/null +++ b/apps/server/src/modules/oauth/loggable/token-request-loggable-exception.ts @@ -0,0 +1,8 @@ +import { AxiosErrorLoggable } from '@src/core/error/loggable'; +import { AxiosError } from 'axios'; + +export class TokenRequestLoggableException extends AxiosErrorLoggable { + constructor(error: AxiosError) { + super(error, 'OAUTH_TOKEN_REQUEST_ERROR'); + } +} diff --git a/apps/server/src/modules/oauth/service/oauth-adapter.service.spec.ts b/apps/server/src/modules/oauth/service/oauth-adapter.service.spec.ts index 12c0a381d8b..af03a6fdda2 100644 --- a/apps/server/src/modules/oauth/service/oauth-adapter.service.spec.ts +++ b/apps/server/src/modules/oauth/service/oauth-adapter.service.spec.ts @@ -2,9 +2,11 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { HttpService } from '@nestjs/axios'; import { Test, TestingModule } from '@nestjs/testing'; import { axiosResponseFactory } from '@shared/testing'; +import { axiosErrorFactory } from '@shared/testing/factory'; +import { AxiosError } from 'axios'; import { of, throwError } from 'rxjs'; import { OAuthGrantType } from '../interface/oauth-grant-type.enum'; -import { OAuthSSOError } from '../loggable'; +import { TokenRequestLoggableException } from '../loggable'; import { AuthenticationCodeGrantTokenRequest, OauthTokenResponse } from './dto'; import { OauthAdapterService } from './oauth-adapter.service'; @@ -93,12 +95,65 @@ describe('OauthAdapterServive', () => { }); describe('when no token got returned', () => { + const setup = () => { + const error = new Error('unknown error'); + httpService.post.mockReturnValueOnce(throwError(() => error)); + + return { + error, + }; + }; + it('should throw an error', async () => { - httpService.post.mockReturnValueOnce(throwError(() => 'error')); + const { error } = setup(); const resp = service.sendAuthenticationCodeTokenRequest('tokenEndpoint', testPayload); - await expect(resp).rejects.toEqual(new OAuthSSOError('Requesting token failed.', 'sso_auth_code_step')); + await expect(resp).rejects.toEqual(error); + }); + }); + + describe('when error got returned', () => { + describe('when error is a unknown error', () => { + const setup = () => { + const error = new Error('unknown error'); + httpService.post.mockReturnValueOnce(throwError(() => error)); + + return { + error, + }; + }; + + it('should throw the default sso error', async () => { + const { error } = setup(); + + const resp = service.sendAuthenticationCodeTokenRequest('tokenEndpoint', testPayload); + + await expect(resp).rejects.toEqual(error); + }); + }); + + describe('when error is a axios error', () => { + const setup = () => { + const error = { + error: 'invalid_request', + }; + const axiosError: AxiosError = axiosErrorFactory.withError(error).build(); + + httpService.post.mockReturnValueOnce(throwError(() => axiosError)); + + return { + axiosError, + }; + }; + + it('should throw an error', async () => { + const { axiosError } = setup(); + + const resp = service.sendAuthenticationCodeTokenRequest('tokenEndpoint', testPayload); + + await expect(resp).rejects.toEqual(new TokenRequestLoggableException(axiosError)); + }); }); }); }); diff --git a/apps/server/src/modules/oauth/service/oauth-adapter.service.ts b/apps/server/src/modules/oauth/service/oauth-adapter.service.ts index 6b008b610cf..4ab048b84c4 100644 --- a/apps/server/src/modules/oauth/service/oauth-adapter.service.ts +++ b/apps/server/src/modules/oauth/service/oauth-adapter.service.ts @@ -1,10 +1,10 @@ import { HttpService } from '@nestjs/axios'; import { Injectable } from '@nestjs/common/decorators'; -import { AxiosResponse } from 'axios'; +import { AxiosResponse, isAxiosError } from 'axios'; import JwksRsa from 'jwks-rsa'; import QueryString from 'qs'; import { lastValueFrom, Observable } from 'rxjs'; -import { OAuthSSOError } from '../loggable'; +import { TokenRequestLoggableException } from '../loggable'; import { AuthenticationCodeGrantTokenRequest, OauthTokenResponse } from './dto'; @Injectable() @@ -40,8 +40,11 @@ export class OauthAdapterService { let responseToken: AxiosResponse; try { responseToken = await lastValueFrom(observable); - } catch (error) { - throw new OAuthSSOError('Requesting token failed.', 'sso_auth_code_step'); + } catch (error: unknown) { + if (isAxiosError(error)) { + throw new TokenRequestLoggableException(error); + } + throw error; } return responseToken.data; diff --git a/apps/server/src/shared/testing/factory/axios-error.factory.ts b/apps/server/src/shared/testing/factory/axios-error.factory.ts new file mode 100644 index 00000000000..089179dafef --- /dev/null +++ b/apps/server/src/shared/testing/factory/axios-error.factory.ts @@ -0,0 +1,28 @@ +import { HttpStatus } from '@nestjs/common'; +import { axiosResponseFactory } from '@shared/testing'; +import { AxiosError, AxiosHeaders } from 'axios'; +import { Factory } from 'fishery'; + +class AxiosErrorFactory extends Factory { + withError(error: unknown): this { + return this.params({ + response: axiosResponseFactory.build({ status: HttpStatus.BAD_REQUEST, data: error }), + }); + } +} + +export const axiosErrorFactory = AxiosErrorFactory.define(() => { + return { + status: HttpStatus.BAD_REQUEST, + config: { headers: new AxiosHeaders() }, + isAxiosError: true, + code: HttpStatus.BAD_REQUEST.toString(), + message: 'Bad Request', + name: 'BadRequest', + response: axiosResponseFactory.build({ status: HttpStatus.BAD_REQUEST }), + stack: 'mockStack', + toJSON: () => { + return { someJson: 'someJson' }; + }, + }; +}); diff --git a/apps/server/src/shared/testing/factory/index.ts b/apps/server/src/shared/testing/factory/index.ts index bd8d0913a72..54fac672098 100644 --- a/apps/server/src/shared/testing/factory/index.ts +++ b/apps/server/src/shared/testing/factory/index.ts @@ -39,3 +39,4 @@ export * from './user.do.factory'; export * from './user.factory'; export * from './legacy-file-entity-mock.factory'; export * from './jwt.test.factory'; +export * from './axios-error.factory';