Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into feat/user-auth
Browse files Browse the repository at this point in the history
# Conflicts:
#	libs/api/auth/src/lib/interceptors/auth.interceptor.spec.ts
#	package-lock.json
  • Loading branch information
timonmasberg committed Feb 1, 2024
2 parents 41a5ed4 + 06f1d93 commit 48061a5
Show file tree
Hide file tree
Showing 18 changed files with 775 additions and 619 deletions.
10 changes: 7 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
with:
fetch-depth: 0
- name: Ensure Conventional Commits
uses: webiny/action-conventional-commits@v1.2.0
uses: webiny/action-conventional-commits@v1.3.0
- name: Setup node
uses: actions/setup-node@v4
with:
Expand All @@ -34,8 +34,12 @@ jobs:
- name: Lint
run: npx nx affected --target=lint --parallel=3

- name: Run Tests with Code Coverage
run: npx nx affected --target=test --parallel=3 --ci --coverage --coverageReporters=lcov
- name: Run all tests
if: github.event_name == 'push'
run: npx nx run-many --all --target=test --parallel --ci --coverage --coverageReporters=lcov
- name: Run affected tests
if: github.event_name == 'pull_request'
run: npx nx affected --target=test --parallel --ci --coverage --coverageReporters=lcov
- name: Merge Coverage files
run: '[ -d "./coverage/" ] && ./node_modules/.bin/lcov-result-merger ./coverage/**/lcov.info ./coverage/lcov.info || exit 0'

Expand Down
14 changes: 7 additions & 7 deletions .github/workflows/preview-deployment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ jobs:
cache: 'npm'
- run: npm ci --ignore-scripts
- name: Initial Deployment Preview Comment
uses: peter-evans/create-or-update-comment@v3.1.0
uses: peter-evans/create-or-update-comment@v4.0.0
id: pr-preview-comment
with:
issue-number: ${{ github.event.issue.number }}
Expand Down Expand Up @@ -101,7 +101,7 @@ jobs:
containerRegistryPassword: ${{ secrets.GITHUB_TOKEN }}
containerTag: ghcr.io/kordis-leitstelle/kordis-spa:${{ steps.set-pr-sha.outputs.head_sha }}
- name: Update PR Preview Comment
uses: peter-evans/create-or-update-comment@v3.1.0
uses: peter-evans/create-or-update-comment@v4.0.0
with:
comment-id: ${{ steps.pr-preview-comment.outputs.comment-id }}
edit-mode: replace
Expand Down Expand Up @@ -145,15 +145,15 @@ jobs:
cache: 'npm'
- run: npm ci --ignore-scripts
- name: Find PR Preview Comment
uses: peter-evans/find-comment@v2
uses: peter-evans/find-comment@v3
id: deploy-preview-comment
with:
issue-number: ${{ github.event.pull_request.number }}
comment-author: "github-actions[bot]"
body-includes: Deployment Preview
- name: Update PR Preview Comment
if: steps.deploy-preview-comment.outputs.comment-id != ''
uses: peter-evans/create-or-update-comment@v3.1.0
uses: peter-evans/create-or-update-comment@v4.0.0
with:
comment-id: ${{ steps.deploy-preview-comment.outputs.comment-id }}
edit-mode: replace
Expand Down Expand Up @@ -185,7 +185,7 @@ jobs:
containerTag: ghcr.io/kordis-leitstelle/kordis-spa:${{ github.event.pull_request.head.sha }}

- name: Update PR Preview Comment
uses: peter-evans/create-or-update-comment@v3.1.0
uses: peter-evans/create-or-update-comment@v4.0.0
with:
comment-id: ${{ steps.deploy-preview-comment.outputs.comment-id }}
edit-mode: replace
Expand All @@ -204,15 +204,15 @@ jobs:
(needs.has-deployment.outputs.has-swa == 'true' || needs.has-deployment.outputs.has-wa == 'true') && false
steps:
- name: Find PR Preview Comment
uses: peter-evans/find-comment@v2
uses: peter-evans/find-comment@v3
id: deploy-preview-comment
with:
issue-number: ${{ github.event.pull_request.number }}
comment-author: "github-actions[bot]"
body-includes: Deployment Preview
- name: Update PR Preview Comment
if: steps.deploy-preview-comment.outputs.comment-id != ''
uses: peter-evans/create-or-update-comment@v3.1.0
uses: peter-evans/create-or-update-comment@v4.0.0
with:
comment-id: ${{ steps.deploy-preview-comment.outputs.comment-id }}
edit-mode: replace
Expand Down
4 changes: 2 additions & 2 deletions apps/api/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ RUN npm --omit=dev ci
COPY ./dist/apps/api ./

# Use distroless for maximum security: https://github.com/GoogleContainerTools/distroless
FROM gcr.io/distroless/nodejs${NODE_VERSION}-debian11
FROM gcr.io/distroless/nodejs${NODE_VERSION}-debian12:nonroot

COPY --from=builder /app /app
COPY --chown=root:root --chmod=655 --from=builder /app /app
WORKDIR /app

ENV PORT=3333
Expand Down
5 changes: 3 additions & 2 deletions apps/api/src/app/app.module.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { classes } from '@automapper/classes';
import { AutomapperModule } from '@automapper/nestjs';
import { ApolloDriver, ApolloDriverConfig } from '@nestjs/apollo';
import { Module } from '@nestjs/common';
import { ConfigModule, ConfigService } from '@nestjs/config';
import { GraphQLModule } from '@nestjs/graphql';
import { MongooseModule } from '@nestjs/mongoose';
import { AutomapperModule } from '@timonmasberg/automapper-nestjs';
import * as path from 'path';

import { AuthModule } from '@kordis/api/auth';
Expand All @@ -15,6 +15,7 @@ import { SharedKernel, errorFormatterFactory } from '@kordis/api/shared';
import { AppResolver } from './app.resolver';
import { AppService } from './app.service';
import { GraphqlSubscriptionsController } from './controllers/graphql-subscriptions.controller';
import { HealthCheckController } from './controllers/health-check.controller';
import environment from './environment';

// todo: narrow this down, we should have either an explicit way of defining when to use what (currently hard, since ConfigService not available in forRoot), or we should have an environment variable that defines the environment
Expand Down Expand Up @@ -67,6 +68,6 @@ const UTILITY_MODULES = [
...FEATURE_MODULES,
],
providers: [AppService, AppResolver],
controllers: [GraphqlSubscriptionsController],
controllers: [GraphqlSubscriptionsController, HealthCheckController],
})
export class AppModule {}
19 changes: 19 additions & 0 deletions apps/api/src/app/controllers/health-check.controller.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { Test, TestingModule } from '@nestjs/testing';

import { HealthCheckController } from './health-check.controller';

describe('HealthCheckController', () => {
let controller: HealthCheckController;

beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
controllers: [HealthCheckController],
}).compile();

controller = module.get<HealthCheckController>(HealthCheckController);
});

it('should return a resolved promise for the healthCheck method', async () => {
await expect(controller.healthCheck()).resolves.toBeUndefined();
});
});
10 changes: 10 additions & 0 deletions apps/api/src/app/controllers/health-check.controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { All, Controller, HttpCode } from '@nestjs/common';

@Controller('health-check')
export class HealthCheckController {
@All()
@HttpCode(200)
healthCheck(): Promise<void> {
return Promise.resolve();
}
}
32 changes: 26 additions & 6 deletions libs/api/auth/src/lib/interceptors/auth.interceptor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { CallHandler, UnauthorizedException } from '@nestjs/common';
import { Observable, firstValueFrom, of } from 'rxjs';

import { KordisRequest } from '@kordis/api/shared';
import { createGqlContextForRequest } from '@kordis/api/test-helpers';
import {
createGqlContextForRequest,
createHttpContextForRequest,
} from '@kordis/api/test-helpers';
import { AuthUser } from '@kordis/shared/auth';

import { VerifyAuthUserStrategy } from '../auth-strategies/verify-auth-user.strategy';
Expand All @@ -23,10 +26,6 @@ describe('AuthInterceptor', () => {
service = new AuthInterceptor(mockAuthUserExtractor);
});

it('should be defined', () => {
expect(service).toBeDefined();
});

it('should throw unauthorized http exception', async () => {
jest
.spyOn(mockAuthUserExtractor, 'verifyUserFromRequest')
Expand All @@ -42,7 +41,7 @@ describe('AuthInterceptor', () => {
).rejects.toThrow(UnauthorizedException);
});

it('should continue request pipeline', async () => {
it('should continue request pipeline for authenticated request', async () => {
jest
.spyOn(mockAuthUserExtractor, 'verifyUserFromRequest')
.mockResolvedValue({
Expand Down Expand Up @@ -71,4 +70,25 @@ describe('AuthInterceptor', () => {
firstValueFrom(await service.intercept(httpCtx, handler)),
).resolves.toBeTruthy();
});

it('should continue request pipeline for health-check request', async () => {
const handler = createMock<CallHandler>({
handle(): Observable<boolean> {
return of(true);
},
});

await expect(
firstValueFrom(
await service.intercept(
createHttpContextForRequest(
createMock<KordisRequest>({
path: '/health-check',
}),
),
handler,
),
),
).resolves.toBeTruthy();
});
});
4 changes: 4 additions & 0 deletions libs/api/auth/src/lib/interceptors/auth.interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ export class AuthInterceptor implements NestInterceptor {
req = ctx.getContext<KordisGqlContext>().req;
} else {
req = context.switchToHttp().getRequest<KordisRequest>();

if (req.path === '/health-check') {
return next.handle();
}
}

const possibleAuthUser =
Expand Down
6 changes: 3 additions & 3 deletions libs/api/observability/jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
export default {
displayName: 'api-observability',
preset: '../../../jest.preset.js',
globals: {},
testEnvironment: 'node',
transform: {
'^.+\\.(ts|mjs|js|html)$': [
'jest-preset-angular',
'^.+\\.[tj]s$': [
'ts-jest',
{
tsconfig: '<rootDir>/tsconfig.spec.json',
stringifyContentPathRegex: '\\.(html|svg)$',
},
],
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,25 @@
import { DeepMocked, createMock } from '@golevelup/ts-jest';
import { Logger } from '@nestjs/common';
import * as Sentry from '@sentry/node';

import { PresentableException } from '@kordis/api/shared';

import { SentryExceptionsFilter } from './sentry-exceptions.filter';

describe('ExceptionsFilter', () => {
describe('SentryExceptionsFilter', () => {
let sentryExceptionsFilter: SentryExceptionsFilter;
let addBreadcrumbMock: jest.Mock;
let captureExceptionMock: jest.Mock;

beforeEach(() => {
let logger: DeepMocked<Logger>;
beforeEach(async () => {
addBreadcrumbMock = jest.fn();
captureExceptionMock = jest.fn();
logger = createMock<Logger>();

(Sentry.addBreadcrumb as jest.Mock) = addBreadcrumbMock;
(Sentry.captureException as jest.Mock) = captureExceptionMock;

sentryExceptionsFilter = new SentryExceptionsFilter();
sentryExceptionsFilter = new SentryExceptionsFilter(logger);
});

afterEach(() => {
Expand All @@ -36,15 +39,11 @@ describe('ExceptionsFilter', () => {

sentryExceptionsFilter.catch(presentableException);

expect(addBreadcrumbMock).toHaveBeenCalledTimes(1);
expect(addBreadcrumbMock).toHaveBeenCalledWith({
level: 'error',
message: 'message',
data: {
name: 'Error',
code: 'code',
stack: expect.any(String),
},
expect(logger.log).toHaveBeenCalledTimes(1);
expect(logger.log).toHaveBeenCalledWith('message', {
name: 'Error',
code: 'code',
stack: expect.any(String),
});
expect(captureExceptionMock).not.toHaveBeenCalled();
});
Expand All @@ -58,6 +57,14 @@ describe('ExceptionsFilter', () => {
expect(captureExceptionMock).toHaveBeenCalledWith(exception, {
level: 'error',
});
expect(logger.error).toHaveBeenCalledTimes(1);
expect(logger.error).toHaveBeenCalledWith(
'Caught unhandled exception that was not presentable',
undefined,
{
exception,
},
);
expect(addBreadcrumbMock).not.toHaveBeenCalled();
});
});
30 changes: 21 additions & 9 deletions libs/api/observability/src/lib/filters/sentry-exceptions.filter.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,35 @@
import { Catch, ExceptionFilter } from '@nestjs/common';
import { Catch, ExceptionFilter, Logger } from '@nestjs/common';
import * as Sentry from '@sentry/node';

import { PresentableException } from '@kordis/api/shared';

import { KordisLogger } from '../services/kordis-logger.interface';

@Catch()
export class SentryExceptionsFilter implements ExceptionFilter {
readonly logger: KordisLogger;

constructor(_logger: Logger) {
this.logger = _logger;
}

catch(exception: unknown): void {
if (exception instanceof PresentableException) {
// if this is a presentable error, such as a validation error, we don't want to log it as an error but rather as an information to have the context for possible future debugging
Sentry.addBreadcrumb({
level: 'error',
message: exception.message,
data: {
code: exception.code,
name: exception.name,
stack: exception.stack,
},
this.logger.log(exception.message, {
code: exception.code,
name: exception.name,
stack: exception.stack,
});
} else {
this.logger.error(
'Caught unhandled exception that was not presentable',
undefined,
{
exception,
},
);

Sentry.captureException(exception, {
level: 'error',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ describe('SentryOTelUserContextInterceptor', () => {
interceptor = new SentryOTelUserContextInterceptor();
});

it('should be defined', () => {
expect(interceptor).toBeDefined();
afterEach(() => {
jest.clearAllMocks();
});

it('should set user context and attributes for Sentry and OpenTelemetry', async () => {
Expand Down Expand Up @@ -66,4 +66,27 @@ describe('SentryOTelUserContextInterceptor', () => {
username: `${user.firstName} ${user.lastName}`,
});
});

it('should ignore user context and attributes for Sentry and OpenTelemetry if no user is present', async () => {
const ctx = createGqlContextForRequest(
createMock<KordisRequest>({
user: undefined,
}),
);
const handler = createMock<CallHandler>({
handle(): Observable<boolean> {
return of(true);
},
});

const sentrySetUserSpy = jest.spyOn(Sentry, 'setUser');
const getActiveSpanSpy = jest.spyOn(trace, 'getActiveSpan');

await expect(
firstValueFrom(interceptor.intercept(ctx, handler)),
).resolves.toBeTruthy();

expect(sentrySetUserSpy).not.toHaveBeenCalled();
expect(getActiveSpanSpy).not.toHaveBeenCalled();
});
});
Loading

0 comments on commit 48061a5

Please sign in to comment.