Skip to content

Commit

Permalink
chore(api): Remove environmentId from auth JWT (#5768)
Browse files Browse the repository at this point in the history
Co-authored-by: Denis Kralj <[email protected]>
  • Loading branch information
SokratisVidros and denis-kralj-novu authored Jul 15, 2024
1 parent 05143b4 commit 3ea3c76
Show file tree
Hide file tree
Showing 151 changed files with 1,396 additions and 1,455 deletions.
2 changes: 1 addition & 1 deletion .source
18 changes: 6 additions & 12 deletions apps/api/src/app/auth/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
UseGuards,
UseInterceptors,
} from '@nestjs/common';
import { MemberEntity, MemberRepository, OrganizationRepository, UserRepository } from '@novu/dal';
import { MemberEntity, MemberRepository, UserRepository } from '@novu/dal';
import { AuthGuard } from '@nestjs/passport';
import { PasswordResetFlowEnum, UserSessionData } from '@novu/shared';
import { UserRegistrationBodyDto } from './dtos/user-registration.dto';
Expand Down Expand Up @@ -154,18 +154,16 @@ export class AuthController {
@UserAuthentication()
@HttpCode(200)
@Header('Cache-Control', 'no-store')
async organizationSwitch(
@UserSession() user: UserSessionData,
@Param('organizationId') organizationId: string
): Promise<string> {
async organizationSwitch(@UserSession() user: UserSessionData, @Param('organizationId') organizationId: string) {
const command = SwitchOrganizationCommand.create({
userId: user._id,
newOrganizationId: organizationId,
});

return await this.switchOrganizationUsecase.execute(command);
return this.switchOrganizationUsecase.execute(command);
}

// @deprecated - Will be removed after full deployment of Api and Dashboard.
@Post('/environments/:environmentId/switch')
@Header('Cache-Control', 'no-store')
@UserAuthentication()
Expand Down Expand Up @@ -203,18 +201,14 @@ export class AuthController {
}

@Get('/test/token/:userId')
async authenticateTest(
@Param('userId') userId: string,
@Query('organizationId') organizationId: string,
@Query('environmentId') environmentId: string
) {
async authenticateTest(@Param('userId') userId: string, @Query('organizationId') organizationId: string) {
if (process.env.NODE_ENV !== 'test') throw new NotFoundException();

const user = await this.userRepository.findById(userId);
if (!user) throw new BadRequestException('No user found');

const member = organizationId ? await this.memberRepository.findMemberByUserId(organizationId, user._id) : null;

return await this.authService.getSignedToken(user, organizationId, member as MemberEntity, environmentId);
return await this.authService.getSignedToken(user, organizationId, member as MemberEntity);
}
}
3 changes: 1 addition & 2 deletions apps/api/src/app/auth/e2e/user.auth.guard.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { UserSession } from '@novu/testing';
import { expect } from 'chai';
import { ApiAuthSchemeEnum } from '@novu/shared';
import { HttpRequestHeaderKeysEnum } from '../../shared/framework/types';
import { ApiAuthSchemeEnum, HttpRequestHeaderKeysEnum } from '@novu/shared';

describe('UserAuthGuard', () => {
let session: UserSession;
Expand Down
2 changes: 1 addition & 1 deletion apps/api/src/app/auth/services/passport/apikey.strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { PassportStrategy } from '@nestjs/passport';
import { Injectable } from '@nestjs/common';
import { AuthService } from '@novu/application-generic';
import { ApiAuthSchemeEnum, UserSessionData } from '@novu/shared';
import { HttpRequestHeaderKeysEnum } from '../../../shared/framework/types';
import { HttpRequestHeaderKeysEnum } from '@novu/shared';

@Injectable()
export class ApiKeyStrategy extends PassportStrategy(HeaderAPIKeyStrategy) {
Expand Down
24 changes: 20 additions & 4 deletions apps/api/src/app/auth/services/passport/jwt.strategy.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type http from 'http';
import { ExtractJwt, Strategy } from 'passport-jwt';
import { PassportStrategy } from '@nestjs/passport';
import { Injectable, UnauthorizedException } from '@nestjs/common';
import { UserSessionData } from '@novu/shared';
import { ApiAuthSchemeEnum, HttpRequestHeaderKeysEnum, UserSessionData } from '@novu/shared';
import { AuthService, Instrument } from '@novu/application-generic';

@Injectable()
Expand All @@ -10,16 +11,31 @@ export class JwtStrategy extends PassportStrategy(Strategy) {
super({
jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(),
secretOrKey: process.env.JWT_SECRET,
passReqToCallback: true,
});
}

@Instrument()
async validate(payload: UserSessionData) {
const user = await this.authService.validateUser(payload);
async validate(req: http.IncomingMessage, session: UserSessionData) {
// Set the scheme to Bearer, meaning the user is authenticated via a JWT coming from Dashboard
session.scheme = ApiAuthSchemeEnum.BEARER;

// Fetch the environmentId from the request header
const environmentIdFromHeader =
(req.headers[HttpRequestHeaderKeysEnum.NOVU_ENVIRONMENT_ID.toLowerCase()] as string) || '';

/*
* Ensure backwards compatibility with existing JWTs that contain environmentId
* or cached SPA versions of Dashboard as there is no guarantee all current users
* will have environmentId in localStorage instantly after the deployment.
*/
session.environmentId = session.environmentId || environmentIdFromHeader;

const user = await this.authService.validateUser(session);
if (!user) {
throw new UnauthorizedException();
}

return payload;
return session;
}
}
14 changes: 6 additions & 8 deletions apps/api/src/app/blueprint/e2e/get-grouped-blueprints.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,19 @@ describe('Get grouped notification template blueprints - /blueprints/group-by-ca
prodEnv,
});

const data = await session.testAgent.get(`/v1/blueprints/group-by-category`).send();

expect(data.statusCode).to.equal(200);

const groupedBlueprints = (data.body.data as GroupedBlueprintResponse).general;
const res1 = await session.testAgent.get(`/v1/blueprints/group-by-category`).send();
expect(res1.statusCode).to.equal(200);
const groupedBlueprints = (res1.body.data as GroupedBlueprintResponse).general;

expect(groupedBlueprints.length).to.equal(1);
expect(groupedBlueprints[0].name).to.equal('General');

const categoryName = 'Life Style';
await updateBlueprintCategory({ categoryName });

let updatedGroupedBluePrints = await session.testAgent.get(`/v1/blueprints/group-by-category`).send();

updatedGroupedBluePrints = (updatedGroupedBluePrints.body.data as GroupedBlueprintResponse).general;
const res2 = await session.testAgent.get(`/v1/blueprints/group-by-category`).send();
expect(res2.statusCode).to.equal(200);
const updatedGroupedBluePrints = (res2.body.data as GroupedBlueprintResponse).general;

expect(updatedGroupedBluePrints.length).to.equal(2);
expect(updatedGroupedBluePrints[0].name).to.equal('General');
Expand Down
6 changes: 2 additions & 4 deletions apps/api/src/app/environments/e2e/guard-check.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,11 @@ describe('Environment - Check Root Environment Guard', async () => {
const { body: envsBody } = await session.testAgent.get('/v1/environments');
const envs = envsBody.data;

const devEnvironment = envs.find((i) => i.name === 'Development');
await session.switchEnvironment(devEnvironment._id);
await session.switchToDevEnvironment();
const { body: devBody } = await session.testAgent.post(`/v1/workflows`).send(testTemplate);
expect(devBody.data).to.be.ok;

const prodEnvironment = envs.find((i) => !!i._parentId);
await session.switchEnvironment(prodEnvironment._id);
await session.switchToProdEnvironment();
const { body: prodBody } = await session.testAgent.post(`/v1/workflows`).send(testTemplate);
expect(prodBody.message).to.contain('This action is only allowed in Development');
expect(prodBody.statusCode).to.eq(401);
Expand Down
8 changes: 3 additions & 5 deletions apps/api/src/app/environments/environments.controller.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Body, ClassSerializerInterceptor, Controller, Get, Param, Post, Put, UseInterceptors } from '@nestjs/common';
import { UserSessionData } from '@novu/shared';
import { ApiAuthSchemeEnum, UserSessionData } from '@novu/shared';
import { UserSession } from '../shared/framework/user.decorator';
import { CreateEnvironment } from './usecases/create-environment/create-environment.usecase';
import { CreateEnvironmentCommand } from './usecases/create-environment/create-environment.command';
Expand Down Expand Up @@ -80,11 +80,9 @@ export class EnvironmentsController {
async listMyEnvironments(@UserSession() user: UserSessionData): Promise<EnvironmentResponseDto[]> {
return await this.getMyEnvironmentsUsecase.execute(
GetMyEnvironmentsCommand.create({
environmentId: user.environmentId,
userId: user._id,
organizationId: user.organizationId,
// TODO: This is a temporary patch to include API keys when Novu API is accessed via a user JWT token
includeApiKeys: user?.iss === 'novu_api' || user?.iss === process.env.CLERK_ISSUER_URL,
environmentId: user.environmentId,
includeAllApiKeys: user.scheme === ApiAuthSchemeEnum.BEARER,
})
);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { IsNotEmpty } from 'class-validator';
import { EnvironmentWithUserCommand } from '../../../shared/commands/project.command';
import { IsNotEmpty, IsOptional } from 'class-validator';
import { BaseCommand } from '../../../shared/commands/base.command';

export class GetMyEnvironmentsCommand extends EnvironmentWithUserCommand {
export class GetMyEnvironmentsCommand extends BaseCommand {
@IsNotEmpty()
readonly includeApiKeys: boolean;
readonly organizationId: string;

@IsOptional()
readonly environmentId: string;

@IsOptional()
readonly includeAllApiKeys: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ export class GetMyEnvironments {
const environments = await this.environmentRepository.findOrganizationEnvironments(command.organizationId);

if (!environments?.length)
throw new NotFoundException(`Environments for organization ${command.organizationId} not found`);
throw new NotFoundException(`No environments were found for organization ${command.organizationId}`);

return environments.map((environment) => {
if (command.includeApiKeys || environment._id === command.environmentId) {
if (command.includeAllApiKeys || environment._id === command.environmentId) {
return this.decryptApiKeys(environment);
}

// TODO: For api_v2: Remove the key from the response. This was not done yet as it's a breaking change.
environment.apiKeys = [];

return environment;
Expand Down
10 changes: 7 additions & 3 deletions apps/api/src/app/rate-limiting/guards/throttler.guard.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { UserSession } from '@novu/testing';
import { expect } from 'chai';
import { ApiRateLimitCategoryEnum, ApiRateLimitCostEnum, ApiServiceLevelEnum } from '@novu/shared';
import { HttpResponseHeaderKeysEnum } from '../../shared/framework/types';
import {
ApiRateLimitCategoryEnum,
ApiRateLimitCostEnum,
ApiServiceLevelEnum,
HttpResponseHeaderKeysEnum,
} from '@novu/shared';

const mockSingleCost = 1;
const mockBulkCost = 5;
Expand Down Expand Up @@ -309,7 +313,7 @@ describe('API Rate Limiting', () => {
}) => {
return () => {
describe(`${expectedStatus === 429 ? 'Throttled' : 'Allowed'} ${name}`, () => {
let lastResponse: ReturnType<typeof UserSession.prototype.testAgent.get>;
let lastResponse;
let throttledResponseCount = 0;
const throttledResponseCountTolerance = 0.5;
const expectedWindowLimit = expectedLimit * mockWindowDuration;
Expand Down
3 changes: 2 additions & 1 deletion apps/api/src/app/rate-limiting/guards/throttler.guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ import {
ApiAuthSchemeEnum,
ApiRateLimitCategoryEnum,
ApiRateLimitCostEnum,
HttpRequestHeaderKeysEnum,
HttpResponseHeaderKeysEnum,
FeatureFlagsKeysEnum,
UserSessionData,
} from '@novu/shared';
import { ThrottlerCategory, ThrottlerCost } from './throttler.decorator';
import { HttpRequestHeaderKeysEnum, HttpResponseHeaderKeysEnum } from '../../shared/framework/types';

export const THROTTLED_EXCEPTION_MESSAGE = 'API rate limit exceeded';
export const ALLOWED_AUTH_SCHEMES = [ApiAuthSchemeEnum.API_KEY];
Expand Down
14 changes: 4 additions & 10 deletions apps/api/src/app/shared/commands/project.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@ import { BaseCommand } from './base.command';

export { BaseCommand };

export abstract class EnvironmentWithUserCommand extends BaseCommand {
export abstract class EnvironmentCommand extends BaseCommand {
@IsNotEmpty()
readonly environmentId: string;

@IsNotEmpty()
readonly organizationId: string;
}

export abstract class EnvironmentWithUserCommand extends EnvironmentCommand {
@IsNotEmpty()
readonly userId: string;
}

export abstract class EnvironmentWithSubscriber extends BaseCommand {
export abstract class EnvironmentWithSubscriber extends EnvironmentCommand {
@IsNotEmpty()
readonly environmentId: string;

Expand All @@ -24,11 +26,3 @@ export abstract class EnvironmentWithSubscriber extends BaseCommand {
@IsNotEmpty()
readonly subscriberId: string;
}

export abstract class EnvironmentCommand extends BaseCommand {
@IsNotEmpty()
readonly environmentId: string;

@IsNotEmpty()
readonly organizationId: string;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { HeaderObject, HttpResponseHeaderKeysEnum } from '../types/headers.types';
import { HeaderObject, HttpResponseHeaderKeysEnum } from '@novu/shared';

export const COMMON_RESPONSE_HEADERS: Array<HttpResponseHeaderKeysEnum> = [
HttpResponseHeaderKeysEnum.CONTENT_TYPE,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ApiResponseOptions } from '@nestjs/swagger';
import { THROTTLED_EXCEPTION_MESSAGE } from '../../../rate-limiting/guards';
import { createReusableHeaders } from '../swagger';
import { ApiResponseDecoratorName, HttpResponseHeaderKeysEnum } from '../types';
import { ApiResponseDecoratorName, HttpResponseHeaderKeysEnum } from '@novu/shared';

export const COMMON_RESPONSES: Partial<Record<ApiResponseDecoratorName, ApiResponseOptions>> = {
ApiConflictResponse: {
Expand Down
2 changes: 1 addition & 1 deletion apps/api/src/app/shared/framework/idempotency.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { UserSession } from '@novu/testing';
import { HttpResponseHeaderKeysEnum } from '@novu/shared';
import { CacheService } from '@novu/application-generic';
import { expect } from 'chai';
import { HttpResponseHeaderKeysEnum } from './types';
import { DOCS_LINK } from './idempotency.interceptor';

process.env.LAUNCH_DARKLY_SDK_KEY = ''; // disable Launch Darkly to allow test to define FF state
Expand Down
3 changes: 1 addition & 2 deletions apps/api/src/app/shared/framework/idempotency.interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ import { CacheService, GetFeatureFlag, GetFeatureFlagCommand, Instrument } from
import { Observable, of, throwError } from 'rxjs';
import { catchError, map } from 'rxjs/operators';
import { createHash } from 'crypto';
import { ApiAuthSchemeEnum, FeatureFlagsKeysEnum, UserSessionData } from '@novu/shared';
import { HttpResponseHeaderKeysEnum } from './types';
import { ApiAuthSchemeEnum, HttpResponseHeaderKeysEnum, FeatureFlagsKeysEnum, UserSessionData } from '@novu/shared';

const LOG_CONTEXT = 'IdempotencyInterceptor';
const IDEMPOTENCY_CACHE_TTL = 60 * 60 * 24; //24h
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { HeaderObjects, HttpResponseHeaderKeysEnum } from '../types';
import { HeaderObjects, HttpResponseHeaderKeysEnum } from '@novu/shared';
import { RESPONSE_HEADER_CONFIG } from '../constants/headers.schema';
import { OpenAPIObject } from '@nestjs/swagger';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { applyDecorators } from '@nestjs/common';
import * as nestSwagger from '@nestjs/swagger';
import { ApiResponseOptions } from '@nestjs/swagger';
import { COMMON_RESPONSE_HEADERS, COMMON_RESPONSES } from '../constants';
import { ApiResponseDecoratorName } from '../types';
import { ApiResponseDecoratorName } from '@novu/shared';
import { createReusableHeaders } from './headers.decorator';

const createCustomResponseDecorator = (decoratorName: ApiResponseDecoratorName) => {
Expand Down
2 changes: 1 addition & 1 deletion apps/api/src/config/cors.config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { INestApplication, Logger } from '@nestjs/common';
import { HttpRequestHeaderKeysEnum } from '../app/shared/framework/types';
import { HttpRequestHeaderKeysEnum } from '@novu/shared';

export const corsOptionsDelegate: Parameters<INestApplication['enableCors']>[0] = function (req: Request, callback) {
const corsOptions: Parameters<typeof callback>[1] = {
Expand Down
1 change: 1 addition & 0 deletions apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"start:test": "http-server build -p 4200 --proxy http://127.0.0.1:4200?",
"test:e2e": "playwright test",
"test:e2e:ui": "playwright test --ui",
"test:e2e:debug": "playwright test --debug",
"test:e2e:install": "playwright install --with-deps",
"test:e2e:codegen": "playwright codegen",
"test:e2e:show-report": "npx playwright show-report",
Expand Down
6 changes: 4 additions & 2 deletions apps/web/src/AppRoutes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,10 @@ export const AppRoutes = () => {
<Route path={ROUTES.SUBSCRIBERS} element={<SubscribersList />} />
<Route path="/translations/*" element={<TranslationRoutes />} />
<Route path={ROUTES.LAYOUT} element={<LayoutsPage />} />
<Route path={ROUTES.API_KEYS} element={<ApiKeysPage />} />
<Route path={ROUTES.WEBHOOK} element={<WebhookPage />} />
{/* The * in this case is for backwards compatibility with the previous version of these paths
that included the environment name in the URL i.e. /api-keys/Development */}
<Route path={`${ROUTES.API_KEYS}/*`} element={<ApiKeysPage />} />
<Route path={`${ROUTES.WEBHOOK}/*`} element={<WebhookPage />} />
<Route path={ROUTES.ANY} element={<HomePage />} />
</Route>

Expand Down
7 changes: 5 additions & 2 deletions apps/web/src/ApplicationReadyGuard.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { type PropsWithChildren, useLayoutEffect } from 'react';
import { useAuth } from './hooks/useAuth';
import { useAuth, useEnvironment } from './hooks';

export function ApplicationReadyGuard({ children }: PropsWithChildren<{}>) {
const { isLoading, inPublicRoute } = useAuth();
const { isLoading: isLoadingAuth, inPublicRoute } = useAuth();
const { isLoading: isLoadingEnvironment } = useEnvironment();

const isLoading = isLoadingAuth || isLoadingEnvironment;

// Clean up the skeleton loader when the app is ready
useLayoutEffect(() => {
Expand Down
Loading

0 comments on commit 3ea3c76

Please sign in to comment.