diff --git a/packages/cli/package.json b/packages/cli/package.json index f30e78827d..8ffaae01b1 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -72,10 +72,8 @@ "@types/json-diff": "^1.0.0", "@types/jsonwebtoken": "^9.0.1", "@types/lodash": "^4.14.195", - "@types/passport-jwt": "^3.0.6", "@types/psl": "^1.1.0", "@types/replacestream": "^4.0.1", - "@types/send": "^0.17.1", "@types/shelljs": "^0.8.11", "@types/sshpk": "^1.17.1", "@types/superagent": "4.1.13", @@ -153,9 +151,6 @@ "otpauth": "9.1.1", "p-cancelable": "2.1.1", "p-lazy": "3.1.0", - "passport": "0.6.0", - "passport-cookie": "1.0.9", - "passport-jwt": "4.0.1", "pg": "8.11.3", "picocolors": "1.0.0", "pkce-challenge": "3.0.0", diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 6bb0d1b933..9d1867f1c8 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -613,18 +613,6 @@ export interface ILicensePostResponse extends ILicenseReadResponse { managementToken: string; } -export interface JwtToken { - token: string; - /** The amount of seconds after which the JWT will expire. **/ - expiresIn: number; -} - -export interface JwtPayload { - id: string; - email: string | null; - password: string | null; -} - export interface PublicUser { id: string; email?: string; diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 357ae8ec44..852def1531 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -62,7 +62,6 @@ import { EventBusController } from '@/eventbus/eventBus.controller'; import { EventBusControllerEE } from '@/eventbus/eventBus.controller.ee'; import { LicenseController } from '@/license/license.controller'; import { setupPushServer, setupPushHandler } from '@/push'; -import { setupAuthMiddlewares } from './middlewares'; import { isLdapEnabled } from './Ldap/helpers'; import { AbstractServer } from './AbstractServer'; import { PostHogClient } from './posthog'; @@ -129,9 +128,8 @@ export class Server extends AbstractServer { Container.get(CollaborationService); } - private async registerControllers(ignoredEndpoints: Readonly) { + private async registerControllers() { const { app } = this; - setupAuthMiddlewares(app, ignoredEndpoints, this.restEndpoint); const controllers: Array> = [ EventBusController, @@ -278,7 +276,7 @@ export class Server extends AbstractServer { await handleMfaDisable(); - await this.registerControllers(ignoredEndpoints); + await this.registerControllers(); // ---------------------------------------- // SAML diff --git a/packages/cli/src/auth/auth.service.ts b/packages/cli/src/auth/auth.service.ts new file mode 100644 index 0000000000..a78360ddc6 --- /dev/null +++ b/packages/cli/src/auth/auth.service.ts @@ -0,0 +1,224 @@ +import { Service } from 'typedi'; +import type { Response } from 'express'; +import { createHash } from 'crypto'; +import { JsonWebTokenError, TokenExpiredError, type JwtPayload } from 'jsonwebtoken'; + +import config from '@/config'; +import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES, Time } from '@/constants'; +import type { User } from '@db/entities/User'; +import { UserRepository } from '@db/repositories/user.repository'; +import type { AuthRole } from '@/decorators/types'; +import { AuthError } from '@/errors/response-errors/auth.error'; +import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; +import { License } from '@/License'; +import { Logger } from '@/Logger'; +import type { AuthenticatedRequest, AsyncRequestHandler } from '@/requests'; +import { JwtService } from '@/services/jwt.service'; +import { UrlService } from '@/services/url.service'; + +interface AuthJwtPayload { + /** User Id */ + id: string; + /** User's email */ + email: string | null; + /** SHA-256 hash of bcrypt hash of the user's password */ + password: string | null; +} + +interface IssuedJWT extends AuthJwtPayload { + exp: number; +} + +@Service() +export class AuthService { + private middlewareCache = new Map(); + + constructor( + private readonly logger: Logger, + private readonly license: License, + private readonly jwtService: JwtService, + private readonly urlService: UrlService, + private readonly userRepository: UserRepository, + ) {} + + createAuthMiddleware(authRole: AuthRole): AsyncRequestHandler { + const { middlewareCache: cache } = this; + let authMiddleware = cache.get(authRole); + if (authMiddleware) return authMiddleware; + + authMiddleware = async (req: AuthenticatedRequest, res, next) => { + if (authRole === 'none') { + next(); + return; + } + + const token = req.cookies[AUTH_COOKIE_NAME]; + if (token) { + try { + req.user = await this.resolveJwt(token, res); + } catch (error) { + if (error instanceof JsonWebTokenError || error instanceof AuthError) { + this.clearCookie(res); + } else { + throw error; + } + } + } + + if (!req.user) { + res.status(401).json({ status: 'error', message: 'Unauthorized' }); + return; + } + + if (authRole === 'any' || authRole === req.user.role) { + next(); + } else { + res.status(403).json({ status: 'error', message: 'Forbidden' }); + } + }; + + cache.set(authRole, authMiddleware); + return authMiddleware; + } + + clearCookie(res: Response) { + res.clearCookie(AUTH_COOKIE_NAME); + } + + issueCookie(res: Response, user: User) { + // If the instance has exceeded its user quota, prevent non-owners from logging in + const isWithinUsersLimit = this.license.isWithinUsersLimit(); + if ( + config.getEnv('userManagement.isInstanceOwnerSetUp') && + !user.isOwner && + !isWithinUsersLimit + ) { + throw new UnauthorizedError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED); + } + + const token = this.issueJWT(user); + res.cookie(AUTH_COOKIE_NAME, token, { + maxAge: this.jwtExpiration * Time.seconds.toMilliseconds, + httpOnly: true, + sameSite: 'lax', + }); + } + + issueJWT(user: User) { + const { id, email, password } = user; + const payload: AuthJwtPayload = { + id, + email, + password: password ? this.createPasswordSha(user) : null, + }; + return this.jwtService.sign(payload, { + expiresIn: this.jwtExpiration, + }); + } + + async resolveJwt(token: string, res: Response): Promise { + const jwtPayload: IssuedJWT = this.jwtService.verify(token, { + algorithms: ['HS256'], + }); + + // TODO: Use an in-memory ttl-cache to cache the User object for upto a minute + const user = await this.userRepository.findOne({ + where: { id: jwtPayload.id }, + }); + + // TODO: include these checks in the cache, to avoid computed this over and over again + const passwordHash = user?.password ? this.createPasswordSha(user) : null; + + if ( + // If not user is found + !user || + // or, If the user has been deactivated (i.e. LDAP users) + user.disabled || + // or, If the password has been updated + jwtPayload.password !== passwordHash || + // or, If the email has been updated + user.email !== jwtPayload.email + ) { + throw new AuthError('Unauthorized'); + } + + if (jwtPayload.exp * 1000 - Date.now() < this.jwtRefreshTimeout) { + this.logger.debug('JWT about to expire. Will be refreshed'); + this.issueCookie(res, user); + } + + return user; + } + + generatePasswordResetToken(user: User, expiresIn = '20m') { + return this.jwtService.sign( + { sub: user.id, passwordSha: this.createPasswordSha(user) }, + { expiresIn }, + ); + } + + generatePasswordResetUrl(user: User) { + const instanceBaseUrl = this.urlService.getInstanceBaseUrl(); + const url = new URL(`${instanceBaseUrl}/change-password`); + + url.searchParams.append('token', this.generatePasswordResetToken(user)); + url.searchParams.append('mfaEnabled', user.mfaEnabled.toString()); + + return url.toString(); + } + + async resolvePasswordResetToken(token: string): Promise { + let decodedToken: JwtPayload & { passwordSha: string }; + try { + decodedToken = this.jwtService.verify(token); + } catch (e) { + if (e instanceof TokenExpiredError) { + this.logger.debug('Reset password token expired', { token }); + } else { + this.logger.debug('Error verifying token', { token }); + } + return; + } + + const user = await this.userRepository.findOne({ + where: { id: decodedToken.sub }, + relations: ['authIdentities'], + }); + + if (!user) { + this.logger.debug( + 'Request to resolve password token failed because no user was found for the provided user ID', + { userId: decodedToken.sub, token }, + ); + return; + } + + if (this.createPasswordSha(user) !== decodedToken.passwordSha) { + this.logger.debug('Password updated since this token was generated'); + return; + } + + return user; + } + + private createPasswordSha({ password }: User) { + return createHash('sha256') + .update(password.slice(password.length / 2)) + .digest('hex'); + } + + /** How many **milliseconds** before expiration should a JWT be renewed */ + get jwtRefreshTimeout() { + const { jwtRefreshTimeoutHours, jwtSessionDurationHours } = config.get('userManagement'); + if (jwtRefreshTimeoutHours === 0) { + return Math.floor(jwtSessionDurationHours * 0.25 * Time.hours.toMilliseconds); + } else { + return Math.floor(jwtRefreshTimeoutHours * Time.hours.toMilliseconds); + } + } + + /** How many **seconds** is an issued JWT valid for */ + get jwtExpiration() { + return config.get('userManagement.jwtSessionDurationHours') * Time.hours.toSeconds; + } +} diff --git a/packages/cli/src/auth/jwt.ts b/packages/cli/src/auth/jwt.ts index affc9ea75f..1d63f97fa3 100644 --- a/packages/cli/src/auth/jwt.ts +++ b/packages/cli/src/auth/jwt.ts @@ -1,94 +1,12 @@ -import type { Response } from 'express'; -import { createHash } from 'crypto'; -import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES, Time } from '@/constants'; -import type { JwtPayload, JwtToken } from '@/Interfaces'; -import type { User } from '@db/entities/User'; -import config from '@/config'; -import { License } from '@/License'; import { Container } from 'typedi'; -import { UserRepository } from '@db/repositories/user.repository'; -import { JwtService } from '@/services/jwt.service'; -import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; -import { AuthError } from '@/errors/response-errors/auth.error'; -import { ApplicationError } from 'n8n-workflow'; +import type { Response } from 'express'; -export function issueJWT(user: User): JwtToken { - const { id, email, password } = user; - const expiresInHours = config.getEnv('userManagement.jwtSessionDurationHours'); - const expiresInSeconds = expiresInHours * Time.hours.toSeconds; +import type { User } from '@db/entities/User'; +import { AuthService } from './auth.service'; - const isWithinUsersLimit = Container.get(License).isWithinUsersLimit(); - - const payload: JwtPayload = { - id, - email, - password: password ?? null, - }; - - if ( - config.getEnv('userManagement.isInstanceOwnerSetUp') && - !user.isOwner && - !isWithinUsersLimit - ) { - throw new UnauthorizedError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED); - } - if (password) { - payload.password = createHash('sha256') - .update(password.slice(password.length / 2)) - .digest('hex'); - } - - const signedToken = Container.get(JwtService).sign(payload, { - expiresIn: expiresInSeconds, - }); - - return { - token: signedToken, - expiresIn: expiresInSeconds, - }; -} - -export const createPasswordSha = (user: User) => - createHash('sha256') - .update(user.password.slice(user.password.length / 2)) - .digest('hex'); - -export async function resolveJwtContent(jwtPayload: JwtPayload): Promise { - const user = await Container.get(UserRepository).findOne({ - where: { id: jwtPayload.id }, - }); - - let passwordHash = null; - if (user?.password) { - passwordHash = createPasswordSha(user); - } - - // currently only LDAP users during synchronization - // can be set to disabled - if (user?.disabled) { - throw new AuthError('Unauthorized'); - } - - if (!user || jwtPayload.password !== passwordHash || user.email !== jwtPayload.email) { - // When owner hasn't been set up, the default user - // won't have email nor password (both equals null) - throw new ApplicationError('Invalid token content'); - } - return user; -} - -export async function resolveJwt(token: string): Promise { - const jwtPayload: JwtPayload = Container.get(JwtService).verify(token, { - algorithms: ['HS256'], - }); - return await resolveJwtContent(jwtPayload); -} - -export async function issueCookie(res: Response, user: User): Promise { - const userData = issueJWT(user); - res.cookie(AUTH_COOKIE_NAME, userData.token, { - maxAge: userData.expiresIn * Time.seconds.toMilliseconds, - httpOnly: true, - sameSite: 'lax', - }); +// This method is still used by cloud hooks. +// DO NOT DELETE until the hooks have been updated +/** @deprecated Use `AuthService` instead */ +export function issueCookie(res: Response, user: User) { + return Container.get(AuthService).issueCookie(res, user); } diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index 9e65e4c342..aecd7cd1ef 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -131,6 +131,7 @@ export const Time = { }, days: { toSeconds: 24 * 60 * 60, + toMilliseconds: 24 * 60 * 60 * 1000, }, }; diff --git a/packages/cli/src/controllers/auth.controller.ts b/packages/cli/src/controllers/auth.controller.ts index 7ba13a2677..eda3914b36 100644 --- a/packages/cli/src/controllers/auth.controller.ts +++ b/packages/cli/src/controllers/auth.controller.ts @@ -1,12 +1,12 @@ import validator from 'validator'; + +import { AuthService } from '@/auth/auth.service'; import { Authorized, Get, Post, RestController } from '@/decorators'; -import { issueCookie, resolveJwt } from '@/auth/jwt'; -import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from '@/constants'; +import { RESPONSE_ERROR_MESSAGES } from '@/constants'; import { Request, Response } from 'express'; import type { User } from '@db/entities/User'; -import { LoginRequest, UserRequest } from '@/requests'; +import { AuthenticatedRequest, LoginRequest, UserRequest } from '@/requests'; import type { PublicUser } from '@/Interfaces'; -import config from '@/config'; import { handleEmailLogin, handleLdapLogin } from '@/auth'; import { PostHogClient } from '@/posthog'; import { @@ -20,7 +20,6 @@ import { UserService } from '@/services/user.service'; import { MfaService } from '@/Mfa/mfa.service'; import { Logger } from '@/Logger'; import { AuthError } from '@/errors/response-errors/auth.error'; -import { InternalServerError } from '@/errors/response-errors/internal-server.error'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; import { ApplicationError } from 'n8n-workflow'; @@ -31,6 +30,7 @@ export class AuthController { constructor( private readonly logger: Logger, private readonly internalHooks: InternalHooks, + private readonly authService: AuthService, private readonly mfaService: MfaService, private readonly userService: UserService, private readonly license: License, @@ -96,7 +96,7 @@ export class AuthController { } } - await issueCookie(res, user); + this.authService.issueCookie(res, user); void this.internalHooks.onUserLoginSuccess({ user, authenticationMethod: usedAuthenticationMethod, @@ -112,45 +112,14 @@ export class AuthController { throw new AuthError('Wrong username or password. Do you have caps lock on?'); } - /** - * Manually check the `n8n-auth` cookie. - */ + /** Check if the user is already logged in */ + @Authorized() @Get('/login') - async currentUser(req: Request, res: Response): Promise { - // Manually check the existing cookie. - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - const cookieContents = req.cookies?.[AUTH_COOKIE_NAME] as string | undefined; - - let user: User; - if (cookieContents) { - // If logged in, return user - try { - user = await resolveJwt(cookieContents); - - return await this.userService.toPublic(user, { posthog: this.postHog, withScopes: true }); - } catch (error) { - res.clearCookie(AUTH_COOKIE_NAME); - } - } - - if (config.getEnv('userManagement.isInstanceOwnerSetUp')) { - throw new AuthError('Not logged in'); - } - - try { - user = await this.userRepository.findOneOrFail({ where: {} }); - } catch (error) { - throw new InternalServerError( - 'No users found in database - did you wipe the users table? Create at least one user.', - ); - } - - if (user.email || user.password) { - throw new InternalServerError('Invalid database state - user has password set.'); - } - - await issueCookie(res, user); - return await this.userService.toPublic(user, { posthog: this.postHog, withScopes: true }); + async currentUser(req: AuthenticatedRequest): Promise { + return await this.userService.toPublic(req.user, { + posthog: this.postHog, + withScopes: true, + }); } /** @@ -228,8 +197,8 @@ export class AuthController { */ @Authorized() @Post('/logout') - logout(req: Request, res: Response) { - res.clearCookie(AUTH_COOKIE_NAME); + logout(_: Request, res: Response) { + this.authService.clearCookie(res); return { loggedOut: true }; } diff --git a/packages/cli/src/controllers/invitation.controller.ts b/packages/cli/src/controllers/invitation.controller.ts index 022ead6ed1..3a96e594f3 100644 --- a/packages/cli/src/controllers/invitation.controller.ts +++ b/packages/cli/src/controllers/invitation.controller.ts @@ -1,9 +1,9 @@ import { Response } from 'express'; import validator from 'validator'; +import { AuthService } from '@/auth/auth.service'; import config from '@/config'; import { Authorized, NoAuthRequired, Post, RequireGlobalScope, RestController } from '@/decorators'; -import { issueCookie } from '@/auth/jwt'; import { RESPONSE_ERROR_MESSAGES } from '@/constants'; import { UserRequest } from '@/requests'; import { License } from '@/License'; @@ -26,6 +26,7 @@ export class InvitationController { private readonly logger: Logger, private readonly internalHooks: InternalHooks, private readonly externalHooks: ExternalHooks, + private readonly authService: AuthService, private readonly userService: UserService, private readonly license: License, private readonly passwordUtility: PasswordUtility, @@ -165,7 +166,7 @@ export class InvitationController { const updatedUser = await this.userRepository.save(invitee, { transaction: false }); - await issueCookie(res, updatedUser); + this.authService.issueCookie(res, updatedUser); void this.internalHooks.onUserSignup(updatedUser, { user_type: 'email', diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index 1d09cb8ce6..5c5d8a9657 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -2,10 +2,11 @@ import validator from 'validator'; import { plainToInstance } from 'class-transformer'; import { Response } from 'express'; import { randomBytes } from 'crypto'; + +import { AuthService } from '@/auth/auth.service'; import { Authorized, Delete, Get, Patch, Post, RestController } from '@/decorators'; import { PasswordUtility } from '@/services/password.utility'; import { validateEntity } from '@/GenericHelpers'; -import { issueCookie } from '@/auth/jwt'; import type { User } from '@db/entities/User'; import { AuthenticatedRequest, @@ -14,7 +15,7 @@ import { UserUpdatePayload, } from '@/requests'; import type { PublicUser } from '@/Interfaces'; -import { isSamlLicensedAndEnabled } from '../sso/saml/samlHelpers'; +import { isSamlLicensedAndEnabled } from '@/sso/saml/samlHelpers'; import { UserService } from '@/services/user.service'; import { Logger } from '@/Logger'; import { ExternalHooks } from '@/ExternalHooks'; @@ -29,6 +30,7 @@ export class MeController { private readonly logger: Logger, private readonly externalHooks: ExternalHooks, private readonly internalHooks: InternalHooks, + private readonly authService: AuthService, private readonly userService: UserService, private readonly passwordUtility: PasswordUtility, private readonly userRepository: UserRepository, @@ -84,7 +86,7 @@ export class MeController { this.logger.info('User updated successfully', { userId }); - await issueCookie(res, user); + this.authService.issueCookie(res, user); const updatedKeys = Object.keys(payload); void this.internalHooks.onUserUpdate({ @@ -137,7 +139,7 @@ export class MeController { const updatedUser = await this.userRepository.save(user, { transaction: false }); this.logger.info('Password updated successfully', { userId: user.id }); - await issueCookie(res, updatedUser); + this.authService.issueCookie(res, updatedUser); void this.internalHooks.onUserUpdate({ user: updatedUser, diff --git a/packages/cli/src/controllers/owner.controller.ts b/packages/cli/src/controllers/owner.controller.ts index 15d0bc2c5c..f1b24a2853 100644 --- a/packages/cli/src/controllers/owner.controller.ts +++ b/packages/cli/src/controllers/owner.controller.ts @@ -1,19 +1,19 @@ import validator from 'validator'; import { Response } from 'express'; +import { AuthService } from '@/auth/auth.service'; import config from '@/config'; import { validateEntity } from '@/GenericHelpers'; import { Authorized, Post, RestController } from '@/decorators'; import { PasswordUtility } from '@/services/password.utility'; -import { issueCookie } from '@/auth/jwt'; import { OwnerRequest } from '@/requests'; import { SettingsRepository } from '@db/repositories/settings.repository'; +import { UserRepository } from '@db/repositories/user.repository'; import { PostHogClient } from '@/posthog'; import { UserService } from '@/services/user.service'; import { Logger } from '@/Logger'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { InternalHooks } from '@/InternalHooks'; -import { UserRepository } from '@/databases/repositories/user.repository'; @Authorized('global:owner') @RestController('/owner') @@ -22,6 +22,7 @@ export class OwnerController { private readonly logger: Logger, private readonly internalHooks: InternalHooks, private readonly settingsRepository: SettingsRepository, + private readonly authService: AuthService, private readonly userService: UserService, private readonly passwordUtility: PasswordUtility, private readonly postHog: PostHogClient, @@ -89,7 +90,7 @@ export class OwnerController { this.logger.debug('Setting isInstanceOwnerSetUp updated successfully', { userId }); - await issueCookie(res, owner); + this.authService.issueCookie(res, owner); void this.internalHooks.onInstanceOwnerSetup({ user_id: userId }); diff --git a/packages/cli/src/controllers/passwordReset.controller.ts b/packages/cli/src/controllers/passwordReset.controller.ts index 806d892d26..6eb073be13 100644 --- a/packages/cli/src/controllers/passwordReset.controller.ts +++ b/packages/cli/src/controllers/passwordReset.controller.ts @@ -2,11 +2,11 @@ import { Response } from 'express'; import { rateLimit } from 'express-rate-limit'; import validator from 'validator'; +import { AuthService } from '@/auth/auth.service'; import { Get, Post, RestController } from '@/decorators'; import { PasswordUtility } from '@/services/password.utility'; import { UserManagementMailer } from '@/UserManagement/email'; import { PasswordResetRequest } from '@/requests'; -import { issueCookie } from '@/auth/jwt'; import { isSamlCurrentAuthenticationMethod } from '@/sso/ssoHelpers'; import { UserService } from '@/services/user.service'; import { License } from '@/License'; @@ -36,6 +36,7 @@ export class PasswordResetController { private readonly externalHooks: ExternalHooks, private readonly internalHooks: InternalHooks, private readonly mailer: UserManagementMailer, + private readonly authService: AuthService, private readonly userService: UserService, private readonly mfaService: MfaService, private readonly urlService: UrlService, @@ -114,7 +115,7 @@ export class PasswordResetController { throw new UnprocessableRequestError('forgotPassword.ldapUserPasswordResetUnavailable'); } - const url = this.userService.generatePasswordResetUrl(user); + const url = this.authService.generatePasswordResetUrl(user); const { id, firstName, lastName } = user; try { @@ -163,7 +164,7 @@ export class PasswordResetController { throw new BadRequestError(''); } - const user = await this.userService.resolvePasswordResetToken(token); + const user = await this.authService.resolvePasswordResetToken(token); if (!user) throw new NotFoundError(''); if (!user?.isOwner && !this.license.isWithinUsersLimit()) { @@ -197,7 +198,7 @@ export class PasswordResetController { const validPassword = this.passwordUtility.validate(password); - const user = await this.userService.resolvePasswordResetToken(token); + const user = await this.authService.resolvePasswordResetToken(token); if (!user) throw new NotFoundError(''); if (user.mfaEnabled) { @@ -216,7 +217,7 @@ export class PasswordResetController { this.logger.info('User password updated successfully', { userId: user.id }); - await issueCookie(res, user); + this.authService.issueCookie(res, user); void this.internalHooks.onUserUpdate({ user, diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts index 2ed94e1345..90b8e53631 100644 --- a/packages/cli/src/controllers/users.controller.ts +++ b/packages/cli/src/controllers/users.controller.ts @@ -1,3 +1,6 @@ +import { plainToInstance } from 'class-transformer'; + +import { AuthService } from '@/auth/auth.service'; import { User } from '@db/entities/User'; import { SharedCredentials } from '@db/entities/SharedCredentials'; import { SharedWorkflow } from '@db/entities/SharedWorkflow'; @@ -22,7 +25,6 @@ import { AuthIdentity } from '@db/entities/AuthIdentity'; import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; import { UserRepository } from '@db/repositories/user.repository'; -import { plainToInstance } from 'class-transformer'; import { UserService } from '@/services/user.service'; import { listQueryMiddleware } from '@/middlewares'; import { Logger } from '@/Logger'; @@ -44,6 +46,7 @@ export class UsersController { private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly userRepository: UserRepository, private readonly activeWorkflowRunner: ActiveWorkflowRunner, + private readonly authService: AuthService, private readonly userService: UserService, ) {} @@ -116,7 +119,7 @@ export class UsersController { throw new NotFoundError('User not found'); } - const link = this.userService.generatePasswordResetUrl(user); + const link = this.authService.generatePasswordResetUrl(user); return { link }; } diff --git a/packages/cli/src/decorators/registerController.ts b/packages/cli/src/decorators/registerController.ts index f6abd260fa..3bb04b7d77 100644 --- a/packages/cli/src/decorators/registerController.ts +++ b/packages/cli/src/decorators/registerController.ts @@ -1,9 +1,14 @@ import { Container } from 'typedi'; import { Router } from 'express'; import type { Application, Request, Response, RequestHandler } from 'express'; +import type { Scope } from '@n8n/permissions'; +import { ApplicationError } from 'n8n-workflow'; import type { Class } from 'n8n-core'; +import { AuthService } from '@/auth/auth.service'; import config from '@/config'; +import type { BooleanLicenseFeature } from '@/Interfaces'; +import { License } from '@/License'; import type { AuthenticatedRequest } from '@/requests'; import { send } from '@/ResponseHelper'; // TODO: move `ResponseHelper.send` to this file import { @@ -15,7 +20,6 @@ import { CONTROLLER_ROUTES, } from './constants'; import type { - AuthRole, AuthRoleMetadata, Controller, LicenseMetadata, @@ -23,23 +27,6 @@ import type { RouteMetadata, ScopeMetadata, } from './types'; -import type { BooleanLicenseFeature } from '@/Interfaces'; - -import { License } from '@/License'; -import type { Scope } from '@n8n/permissions'; -import { ApplicationError } from 'n8n-workflow'; - -export const createAuthMiddleware = - (authRole: AuthRole): RequestHandler => - ({ user }: AuthenticatedRequest, res, next) => { - if (authRole === 'none') return next(); - - if (!user) return res.status(401).json({ status: 'error', message: 'Unauthorized' }); - - if (authRole === 'any' || authRole === user.role) return next(); - - res.status(403).json({ status: 'error', message: 'Unauthorized' }); - }; export const createLicenseMiddleware = (features: BooleanLicenseFeature[]): RequestHandler => @@ -77,11 +64,6 @@ export const createGlobalScopeMiddleware = return next(); }; -const authFreeRoutes: string[] = []; - -export const canSkipAuth = (method: string, path: string): boolean => - authFreeRoutes.includes(`${method.toLowerCase()} ${path}`); - export const registerController = (app: Application, controllerClass: Class) => { const controller = Container.get(controllerClass as Class); const controllerBasePath = Reflect.getMetadata(CONTROLLER_BASE_PATH, controllerClass) as @@ -114,6 +96,8 @@ export const registerController = (app: Application, controllerClass: Class controller[handlerName].bind(controller) as RequestHandler); + const authService = Container.get(AuthService); + routes.forEach( ({ method, path, middlewares: routeMiddlewares, handlerName, usesTemplates }) => { const authRole = authRoles?.[handlerName] ?? authRoles?.['*']; @@ -123,14 +107,13 @@ export const registerController = (app: Application, controllerClass: Class { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return (req.cookies?.[AUTH_COOKIE_NAME] as string | undefined) ?? null; -}; - -const userManagementJwtAuth = (): RequestHandler => { - const jwtStrategy = new Strategy( - { - jwtFromRequest, - secretOrKey: Container.get(JwtService).jwtSecret, - }, - async (jwtPayload: JwtPayload, done) => { - try { - const user = await resolveJwtContent(jwtPayload); - return done(null, user); - } catch (error) { - Container.get(Logger).debug('Failed to extract user from JWT payload', { jwtPayload }); - return done(null, false, { message: 'User not found' }); - } - }, - ); - - passport.use(jwtStrategy); - return passport.initialize(); -}; - -/** - * middleware to refresh cookie before it expires - */ -export const refreshExpiringCookie = (async (req: AuthenticatedRequest, res, next) => { - const jwtRefreshTimeoutHours = config.get('userManagement.jwtRefreshTimeoutHours'); - - let jwtRefreshTimeoutMilliSeconds: number; - - if (jwtRefreshTimeoutHours === 0) { - const jwtSessionDurationHours = config.get('userManagement.jwtSessionDurationHours'); - - jwtRefreshTimeoutMilliSeconds = Math.floor(jwtSessionDurationHours * 0.25 * 60 * 60 * 1000); - } else { - jwtRefreshTimeoutMilliSeconds = Math.floor(jwtRefreshTimeoutHours * 60 * 60 * 1000); - } - - const cookieAuth = jwtFromRequest(req); - - if (cookieAuth && req.user && jwtRefreshTimeoutHours !== -1) { - const cookieContents = jwt.decode(cookieAuth) as JwtPayload & { exp: number }; - if (cookieContents.exp * 1000 - Date.now() < jwtRefreshTimeoutMilliSeconds) { - await issueCookie(res, req.user); - } - } - next(); -}) satisfies RequestHandler; - -const passportMiddleware = passport.authenticate('jwt', { session: false }) as RequestHandler; - -const staticAssets = globSync(['**/*.html', '**/*.svg', '**/*.png', '**/*.ico'], { - cwd: EDITOR_UI_DIST_DIR, -}); - -// TODO: delete this -const isPostInvitationAccept = (req: Request, restEndpoint: string): boolean => - req.method === 'POST' && - new RegExp(`/${restEndpoint}/invitations/[\\w\\d-]*`).test(req.url) && - req.url.includes('accept'); - -const isAuthExcluded = (url: string, ignoredEndpoints: Readonly): boolean => - !!ignoredEndpoints - .filter(Boolean) // skip empty paths - .find((ignoredEndpoint) => url.startsWith(`/${ignoredEndpoint}`)); - -/** - * This sets up the auth middlewares in the correct order - */ -export const setupAuthMiddlewares = ( - app: Application, - ignoredEndpoints: Readonly, - restEndpoint: string, -) => { - app.use(userManagementJwtAuth()); - - app.use(async (req: Request, res: Response, next: NextFunction) => { - if ( - // TODO: refactor me!!! - // skip authentication for preflight requests - req.method === 'OPTIONS' || - staticAssets.includes(req.url.slice(1)) || - canSkipAuth(req.method, req.path) || - isAuthExcluded(req.url, ignoredEndpoints) || - req.url.startsWith(`/${restEndpoint}/settings`) || - isPostInvitationAccept(req, restEndpoint) - ) { - return next(); - } - - return passportMiddleware(req, res, next); - }); - - app.use(refreshExpiringCookie); -}; diff --git a/packages/cli/src/middlewares/index.ts b/packages/cli/src/middlewares/index.ts index c9390709df..75ebae01c6 100644 --- a/packages/cli/src/middlewares/index.ts +++ b/packages/cli/src/middlewares/index.ts @@ -1,4 +1,3 @@ -export * from './auth'; export * from './bodyParser'; export * from './cors'; export * from './listQuery'; diff --git a/packages/cli/src/push/index.ts b/packages/cli/src/push/index.ts index e740821321..14cff7887e 100644 --- a/packages/cli/src/push/index.ts +++ b/packages/cli/src/push/index.ts @@ -7,14 +7,13 @@ import { Server as WSServer } from 'ws'; import { parse as parseUrl } from 'url'; import { Container, Service } from 'typedi'; import config from '@/config'; -import { resolveJwt } from '@/auth/jwt'; -import { AUTH_COOKIE_NAME } from '@/constants'; import { SSEPush } from './sse.push'; import { WebSocketPush } from './websocket.push'; import type { PushResponse, SSEPushRequest, WebSocketPushRequest } from './types'; import type { IPushDataType } from '@/Interfaces'; import type { User } from '@db/entities/User'; import { OnShutdown } from '@/decorators/OnShutdown'; +import { AuthService } from '@/auth/auth.service'; const useWebSockets = config.getEnv('push.backend') === 'websocket'; @@ -120,27 +119,15 @@ export const setupPushHandler = (restEndpoint: string, app: Application) => { } return; } - try { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access - const authCookie: string = req.cookies?.[AUTH_COOKIE_NAME] ?? ''; - const user = await resolveJwt(authCookie); - req.userId = user.id; - } catch (error) { - if (ws) { - ws.send(`Unauthorized: ${(error as Error).message}`); - ws.close(1008); - } else { - res.status(401).send('Unauthorized'); - } - return; - } next(); }; const push = Container.get(Push); + const authService = Container.get(AuthService); app.use( endpoint, + authService.createAuthMiddleware('any'), pushValidationMiddleware, (req: SSEPushRequest | WebSocketPushRequest, res: PushResponse) => push.handleRequest(req, res), ); diff --git a/packages/cli/src/push/types.ts b/packages/cli/src/push/types.ts index 2a65b08131..791604de28 100644 --- a/packages/cli/src/push/types.ts +++ b/packages/cli/src/push/types.ts @@ -1,10 +1,12 @@ -import type { User } from '@db/entities/User'; -import type { Request, Response } from 'express'; +import type { Response } from 'express'; import type { WebSocket } from 'ws'; +import type { User } from '@db/entities/User'; +import type { AuthenticatedRequest } from '@/requests'; + // TODO: move all push related types here -export type PushRequest = Request<{}, {}, {}, { sessionId: string }>; +export type PushRequest = AuthenticatedRequest<{}, {}, {}, { sessionId: string }>; export type SSEPushRequest = PushRequest & { ws: undefined; userId: User['id'] }; export type WebSocketPushRequest = PushRequest & { ws: WebSocket; userId: User['id'] }; diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index 4b64f844a9..02436390b5 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -1,3 +1,4 @@ +import type { ParsedQs } from 'qs'; import type express from 'express'; import type { BannerName, @@ -20,6 +21,17 @@ import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; import type { WorkflowHistory } from '@db/entities/WorkflowHistory'; +export type AsyncRequestHandler< + Params = Record, + ResBody = unknown, + ReqBody = unknown, + ReqQuery = ParsedQs, +> = ( + req: express.Request, + res: express.Response, + next: () => void, +) => Promise; + export class UserUpdatePayload implements Pick { @IsEmail() email: string; @@ -62,8 +74,12 @@ export type AuthenticatedRequest< ResponseBody = {}, RequestBody = {}, RequestQuery = {}, -> = Omit, 'user'> & { +> = Omit< + express.Request, + 'user' | 'cookies' +> & { user: User; + cookies: Record; }; // ---------------------------------- diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts index 6c753788fa..0ce290da8b 100644 --- a/packages/cli/src/services/user.service.ts +++ b/packages/cli/src/services/user.service.ts @@ -1,17 +1,15 @@ import { Container, Service } from 'typedi'; -import { type AssignableRole, User } from '@db/entities/User'; import type { IUserSettings } from 'n8n-workflow'; +import { ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; + +import { type AssignableRole, User } from '@db/entities/User'; import { UserRepository } from '@db/repositories/user.repository'; import type { PublicUser } from '@/Interfaces'; import type { PostHogClient } from '@/posthog'; -import { type JwtPayload, JwtService } from './jwt.service'; -import { TokenExpiredError } from 'jsonwebtoken'; import { Logger } from '@/Logger'; -import { createPasswordSha } from '@/auth/jwt'; import { UserManagementMailer } from '@/UserManagement/email'; import { InternalHooks } from '@/InternalHooks'; import { UrlService } from '@/services/url.service'; -import { ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; import type { UserRequest } from '@/requests'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; @@ -20,7 +18,6 @@ export class UserService { constructor( private readonly logger: Logger, private readonly userRepository: UserRepository, - private readonly jwtService: JwtService, private readonly mailer: UserManagementMailer, private readonly urlService: UrlService, ) {} @@ -39,57 +36,6 @@ export class UserService { return await this.userRepository.update(userId, { settings: { ...settings, ...newSettings } }); } - generatePasswordResetToken(user: User, expiresIn = '20m') { - return this.jwtService.sign( - { sub: user.id, passwordSha: createPasswordSha(user) }, - { expiresIn }, - ); - } - - generatePasswordResetUrl(user: User) { - const instanceBaseUrl = this.urlService.getInstanceBaseUrl(); - const url = new URL(`${instanceBaseUrl}/change-password`); - - url.searchParams.append('token', this.generatePasswordResetToken(user)); - url.searchParams.append('mfaEnabled', user.mfaEnabled.toString()); - - return url.toString(); - } - - async resolvePasswordResetToken(token: string): Promise { - let decodedToken: JwtPayload & { passwordSha: string }; - try { - decodedToken = this.jwtService.verify(token); - } catch (e) { - if (e instanceof TokenExpiredError) { - this.logger.debug('Reset password token expired', { token }); - } else { - this.logger.debug('Error verifying token', { token }); - } - return; - } - - const user = await this.userRepository.findOne({ - where: { id: decodedToken.sub }, - relations: ['authIdentities'], - }); - - if (!user) { - this.logger.debug( - 'Request to resolve password token failed because no user was found for the provided user ID', - { userId: decodedToken.sub, token }, - ); - return; - } - - if (createPasswordSha(user) !== decodedToken.passwordSha) { - this.logger.debug('Password updated since this token was generated'); - return; - } - - return user; - } - async toPublic( user: User, options?: { diff --git a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts index cfa68bfdd6..5b567018dc 100644 --- a/packages/cli/src/sso/saml/routes/saml.controller.ee.ts +++ b/packages/cli/src/sso/saml/routes/saml.controller.ee.ts @@ -1,4 +1,8 @@ import express from 'express'; +import { validate } from 'class-validator'; +import type { PostBindingContext } from 'samlify/types/src/entity'; +import url from 'url'; + import { Authorized, Get, @@ -7,20 +11,16 @@ import { RestController, RequireGlobalScope, } from '@/decorators'; -import { SamlUrls } from '../constants'; -import { - samlLicensedAndEnabledMiddleware, - samlLicensedMiddleware, -} from '../middleware/samlEnabledMiddleware'; -import { SamlService } from '../saml.service.ee'; -import { SamlConfiguration } from '../types/requests'; -import { getInitSSOFormView } from '../views/initSsoPost'; -import { issueCookie } from '@/auth/jwt'; -import { validate } from 'class-validator'; -import type { PostBindingContext } from 'samlify/types/src/entity'; -import { isConnectionTestRequest, isSamlLicensedAndEnabled } from '../samlHelpers'; -import type { SamlLoginBinding } from '../types'; + +import { AuthService } from '@/auth/auth.service'; import { AuthenticatedRequest } from '@/requests'; +import { InternalHooks } from '@/InternalHooks'; +import querystring from 'querystring'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { AuthError } from '@/errors/response-errors/auth.error'; +import { UrlService } from '@/services/url.service'; + +import { SamlUrls } from '../constants'; import { getServiceProviderConfigTestReturnUrl, getServiceProviderEntityId, @@ -28,17 +28,21 @@ import { } from '../serviceProvider.ee'; import { getSamlConnectionTestSuccessView } from '../views/samlConnectionTestSuccess'; import { getSamlConnectionTestFailedView } from '../views/samlConnectionTestFailed'; -import { InternalHooks } from '@/InternalHooks'; -import url from 'url'; -import querystring from 'querystring'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { AuthError } from '@/errors/response-errors/auth.error'; -import { UrlService } from '@/services/url.service'; +import { isConnectionTestRequest, isSamlLicensedAndEnabled } from '../samlHelpers'; +import type { SamlLoginBinding } from '../types'; +import { + samlLicensedAndEnabledMiddleware, + samlLicensedMiddleware, +} from '../middleware/samlEnabledMiddleware'; +import { SamlService } from '../saml.service.ee'; +import { SamlConfiguration } from '../types/requests'; +import { getInitSSOFormView } from '../views/initSsoPost'; @Authorized() @RestController('/sso/saml') export class SamlController { constructor( + private readonly authService: AuthService, private readonly samlService: SamlService, private readonly urlService: UrlService, private readonly internalHooks: InternalHooks, @@ -46,7 +50,7 @@ export class SamlController { @NoAuthRequired() @Get(SamlUrls.metadata) - async getServiceProviderMetadata(req: express.Request, res: express.Response) { + async getServiceProviderMetadata(_: express.Request, res: express.Response) { return res .header('Content-Type', 'text/xml') .send(this.samlService.getServiceProviderInstance().getMetadata()); @@ -147,7 +151,7 @@ export class SamlController { }); // Only sign in user if SAML is enabled, otherwise treat as test connection if (isSamlLicensedAndEnabled()) { - await issueCookie(res, loginResult.authenticatedUser); + this.authService.issueCookie(res, loginResult.authenticatedUser); if (loginResult.onboardingRequired) { return res.redirect(this.urlService.getInstanceBaseUrl() + SamlUrls.samlOnboarding); } else { diff --git a/packages/cli/test/integration/auth.api.test.ts b/packages/cli/test/integration/auth.api.test.ts index 768acc467d..9435fa7a7d 100644 --- a/packages/cli/test/integration/auth.api.test.ts +++ b/packages/cli/test/integration/auth.api.test.ts @@ -157,18 +157,6 @@ describe('GET /login', () => { expect(authToken).toBeUndefined(); }); - test('should return cookie if UM is disabled and no cookie is already set', async () => { - await createUserShell('global:owner'); - await utils.setInstanceOwnerSetUp(false); - - const response = await testServer.authlessAgent.get('/login'); - - expect(response.statusCode).toBe(200); - - const authToken = utils.getAuthToken(response); - expect(authToken).toBeDefined(); - }); - test('should return 401 Unauthorized if invalid cookie', async () => { testServer.authlessAgent.jar.setCookie(`${AUTH_COOKIE_NAME}=invalid`); diff --git a/packages/cli/test/integration/auth.mw.test.ts b/packages/cli/test/integration/auth.mw.test.ts index 7a117e95be..e4b1b974bb 100644 --- a/packages/cli/test/integration/auth.mw.test.ts +++ b/packages/cli/test/integration/auth.mw.test.ts @@ -18,7 +18,6 @@ describe('Auth Middleware', () => { ['PATCH', '/me/password'], ['POST', '/me/survey'], ['POST', '/owner/setup'], - ['GET', '/non-existent'], ]; /** Routes requiring a valid `n8n-auth` cookie for an owner. */ diff --git a/packages/cli/test/integration/ldap/ldap.api.test.ts b/packages/cli/test/integration/ldap/ldap.api.test.ts index f3a923d4bd..cc28d7b748 100644 --- a/packages/cli/test/integration/ldap/ldap.api.test.ts +++ b/packages/cli/test/integration/ldap/ldap.api.test.ts @@ -453,7 +453,7 @@ describe('POST /ldap/sync', () => { await authOwnerAgent.post('/ldap/sync').send({ type: 'live' }); const response = await testServer.authAgentFor(member).get('/login'); - expect(response.body.code).toBe(401); + expect(response.status).toBe(401); }); }); }); diff --git a/packages/cli/test/integration/me.api.test.ts b/packages/cli/test/integration/me.api.test.ts index 08261b7b08..53ee823430 100644 --- a/packages/cli/test/integration/me.api.test.ts +++ b/packages/cli/test/integration/me.api.test.ts @@ -300,10 +300,6 @@ describe('Member', () => { }); describe('Owner', () => { - beforeEach(async () => { - await utils.setInstanceOwnerSetUp(true); - }); - test('PATCH /me should succeed with valid inputs', async () => { const owner = await createUser({ role: 'global:owner' }); const authOwnerAgent = testServer.authAgentFor(owner); diff --git a/packages/cli/test/integration/mfa/mfa.api.test.ts b/packages/cli/test/integration/mfa/mfa.api.test.ts index 2e7df144ca..d66261f60e 100644 --- a/packages/cli/test/integration/mfa/mfa.api.test.ts +++ b/packages/cli/test/integration/mfa/mfa.api.test.ts @@ -1,14 +1,16 @@ import Container from 'typedi'; + +import { AuthService } from '@/auth/auth.service'; import config from '@/config'; import type { User } from '@db/entities/User'; +import { UserRepository } from '@db/repositories/user.repository'; import { randomPassword } from '@/Ldap/helpers'; import { TOTPService } from '@/Mfa/totp.service'; -import { UserService } from '@/services/user.service'; -import { randomDigit, randomString, randomValidPassword, uniqueId } from '../shared/random'; + import * as testDb from '../shared/testDb'; import * as utils from '../shared/utils'; +import { randomDigit, randomString, randomValidPassword, uniqueId } from '../shared/random'; import { createUser, createUserWithMfaEnabled } from '../shared/db/users'; -import { UserRepository } from '@db/repositories/user.repository'; jest.mock('@/telemetry'); @@ -241,7 +243,7 @@ describe('Change password with MFA enabled', () => { config.set('userManagement.jwtSecret', randomString(5, 10)); - const resetPasswordToken = Container.get(UserService).generatePasswordResetToken(user); + const resetPasswordToken = Container.get(AuthService).generatePasswordResetToken(user); const mfaToken = new TOTPService().generateTOTP(rawSecret); diff --git a/packages/cli/test/integration/passwordReset.api.test.ts b/packages/cli/test/integration/passwordReset.api.test.ts index 996fc0ab77..6aca848e84 100644 --- a/packages/cli/test/integration/passwordReset.api.test.ts +++ b/packages/cli/test/integration/passwordReset.api.test.ts @@ -3,13 +3,13 @@ import { compare } from 'bcryptjs'; import { Container } from 'typedi'; import { mock } from 'jest-mock-extended'; +import { AuthService } from '@/auth/auth.service'; import { License } from '@/License'; import config from '@/config'; import type { User } from '@db/entities/User'; import { setCurrentAuthenticationMethod } from '@/sso/ssoHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import { JwtService } from '@/services/jwt.service'; -import { UserService } from '@/services/user.service'; import { UserManagementMailer } from '@/UserManagement/email'; import { UserRepository } from '@db/repositories/user.repository'; @@ -35,7 +35,7 @@ const externalHooks = mockInstance(ExternalHooks); const mailer = mockInstance(UserManagementMailer, { isEmailSetUp: true }); const testServer = setupTestServer({ endpointGroups: ['passwordReset'] }); const jwtService = Container.get(JwtService); -let userService: UserService; +let authService: AuthService; beforeEach(async () => { await testDb.truncate(['User']); @@ -43,7 +43,7 @@ beforeEach(async () => { member = await createUser({ role: 'global:member' }); externalHooks.run.mockReset(); jest.replaceProperty(mailer, 'isEmailSetUp', true); - userService = Container.get(UserService); + authService = Container.get(AuthService); }); describe('POST /forgot-password', () => { @@ -126,7 +126,7 @@ describe('POST /forgot-password', () => { describe('GET /resolve-password-token', () => { test('should succeed with valid inputs', async () => { - const resetPasswordToken = userService.generatePasswordResetToken(owner); + const resetPasswordToken = authService.generatePasswordResetToken(owner); const response = await testServer.authlessAgent .get('/resolve-password-token') @@ -158,7 +158,7 @@ describe('GET /resolve-password-token', () => { }); test('should fail if token is expired', async () => { - const resetPasswordToken = userService.generatePasswordResetToken(owner, '-1h'); + const resetPasswordToken = authService.generatePasswordResetToken(owner, '-1h'); const response = await testServer.authlessAgent .get('/resolve-password-token') @@ -169,7 +169,7 @@ describe('GET /resolve-password-token', () => { test('should fail after password has changed', async () => { const updatedUser = mock({ ...owner, password: 'another-password' }); - const resetPasswordToken = userService.generatePasswordResetToken(updatedUser); + const resetPasswordToken = authService.generatePasswordResetToken(updatedUser); const response = await testServer.authlessAgent .get('/resolve-password-token') @@ -183,7 +183,7 @@ describe('POST /change-password', () => { const passwordToStore = randomValidPassword(); test('should succeed with valid inputs', async () => { - const resetPasswordToken = userService.generatePasswordResetToken(owner); + const resetPasswordToken = authService.generatePasswordResetToken(owner); const response = await testServer.authlessAgent.post('/change-password').send({ token: resetPasswordToken, userId: owner.id, @@ -213,7 +213,7 @@ describe('POST /change-password', () => { }); test('should fail with invalid inputs', async () => { - const resetPasswordToken = userService.generatePasswordResetToken(owner); + const resetPasswordToken = authService.generatePasswordResetToken(owner); const invalidPayloads = [ { token: uuid() }, @@ -247,7 +247,7 @@ describe('POST /change-password', () => { }); test('should fail when token has expired', async () => { - const resetPasswordToken = userService.generatePasswordResetToken(owner, '-1h'); + const resetPasswordToken = authService.generatePasswordResetToken(owner, '-1h'); const response = await testServer.authlessAgent.post('/change-password').send({ token: resetPasswordToken, @@ -263,7 +263,7 @@ describe('POST /change-password', () => { test('owner should be able to reset its password when quota:users = 1', async () => { jest.spyOn(Container.get(License), 'getUsersLimit').mockReturnValueOnce(1); - const resetPasswordToken = userService.generatePasswordResetToken(owner); + const resetPasswordToken = authService.generatePasswordResetToken(owner); const response = await testServer.authlessAgent.post('/change-password').send({ token: resetPasswordToken, userId: owner.id, @@ -292,7 +292,7 @@ describe('POST /change-password', () => { test('member should not be able to reset its password when quota:users = 1', async () => { jest.spyOn(Container.get(License), 'getUsersLimit').mockReturnValueOnce(1); - const resetPasswordToken = userService.generatePasswordResetToken(member); + const resetPasswordToken = authService.generatePasswordResetToken(member); const response = await testServer.authlessAgent.post('/change-password').send({ token: resetPasswordToken, userId: member.id, diff --git a/packages/cli/test/integration/shared/constants.ts b/packages/cli/test/integration/shared/constants.ts index 5fc7149f8c..7b32134cab 100644 --- a/packages/cli/test/integration/shared/constants.ts +++ b/packages/cli/test/integration/shared/constants.ts @@ -4,14 +4,6 @@ export const REST_PATH_SEGMENT = config.getEnv('endpoints.rest'); export const PUBLIC_API_REST_PATH_SEGMENT = config.getEnv('publicApi.path'); -export const AUTHLESS_ENDPOINTS: Readonly = [ - 'healthz', - 'metrics', - config.getEnv('endpoints.webhook'), - config.getEnv('endpoints.webhookWaiting'), - config.getEnv('endpoints.webhookTest'), -]; - export const SUCCESS_RESPONSE_BODY = { data: { success: true, diff --git a/packages/cli/test/integration/shared/types.ts b/packages/cli/test/integration/shared/types.ts index 6393e79143..2482beb95c 100644 --- a/packages/cli/test/integration/shared/types.ts +++ b/packages/cli/test/integration/shared/types.ts @@ -35,7 +35,6 @@ type EndpointGroup = | 'debug'; export interface SetupProps { - applyAuth?: boolean; endpointGroups?: EndpointGroup[]; enabledFeatures?: BooleanLicenseFeature[]; quotas?: Partial<{ [K in NumericLicenseFeature]: number }>; diff --git a/packages/cli/test/integration/shared/utils/testServer.ts b/packages/cli/test/integration/shared/utils/testServer.ts index d791d7dc89..829fd0214c 100644 --- a/packages/cli/test/integration/shared/utils/testServer.ts +++ b/packages/cli/test/integration/shared/utils/testServer.ts @@ -8,9 +8,8 @@ import { URL } from 'url'; import config from '@/config'; import { AUTH_COOKIE_NAME } from '@/constants'; import type { User } from '@db/entities/User'; -import { issueJWT } from '@/auth/jwt'; import { registerController } from '@/decorators'; -import { rawBodyReader, bodyParser, setupAuthMiddlewares } from '@/middlewares'; +import { rawBodyReader, bodyParser } from '@/middlewares'; import { PostHogClient } from '@/posthog'; import { Push } from '@/push'; import { License } from '@/License'; @@ -19,9 +18,10 @@ import { InternalHooks } from '@/InternalHooks'; import { mockInstance } from '../../../shared/mocking'; import * as testDb from '../../shared/testDb'; -import { AUTHLESS_ENDPOINTS, PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from '../constants'; +import { PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from '../constants'; import type { SetupProps, TestServer } from '../types'; import { LicenseMocker } from '../license'; +import { AuthService } from '@/auth/auth.service'; /** * Plugin to prefix a path segment into a request URL pathname. @@ -47,7 +47,7 @@ function createAgent(app: express.Application, options?: { auth: boolean; user: const agent = request.agent(app); void agent.use(prefix(REST_PATH_SEGMENT)); if (options?.auth && options?.user) { - const { token } = issueJWT(options.user); + const token = Container.get(AuthService).issueJWT(options.user); agent.jar.setCookie(`${AUTH_COOKIE_NAME}=${token}`); } return agent; @@ -67,7 +67,6 @@ function publicApiAgent( export const setupTestServer = ({ endpointGroups, - applyAuth = true, enabledFeatures, quotas, }: SetupProps): TestServer => { @@ -104,15 +103,11 @@ export const setupTestServer = ({ }); } - const enablePublicAPI = endpointGroups?.includes('publicApi'); - if (applyAuth && !enablePublicAPI) { - setupAuthMiddlewares(app, AUTHLESS_ENDPOINTS, REST_PATH_SEGMENT); - } - if (!endpointGroups) return; app.use(bodyParser); + const enablePublicAPI = endpointGroups?.includes('publicApi'); if (enablePublicAPI) { const { loadPublicApiVersions } = await import('@/PublicApi'); const { apiRouters } = await loadPublicApiVersions(PUBLIC_API_REST_PATH_SEGMENT); diff --git a/packages/cli/test/integration/workflows/workflow.service.ee.test.ts b/packages/cli/test/integration/workflows/workflow.service.ee.test.ts index 4cb823b7c0..4504775f2e 100644 --- a/packages/cli/test/integration/workflows/workflow.service.ee.test.ts +++ b/packages/cli/test/integration/workflows/workflow.service.ee.test.ts @@ -28,6 +28,7 @@ describe('EnterpriseWorkflowService', () => { Container.get(SharedWorkflowRepository), Container.get(WorkflowRepository), Container.get(CredentialsRepository), + mock(), ); }); diff --git a/packages/cli/test/unit/UserManagementMailer.test.ts b/packages/cli/test/unit/UserManagementMailer.test.ts index 8e9de8d3a9..10daf984c9 100644 --- a/packages/cli/test/unit/UserManagementMailer.test.ts +++ b/packages/cli/test/unit/UserManagementMailer.test.ts @@ -1,6 +1,7 @@ import config from '@/config'; import { NodeMailer } from '@/UserManagement/email/NodeMailer'; import { UserManagementMailer } from '@/UserManagement/email/UserManagementMailer'; +import { mock } from 'jest-mock-extended'; describe('UserManagementMailer', () => { describe('expect NodeMailer.verifyConnection', () => { @@ -20,7 +21,7 @@ describe('UserManagementMailer', () => { }); test('not be called when SMTP not set up', async () => { - const userManagementMailer = new UserManagementMailer(); + const userManagementMailer = new UserManagementMailer(mock(), mock(), mock()); // NodeMailer.verifyConnection gets called only explicitly await expect(async () => await userManagementMailer.verifyConnection()).rejects.toThrow(); @@ -32,7 +33,7 @@ describe('UserManagementMailer', () => { config.set('userManagement.emails.smtp.host', 'host'); config.set('userManagement.emails.mode', 'smtp'); - const userManagementMailer = new UserManagementMailer(); + const userManagementMailer = new UserManagementMailer(mock(), mock(), mock()); // NodeMailer.verifyConnection gets called only explicitly expect(async () => await userManagementMailer.verifyConnection()).not.toThrow(); }); diff --git a/packages/cli/test/unit/auth/auth.service.test.ts b/packages/cli/test/unit/auth/auth.service.test.ts new file mode 100644 index 0000000000..e5d873a543 --- /dev/null +++ b/packages/cli/test/unit/auth/auth.service.test.ts @@ -0,0 +1,292 @@ +import jwt from 'jsonwebtoken'; +import { mock } from 'jest-mock-extended'; +import { type NextFunction, type Response } from 'express'; + +import { AuthService } from '@/auth/auth.service'; +import config from '@/config'; +import { AUTH_COOKIE_NAME, Time } from '@/constants'; +import type { User } from '@db/entities/User'; +import type { UserRepository } from '@db/repositories/user.repository'; +import { JwtService } from '@/services/jwt.service'; +import type { UrlService } from '@/services/url.service'; +import type { AuthenticatedRequest } from '@/requests'; + +describe('AuthService', () => { + config.set('userManagement.jwtSecret', 'random-secret'); + + const userData = { + id: '123', + email: 'test@example.com', + password: 'passwordHash', + disabled: false, + mfaEnabled: false, + }; + const validToken = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpZCI6IjEyMyIsImVtYWlsIjoidGVzdEBleGFtcGxlLmNvbSIsInBhc3N3b3JkIjoiMzE1MTNjNWE5ZTNjNWFmZTVjMDZkNTY3NWFjZTc0ZThiYzNmYWRkOTc0NGFiNWQ4OWMzMTFmMmE2MmNjYmQzOSIsImlhdCI6MTcwNjc1MDYyNSwiZXhwIjoxNzA3MzU1NDI1fQ.mtXKUwQDHOhiHn0YNuCeybmxevtNG6LXTAv_sQL63Zc'; + + const user = mock(userData); + const jwtService = new JwtService(mock()); + const urlService = mock(); + const userRepository = mock(); + const authService = new AuthService(mock(), mock(), jwtService, urlService, userRepository); + + jest.useFakeTimers(); + const now = new Date('2024-02-01T01:23:45.678Z'); + beforeEach(() => { + jest.clearAllMocks(); + jest.setSystemTime(now); + config.set('userManagement.jwtSessionDurationHours', 168); + config.set('userManagement.jwtRefreshTimeoutHours', 0); + }); + + describe('createAuthMiddleware', () => { + const req = mock({ cookies: {}, user: undefined }); + const res = mock(); + const next = jest.fn() as NextFunction; + + beforeEach(() => { + res.status.mockReturnThis(); + }); + + describe('authRole = none', () => { + const authMiddleware = authService.createAuthMiddleware('none'); + + it('should just skips auth checks', async () => { + await authMiddleware(req, res, next); + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + }); + + describe('authRole = any', () => { + const authMiddleware = authService.createAuthMiddleware('any'); + + it('should 401 if no cookie is set', async () => { + req.cookies[AUTH_COOKIE_NAME] = undefined; + await authMiddleware(req, res, next); + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + }); + + it('should 401 and clear the cookie if the JWT is expired', async () => { + req.cookies[AUTH_COOKIE_NAME] = validToken; + jest.advanceTimersByTime(365 * Time.days.toMilliseconds); + + await authMiddleware(req, res, next); + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(401); + expect(res.clearCookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME); + }); + + it('should refresh the cookie before it expires', async () => { + req.cookies[AUTH_COOKIE_NAME] = validToken; + jest.advanceTimersByTime(6 * Time.days.toMilliseconds); + userRepository.findOne.mockResolvedValue(user); + + await authMiddleware(req, res, next); + expect(next).toHaveBeenCalled(); + expect(res.cookie).toHaveBeenCalledWith('n8n-auth', expect.any(String), { + httpOnly: true, + maxAge: 604800000, + sameSite: 'lax', + }); + }); + }); + + describe('authRole = global:owner', () => { + const authMiddleware = authService.createAuthMiddleware('global:owner'); + + it('should 403 if the user does not have the correct role', async () => { + req.cookies[AUTH_COOKIE_NAME] = validToken; + jest.advanceTimersByTime(6 * Time.days.toMilliseconds); + userRepository.findOne.mockResolvedValue( + mock({ ...userData, role: 'global:member' }), + ); + + await authMiddleware(req, res, next); + expect(next).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(403); + }); + }); + }); + + describe('issueJWT', () => { + describe('when not setting userManagement.jwtSessionDuration', () => { + it('should default to expire in 7 days', () => { + const defaultInSeconds = 7 * Time.days.toSeconds; + const token = authService.issueJWT(user); + + expect(authService.jwtExpiration).toBe(defaultInSeconds); + const decodedToken = jwtService.verify(token); + if (decodedToken.exp === undefined || decodedToken.iat === undefined) { + fail('Expected exp and iat to be defined'); + } + + expect(decodedToken.exp - decodedToken.iat).toBe(defaultInSeconds); + }); + }); + + describe('when setting userManagement.jwtSessionDuration', () => { + const testDurationHours = 1; + const testDurationSeconds = testDurationHours * Time.hours.toSeconds; + + it('should apply it to tokens', () => { + config.set('userManagement.jwtSessionDurationHours', testDurationHours); + const token = authService.issueJWT(user); + + const decodedToken = jwtService.verify(token); + if (decodedToken.exp === undefined || decodedToken.iat === undefined) { + fail('Expected exp and iat to be defined on decodedToken'); + } + expect(decodedToken.exp - decodedToken.iat).toBe(testDurationSeconds); + }); + }); + }); + + describe('resolveJwt', () => { + const res = mock(); + + it('should throw on invalid tokens', async () => { + await expect(authService.resolveJwt('random-string', res)).rejects.toThrow('jwt malformed'); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + it('should throw on expired tokens', async () => { + jest.advanceTimersByTime(365 * Time.days.toMilliseconds); + + await expect(authService.resolveJwt(validToken, res)).rejects.toThrow('jwt expired'); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + it('should throw on tampered tokens', async () => { + const [header, payload, signature] = validToken.split('.'); + const tamperedToken = [header, payload, signature + '123'].join('.'); + await expect(authService.resolveJwt(tamperedToken, res)).rejects.toThrow('invalid signature'); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + test.each([ + ['no user is found', null], + ['the user is disabled', { ...userData, disabled: true }], + [ + 'user password does not match the one on the token', + { ...userData, password: 'something else' }, + ], + [ + 'user email does not match the one on the token', + { ...userData, email: 'someone@example.com' }, + ], + ])('should throw if %s', async (_, data) => { + userRepository.findOne.mockResolvedValueOnce(data && mock(data)); + await expect(authService.resolveJwt(validToken, res)).rejects.toThrow('Unauthorized'); + expect(res.cookie).not.toHaveBeenCalled(); + }); + + it('should refresh the cookie before it expires', async () => { + userRepository.findOne.mockResolvedValue(user); + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(6 * Time.days.toMilliseconds); // 6 Days + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).toHaveBeenCalledWith('n8n-auth', expect.any(String), { + httpOnly: true, + maxAge: 604800000, + sameSite: 'lax', + }); + }); + + it('should refresh the cookie only if less than 1/4th of time is left', async () => { + userRepository.findOne.mockResolvedValue(user); + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(5 * Time.days.toMilliseconds); + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(1 * Time.days.toMilliseconds); + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).toHaveBeenCalled(); + }); + + it('should not refresh the cookie if jwtRefreshTimeoutHours is set to -1', async () => { + config.set('userManagement.jwtRefreshTimeoutHours', -1); + + userRepository.findOne.mockResolvedValue(user); + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(6 * Time.days.toMilliseconds); // 6 Days + expect(await authService.resolveJwt(validToken, res)).toEqual(user); + expect(res.cookie).not.toHaveBeenCalled(); + }); + }); + + describe('generatePasswordResetUrl', () => { + it('should generate a valid url', () => { + urlService.getInstanceBaseUrl.mockReturnValue('https://n8n.instance'); + const url = authService.generatePasswordResetUrl(user); + expect(url).toEqual( + 'https://n8n.instance/change-password?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjMiLCJwYXNzd29yZFNoYSI6IjMxNTEzYzVhOWUzYzVhZmU1YzA2ZDU2NzVhY2U3NGU4YmMzZmFkZDk3NDRhYjVkODljMzExZjJhNjJjY2JkMzkiLCJpYXQiOjE3MDY3NTA2MjUsImV4cCI6MTcwNjc1MTgyNX0.wsdEpbK2zhFucaPwga7f8EOcwiJcv0iW23HcnvJs-s8&mfaEnabled=false', + ); + }); + }); + + describe('generatePasswordResetToken', () => { + it('should generate valid password-reset tokens', () => { + const token = authService.generatePasswordResetToken(user); + + const decoded = jwt.decode(token) as jwt.JwtPayload; + + if (!decoded.exp) fail('Token does not contain expiry'); + if (!decoded.iat) fail('Token does not contain issued-at'); + + expect(decoded.sub).toEqual(user.id); + expect(decoded.exp - decoded.iat).toEqual(1200); // Expires in 20 minutes + expect(decoded.passwordSha).toEqual( + '31513c5a9e3c5afe5c06d5675ace74e8bc3fadd9744ab5d89c311f2a62ccbd39', + ); + }); + }); + + describe('resolvePasswordResetToken', () => { + it('should not return a user if the token in invalid', async () => { + const resolvedUser = await authService.resolvePasswordResetToken('invalid-token'); + expect(resolvedUser).toBeUndefined(); + }); + + it('should not return a user if the token in expired', async () => { + const token = authService.generatePasswordResetToken(user, '-1h'); + + const resolvedUser = await authService.resolvePasswordResetToken(token); + expect(resolvedUser).toBeUndefined(); + }); + + it('should not return a user if the user does not exist in the DB', async () => { + userRepository.findOne.mockResolvedValueOnce(null); + const token = authService.generatePasswordResetToken(user); + + const resolvedUser = await authService.resolvePasswordResetToken(token); + expect(resolvedUser).toBeUndefined(); + }); + + it('should not return a user if the password sha does not match', async () => { + const token = authService.generatePasswordResetToken(user); + const updatedUser = Object.create(user); + updatedUser.password = 'something-else'; + userRepository.findOne.mockResolvedValueOnce(updatedUser); + + const resolvedUser = await authService.resolvePasswordResetToken(token); + expect(resolvedUser).toBeUndefined(); + }); + + it('should not return the user if all checks pass', async () => { + const token = authService.generatePasswordResetToken(user); + userRepository.findOne.mockResolvedValueOnce(user); + + const resolvedUser = await authService.resolvePasswordResetToken(token); + expect(resolvedUser).toEqual(user); + }); + }); +}); diff --git a/packages/cli/test/unit/auth/jwt.test.ts b/packages/cli/test/unit/auth/jwt.test.ts deleted file mode 100644 index dd9c65116a..0000000000 --- a/packages/cli/test/unit/auth/jwt.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { Container } from 'typedi'; -import { mock } from 'jest-mock-extended'; - -import config from '@/config'; -import { JwtService } from '@/services/jwt.service'; -import { License } from '@/License'; -import { Time } from '@/constants'; -import { issueJWT } from '@/auth/jwt'; - -import { mockInstance } from '../../shared/mocking'; - -import type { User } from '@db/entities/User'; - -mockInstance(License); - -describe('jwt.issueJWT', () => { - const jwtService = Container.get(JwtService); - - describe('when not setting userManagement.jwtSessionDuration', () => { - it('should default to expire in 7 days', () => { - const defaultInSeconds = 7 * Time.days.toSeconds; - const mockUser = mock({ password: 'passwordHash' }); - const { token, expiresIn } = issueJWT(mockUser); - - expect(expiresIn).toBe(defaultInSeconds); - const decodedToken = jwtService.verify(token); - if (decodedToken.exp === undefined || decodedToken.iat === undefined) { - fail('Expected exp and iat to be defined'); - } - - expect(decodedToken.exp - decodedToken.iat).toBe(defaultInSeconds); - }); - }); - - describe('when setting userManagement.jwtSessionDuration', () => { - const oldDuration = config.get('userManagement.jwtSessionDurationHours'); - const testDurationHours = 1; - const testDurationSeconds = testDurationHours * Time.hours.toSeconds; - - beforeEach(() => { - mockInstance(License); - config.set('userManagement.jwtSessionDurationHours', testDurationHours); - }); - - afterEach(() => { - config.set('userManagement.jwtSessionDuration', oldDuration); - }); - - it('should apply it to tokens', () => { - const mockUser = mock({ password: 'passwordHash' }); - const { token, expiresIn } = issueJWT(mockUser); - - expect(expiresIn).toBe(testDurationSeconds); - const decodedToken = jwtService.verify(token); - if (decodedToken.exp === undefined || decodedToken.iat === undefined) { - fail('Expected exp and iat to be defined on decodedToken'); - } - expect(decodedToken.exp - decodedToken.iat).toBe(testDurationSeconds); - }); - }); -}); diff --git a/packages/cli/test/unit/controllers/owner.controller.test.ts b/packages/cli/test/unit/controllers/owner.controller.test.ts index f142e16b99..ffa6f1aa48 100644 --- a/packages/cli/test/unit/controllers/owner.controller.test.ts +++ b/packages/cli/test/unit/controllers/owner.controller.test.ts @@ -1,34 +1,37 @@ -import type { CookieOptions, Response } from 'express'; -import { anyObject, captor, mock } from 'jest-mock-extended'; +import Container from 'typedi'; +import type { Response } from 'express'; +import { anyObject, mock } from 'jest-mock-extended'; import jwt from 'jsonwebtoken'; + +import type { AuthService } from '@/auth/auth.service'; +import config from '@/config'; +import { OwnerController } from '@/controllers/owner.controller'; import type { User } from '@db/entities/User'; import type { SettingsRepository } from '@db/repositories/settings.repository'; -import config from '@/config'; -import type { OwnerRequest } from '@/requests'; -import { OwnerController } from '@/controllers/owner.controller'; -import { AUTH_COOKIE_NAME } from '@/constants'; -import { UserService } from '@/services/user.service'; +import type { UserRepository } from '@db/repositories/user.repository'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import type { InternalHooks } from '@/InternalHooks'; import { License } from '@/License'; +import type { OwnerRequest } from '@/requests'; +import type { UserService } from '@/services/user.service'; +import { PasswordUtility } from '@/services/password.utility'; import { mockInstance } from '../../shared/mocking'; import { badPasswords } from '../shared/testData'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { PasswordUtility } from '@/services/password.utility'; -import Container from 'typedi'; -import type { InternalHooks } from '@/InternalHooks'; -import { UserRepository } from '@/databases/repositories/user.repository'; describe('OwnerController', () => { const configGetSpy = jest.spyOn(config, 'getEnv'); const internalHooks = mock(); - const userService = mockInstance(UserService); - const userRepository = mockInstance(UserRepository); + const authService = mock(); + const userService = mock(); + const userRepository = mock(); const settingsRepository = mock(); mockInstance(License).isWithinUsersLimit.mockReturnValue(true); const controller = new OwnerController( mock(), internalHooks, settingsRepository, + authService, userService, Container.get(PasswordUtility), mock(), @@ -96,11 +99,7 @@ describe('OwnerController', () => { await controller.setupOwner(req, res); expect(userRepository.save).toHaveBeenCalledWith(user, { transaction: false }); - - const cookieOptions = captor(); - expect(res.cookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME, 'signed-token', cookieOptions); - expect(cookieOptions.value.httpOnly).toBe(true); - expect(cookieOptions.value.sameSite).toBe('lax'); + expect(authService.issueCookie).toHaveBeenCalledWith(res, user); }); }); }); diff --git a/packages/cli/test/unit/middleware/auth.test.ts b/packages/cli/test/unit/middleware/auth.test.ts deleted file mode 100644 index dfd6e0e8c1..0000000000 --- a/packages/cli/test/unit/middleware/auth.test.ts +++ /dev/null @@ -1,162 +0,0 @@ -import { mock } from 'jest-mock-extended'; - -import config from '@/config'; -import { AUTH_COOKIE_NAME, Time } from '@/constants'; -import { License } from '@/License'; -import { issueJWT } from '@/auth/jwt'; -import { refreshExpiringCookie } from '@/middlewares'; - -import { mockInstance } from '../../shared/mocking'; - -import type { AuthenticatedRequest } from '@/requests'; -import type { NextFunction, Response } from 'express'; -import type { User } from '@/databases/entities/User'; - -mockInstance(License); - -jest.useFakeTimers(); - -describe('refreshExpiringCookie', () => { - const oldDuration = config.getEnv('userManagement.jwtSessionDurationHours'); - const oldTimeout = config.getEnv('userManagement.jwtRefreshTimeoutHours'); - let mockUser: User; - - beforeEach(() => { - mockUser = mock({ password: 'passwordHash' }); - }); - - afterEach(() => { - config.set('userManagement.jwtSessionDuration', oldDuration); - config.set('userManagement.jwtRefreshTimeoutHours', oldTimeout); - }); - - it('does not do anything if the user is not authorized', async () => { - const req = mock(); - const res = mock({ cookie: jest.fn() }); - const next = jest.fn(); - - await refreshExpiringCookie(req, res, next); - - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).not.toHaveBeenCalled(); - }); - - describe('with N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS=-1', () => { - it('does not refresh the cookie, ever', async () => { - config.set('userManagement.jwtSessionDurationHours', 1); - config.set('userManagement.jwtRefreshTimeoutHours', -1); - const { token } = issueJWT(mockUser); - - jest.advanceTimersByTime(1000 * 60 * 55); /* 55 minutes */ - - const req = mock({ - cookies: { [AUTH_COOKIE_NAME]: token }, - user: mockUser, - }); - const res = mock({ cookie: jest.fn() }); - const next = jest.fn(); - await refreshExpiringCookie(req, res, next); - - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).not.toHaveBeenCalled(); - }); - }); - - describe('with N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS=0', () => { - let token: string; - let req: AuthenticatedRequest; - let res: Response; - let next: NextFunction; - - beforeEach(() => { - // ARRANGE - config.set('userManagement.jwtSessionDurationHours', 1); - config.set('userManagement.jwtRefreshTimeoutHours', 0); - token = issueJWT(mockUser).token; - - req = mock({ - cookies: { [AUTH_COOKIE_NAME]: token }, - user: mockUser, - }); - res = mock({ cookie: jest.fn() }); - next = jest.fn(); - }); - - it('does not refresh the cookie when more than 1/4th of time is left', async () => { - // ARRANGE - jest.advanceTimersByTime(44 * Time.minutes.toMilliseconds); /* 44 minutes */ - - // ACT - await refreshExpiringCookie(req, res, next); - - // ASSERT - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).not.toHaveBeenCalled(); - }); - - it('refreshes the cookie when 1/4th of time is left', async () => { - // ARRANGE - jest.advanceTimersByTime(46 * Time.minutes.toMilliseconds); /* 46 minutes */ - - // ACT - await refreshExpiringCookie(req, res, next); - - // ASSERT - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).toHaveBeenCalledTimes(1); - }); - }); - - describe('with N8N_USER_MANAGEMENT_JWT_REFRESH_TIMEOUT_HOURS=50', () => { - const jwtSessionDurationHours = 51; - let token: string; - let req: AuthenticatedRequest; - let res: Response; - let next: NextFunction; - - // ARRANGE - beforeEach(() => { - config.set('userManagement.jwtSessionDurationHours', jwtSessionDurationHours); - config.set('userManagement.jwtRefreshTimeoutHours', 50); - - token = issueJWT(mockUser).token; - req = mock({ - cookies: { [AUTH_COOKIE_NAME]: token }, - user: mockUser, - }); - res = mock({ cookie: jest.fn() }); - next = jest.fn(); - }); - - it('does not do anything if the cookie is still valid', async () => { - // ARRANGE - // cookie has 50.5 hours to live: 51 - 0.5 - jest.advanceTimersByTime(30 * Time.minutes.toMilliseconds); - - // ACT - await refreshExpiringCookie(req, res, next); - - // ASSERT - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).not.toHaveBeenCalled(); - }); - - it('refreshes the cookie if it has less than 50 hours to live', async () => { - // ARRANGE - // cookie has 49.5 hours to live: 51 - 1.5 - jest.advanceTimersByTime(1.5 * Time.hours.toMilliseconds); - - // ACT - await refreshExpiringCookie(req, res, next); - - // ASSERT - expect(next).toHaveBeenCalledTimes(1); - expect(res.cookie).toHaveBeenCalledTimes(1); - expect(res.cookie).toHaveBeenCalledWith(AUTH_COOKIE_NAME, expect.any(String), { - httpOnly: true, - maxAge: jwtSessionDurationHours * Time.hours.toMilliseconds, - sameSite: 'lax', - }); - }); - }); -}); diff --git a/packages/cli/test/unit/services/user.service.test.ts b/packages/cli/test/unit/services/user.service.test.ts index 6ab4f79c42..fe5a7c2a80 100644 --- a/packages/cli/test/unit/services/user.service.test.ts +++ b/packages/cli/test/unit/services/user.service.test.ts @@ -1,22 +1,13 @@ -import Container from 'typedi'; -import jwt from 'jsonwebtoken'; -import { Logger } from '@/Logger'; -import config from '@/config'; -import { User } from '@db/entities/User'; -import { UserRepository } from '@db/repositories/user.repository'; -import { UserService } from '@/services/user.service'; -import { mockInstance } from '../../shared/mocking'; +import { mock } from 'jest-mock-extended'; import { v4 as uuid } from 'uuid'; -import { InternalHooks } from '@/InternalHooks'; + +import { User } from '@db/entities/User'; +import { UserService } from '@/services/user.service'; +import { UrlService } from '@/services/url.service'; describe('UserService', () => { - config.set('userManagement.jwtSecret', 'random-secret'); - - mockInstance(Logger); - mockInstance(InternalHooks); - - const userRepository = mockInstance(UserRepository); - const userService = Container.get(UserService); + const urlService = new UrlService(); + const userService = new UserService(mock(), mock(), mock(), urlService); const commonMockUser = Object.assign(new User(), { id: uuid(), @@ -75,66 +66,4 @@ describe('UserService', () => { expect(url.searchParams.get('inviteeId')).toBe(secondUser.id); }); }); - - describe('generatePasswordResetToken', () => { - it('should generate valid password-reset tokens', () => { - const token = userService.generatePasswordResetToken(commonMockUser); - - const decoded = jwt.decode(token) as jwt.JwtPayload; - - if (!decoded.exp) fail('Token does not contain expiry'); - if (!decoded.iat) fail('Token does not contain issued-at'); - - expect(decoded.sub).toEqual(commonMockUser.id); - expect(decoded.exp - decoded.iat).toEqual(1200); // Expires in 20 minutes - expect(decoded.passwordSha).toEqual( - '31513c5a9e3c5afe5c06d5675ace74e8bc3fadd9744ab5d89c311f2a62ccbd39', - ); - }); - }); - - describe('resolvePasswordResetToken', () => { - it('should not return a user if the token in invalid', async () => { - const user = await userService.resolvePasswordResetToken('invalid-token'); - - expect(user).toBeUndefined(); - }); - - it('should not return a user if the token in expired', async () => { - const token = userService.generatePasswordResetToken(commonMockUser, '-1h'); - - const user = await userService.resolvePasswordResetToken(token); - - expect(user).toBeUndefined(); - }); - - it('should not return a user if the user does not exist in the DB', async () => { - userRepository.findOne.mockResolvedValueOnce(null); - const token = userService.generatePasswordResetToken(commonMockUser); - - const user = await userService.resolvePasswordResetToken(token); - - expect(user).toBeUndefined(); - }); - - it('should not return a user if the password sha does not match', async () => { - const token = userService.generatePasswordResetToken(commonMockUser); - const updatedUser = Object.create(commonMockUser); - updatedUser.password = 'something-else'; - userRepository.findOne.mockResolvedValueOnce(updatedUser); - - const user = await userService.resolvePasswordResetToken(token); - - expect(user).toBeUndefined(); - }); - - it('should not return the user if all checks pass', async () => { - const token = userService.generatePasswordResetToken(commonMockUser); - userRepository.findOne.mockResolvedValueOnce(commonMockUser); - - const user = await userService.resolvePasswordResetToken(token); - - expect(user).toEqual(commonMockUser); - }); - }); }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9f217fdee0..2c6f4b2a75 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -548,15 +548,6 @@ importers: p-lazy: specifier: 3.1.0 version: 3.1.0 - passport: - specifier: 0.6.0 - version: 0.6.0 - passport-cookie: - specifier: 1.0.9 - version: 1.0.9 - passport-jwt: - specifier: 4.0.1 - version: 4.0.1 pg: specifier: 8.11.3 version: 8.11.3 @@ -675,18 +666,12 @@ importers: '@types/lodash': specifier: ^4.14.195 version: 4.14.195 - '@types/passport-jwt': - specifier: ^3.0.6 - version: 3.0.7 '@types/psl': specifier: ^1.1.0 version: 1.1.0 '@types/replacestream': specifier: ^4.0.1 version: 4.0.1 - '@types/send': - specifier: ^0.17.1 - version: 0.17.1 '@types/shelljs': specifier: ^0.8.11 version: 0.8.11 @@ -10024,10 +10009,6 @@ packages: resolution: {integrity: sha512-vXOTGVSLR2jMw440moWTC7H19iUyLtP3Z1YTj7cSsubOICinjMxFeb/V57v9QdyyPGbbWolUFSSmSiRSn94tFw==} dev: true - /@types/mime@1.3.2: - resolution: {integrity: sha512-YATxVxgRqNH6nHEIsvg6k2Boc1JHI9ZbH5iWFFv/MTkchz3b1ieGDa5T0a9RznNdI0KhVbdbWSN+KWWrQZRxTw==} - dev: true - /@types/mime@3.0.1: resolution: {integrity: sha512-Y4XFY5VJAuw0FgAqPNd6NNoV44jbq9Bz2L7Rh/J6jLTiHBSBJa9fxqQIvkIld4GsoDOcCbvzOUAbLPsSKKg+uA==} @@ -10082,27 +10063,6 @@ packages: resolution: {integrity: sha512-//oorEZjL6sbPcKUaCdIGlIUeH26mgzimjBB77G6XRgnDl/L5wOnpyBGRe/Mmf5CVW3PwEBE1NjiMZ/ssFh4wA==} dev: true - /@types/passport-jwt@3.0.7: - resolution: {integrity: sha512-qRQ4qlww1Yhs3IaioDKrsDNmKy6gLDLgFsGwpCnc2YqWovO2Oxu9yCQdWHMJafQ7UIuOba4C4/TNXcGkQfEjlQ==} - dependencies: - '@types/express': 4.17.14 - '@types/jsonwebtoken': 9.0.1 - '@types/passport-strategy': 0.2.35 - dev: true - - /@types/passport-strategy@0.2.35: - resolution: {integrity: sha512-o5D19Jy2XPFoX2rKApykY15et3Apgax00RRLf0RUotPDUsYrQa7x4howLYr9El2mlUApHmCMv5CZ1IXqKFQ2+g==} - dependencies: - '@types/express': 4.17.14 - '@types/passport': 1.0.11 - dev: true - - /@types/passport@1.0.11: - resolution: {integrity: sha512-pz1cx9ptZvozyGKKKIPLcVDVHwae4hrH5d6g5J+DkMRRjR3cVETb4jMabhXAUbg3Ov7T22nFHEgaK2jj+5CBpw==} - dependencies: - '@types/express': 4.17.14 - dev: true - /@types/phoenix@1.6.4: resolution: {integrity: sha512-B34A7uot1Cv0XtaHRYDATltAdKx0BvVKNgYNqE4WjtPUa4VQJM7kxeXcVKaH+KS+kCmZ+6w+QaUdcljiheiBJA==} dev: false @@ -10183,13 +10143,6 @@ packages: resolution: {integrity: sha512-G8hZ6XJiHnuhQKR7ZmysCeJWE08o8T0AXtk5darsCaTVsYZhhgUrq53jizaR2FvsoeCwJhlmwTjkXBY5Pn/ZHw==} dev: true - /@types/send@0.17.1: - resolution: {integrity: sha512-Cwo8LE/0rnvX7kIIa3QHCkcuF21c05Ayb0ZfxPiv0W8VRiZiNW/WuRupHKpqqGVGf7SUA44QSOUKaEd9lIrd/Q==} - dependencies: - '@types/mime': 1.3.2 - '@types/node': 18.16.16 - dev: true - /@types/serve-static@1.15.0: resolution: {integrity: sha512-z5xyF6uh8CbjAu9760KDKsH2FcDxZ2tFCsA4HIMWE6IkiYMXfVoa+4f9KX+FN0ZLsaMw1WNG2ETLA6N+/YA+cg==} dependencies: @@ -20984,34 +20937,6 @@ packages: engines: {node: '>=0.10.0'} dev: true - /passport-cookie@1.0.9: - resolution: {integrity: sha512-8a6foX2bbGoJzup0RAiNcC2tTqzYS46RQEK3Z4u8p86wesPUjgDaji3C7+5j4TGyCq4ZoOV+3YLw1Hy6cV6kyw==} - engines: {node: '>= 0.10.0'} - dependencies: - passport-strategy: 1.0.0 - dev: false - - /passport-jwt@4.0.1: - resolution: {integrity: sha512-UCKMDYhNuGOBE9/9Ycuoyh7vP6jpeTp/+sfMJl7nLff/t6dps+iaeE0hhNkKN8/HZHcJ7lCdOyDxHdDoxoSvdQ==} - dependencies: - jsonwebtoken: 9.0.0 - passport-strategy: 1.0.0 - dev: false - - /passport-strategy@1.0.0: - resolution: {integrity: sha512-CB97UUvDKJde2V0KDWWB3lyf6PC3FaZP7YxZ2G8OAtn9p4HI9j9JLP9qjOGZFvyl8uwNT8qM+hGnz/n16NI7oA==} - engines: {node: '>= 0.4.0'} - dev: false - - /passport@0.6.0: - resolution: {integrity: sha512-0fe+p3ZnrWRW74fe8+SvCyf4a3Pb2/h7gFkQ8yTJpAO50gDzlfjZUZTO1k5Eg9kUct22OxHLqDZoKUWRHOh9ug==} - engines: {node: '>= 0.4.0'} - dependencies: - passport-strategy: 1.0.0 - pause: 0.0.1 - utils-merge: 1.0.1 - dev: false - /password-prompt@1.1.3: resolution: {integrity: sha512-HkrjG2aJlvF0t2BMH0e2LB/EHf3Lcq3fNMzy4GYHcQblAvOl+QQji1Lx7WRBMqpVK8p+KR7bCg7oqAMXtdgqyw==} dependencies: @@ -21129,10 +21054,6 @@ packages: through: 2.3.8 dev: true - /pause@0.0.1: - resolution: {integrity: sha512-KG8UEiEVkR3wGEb4m5yZkVCzigAD+cVEJck2CzYZO37ZGJfctvVptVO192MwrtPhzONn6go8ylnOdMhKqi4nfg==} - dev: false - /pdf-parse@1.1.1: resolution: {integrity: sha512-v6ZJ/efsBpGrGGknjtq9J/oC8tZWq0KWL5vQrk2GlzLEQPUDB1ex+13Rmidl1neNN358Jn9EHZw5y07FFtaC7A==} engines: {node: '>=6.8.1'}