From fc6f5aa8a80efceefda0cb3f1e66ea5c66051532 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 8 Aug 2021 21:44:54 +0200 Subject: [PATCH 01/18] chore: add passport-local dependency Signed-off-by: Philip Molares --- package.json | 2 ++ yarn.lock | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/package.json b/package.json index 22692a62e..7c76edcda 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,7 @@ "node-fetch": "2.6.2", "passport": "0.4.1", "passport-http-bearer": "1.0.1", + "passport-local": "1.0.0", "raw-body": "2.4.1", "reflect-metadata": "0.1.13", "rimraf": "3.0.2", @@ -72,6 +73,7 @@ "@types/express": "4.17.13", "@types/jest": "27.0.1", "@types/node": "14.17.16", + "@types/passport-local": "^1.0.34", "@types/supertest": "2.0.11", "@typescript-eslint/eslint-plugin": "4.31.1", "@typescript-eslint/parser": "4.31.1", diff --git a/yarn.lock b/yarn.lock index e6de40179..98d1c4f74 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1293,6 +1293,23 @@ "@types/koa" "*" "@types/passport" "*" +"@types/passport-local@^1.0.34": + version "1.0.34" + resolved "https://registry.yarnpkg.com/@types/passport-local/-/passport-local-1.0.34.tgz#84d3b35b2fd4d36295039ded17fe5f3eaa62f4f6" + integrity sha512-PSc07UdYx+jhadySxxIYWuv6sAnY5e+gesn/5lkPKfBeGuIYn9OPR+AAEDq73VRUh6NBTpvE/iPE62rzZUslog== + dependencies: + "@types/express" "*" + "@types/passport" "*" + "@types/passport-strategy" "*" + +"@types/passport-strategy@*": + version "0.2.35" + resolved "https://registry.yarnpkg.com/@types/passport-strategy/-/passport-strategy-0.2.35.tgz#e52f5212279ea73f02d9b06af67efe9cefce2d0c" + integrity sha512-o5D19Jy2XPFoX2rKApykY15et3Apgax00RRLf0RUotPDUsYrQa7x4howLYr9El2mlUApHmCMv5CZ1IXqKFQ2+g== + dependencies: + "@types/express" "*" + "@types/passport" "*" + "@types/passport@*": version "1.0.7" resolved "https://registry.yarnpkg.com/@types/passport/-/passport-1.0.7.tgz#85892f14932168158c86aecafd06b12f5439467a" @@ -5573,6 +5590,13 @@ passport-http-bearer@1.0.1: dependencies: passport-strategy "1.x.x" +passport-local@1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/passport-local/-/passport-local-1.0.0.tgz#1fe63268c92e75606626437e3b906662c15ba6ee" + integrity sha1-H+YyaMkudWBmJkN+O5BmYsFbpu4= + dependencies: + passport-strategy "1.x.x" + passport-strategy@1.x.x: version "1.0.0" resolved "https://registry.yarnpkg.com/passport-strategy/-/passport-strategy-1.0.0.tgz#b5539aa8fc225a3d1ad179476ddf236b440f52e4" From 4938d308b035333c44ad2a9a871e1d3bcc5ada0b Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 8 Aug 2021 21:57:22 +0200 Subject: [PATCH 02/18] feat: add ProviderType enum This is used to give identities a type and to easily get the identity any auth method would need. Signed-off-by: Philip Molares --- src/identity/provider-type.enum.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 src/identity/provider-type.enum.ts diff --git a/src/identity/provider-type.enum.ts b/src/identity/provider-type.enum.ts new file mode 100644 index 000000000..d2032c70c --- /dev/null +++ b/src/identity/provider-type.enum.ts @@ -0,0 +1,18 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +export enum ProviderType { + LOCAL = 'local', + LDAP = 'ldap', + SAML = 'saml', + OAUTH2 = 'oauth2', + GITLAB = 'gitlab', + GITHUB = 'github', + FACEBOOK = 'facebook', + TWITTER = 'twitter', + DROPBOX = 'dropbox', + GOOGLE = 'google', +} From 87a5f77abe63e422a5a9a66dc5f1132edab9820a Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 8 Aug 2021 21:47:13 +0200 Subject: [PATCH 03/18] chore: move password related functions from AuthService to utils file As these methods will be used in both the AuthService and the IdentityService, it makes sense to extract them and use them in this manner. Especially if one considers that they are quite standalone functions. Signed-off-by: Philip Molares --- src/auth/auth.service.spec.ts | 40 +++--------------------- src/auth/auth.service.ts | 43 +++++++++---------------- src/utils/password.spec.ts | 59 +++++++++++++++++++++++++++++++++++ src/utils/password.ts | 30 ++++++++++++++++++ 4 files changed, 109 insertions(+), 63 deletions(-) create mode 100644 src/utils/password.spec.ts create mode 100644 src/utils/password.ts diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index f7aaba855..937a97feb 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -7,16 +7,16 @@ import { ConfigModule } from '@nestjs/config'; import { PassportModule } from '@nestjs/passport'; import { Test, TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; -import { randomBytes } from 'crypto'; import { Repository } from 'typeorm'; import appConfigMock from '../config/mock/app.config.mock'; import { NotInDBError, TokenNotValidError } from '../errors/errors'; +import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; -import { Identity } from '../users/identity.entity'; import { Session } from '../users/session.entity'; import { User } from '../users/user.entity'; import { UsersModule } from '../users/users.module'; +import { hashPassword } from '../utils/password'; import { AuthToken } from './auth-token.entity'; import { AuthService } from './auth.service'; @@ -74,26 +74,6 @@ describe('AuthService', () => { expect(service).toBeDefined(); }); - describe('checkPassword', () => { - it('works', async () => { - const testPassword = 'thisIsATestPassword'; - const hash = await service.hashPassword(testPassword); - await service - .checkPassword(testPassword, hash) - .then((result) => expect(result).toBeTruthy()); - }); - it('fails, if secret is too short', async () => { - const secret = service.bufferToBase64Url(randomBytes(54)); - const hash = await service.hashPassword(secret); - await service - .checkPassword(secret, hash) - .then((result) => expect(result).toBeTruthy()); - await service - .checkPassword(secret.substr(0, secret.length - 1), hash) - .then((result) => expect(result).toBeFalsy()); - }); - }); - describe('getTokensByUsername', () => { it('works', async () => { jest @@ -108,7 +88,7 @@ describe('AuthService', () => { describe('getAuthToken', () => { const token = 'testToken'; it('works', async () => { - const accessTokenHash = await service.hashPassword(token); + const accessTokenHash = await hashPassword(token); jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce({ ...authToken, user: user, @@ -142,7 +122,7 @@ describe('AuthService', () => { ).rejects.toThrow(TokenNotValidError); }); it('AuthToken has wrong validUntil Date', async () => { - const accessTokenHash = await service.hashPassword(token); + const accessTokenHash = await hashPassword(token); jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce({ ...authToken, user: user, @@ -185,7 +165,7 @@ describe('AuthService', () => { describe('validateToken', () => { it('works', async () => { const token = 'testToken'; - const accessTokenHash = await service.hashPassword(token); + const accessTokenHash = await hashPassword(token); jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce({ ...user, authTokens: [authToken], @@ -303,16 +283,6 @@ describe('AuthService', () => { }); }); - describe('bufferToBase64Url', () => { - it('works', () => { - expect( - service.bufferToBase64Url( - Buffer.from('testsentence is a test sentence'), - ), - ).toEqual('dGVzdHNlbnRlbmNlIGlzIGEgdGVzdCBzZW50ZW5jZQ'); - }); - }); - describe('toAuthTokenDto', () => { it('works', () => { const authToken = new AuthToken(); diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 609592beb..5bcc01540 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -6,7 +6,6 @@ import { Injectable } from '@nestjs/common'; import { Cron, Timeout } from '@nestjs/schedule'; import { InjectRepository } from '@nestjs/typeorm'; -import { compare, hash } from 'bcrypt'; import { randomBytes } from 'crypto'; import { Repository } from 'typeorm'; @@ -16,8 +15,13 @@ import { TooManyTokensError, } from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; +import { UserRelationEnum } from '../users/user-relation.enum'; import { User } from '../users/user.entity'; import { UsersService } from '../users/users.service'; +import { + bufferToBase64Url, + hashPassword, +} from '../utils/password'; import { TimestampMillis } from '../utils/timestamp'; import { AuthTokenWithSecretDto } from './auth-token-with-secret.dto'; import { AuthTokenDto } from './auth-token.dto'; @@ -52,33 +56,14 @@ export class AuthService { return await this.usersService.getUserByUsername(accessToken.user.userName); } - async hashPassword(cleartext: string): Promise { - // hash the password with bcrypt and 2^12 iterations - // this was decided on the basis of https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#bcrypt - return await hash(cleartext, 12); - } - - async checkPassword(cleartext: string, password: string): Promise { - return await compare(cleartext, password); - } - - bufferToBase64Url(text: Buffer): string { - // This is necessary as the is no base64url encoding in the toString method - // but as can be seen on https://tools.ietf.org/html/rfc4648#page-7 - // base64url is quite easy buildable from base64 - return text - .toString('base64') - .replace(/\+/g, '-') - .replace(/\//g, '_') - .replace(/=+$/, ''); - } - async createTokenForUser( userName: string, identifier: string, validUntil: TimestampMillis, ): Promise { - const user = await this.usersService.getUserByUsername(userName, true); + const user = await this.usersService.getUserByUsername(userName, [ + UserRelationEnum.AUTHTOKENS, + ]); if (user.authTokens.length >= 200) { // This is a very high ceiling unlikely to hinder legitimate usage, // but should prevent possible attack vectors @@ -86,9 +71,9 @@ export class AuthService { `User '${user.userName}' has already 200 tokens and can't have anymore`, ); } - const secret = this.bufferToBase64Url(randomBytes(54)); - const keyId = this.bufferToBase64Url(randomBytes(8)); - const accessToken = await this.hashPassword(secret); + const secret = bufferToBase64Url(randomBytes(54)); + const keyId = bufferToBase64Url(randomBytes(8)); + const accessToken = await hashPassword(secret); let token; // Tokens can only be valid for a maximum of 2 years const maximumTokenValidity = @@ -138,7 +123,7 @@ export class AuthService { if (accessToken === undefined) { throw new NotInDBError(`AuthToken '${token}' not found`); } - if (!(await this.checkPassword(token, accessToken.accessTokenHash))) { + if (!(await checkPassword(token, accessToken.accessTokenHash))) { // hashes are not the same throw new TokenNotValidError(`AuthToken '${token}' is not valid.`); } @@ -155,7 +140,9 @@ export class AuthService { } async getTokensByUsername(userName: string): Promise { - const user = await this.usersService.getUserByUsername(userName, true); + const user = await this.usersService.getUserByUsername(userName, [ + UserRelationEnum.AUTHTOKENS, + ]); if (user.authTokens === undefined) { return []; } diff --git a/src/utils/password.spec.ts b/src/utils/password.spec.ts new file mode 100644 index 000000000..199c7d16b --- /dev/null +++ b/src/utils/password.spec.ts @@ -0,0 +1,59 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import bcrypt from 'bcrypt'; +import { randomBytes } from 'crypto'; + +import { bufferToBase64Url, checkPassword, hashPassword } from './password'; + +const testPassword = 'thisIsATestPassword'; + +describe('hashPassword', () => { + it('output looks like a bcrypt hash with 2^12 rounds of hashing', async () => { + /* + * a bcrypt hash example with the different parts highlighted: + * $2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy + * \__/\/ \____________________/\_____________________________/ + * Alg Cost Salt Hash + * from https://en.wikipedia.org/wiki/Bcrypt#Description + */ + const regexBcrypt = /^\$2[abxy]\$12\$[A-Za-z0-9/.]{53}$/; + const hash = await hashPassword(testPassword); + expect(regexBcrypt.test(hash)).toBeTruthy(); + }); + it('calls bcrypt.hash with the correct parameters', async () => { + const spy = jest.spyOn(bcrypt, 'hash'); + await hashPassword(testPassword); + expect(spy).toHaveBeenCalledWith(testPassword, 12); + }); +}); + +describe('checkPassword', () => { + it("is returning true if the inputs are a plaintext password and it's bcrypt-hashed version", async () => { + const hashOfTestPassword = + '$2a$12$WHKCq4c0rg19zyx5WgX0p.or0rjSKYpIBcHhQQGLrxrr6FfMPylIW'; + await checkPassword(testPassword, hashOfTestPassword).then((result) => + expect(result).toBeTruthy(), + ); + }); + it('fails, if secret is too short', async () => { + const secret = bufferToBase64Url(randomBytes(54)); + const hash = await hashPassword(secret); + await checkPassword(secret, hash).then((result) => + expect(result).toBeTruthy(), + ); + await checkPassword(secret.substr(0, secret.length - 1), hash).then( + (result) => expect(result).toBeFalsy(), + ); + }); +}); + +describe('bufferToBase64Url', () => { + it('transforms a buffer to the correct base64url encoded string', () => { + expect( + bufferToBase64Url(Buffer.from('testsentence is a test sentence')), + ).toEqual('dGVzdHNlbnRlbmNlIGlzIGEgdGVzdCBzZW50ZW5jZQ'); + }); +}); diff --git a/src/utils/password.ts b/src/utils/password.ts new file mode 100644 index 000000000..a419bf01f --- /dev/null +++ b/src/utils/password.ts @@ -0,0 +1,30 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { compare, hash } from 'bcrypt'; + +export async function hashPassword(cleartext: string): Promise { + // hash the password with bcrypt and 2^12 iterations + // this was decided on the basis of https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#bcrypt + return await hash(cleartext, 12); +} + +export async function checkPassword( + cleartext: string, + password: string, +): Promise { + return await compare(cleartext, password); +} + +export function bufferToBase64Url(text: Buffer): string { + // This is necessary as the is no base64url encoding in the toString method + // but as can be seen on https://tools.ietf.org/html/rfc4648#page-7 + // base64url is quite easy buildable from base64 + return text + .toString('base64') + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/, ''); +} From b2da8a2b950b312c3dbee63020d1633ba316c589 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 8 Aug 2021 21:53:20 +0200 Subject: [PATCH 04/18] chore: move identity entity in its own folder Signed-off-by: Philip Molares --- docs/content/dev/db-schema.plantuml | 3 +- .../me/history/history.controller.spec.ts | 2 +- src/api/private/me/me.controller.spec.ts | 2 +- .../private/media/media.controller.spec.ts | 2 +- .../private/notes/notes.controller.spec.ts | 2 +- .../private/tokens/tokens.controller.spec.ts | 2 +- src/api/public/me/me.controller.spec.ts | 2 +- src/api/public/media/media.controller.spec.ts | 2 +- src/api/public/notes/notes.controller.spec.ts | 2 +- src/history/history.service.spec.ts | 2 +- src/identity/identity.entity.ts | 111 ++++++++++++++++++ src/media/media.service.spec.ts | 2 +- src/notes/notes.service.spec.ts | 2 +- src/permissions/permissions.service.spec.ts | 2 +- src/revisions/revisions.service.spec.ts | 2 +- src/seed.ts | 2 +- src/users/identity.entity.ts | 56 --------- src/users/user.entity.ts | 2 +- src/users/users.module.ts | 2 +- 19 files changed, 129 insertions(+), 73 deletions(-) create mode 100644 src/identity/identity.entity.ts delete mode 100644 src/users/identity.entity.ts diff --git a/docs/content/dev/db-schema.plantuml b/docs/content/dev/db-schema.plantuml index 98ff6a3cb..7a9239184 100644 --- a/docs/content/dev/db-schema.plantuml +++ b/docs/content/dev/db-schema.plantuml @@ -40,8 +40,9 @@ entity "identity" { *id : number -- *userId : uuid <> + *providerType: text ' Identifies the external login provider and is set in the config - *providerName : text + providerName : text *syncSource : boolean *createdAt : date *updatedAt : date diff --git a/src/api/private/me/history/history.controller.spec.ts b/src/api/private/me/history/history.controller.spec.ts index a16de5aa0..410908181 100644 --- a/src/api/private/me/history/history.controller.spec.ts +++ b/src/api/private/me/history/history.controller.spec.ts @@ -17,6 +17,7 @@ import appConfigMock from '../../../../config/mock/app.config.mock'; import { Group } from '../../../../groups/group.entity'; import { HistoryEntry } from '../../../../history/history-entry.entity'; import { HistoryModule } from '../../../../history/history.module'; +import { Identity } from '../../../../identity/identity.entity'; import { LoggerModule } from '../../../../logger/logger.module'; import { Note } from '../../../../notes/note.entity'; import { NotesModule } from '../../../../notes/notes.module'; @@ -25,7 +26,6 @@ import { NoteGroupPermission } from '../../../../permissions/note-group-permissi import { NoteUserPermission } from '../../../../permissions/note-user-permission.entity'; import { Edit } from '../../../../revisions/edit.entity'; import { Revision } from '../../../../revisions/revision.entity'; -import { Identity } from '../../../../users/identity.entity'; import { Session } from '../../../../users/session.entity'; import { User } from '../../../../users/user.entity'; import { UsersModule } from '../../../../users/users.module'; diff --git a/src/api/private/me/me.controller.spec.ts b/src/api/private/me/me.controller.spec.ts index 33aef70e0..d03e7208b 100644 --- a/src/api/private/me/me.controller.spec.ts +++ b/src/api/private/me/me.controller.spec.ts @@ -14,6 +14,7 @@ import customizationConfigMock from '../../../config/mock/customization.config.m import externalServicesConfigMock from '../../../config/mock/external-services.config.mock'; import mediaConfigMock from '../../../config/mock/media.config.mock'; import { Group } from '../../../groups/group.entity'; +import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; @@ -23,7 +24,6 @@ import { NoteGroupPermission } from '../../../permissions/note-group-permission. import { NoteUserPermission } from '../../../permissions/note-user-permission.entity'; import { Edit } from '../../../revisions/edit.entity'; import { Revision } from '../../../revisions/revision.entity'; -import { Identity } from '../../../users/identity.entity'; import { Session } from '../../../users/session.entity'; import { User } from '../../../users/user.entity'; import { UsersModule } from '../../../users/users.module'; diff --git a/src/api/private/media/media.controller.spec.ts b/src/api/private/media/media.controller.spec.ts index cbed058e3..ffb9a9b66 100644 --- a/src/api/private/media/media.controller.spec.ts +++ b/src/api/private/media/media.controller.spec.ts @@ -15,6 +15,7 @@ import customizationConfigMock from '../../../config/mock/customization.config.m import externalConfigMock from '../../../config/mock/external-services.config.mock'; import mediaConfigMock from '../../../config/mock/media.config.mock'; import { Group } from '../../../groups/group.entity'; +import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; @@ -25,7 +26,6 @@ import { NoteGroupPermission } from '../../../permissions/note-group-permission. import { NoteUserPermission } from '../../../permissions/note-user-permission.entity'; import { Edit } from '../../../revisions/edit.entity'; import { Revision } from '../../../revisions/revision.entity'; -import { Identity } from '../../../users/identity.entity'; import { Session } from '../../../users/session.entity'; import { User } from '../../../users/user.entity'; import { UsersModule } from '../../../users/users.module'; diff --git a/src/api/private/notes/notes.controller.spec.ts b/src/api/private/notes/notes.controller.spec.ts index c3d32eff4..eb908ff47 100644 --- a/src/api/private/notes/notes.controller.spec.ts +++ b/src/api/private/notes/notes.controller.spec.ts @@ -19,6 +19,7 @@ import { Group } from '../../../groups/group.entity'; import { GroupsModule } from '../../../groups/groups.module'; import { HistoryEntry } from '../../../history/history-entry.entity'; import { HistoryModule } from '../../../history/history.module'; +import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; @@ -31,7 +32,6 @@ import { PermissionsModule } from '../../../permissions/permissions.module'; import { Edit } from '../../../revisions/edit.entity'; import { Revision } from '../../../revisions/revision.entity'; import { RevisionsModule } from '../../../revisions/revisions.module'; -import { Identity } from '../../../users/identity.entity'; import { Session } from '../../../users/session.entity'; import { User } from '../../../users/user.entity'; import { UsersModule } from '../../../users/users.module'; diff --git a/src/api/private/tokens/tokens.controller.spec.ts b/src/api/private/tokens/tokens.controller.spec.ts index 1f5341bb6..56ecfd57e 100644 --- a/src/api/private/tokens/tokens.controller.spec.ts +++ b/src/api/private/tokens/tokens.controller.spec.ts @@ -10,8 +10,8 @@ import { getRepositoryToken } from '@nestjs/typeorm'; import { AuthToken } from '../../../auth/auth-token.entity'; import { AuthModule } from '../../../auth/auth.module'; import appConfigMock from '../../../config/mock/app.config.mock'; +import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; -import { Identity } from '../../../users/identity.entity'; import { Session } from '../../../users/session.entity'; import { User } from '../../../users/user.entity'; import { TokensController } from './tokens.controller'; diff --git a/src/api/public/me/me.controller.spec.ts b/src/api/public/me/me.controller.spec.ts index d54398fb1..dd5bf3750 100644 --- a/src/api/public/me/me.controller.spec.ts +++ b/src/api/public/me/me.controller.spec.ts @@ -18,6 +18,7 @@ import mediaConfigMock from '../../../config/mock/media.config.mock'; import { Group } from '../../../groups/group.entity'; import { HistoryEntry } from '../../../history/history-entry.entity'; import { HistoryModule } from '../../../history/history.module'; +import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; @@ -28,7 +29,6 @@ import { NoteGroupPermission } from '../../../permissions/note-group-permission. import { NoteUserPermission } from '../../../permissions/note-user-permission.entity'; import { Edit } from '../../../revisions/edit.entity'; import { Revision } from '../../../revisions/revision.entity'; -import { Identity } from '../../../users/identity.entity'; import { Session } from '../../../users/session.entity'; import { User } from '../../../users/user.entity'; import { UsersModule } from '../../../users/users.module'; diff --git a/src/api/public/media/media.controller.spec.ts b/src/api/public/media/media.controller.spec.ts index 466086569..028ccba75 100644 --- a/src/api/public/media/media.controller.spec.ts +++ b/src/api/public/media/media.controller.spec.ts @@ -12,6 +12,7 @@ import { Author } from '../../../authors/author.entity'; import appConfigMock from '../../../config/mock/app.config.mock'; import mediaConfigMock from '../../../config/mock/media.config.mock'; import { Group } from '../../../groups/group.entity'; +import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; @@ -22,7 +23,6 @@ import { NoteGroupPermission } from '../../../permissions/note-group-permission. import { NoteUserPermission } from '../../../permissions/note-user-permission.entity'; import { Edit } from '../../../revisions/edit.entity'; import { Revision } from '../../../revisions/revision.entity'; -import { Identity } from '../../../users/identity.entity'; import { Session } from '../../../users/session.entity'; import { User } from '../../../users/user.entity'; import { MediaController } from './media.controller'; diff --git a/src/api/public/notes/notes.controller.spec.ts b/src/api/public/notes/notes.controller.spec.ts index aeb97ddb1..d4ec8bbf0 100644 --- a/src/api/public/notes/notes.controller.spec.ts +++ b/src/api/public/notes/notes.controller.spec.ts @@ -19,6 +19,7 @@ import { Group } from '../../../groups/group.entity'; import { GroupsModule } from '../../../groups/groups.module'; import { HistoryEntry } from '../../../history/history-entry.entity'; import { HistoryModule } from '../../../history/history.module'; +import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; @@ -31,7 +32,6 @@ import { PermissionsModule } from '../../../permissions/permissions.module'; import { Edit } from '../../../revisions/edit.entity'; import { Revision } from '../../../revisions/revision.entity'; import { RevisionsModule } from '../../../revisions/revisions.module'; -import { Identity } from '../../../users/identity.entity'; import { Session } from '../../../users/session.entity'; import { User } from '../../../users/user.entity'; import { UsersModule } from '../../../users/users.module'; diff --git a/src/history/history.service.spec.ts b/src/history/history.service.spec.ts index ddfa763cb..559b2946b 100644 --- a/src/history/history.service.spec.ts +++ b/src/history/history.service.spec.ts @@ -13,6 +13,7 @@ import { Author } from '../authors/author.entity'; import appConfigMock from '../config/mock/app.config.mock'; import { NotInDBError } from '../errors/errors'; import { Group } from '../groups/group.entity'; +import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; import { Note } from '../notes/note.entity'; import { NotesModule } from '../notes/notes.module'; @@ -21,7 +22,6 @@ import { NoteGroupPermission } from '../permissions/note-group-permission.entity import { NoteUserPermission } from '../permissions/note-user-permission.entity'; import { Edit } from '../revisions/edit.entity'; import { Revision } from '../revisions/revision.entity'; -import { Identity } from '../users/identity.entity'; import { Session } from '../users/session.entity'; import { User } from '../users/user.entity'; import { UsersModule } from '../users/users.module'; diff --git a/src/identity/identity.entity.ts b/src/identity/identity.entity.ts new file mode 100644 index 000000000..bc36e6f10 --- /dev/null +++ b/src/identity/identity.entity.ts @@ -0,0 +1,111 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + Column, + CreateDateColumn, + Entity, + ManyToOne, + PrimaryGeneratedColumn, + UpdateDateColumn, +} from 'typeorm'; + +import { User } from '../users/user.entity'; +import { ProviderType } from './provider-type.enum'; + +/** + * The identity represents a single way for a user to login. + * A 'user' can have any number of these. + * Each one holds a type (local, github, twitter, etc.), if this type can have multiple instances (e.g. gitlab), + * it also saves the name of the instance. Also if this identity shall be the syncSource is saved. + */ +@Entity() +export class Identity { + @PrimaryGeneratedColumn() + id: number; + + /** + * User that this identity corresponds to + */ + @ManyToOne((_) => User, (user) => user.identities, { + onDelete: 'CASCADE', // This deletes the Identity, when the associated User is deleted + }) + user: User; + + /** + * The ProviderType of the identity + */ + @Column() + providerType: string; + + /** + * The name of the provider. + * Only set if there are multiple provider of that type (e.g. gitlab) + */ + @Column({ + nullable: true, + type: 'text', + }) + providerName: string | null; + + /** + * If the identity should be used as the sync source. + * See [authentication doc](../../docs/content/dev/authentication.md) for clarification + */ + @Column() + syncSource: boolean; + + /** + * When the identity was created. + */ + @CreateDateColumn() + createdAt: Date; + + /** + * When the identity was last updated. + */ + @UpdateDateColumn() + updatedAt: Date; + + /** + * The unique identifier of a user from the login provider + */ + @Column({ + nullable: true, + type: 'text', + }) + providerUserId: string | null; + + /** + * Token used to access the OAuth provider in the users name. + */ + @Column({ + nullable: true, + type: 'text', + }) + oAuthAccessToken: string | null; + + /** + * The hash of the password + * Only set when the type of the identity is local + */ + @Column({ + nullable: true, + type: 'text', + }) + passwordHash: string | null; + + public static create( + user: User, + providerType: ProviderType, + syncSource = false, + ): Identity { + const newIdentity = new Identity(); + newIdentity.user = user; + newIdentity.providerType = providerType; + newIdentity.syncSource = syncSource; + return newIdentity; + } +} diff --git a/src/media/media.service.spec.ts b/src/media/media.service.spec.ts index 4dec3d89a..76ff471a3 100644 --- a/src/media/media.service.spec.ts +++ b/src/media/media.service.spec.ts @@ -15,6 +15,7 @@ import { Author } from '../authors/author.entity'; import mediaConfigMock from '../config/mock/media.config.mock'; import { ClientError, NotInDBError } from '../errors/errors'; import { Group } from '../groups/group.entity'; +import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; import { Note } from '../notes/note.entity'; import { NotesModule } from '../notes/notes.module'; @@ -23,7 +24,6 @@ import { NoteGroupPermission } from '../permissions/note-group-permission.entity import { NoteUserPermission } from '../permissions/note-user-permission.entity'; import { Edit } from '../revisions/edit.entity'; import { Revision } from '../revisions/revision.entity'; -import { Identity } from '../users/identity.entity'; import { Session } from '../users/session.entity'; import { User } from '../users/user.entity'; import { UsersModule } from '../users/users.module'; diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 20614a5f1..585a07b07 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -19,13 +19,13 @@ import { } from '../errors/errors'; import { Group } from '../groups/group.entity'; import { GroupsModule } from '../groups/groups.module'; +import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; import { NoteGroupPermission } from '../permissions/note-group-permission.entity'; import { NoteUserPermission } from '../permissions/note-user-permission.entity'; import { Edit } from '../revisions/edit.entity'; import { Revision } from '../revisions/revision.entity'; import { RevisionsModule } from '../revisions/revisions.module'; -import { Identity } from '../users/identity.entity'; import { Session } from '../users/session.entity'; import { User } from '../users/user.entity'; import { UsersModule } from '../users/users.module'; diff --git a/src/permissions/permissions.service.spec.ts b/src/permissions/permissions.service.spec.ts index b6414c467..9413a73f7 100644 --- a/src/permissions/permissions.service.spec.ts +++ b/src/permissions/permissions.service.spec.ts @@ -11,13 +11,13 @@ import { AuthToken } from '../auth/auth-token.entity'; import { Author } from '../authors/author.entity'; import appConfigMock from '../config/mock/app.config.mock'; import { Group } from '../groups/group.entity'; +import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; import { Note } from '../notes/note.entity'; import { NotesModule } from '../notes/notes.module'; import { Tag } from '../notes/tag.entity'; import { Edit } from '../revisions/edit.entity'; import { Revision } from '../revisions/revision.entity'; -import { Identity } from '../users/identity.entity'; import { Session } from '../users/session.entity'; import { User } from '../users/user.entity'; import { UsersModule } from '../users/users.module'; diff --git a/src/revisions/revisions.service.spec.ts b/src/revisions/revisions.service.spec.ts index c7824de63..cee143558 100644 --- a/src/revisions/revisions.service.spec.ts +++ b/src/revisions/revisions.service.spec.ts @@ -13,13 +13,13 @@ import { Author } from '../authors/author.entity'; import appConfigMock from '../config/mock/app.config.mock'; import { NotInDBError } from '../errors/errors'; import { Group } from '../groups/group.entity'; +import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; import { Note } from '../notes/note.entity'; import { NotesModule } from '../notes/notes.module'; import { Tag } from '../notes/tag.entity'; import { NoteGroupPermission } from '../permissions/note-group-permission.entity'; import { NoteUserPermission } from '../permissions/note-user-permission.entity'; -import { Identity } from '../users/identity.entity'; import { Session } from '../users/session.entity'; import { User } from '../users/user.entity'; import { Edit } from './edit.entity'; diff --git a/src/seed.ts b/src/seed.ts index d866f01e3..cd8b7ce98 100644 --- a/src/seed.ts +++ b/src/seed.ts @@ -9,6 +9,7 @@ import { AuthToken } from './auth/auth-token.entity'; import { Author } from './authors/author.entity'; import { Group } from './groups/group.entity'; import { HistoryEntry } from './history/history-entry.entity'; +import { Identity } from './identity/identity.entity'; import { MediaUpload } from './media/media-upload.entity'; import { Note } from './notes/note.entity'; import { Tag } from './notes/tag.entity'; @@ -16,7 +17,6 @@ import { NoteGroupPermission } from './permissions/note-group-permission.entity' import { NoteUserPermission } from './permissions/note-user-permission.entity'; import { Edit } from './revisions/edit.entity'; import { Revision } from './revisions/revision.entity'; -import { Identity } from './users/identity.entity'; import { Session } from './users/session.entity'; import { User } from './users/user.entity'; diff --git a/src/users/identity.entity.ts b/src/users/identity.entity.ts deleted file mode 100644 index 95c25e89c..000000000 --- a/src/users/identity.entity.ts +++ /dev/null @@ -1,56 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) - * - * SPDX-License-Identifier: AGPL-3.0-only - */ -import { - Column, - CreateDateColumn, - Entity, - ManyToOne, - PrimaryGeneratedColumn, - UpdateDateColumn, -} from 'typeorm'; - -import { User } from './user.entity'; - -@Entity() -export class Identity { - @PrimaryGeneratedColumn() - id: number; - - @ManyToOne((_) => User, (user) => user.identities, { - onDelete: 'CASCADE', // This deletes the Identity, when the associated User is deleted - }) - user: User; - - @Column() - providerName: string; - - @Column() - syncSource: boolean; - - @CreateDateColumn() - createdAt: Date; - - @UpdateDateColumn() - updatedAt: Date; - - @Column({ - nullable: true, - type: 'text', - }) - providerUserId: string | null; - - @Column({ - nullable: true, - type: 'text', - }) - oAuthAccessToken: string | null; - - @Column({ - nullable: true, - type: 'text', - }) - passwordHash: string | null; -} diff --git a/src/users/user.entity.ts b/src/users/user.entity.ts index 3d77437ed..3766b02d2 100644 --- a/src/users/user.entity.ts +++ b/src/users/user.entity.ts @@ -17,9 +17,9 @@ import { AuthToken } from '../auth/auth-token.entity'; import { Author } from '../authors/author.entity'; import { Group } from '../groups/group.entity'; import { HistoryEntry } from '../history/history-entry.entity'; +import { Identity } from '../identity/identity.entity'; import { MediaUpload } from '../media/media-upload.entity'; import { Note } from '../notes/note.entity'; -import { Identity } from './identity.entity'; @Entity() export class User { diff --git a/src/users/users.module.ts b/src/users/users.module.ts index 12a926764..29b6d7fe5 100644 --- a/src/users/users.module.ts +++ b/src/users/users.module.ts @@ -6,8 +6,8 @@ import { Module } from '@nestjs/common'; import { TypeOrmModule } from '@nestjs/typeorm'; +import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; -import { Identity } from './identity.entity'; import { Session } from './session.entity'; import { User } from './user.entity'; import { UsersService } from './users.service'; From e37caf1e6a96bfb77201bf2a27fb35b6fa25c1be Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 4 Sep 2021 22:13:16 +0200 Subject: [PATCH 05/18] feat: lazy load identities of user object This makes it possible that we can get identities from any user object even if we didn't specify that while getting them from the orm Signed-off-by: Philip Molares --- src/users/user.entity.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/users/user.entity.ts b/src/users/user.entity.ts index 3766b02d2..27fa63504 100644 --- a/src/users/user.entity.ts +++ b/src/users/user.entity.ts @@ -59,7 +59,7 @@ export class User { authTokens: AuthToken[]; @OneToMany((_) => Identity, (identity) => identity.user) - identities: Identity[]; + identities: Promise; @ManyToMany((_) => Group, (group) => group.members) groups: Group[]; From 5985c4e67d2561c3cfafbaecba544c3a849e4c82 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Tue, 31 Aug 2021 13:39:36 +0200 Subject: [PATCH 06/18] chore: add user relation enum this enum is used to specify which relation of the user object should be populated. Signed-off-by: Philip Molares --- src/auth/auth.service.ts | 1 + src/users/user-relation.enum.ts | 10 ++++++++++ src/users/users.service.ts | 14 +++++++------- 3 files changed, 18 insertions(+), 7 deletions(-) create mode 100644 src/users/user-relation.enum.ts diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 5bcc01540..e196aee6f 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -20,6 +20,7 @@ import { User } from '../users/user.entity'; import { UsersService } from '../users/users.service'; import { bufferToBase64Url, + checkPassword, hashPassword, } from '../utils/password'; import { TimestampMillis } from '../utils/timestamp'; diff --git a/src/users/user-relation.enum.ts b/src/users/user-relation.enum.ts new file mode 100644 index 000000000..bad202ae5 --- /dev/null +++ b/src/users/user-relation.enum.ts @@ -0,0 +1,10 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +export enum UserRelationEnum { + AUTHTOKENS = 'authTokens', + IDENTITIES = 'identities', +} diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 5371b575d..c0f1a3d88 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -10,6 +10,7 @@ import { Repository } from 'typeorm'; import { AlreadyInDBError, NotInDBError } from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { UserInfoDto } from './user-info.dto'; +import { UserRelationEnum } from './user-relation.enum'; import { User } from './user.entity'; @Injectable() @@ -73,17 +74,16 @@ export class UsersService { * @async * Get the user specified by the username * @param {string} userName the username by which the user is specified - * @param {boolean} [withTokens=false] if the returned user object should contain authTokens + * @param {UserRelationEnum[]} [withRelations=[]] if the returned user object should contain certain relations * @return {User} the specified user */ - async getUserByUsername(userName: string, withTokens = false): Promise { - const relations: string[] = []; - if (withTokens) { - relations.push('authTokens'); - } + async getUserByUsername( + userName: string, + withRelations: UserRelationEnum[] = [], + ): Promise { const user = await this.userRepository.findOne({ where: { userName: userName }, - relations: relations, + relations: withRelations, }); if (user === undefined) { throw new NotInDBError(`User with username '${userName}' not found`); From a2e89c7c9730d8ad83e7f0d87b3b9cd0bffe1a59 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 8 Aug 2021 21:54:05 +0200 Subject: [PATCH 07/18] feat: add local auth dtos Signed-off-by: Philip Molares --- src/identity/local/login.dto.ts | 13 +++++++++++++ src/identity/local/register.dto.ts | 17 +++++++++++++++++ src/identity/local/update-password.dto.ts | 11 +++++++++++ 3 files changed, 41 insertions(+) create mode 100644 src/identity/local/login.dto.ts create mode 100644 src/identity/local/register.dto.ts create mode 100644 src/identity/local/update-password.dto.ts diff --git a/src/identity/local/login.dto.ts b/src/identity/local/login.dto.ts new file mode 100644 index 000000000..290c52456 --- /dev/null +++ b/src/identity/local/login.dto.ts @@ -0,0 +1,13 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { IsString } from 'class-validator'; + +export class LoginDto { + @IsString() + username: string; + @IsString() + password: string; +} diff --git a/src/identity/local/register.dto.ts b/src/identity/local/register.dto.ts new file mode 100644 index 000000000..0c2a9e6c1 --- /dev/null +++ b/src/identity/local/register.dto.ts @@ -0,0 +1,17 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { IsString } from 'class-validator'; + +export class RegisterDto { + @IsString() + username: string; + + @IsString() + displayname: string; + + @IsString() + password: string; +} diff --git a/src/identity/local/update-password.dto.ts b/src/identity/local/update-password.dto.ts new file mode 100644 index 000000000..bfe473b32 --- /dev/null +++ b/src/identity/local/update-password.dto.ts @@ -0,0 +1,11 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { IsString } from 'class-validator'; + +export class UpdatePasswordDto { + @IsString() + newPassword: string; +} From df08d56f28ff1e3693cb909a024d8e1afda1be1b Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 4 Sep 2021 17:46:58 +0200 Subject: [PATCH 08/18] feat: add session to AuthConfig this handles the settings for the cookie session. The secret and the lifeTime of the cookie can be configured. Signed-off-by: Philip Molares --- src/config/auth.config.ts | 16 ++++++++++++++++ src/config/mock/auth.config.mock.ts | 4 ++++ .../frontend-config.service.spec.ts | 4 ++++ 3 files changed, 24 insertions(+) diff --git a/src/config/auth.config.ts b/src/config/auth.config.ts index 5670ad14b..57406a0f0 100644 --- a/src/config/auth.config.ts +++ b/src/config/auth.config.ts @@ -9,11 +9,16 @@ import * as Joi from 'joi'; import { GitlabScope, GitlabVersion } from './gitlab.enum'; import { buildErrorMessage, + parseOptionalInt, replaceAuthErrorsWithEnvironmentVariables, toArrayConfig, } from './utils'; export interface AuthConfig { + session: { + secret: string; + lifeTime: number; + }; email: { enableLogin: boolean; enableRegister: boolean; @@ -101,6 +106,13 @@ export interface AuthConfig { } const authSchema = Joi.object({ + session: { + secret: Joi.string().label('HD_SESSION_SECRET'), + lifeTime: Joi.number() + .default(100000) + .optional() + .label('HD_SESSION_LIFE_TIME'), + }, email: { enableLogin: Joi.boolean() .default(false) @@ -332,6 +344,10 @@ export default registerAs('authConfig', () => { const authConfig = authSchema.validate( { + session: { + secret: process.env.HD_SESSION_SECRET, + lifeTime: parseOptionalInt(process.env.HD_SESSION_LIFE_TIME), + }, email: { enableLogin: process.env.HD_AUTH_EMAIL_ENABLE_LOGIN, enableRegister: process.env.HD_AUTH_EMAIL_ENABLE_REGISTER, diff --git a/src/config/mock/auth.config.mock.ts b/src/config/mock/auth.config.mock.ts index 0fdc9b5ae..2b59e7c59 100644 --- a/src/config/mock/auth.config.mock.ts +++ b/src/config/mock/auth.config.mock.ts @@ -6,6 +6,10 @@ import { registerAs } from '@nestjs/config'; export default registerAs('authConfig', () => ({ + session: { + secret: 'my_secret', + lifeTime: 1209600000, + }, email: { enableLogin: true, enableRegister: true, diff --git a/src/frontend-config/frontend-config.service.spec.ts b/src/frontend-config/frontend-config.service.spec.ts index bb52e239e..f2d43a04a 100644 --- a/src/frontend-config/frontend-config.service.spec.ts +++ b/src/frontend-config/frontend-config.service.spec.ts @@ -23,6 +23,10 @@ import { FrontendConfigService } from './frontend-config.service'; describe('FrontendConfigService', () => { const domain = 'http://md.example.com'; const emptyAuthConfig: AuthConfig = { + session: { + secret: 'my-secret', + lifeTime: 1209600000, + }, email: { enableLogin: false, enableRegister: false, From cda8c7ac6385b5f58de205856c7bd67e79ab8b9f Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 4 Sep 2021 19:24:32 +0200 Subject: [PATCH 09/18] feat: change email auth config to local This was done to use the same term. Also email was the old term from HedgeDoc 1 and wildly inaccurate. As we never checked any mail addresses, in fact it was more of a username than anything else. Signed-off-by: Philip Molares --- src/config/auth.config.ts | 28 +++++++++---------- src/config/mock/auth.config.mock.ts | 4 +-- src/frontend-config/frontend-config.dto.ts | 4 +-- .../frontend-config.service.spec.ts | 8 +++--- .../frontend-config.service.ts | 4 +-- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/config/auth.config.ts b/src/config/auth.config.ts index 57406a0f0..a9a98fa40 100644 --- a/src/config/auth.config.ts +++ b/src/config/auth.config.ts @@ -17,9 +17,9 @@ import { export interface AuthConfig { session: { secret: string; - lifeTime: number; + lifetime: number; }; - email: { + local: { enableLogin: boolean; enableRegister: boolean; }; @@ -108,20 +108,20 @@ export interface AuthConfig { const authSchema = Joi.object({ session: { secret: Joi.string().label('HD_SESSION_SECRET'), - lifeTime: Joi.number() - .default(100000) + lifetime: Joi.number() + .default(1209600000) // 14 * 24 * 60 * 60 * 1000ms = 14 days .optional() - .label('HD_SESSION_LIFE_TIME'), + .label('HD_SESSION_LIFETIME'), }, - email: { + local: { enableLogin: Joi.boolean() .default(false) .optional() - .label('HD_AUTH_EMAIL_ENABLE_LOGIN'), + .label('HD_AUTH_LOCAL_ENABLE_LOGIN'), enableRegister: Joi.boolean() .default(false) .optional() - .label('HD_AUTH_EMAIL_ENABLE_REGISTER'), + .label('HD_AUTH_LOCAL_ENABLE_REGISTER'), }, facebook: { clientID: Joi.string().optional().label('HD_AUTH_FACEBOOK_CLIENT_ID'), @@ -211,7 +211,7 @@ const authSchema = Joi.object({ attribute: { id: Joi.string().default('NameId').optional(), username: Joi.string().default('NameId').optional(), - email: Joi.string().default('NameId').optional(), + local: Joi.string().default('NameId').optional(), }, }).optional(), ) @@ -309,7 +309,7 @@ export default registerAs('authConfig', () => { attribute: { id: process.env[`HD_AUTH_SAML_${samlName}_ATTRIBUTE_ID`], username: process.env[`HD_AUTH_SAML_${samlName}_ATTRIBUTE_USERNAME`], - email: process.env[`HD_AUTH_SAML_${samlName}_ATTRIBUTE_USERNAME`], + local: process.env[`HD_AUTH_SAML_${samlName}_ATTRIBUTE_USERNAME`], }, }; }); @@ -346,11 +346,11 @@ export default registerAs('authConfig', () => { { session: { secret: process.env.HD_SESSION_SECRET, - lifeTime: parseOptionalInt(process.env.HD_SESSION_LIFE_TIME), + lifetime: parseOptionalInt(process.env.HD_SESSION_LIFETIME), }, - email: { - enableLogin: process.env.HD_AUTH_EMAIL_ENABLE_LOGIN, - enableRegister: process.env.HD_AUTH_EMAIL_ENABLE_REGISTER, + local: { + enableLogin: process.env.HD_AUTH_LOCAL_ENABLE_LOGIN, + enableRegister: process.env.HD_AUTH_LOCAL_ENABLE_REGISTER, }, facebook: { clientID: process.env.HD_AUTH_FACEBOOK_CLIENT_ID, diff --git a/src/config/mock/auth.config.mock.ts b/src/config/mock/auth.config.mock.ts index 2b59e7c59..9ffada095 100644 --- a/src/config/mock/auth.config.mock.ts +++ b/src/config/mock/auth.config.mock.ts @@ -8,9 +8,9 @@ import { registerAs } from '@nestjs/config'; export default registerAs('authConfig', () => ({ session: { secret: 'my_secret', - lifeTime: 1209600000, + lifetime: 1209600000, }, - email: { + local: { enableLogin: true, enableRegister: true, }, diff --git a/src/frontend-config/frontend-config.dto.ts b/src/frontend-config/frontend-config.dto.ts index 3fd06aa06..6814acf68 100644 --- a/src/frontend-config/frontend-config.dto.ts +++ b/src/frontend-config/frontend-config.dto.ts @@ -71,10 +71,10 @@ export class AuthProviders { oauth2: boolean; /** - * Is internal auth available? + * Is local auth available? */ @IsBoolean() - internal: boolean; + local: boolean; } export class BrandingDto { diff --git a/src/frontend-config/frontend-config.service.spec.ts b/src/frontend-config/frontend-config.service.spec.ts index f2d43a04a..b019f8460 100644 --- a/src/frontend-config/frontend-config.service.spec.ts +++ b/src/frontend-config/frontend-config.service.spec.ts @@ -25,9 +25,9 @@ describe('FrontendConfigService', () => { const emptyAuthConfig: AuthConfig = { session: { secret: 'my-secret', - lifeTime: 1209600000, + lifetime: 1209600000, }, - email: { + local: { enableLogin: false, enableRegister: false, }, @@ -197,7 +197,7 @@ describe('FrontendConfigService', () => { }; const authConfig: AuthConfig = { ...emptyAuthConfig, - email: { + local: { enableLogin, enableRegister, }, @@ -262,7 +262,7 @@ describe('FrontendConfigService', () => { expect(config.authProviders.google).toEqual( !!authConfig.google.clientID, ); - expect(config.authProviders.internal).toEqual( + expect(config.authProviders.local).toEqual( enableLogin, ); expect(config.authProviders.twitter).toEqual( diff --git a/src/frontend-config/frontend-config.service.ts b/src/frontend-config/frontend-config.service.ts index 1492aad8c..f66342188 100644 --- a/src/frontend-config/frontend-config.service.ts +++ b/src/frontend-config/frontend-config.service.ts @@ -44,7 +44,7 @@ export class FrontendConfigService { return { // ToDo: use actual value here allowAnonymous: false, - allowRegister: this.authConfig.email.enableRegister, + allowRegister: this.authConfig.local.enableRegister, authProviders: this.getAuthProviders(), branding: this.getBranding(), customAuthNames: this.getCustomAuthNames(), @@ -66,7 +66,7 @@ export class FrontendConfigService { github: !!this.authConfig.github.clientID, gitlab: this.authConfig.gitlab.length !== 0, google: !!this.authConfig.google.clientID, - internal: this.authConfig.email.enableLogin, + local: this.authConfig.local.enableLogin, ldap: this.authConfig.ldap.length !== 0, oauth2: this.authConfig.oauth2.length !== 0, saml: this.authConfig.saml.length !== 0, From 021a0c9440934ba93db6650d44bd3a1b08384fb9 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 8 Aug 2021 21:57:46 +0200 Subject: [PATCH 10/18] feat: add getFirstIdentityFromUser helper function Signed-off-by: Philip Molares --- src/identity/utils.ts | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 src/identity/utils.ts diff --git a/src/identity/utils.ts b/src/identity/utils.ts new file mode 100644 index 000000000..038250ea0 --- /dev/null +++ b/src/identity/utils.ts @@ -0,0 +1,27 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { User } from '../users/user.entity'; +import { Identity } from './identity.entity'; +import { ProviderType } from './provider-type.enum'; + +/** + * Get the first identity of a given type from the user + * @param {User} user - the user to get the identity from + * @param {ProviderType} providerType - the type of the identity + * @return {Identity | undefined} the first identity of the user or undefined, if such an identity can not be found + */ +export async function getFirstIdentityFromUser( + user: User, + providerType: ProviderType, +): Promise { + const identities = await user.identities; + if (identities === undefined) { + return undefined; + } + return identities.find( + (aIdentity) => aIdentity.providerType === providerType, + ); +} From 6ad11e47cc447504a42b84c595b252c88126cd8d Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 8 Aug 2021 21:58:54 +0200 Subject: [PATCH 11/18] feat: add identity service This service handles all the authentication of the private api. Signed-off-by: Philip Molares --- src/identity/identity.service.spec.ts | 120 ++++++++++++++++++++++++++ src/identity/identity.service.ts | 94 ++++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 src/identity/identity.service.spec.ts create mode 100644 src/identity/identity.service.ts diff --git a/src/identity/identity.service.spec.ts b/src/identity/identity.service.spec.ts new file mode 100644 index 000000000..687b3d122 --- /dev/null +++ b/src/identity/identity.service.spec.ts @@ -0,0 +1,120 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { ConfigModule } from '@nestjs/config'; +import { Test, TestingModule } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { Repository } from 'typeorm'; + +import appConfigMock from '../config/mock/app.config.mock'; +import authConfigMock from '../config/mock/auth.config.mock'; +import { NotInDBError } from '../errors/errors'; +import { LoggerModule } from '../logger/logger.module'; +import { User } from '../users/user.entity'; +import { checkPassword, hashPassword } from '../utils/password'; +import { Identity } from './identity.entity'; +import { IdentityService } from './identity.service'; +import { ProviderType } from './provider-type.enum'; + +describe('IdentityService', () => { + let service: IdentityService; + let user: User; + let identityRepo: Repository; + const password = 'test123'; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + IdentityService, + { + provide: getRepositoryToken(Identity), + useClass: Repository, + }, + ], + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [appConfigMock, authConfigMock], + }), + LoggerModule, + ], + }).compile(); + + service = module.get(IdentityService); + user = User.create('test', 'Testy') as User; + identityRepo = module.get>( + getRepositoryToken(Identity), + ); + }); + + describe('createLocalIdentity', () => { + it('works', async () => { + jest + .spyOn(identityRepo, 'save') + .mockImplementationOnce( + async (identity: Identity): Promise => identity, + ); + const identity = await service.createLocalIdentity(user, password); + await checkPassword(password, identity.passwordHash ?? '').then( + (result) => expect(result).toBeTruthy(), + ); + expect(identity.user).toEqual(user); + }); + }); + + describe('updateLocalPassword', () => { + beforeEach(async () => { + jest + .spyOn(identityRepo, 'save') + .mockImplementationOnce( + async (identity: Identity): Promise => identity, + ) + .mockImplementationOnce( + async (identity: Identity): Promise => identity, + ); + const identity = await service.createLocalIdentity(user, password); + user.identities = Promise.resolve([identity]); + }); + it('works', async () => { + const newPassword = 'newPassword'; + const identity = await service.updateLocalPassword(user, newPassword); + await checkPassword(newPassword, identity.passwordHash ?? '').then( + (result) => expect(result).toBeTruthy(), + ); + expect(identity.user).toEqual(user); + }); + it('fails, when user has no local identity', async () => { + user.identities = Promise.resolve([]); + await expect(service.updateLocalPassword(user, password)).rejects.toThrow( + NotInDBError, + ); + }); + }); + + describe('loginWithLocalIdentity', () => { + it('works', async () => { + const identity = Identity.create(user, ProviderType.LOCAL); + identity.passwordHash = await hashPassword(password); + user.identities = Promise.resolve([identity]); + await expect( + service.loginWithLocalIdentity(user, password), + ).resolves.toEqual(undefined); + }); + describe('fails', () => { + it('when user has no local identity', async () => { + user.identities = Promise.resolve([]); + await expect( + service.updateLocalPassword(user, password), + ).rejects.toThrow(NotInDBError); + }); + it('when the password is wrong', async () => { + user.identities = Promise.resolve([]); + await expect( + service.updateLocalPassword(user, 'wrong_password'), + ).rejects.toThrow(NotInDBError); + }); + }); + }); +}); diff --git a/src/identity/identity.service.ts b/src/identity/identity.service.ts new file mode 100644 index 000000000..10c763d33 --- /dev/null +++ b/src/identity/identity.service.ts @@ -0,0 +1,94 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Inject, Injectable } from '@nestjs/common'; +import { InjectRepository } from '@nestjs/typeorm'; +import { Repository } from 'typeorm'; + +import authConfiguration, { AuthConfig } from '../config/auth.config'; +import { NotInDBError } from '../errors/errors'; +import { ConsoleLoggerService } from '../logger/console-logger.service'; +import { User } from '../users/user.entity'; +import { checkPassword, hashPassword } from '../utils/password'; +import { Identity } from './identity.entity'; +import { ProviderType } from './provider-type.enum'; +import { getFirstIdentityFromUser } from './utils'; + +@Injectable() +export class IdentityService { + constructor( + private readonly logger: ConsoleLoggerService, + @InjectRepository(Identity) + private identityRepository: Repository, + @Inject(authConfiguration.KEY) + private authConfig: AuthConfig, + ) { + this.logger.setContext(IdentityService.name); + } + + /** + * @async + * Create a new identity for internal auth + * @param {User} user - the user the identity should be added to + * @param {string} password - the password the identity should have + * @return {Identity} the new local identity + */ + async createLocalIdentity(user: User, password: string): Promise { + const identity = Identity.create(user, ProviderType.LOCAL); + identity.passwordHash = await hashPassword(password); + return await this.identityRepository.save(identity); + } + + /** + * @async + * Update the internal password of the specified the user + * @param {User} user - the user, which identity should be updated + * @param {string} newPassword - the new password + * @throws {NotInDBError} the specified user has no internal identity + * @return {Identity} the changed identity + */ + async updateLocalPassword( + user: User, + newPassword: string, + ): Promise { + const internalIdentity: Identity | undefined = + await getFirstIdentityFromUser(user, ProviderType.LOCAL); + if (internalIdentity === undefined) { + this.logger.debug( + `The user with the username ${user.userName} does not have a internal identity.`, + 'updateLocalPassword', + ); + throw new NotInDBError('This user has no internal identity.'); + } + internalIdentity.passwordHash = await hashPassword(newPassword); + return await this.identityRepository.save(internalIdentity); + } + + /** + * @async + * Login the user with their username and password + * @param {User} user - the user to use + * @param {string} password - the password to use + * @throws {NotInDBError} the specified user can't be logged in + */ + async loginWithLocalIdentity(user: User, password: string): Promise { + const internalIdentity: Identity | undefined = + await getFirstIdentityFromUser(user, ProviderType.LOCAL); + if (internalIdentity === undefined) { + this.logger.debug( + `The user with the username ${user.userName} does not have a internal identity.`, + 'loginWithLocalIdentity', + ); + throw new NotInDBError(); + } + if (!(await checkPassword(password, internalIdentity.passwordHash ?? ''))) { + this.logger.debug( + `Password check for ${user.userName} did not succeed.`, + 'loginWithLocalIdentity', + ); + throw new NotInDBError(); + } + } +} From 9fa099449730660a2abd59e8aff80e5874e4b638 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 8 Aug 2021 21:59:12 +0200 Subject: [PATCH 12/18] feat: add local auth strategy Signed-off-by: Philip Molares --- src/identity/local/local.strategy.ts | 44 ++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 src/identity/local/local.strategy.ts diff --git a/src/identity/local/local.strategy.ts b/src/identity/local/local.strategy.ts new file mode 100644 index 000000000..3f927577b --- /dev/null +++ b/src/identity/local/local.strategy.ts @@ -0,0 +1,44 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Injectable, UnauthorizedException } from '@nestjs/common'; +import { AuthGuard, PassportStrategy } from '@nestjs/passport'; +import { Strategy } from 'passport-local'; + +import { NotInDBError } from '../../errors/errors'; +import { UserRelationEnum } from '../../users/user-relation.enum'; +import { User } from '../../users/user.entity'; +import { UsersService } from '../../users/users.service'; +import { IdentityService } from '../identity.service'; + +@Injectable() +export class LocalAuthGuard extends AuthGuard('local') {} + +@Injectable() +export class LocalStrategy extends PassportStrategy(Strategy, 'local') { + constructor( + private userService: UsersService, + private identityService: IdentityService, + ) { + super(); + } + + async validate(username: string, password: string): Promise { + try { + const user = await this.userService.getUserByUsername(username, [ + UserRelationEnum.IDENTITIES, + ]); + await this.identityService.loginWithLocalIdentity(user, password); + return user; + } catch (e) { + if (e instanceof NotInDBError) { + throw new UnauthorizedException( + 'This username and password combination did not work.', + ); + } + throw e; + } + } +} From ce681845789c6de5218cbfab04973c9832b62417 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 8 Aug 2021 21:59:23 +0200 Subject: [PATCH 13/18] feat: add identity module Signed-off-by: Philip Molares --- src/app.module.ts | 2 ++ src/identity/identity.module.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 src/identity/identity.module.ts diff --git a/src/app.module.ts b/src/app.module.ts index 00add89d1..e3de9d89c 100644 --- a/src/app.module.ts +++ b/src/app.module.ts @@ -25,6 +25,7 @@ import { FrontendConfigModule } from './frontend-config/frontend-config.module'; import { FrontendConfigService } from './frontend-config/frontend-config.service'; import { GroupsModule } from './groups/groups.module'; import { HistoryModule } from './history/history.module'; +import { IdentityModule } from './identity/identity.module'; import { LoggerModule } from './logger/logger.module'; import { MediaModule } from './media/media.module'; import { MonitoringModule } from './monitoring/monitoring.module'; @@ -81,6 +82,7 @@ const routes: Routes = [ MediaModule, AuthModule, FrontendConfigModule, + IdentityModule, ], controllers: [], providers: [FrontendConfigService], diff --git a/src/identity/identity.module.ts b/src/identity/identity.module.ts new file mode 100644 index 000000000..2c5f9a9fa --- /dev/null +++ b/src/identity/identity.module.ts @@ -0,0 +1,28 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Module } from '@nestjs/common'; +import { PassportModule } from '@nestjs/passport'; +import { TypeOrmModule } from '@nestjs/typeorm'; + +import { LoggerModule } from '../logger/logger.module'; +import { User } from '../users/user.entity'; +import { UsersModule } from '../users/users.module'; +import { Identity } from './identity.entity'; +import { IdentityService } from './identity.service'; +import { LocalStrategy } from './local/local.strategy'; + +@Module({ + imports: [ + TypeOrmModule.forFeature([Identity, User]), + UsersModule, + PassportModule, + LoggerModule, + ], + controllers: [], + providers: [IdentityService, LocalStrategy], + exports: [IdentityService, LocalStrategy], +}) +export class IdentityModule {} From 28be215aaddd6f5b4d47e6847f55a040e7ce54b6 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Tue, 31 Aug 2021 13:36:13 +0200 Subject: [PATCH 14/18] feat: add session handling Signed-off-by: Philip Molares --- package.json | 2 ++ src/identity/session.guard.ts | 49 +++++++++++++++++++++++++++++++++++ src/main.ts | 7 ++++- src/utils/session.ts | 39 ++++++++++++++++++++++++++++ yarn.lock | 4 +-- 5 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 src/identity/session.guard.ts create mode 100644 src/utils/session.ts diff --git a/package.json b/package.json index 7c76edcda..834f68862 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "connect-typeorm": "1.1.4", "eslint-plugin-jest": "24.4.0", "eslint-plugin-local-rules": "1.1.0", + "express-session": "1.17.2", "file-type": "16.5.3", "joi": "17.4.2", "minio": "7.0.19", @@ -71,6 +72,7 @@ "@tsconfig/node12": "1.0.9", "@types/cli-color": "2.0.1", "@types/express": "4.17.13", + "@types/express-session": "^1.17.4", "@types/jest": "27.0.1", "@types/node": "14.17.16", "@types/passport-local": "^1.0.34", diff --git a/src/identity/session.guard.ts b/src/identity/session.guard.ts new file mode 100644 index 000000000..c263596b2 --- /dev/null +++ b/src/identity/session.guard.ts @@ -0,0 +1,49 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + CanActivate, + ExecutionContext, + Injectable, + UnauthorizedException, +} from '@nestjs/common'; + +import { NotInDBError } from '../errors/errors'; +import { ConsoleLoggerService } from '../logger/console-logger.service'; +import { User } from '../users/user.entity'; +import { UsersService } from '../users/users.service'; + +@Injectable() +export class SessionGuard implements CanActivate { + constructor( + private readonly logger: ConsoleLoggerService, + private userService: UsersService, + ) { + this.logger.setContext(SessionGuard.name); + } + + async canActivate(context: ExecutionContext): Promise { + const request: Request & { session?: { user: string }; user?: User } = + context.switchToHttp().getRequest(); + if (!request.session) { + this.logger.debug('The user has no session.'); + throw new UnauthorizedException("You're not logged in"); + } + try { + request.user = await this.userService.getUserByUsername( + request.session.user, + ); + return true; + } catch (e) { + if (e instanceof NotInDBError) { + this.logger.debug( + `The user '${request.session.user}' does not exist, but has a session.`, + ); + throw new UnauthorizedException("You're not logged in"); + } + throw e; + } + } +} diff --git a/src/main.ts b/src/main.ts index d0d883dbb..289138a13 100644 --- a/src/main.ts +++ b/src/main.ts @@ -10,9 +10,11 @@ import { NestExpressApplication } from '@nestjs/platform-express'; import { AppModule } from './app.module'; import { AppConfig } from './config/app.config'; +import { AuthConfig } from './config/auth.config'; import { MediaConfig } from './config/media.config'; import { ConsoleLoggerService } from './logger/console-logger.service'; import { BackendType } from './media/backends/backend-type.enum'; +import { setupSessionMiddleware } from './utils/session'; import { setupPrivateApiDocs, setupPublicApiDocs } from './utils/swagger'; async function bootstrap(): Promise { @@ -25,9 +27,10 @@ async function bootstrap(): Promise { app.useLogger(logger); const configService = app.get(ConfigService); const appConfig = configService.get('appConfig'); + const authConfig = configService.get('authConfig'); const mediaConfig = configService.get('mediaConfig'); - if (!appConfig || !mediaConfig) { + if (!appConfig || !authConfig || !mediaConfig) { logger.error('Could not initialize config, aborting.', 'AppBootstrap'); process.exit(1); } @@ -45,6 +48,8 @@ async function bootstrap(): Promise { ); } + setupSessionMiddleware(app, authConfig); + app.enableCors({ origin: appConfig.rendererOrigin, }); diff --git a/src/utils/session.ts b/src/utils/session.ts new file mode 100644 index 000000000..d8b10fd2c --- /dev/null +++ b/src/utils/session.ts @@ -0,0 +1,39 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { INestApplication } from '@nestjs/common'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { TypeormStore } from 'connect-typeorm'; +import session from 'express-session'; +import { Repository } from 'typeorm'; + +import { AuthConfig } from '../config/auth.config'; +import { Session } from '../users/session.entity'; + +/** + * Setup the session middleware via the given authConfig. + * @param {INestApplication} app - the nest application to configure the middleware for. + * @param {AuthConfig} authConfig - the authConfig to configure the middleware with. + */ +export function setupSessionMiddleware( + app: INestApplication, + authConfig: AuthConfig, +): void { + app.use( + session({ + name: 'hedgedoc-session', + secret: authConfig.session.secret, + cookie: { + maxAge: authConfig.session.lifetime, + }, + resave: false, + saveUninitialized: false, + store: new TypeormStore({ + cleanupLimit: 2, + ttl: 86400, + }).connect(app.get>(getRepositoryToken(Session))), + }), + ); +} diff --git a/yarn.lock b/yarn.lock index 98d1c4f74..325f58c14 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1147,7 +1147,7 @@ "@types/qs" "*" "@types/range-parser" "*" -"@types/express-session@^1.15.5": +"@types/express-session@^1.15.5", "@types/express-session@^1.17.4": version "1.17.4" resolved "https://registry.yarnpkg.com/@types/express-session/-/express-session-1.17.4.tgz#97a30a35e853a61bdd26e727453b8ed314d6166b" integrity sha512-7cNlSI8+oOBUHTfPXMwDxF/Lchx5aJ3ho7+p9jJZYVg9dVDJFh3qdMXmJtRsysnvS+C6x46k9DRYmrmCkE+MVg== @@ -3175,7 +3175,7 @@ expect@^27.2.0: jest-message-util "^27.2.0" jest-regex-util "^27.0.6" -express-session@^1.15.6: +express-session@1.17.2, express-session@^1.15.6: version "1.17.2" resolved "https://registry.yarnpkg.com/express-session/-/express-session-1.17.2.tgz#397020374f9bf7997f891b85ea338767b30d0efd" integrity sha512-mPcYcLA0lvh7D4Oqr5aNJFMtBMKPLl++OKKxkHzZ0U0oDq1rpKBnkR5f5vCHR26VeArlTOEF9td4x5IjICksRQ== From 53f5713905a9e09901208324148ecade4255e215 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Tue, 31 Aug 2021 13:39:51 +0200 Subject: [PATCH 15/18] fix: update seed.ts Signed-off-by: Philip Molares --- src/seed.ts | 59 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 14 deletions(-) diff --git a/src/seed.ts b/src/seed.ts index cd8b7ce98..3275640e1 100644 --- a/src/seed.ts +++ b/src/seed.ts @@ -10,6 +10,7 @@ import { Author } from './authors/author.entity'; import { Group } from './groups/group.entity'; import { HistoryEntry } from './history/history-entry.entity'; import { Identity } from './identity/identity.entity'; +import { ProviderType } from './identity/provider-type.enum'; import { MediaUpload } from './media/media-upload.entity'; import { Note } from './notes/note.entity'; import { Tag } from './notes/tag.entity'; @@ -19,6 +20,7 @@ import { Edit } from './revisions/edit.entity'; import { Revision } from './revisions/revision.entity'; import { Session } from './users/session.entity'; import { User } from './users/user.entity'; +import { hashPassword } from './utils/password'; /** * This function creates and populates a sqlite db for manual testing @@ -47,6 +49,7 @@ createConnection({ dropSchema: true, }) .then(async (connection) => { + const password = 'test_password'; const users = []; users.push(User.create('hardcoded', 'Test User 1')); users.push(User.create('hardcoded_2', 'Test User 2')); @@ -59,6 +62,9 @@ createConnection({ for (let i = 0; i < 3; i++) { const author = connection.manager.create(Author, Author.create(1)); const user = connection.manager.create(User, users[i]); + const identity = Identity.create(user, ProviderType.LOCAL); + identity.passwordHash = await hashPassword(password); + connection.manager.create(Identity, identity); author.user = user; const revision = Revision.create( 'This is a test note', @@ -70,23 +76,48 @@ createConnection({ notes[i].userPermissions = []; notes[i].groupPermissions = []; user.ownedNotes = [notes[i]]; - await connection.manager.save([notes[i], user, revision, edit, author]); + await connection.manager.save([ + notes[i], + user, + revision, + edit, + author, + identity, + ]); } - const foundUser = await connection.manager.findOne(User); - if (!foundUser) { - throw new Error('Could not find freshly seeded user. Aborting.'); + const foundUsers = await connection.manager.find(User); + if (!foundUsers) { + throw new Error('Could not find freshly seeded users. Aborting.'); } - const foundNote = await connection.manager.findOne(Note); - if (!foundNote) { - throw new Error('Could not find freshly seeded note. Aborting.'); + const foundNotes = await connection.manager.find(Note); + if (!foundNotes) { + throw new Error('Could not find freshly seeded notes. Aborting.'); } - if (!foundNote.alias) { - throw new Error('Could not find alias of freshly seeded note. Aborting.'); + for (const note of foundNotes) { + if (!note.alias) { + throw new Error( + 'Could not find alias of freshly seeded notes. Aborting.', + ); + } + } + for (const user of foundUsers) { + console.log( + `Created User '${user.userName}' with password '${password}'`, + ); + } + for (const note of foundNotes) { + console.log(`Created Note '${note.alias ?? ''}'`); + } + for (const user of foundUsers) { + for (const note of foundNotes) { + const historyEntry = HistoryEntry.create(user, note); + await connection.manager.save(historyEntry); + console.log( + `Created HistoryEntry for user '${user.userName}' and note '${ + note.alias ?? '' + }'`, + ); + } } - const historyEntry = HistoryEntry.create(foundUser, foundNote); - await connection.manager.save(historyEntry); - console.log(`Created User '${foundUser.userName}'`); - console.log(`Created Note '${foundNote.alias}'`); - console.log(`Created HistoryEntry`); }) .catch((error) => console.log(error)); From 1a96900224786fc1b8f429dfd4bdeb25d39742ec Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 4 Sep 2021 18:31:01 +0200 Subject: [PATCH 16/18] feat: add LoginEnabledGuard and RegistrationEnabledGuard These guards check if the login or registration are enabled in the config. If so the guarded method is executed, if not the client will get the HTTP Error 400 Forbidden as an answer Signed-off-by: Philip Molares --- src/api/utils/login-enabled.guard.ts | 33 +++++++++++++++++++++ src/api/utils/registration-enabled.guard.ts | 33 +++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 src/api/utils/login-enabled.guard.ts create mode 100644 src/api/utils/registration-enabled.guard.ts diff --git a/src/api/utils/login-enabled.guard.ts b/src/api/utils/login-enabled.guard.ts new file mode 100644 index 000000000..fe1b98eb2 --- /dev/null +++ b/src/api/utils/login-enabled.guard.ts @@ -0,0 +1,33 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + BadRequestException, + CanActivate, + Inject, + Injectable, +} from '@nestjs/common'; + +import authConfiguration, { AuthConfig } from '../../config/auth.config'; +import { ConsoleLoggerService } from '../../logger/console-logger.service'; + +@Injectable() +export class LoginEnabledGuard implements CanActivate { + constructor( + private readonly logger: ConsoleLoggerService, + @Inject(authConfiguration.KEY) + private authConfig: AuthConfig, + ) { + this.logger.setContext(LoginEnabledGuard.name); + } + + canActivate(): boolean { + if (!this.authConfig.local.enableLogin) { + this.logger.debug('Local auth is disabled.', 'canActivate'); + throw new BadRequestException('Local auth is disabled.'); + } + return true; + } +} diff --git a/src/api/utils/registration-enabled.guard.ts b/src/api/utils/registration-enabled.guard.ts new file mode 100644 index 000000000..6289f857f --- /dev/null +++ b/src/api/utils/registration-enabled.guard.ts @@ -0,0 +1,33 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + BadRequestException, + CanActivate, + Inject, + Injectable, +} from '@nestjs/common'; + +import authConfiguration, { AuthConfig } from '../../config/auth.config'; +import { ConsoleLoggerService } from '../../logger/console-logger.service'; + +@Injectable() +export class RegistrationEnabledGuard implements CanActivate { + constructor( + private readonly logger: ConsoleLoggerService, + @Inject(authConfiguration.KEY) + private authConfig: AuthConfig, + ) { + this.logger.setContext(RegistrationEnabledGuard.name); + } + + canActivate(): boolean { + if (!this.authConfig.local.enableRegister) { + this.logger.debug('User registration is disabled.', 'canActivate'); + throw new BadRequestException('User registration is disabled.'); + } + return true; + } +} From b15361563787b4b218b482879fa37e25b4de9741 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 8 Aug 2021 22:00:14 +0200 Subject: [PATCH 17/18] feat: add auth controller with internal login, registration, password change and logout Signed-off-by: Philip Molares --- src/api/private/auth/auth.controller.spec.ts | 71 +++++++++++++ src/api/private/auth/auth.controller.ts | 105 +++++++++++++++++++ src/api/private/private-api.module.ts | 4 + 3 files changed, 180 insertions(+) create mode 100644 src/api/private/auth/auth.controller.spec.ts create mode 100644 src/api/private/auth/auth.controller.ts diff --git a/src/api/private/auth/auth.controller.spec.ts b/src/api/private/auth/auth.controller.spec.ts new file mode 100644 index 000000000..42fbcbb57 --- /dev/null +++ b/src/api/private/auth/auth.controller.spec.ts @@ -0,0 +1,71 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { ConfigModule } from '@nestjs/config'; +import { Test, TestingModule } from '@nestjs/testing'; +import { getConnectionToken, getRepositoryToken } from '@nestjs/typeorm'; +import { Repository } from 'typeorm'; + +import appConfigMock from '../../../config/mock/app.config.mock'; +import authConfigMock from '../../../config/mock/auth.config.mock'; +import { Identity } from '../../../identity/identity.entity'; +import { IdentityModule } from '../../../identity/identity.module'; +import { LoggerModule } from '../../../logger/logger.module'; +import { Session } from '../../../users/session.entity'; +import { User } from '../../../users/user.entity'; +import { UsersModule } from '../../../users/users.module'; +import { AuthController } from './auth.controller'; + +describe('AuthController', () => { + let controller: AuthController; + + type MockConnection = { + transaction: () => void; + }; + + function mockConnection(): MockConnection { + return { + transaction: jest.fn(), + }; + } + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + { + provide: getRepositoryToken(Identity), + useClass: Repository, + }, + { + provide: getConnectionToken(), + useFactory: mockConnection, + }, + ], + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [appConfigMock, authConfigMock], + }), + LoggerModule, + UsersModule, + IdentityModule, + ], + controllers: [AuthController], + }) + .overrideProvider(getRepositoryToken(Identity)) + .useClass(Repository) + .overrideProvider(getRepositoryToken(Session)) + .useValue({}) + .overrideProvider(getRepositoryToken(User)) + .useValue({}) + .compile(); + + controller = module.get(AuthController); + }); + + it('should be defined', () => { + expect(controller).toBeDefined(); + }); +}); diff --git a/src/api/private/auth/auth.controller.ts b/src/api/private/auth/auth.controller.ts new file mode 100644 index 000000000..334a1d29b --- /dev/null +++ b/src/api/private/auth/auth.controller.ts @@ -0,0 +1,105 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + BadRequestException, + Body, + Controller, + Delete, + NotFoundException, + Post, + Put, + Req, + UseGuards, +} from '@nestjs/common'; +import { Session } from 'express-session'; + +import { AlreadyInDBError, NotInDBError } from '../../../errors/errors'; +import { IdentityService } from '../../../identity/identity.service'; +import { LocalAuthGuard } from '../../../identity/local/local.strategy'; +import { LoginDto } from '../../../identity/local/login.dto'; +import { RegisterDto } from '../../../identity/local/register.dto'; +import { UpdatePasswordDto } from '../../../identity/local/update-password.dto'; +import { SessionGuard } from '../../../identity/session.guard'; +import { ConsoleLoggerService } from '../../../logger/console-logger.service'; +import { User } from '../../../users/user.entity'; +import { UsersService } from '../../../users/users.service'; +import { LoginEnabledGuard } from '../../utils/login-enabled.guard'; +import { RegistrationEnabledGuard } from '../../utils/registration-enabled.guard'; +import { RequestUser } from '../../utils/request-user.decorator'; + +@Controller('auth') +export class AuthController { + constructor( + private readonly logger: ConsoleLoggerService, + private usersService: UsersService, + private identityService: IdentityService, + ) { + this.logger.setContext(AuthController.name); + } + + @UseGuards(RegistrationEnabledGuard) + @Post('local') + async registerUser(@Body() registerDto: RegisterDto): Promise { + try { + const user = await this.usersService.createUser( + registerDto.username, + registerDto.displayname, + ); + // ToDo: Figure out how to rollback user if anything with this calls goes wrong + await this.identityService.createLocalIdentity( + user, + registerDto.password, + ); + return; + } catch (e) { + if (e instanceof AlreadyInDBError) { + throw new BadRequestException(e.message); + } + throw e; + } + } + + @UseGuards(LoginEnabledGuard, SessionGuard) + @Put('local') + async updatePassword( + @RequestUser() user: User, + @Body() changePasswordDto: UpdatePasswordDto, + ): Promise { + try { + await this.identityService.updateLocalPassword( + user, + changePasswordDto.newPassword, + ); + return; + } catch (e) { + if (e instanceof NotInDBError) { + throw new NotFoundException(e.message); + } + throw e; + } + } + + @UseGuards(LoginEnabledGuard, LocalAuthGuard) + @Post('local/login') + login( + @Req() request: Request & { session: { user: string } }, + @Body() loginDto: LoginDto, + ): void { + // There is no further testing needed as we only get to this point if LocalAuthGuard was successful + request.session.user = loginDto.username; + } + + @UseGuards(SessionGuard) + @Delete('logout') + logout(@Req() request: Request & { session: Session }): void { + request.session.destroy((err) => { + if (err) { + this.logger.error('Encountered an error while logging out: ${err}'); + throw new BadRequestException('Unable to log out'); + } + }); + } +} diff --git a/src/api/private/private-api.module.ts b/src/api/private/private-api.module.ts index 98c113e66..625cfbe13 100644 --- a/src/api/private/private-api.module.ts +++ b/src/api/private/private-api.module.ts @@ -8,12 +8,14 @@ import { Module } from '@nestjs/common'; import { AuthModule } from '../../auth/auth.module'; import { FrontendConfigModule } from '../../frontend-config/frontend-config.module'; import { HistoryModule } from '../../history/history.module'; +import { IdentityModule } from '../../identity/identity.module'; import { LoggerModule } from '../../logger/logger.module'; import { MediaModule } from '../../media/media.module'; import { NotesModule } from '../../notes/notes.module'; import { PermissionsModule } from '../../permissions/permissions.module'; import { RevisionsModule } from '../../revisions/revisions.module'; import { UsersModule } from '../../users/users.module'; +import { AuthController } from './auth/auth.controller'; import { ConfigController } from './config/config.controller'; import { HistoryController } from './me/history/history.controller'; import { MeController } from './me/me.controller'; @@ -32,6 +34,7 @@ import { TokensController } from './tokens/tokens.controller'; NotesModule, MediaModule, RevisionsModule, + IdentityModule, ], controllers: [ TokensController, @@ -40,6 +43,7 @@ import { TokensController } from './tokens/tokens.controller'; HistoryController, MeController, NotesController, + AuthController, ], }) export class PrivateApiModule {} From 67baa51b936915df0b1d7186baad6fb70c647b02 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Thu, 2 Sep 2021 23:41:32 +0200 Subject: [PATCH 18/18] feat: add auth e2e tests Signed-off-by: Philip Molares --- test/private-api/auth.e2e-spec.ts | 265 ++++++++++++++++++++++++++++++ 1 file changed, 265 insertions(+) create mode 100644 test/private-api/auth.e2e-spec.ts diff --git a/test/private-api/auth.e2e-spec.ts b/test/private-api/auth.e2e-spec.ts new file mode 100644 index 000000000..3dd887fa9 --- /dev/null +++ b/test/private-api/auth.e2e-spec.ts @@ -0,0 +1,265 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +/* eslint-disable +@typescript-eslint/no-unsafe-assignment, +@typescript-eslint/no-unsafe-member-access +*/ +import { INestApplication } from '@nestjs/common'; +import { ConfigModule, ConfigService } from '@nestjs/config'; +import { Test } from '@nestjs/testing'; +import { TypeOrmModule } from '@nestjs/typeorm'; +import request from 'supertest'; + +import { PrivateApiModule } from '../../src/api/private/private-api.module'; +import { AuthModule } from '../../src/auth/auth.module'; +import { AuthConfig } from '../../src/config/auth.config'; +import appConfigMock from '../../src/config/mock/app.config.mock'; +import authConfigMock from '../../src/config/mock/auth.config.mock'; +import customizationConfigMock from '../../src/config/mock/customization.config.mock'; +import externalServicesConfigMock from '../../src/config/mock/external-services.config.mock'; +import mediaConfigMock from '../../src/config/mock/media.config.mock'; +import { GroupsModule } from '../../src/groups/groups.module'; +import { HistoryModule } from '../../src/history/history.module'; +import { LoginDto } from '../../src/identity/local/login.dto'; +import { RegisterDto } from '../../src/identity/local/register.dto'; +import { UpdatePasswordDto } from '../../src/identity/local/update-password.dto'; +import { LoggerModule } from '../../src/logger/logger.module'; +import { MediaModule } from '../../src/media/media.module'; +import { NotesModule } from '../../src/notes/notes.module'; +import { PermissionsModule } from '../../src/permissions/permissions.module'; +import { UserRelationEnum } from '../../src/users/user-relation.enum'; +import { UsersModule } from '../../src/users/users.module'; +import { UsersService } from '../../src/users/users.service'; +import { checkPassword } from '../../src/utils/password'; +import { setupSessionMiddleware } from '../../src/utils/session'; + +describe('Auth', () => { + let app: INestApplication; + let userService: UsersService; + let username: string; + let displayname: string; + let password: string; + let config: ConfigService; + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [ + appConfigMock, + authConfigMock, + mediaConfigMock, + customizationConfigMock, + externalServicesConfigMock, + ], + }), + PrivateApiModule, + NotesModule, + PermissionsModule, + GroupsModule, + TypeOrmModule.forRoot({ + type: 'sqlite', + database: './hedgedoc-e2e-private-auth.sqlite', + autoLoadEntities: true, + synchronize: true, + dropSchema: true, + }), + LoggerModule, + AuthModule, + UsersModule, + MediaModule, + HistoryModule, + ], + }).compile(); + config = moduleRef.get(ConfigService); + app = moduleRef.createNestApplication(); + const authConfig = config.get('authConfig') as AuthConfig; + setupSessionMiddleware(app, authConfig); + await app.init(); + userService = moduleRef.get(UsersService); + username = 'hardcoded'; + displayname = 'Testy'; + password = 'test_password'; + }); + + describe('POST /auth/local', () => { + it('works', async () => { + const registrationDto: RegisterDto = { + displayname: displayname, + password: password, + username: username, + }; + await request(app.getHttpServer()) + .post('/auth/local') + .set('Content-Type', 'application/json') + .send(JSON.stringify(registrationDto)) + .expect(201); + const newUser = await userService.getUserByUsername(username, [ + UserRelationEnum.IDENTITIES, + ]); + expect(newUser.displayName).toEqual(displayname); + await expect(newUser.identities).resolves.toHaveLength(1); + await expect( + checkPassword( + password, + (await newUser.identities)[0].passwordHash ?? '', + ), + ).resolves.toBeTruthy(); + }); + describe('fails', () => { + it('when the user already exits', async () => { + const username2 = 'already_existing'; + await userService.createUser(username2, displayname); + const registrationDto: RegisterDto = { + displayname: displayname, + password: password, + username: username2, + }; + await request(app.getHttpServer()) + .post('/auth/local') + .set('Content-Type', 'application/json') + .send(JSON.stringify(registrationDto)) + .expect(400); + }); + it('when registration is disabled', async () => { + config.get('authConfig').local.enableRegister = false; + const registrationDto: RegisterDto = { + displayname: displayname, + password: password, + username: username, + }; + await request(app.getHttpServer()) + .post('/auth/local') + .set('Content-Type', 'application/json') + .send(JSON.stringify(registrationDto)) + .expect(400); + config.get('authConfig').local.enableRegister = true; + }); + }); + }); + + describe('PUT /auth/local', () => { + const newPassword = 'new_password'; + let cookie = ''; + beforeEach(async () => { + const loginDto: LoginDto = { + password: password, + username: username, + }; + const response = await request(app.getHttpServer()) + .post('/auth/local/login') + .set('Content-Type', 'application/json') + .send(JSON.stringify(loginDto)) + .expect(201); + cookie = response.get('Set-Cookie')[0]; + }); + it('works', async () => { + // Change password + const changePasswordDto: UpdatePasswordDto = { + newPassword: newPassword, + }; + await request(app.getHttpServer()) + .put('/auth/local') + .set('Content-Type', 'application/json') + .set('Cookie', cookie) + .send(JSON.stringify(changePasswordDto)) + .expect(200); + // Successfully login with new password + const loginDto: LoginDto = { + password: newPassword, + username: username, + }; + const response = await request(app.getHttpServer()) + .post('/auth/local/login') + .set('Content-Type', 'application/json') + .send(JSON.stringify(loginDto)) + .expect(201); + cookie = response.get('Set-Cookie')[0]; + // Reset password + const changePasswordBackDto: UpdatePasswordDto = { + newPassword: password, + }; + await request(app.getHttpServer()) + .put('/auth/local') + .set('Content-Type', 'application/json') + .set('Cookie', cookie) + .send(JSON.stringify(changePasswordBackDto)) + .expect(200); + }); + it('fails, when registration is disabled', async () => { + config.get('authConfig').local.enableLogin = false; + // Try to change password + const changePasswordDto: UpdatePasswordDto = { + newPassword: newPassword, + }; + await request(app.getHttpServer()) + .put('/auth/local') + .set('Content-Type', 'application/json') + .set('Cookie', cookie) + .send(JSON.stringify(changePasswordDto)) + .expect(400); + // enable login again + config.get('authConfig').local.enableLogin = true; + // new password doesn't work for login + const loginNewPasswordDto: LoginDto = { + password: newPassword, + username: username, + }; + await request(app.getHttpServer()) + .post('/auth/local/login') + .set('Content-Type', 'application/json') + .send(JSON.stringify(loginNewPasswordDto)) + .expect(401); + // old password does work for login + const loginOldPasswordDto: LoginDto = { + password: password, + username: username, + }; + await request(app.getHttpServer()) + .post('/auth/local/login') + .set('Content-Type', 'application/json') + .send(JSON.stringify(loginOldPasswordDto)) + .expect(201); + }); + }); + + describe('POST /auth/local/login', () => { + it('works', async () => { + config.get('authConfig').local.enableLogin = true; + const loginDto: LoginDto = { + password: password, + username: username, + }; + await request(app.getHttpServer()) + .post('/auth/local/login') + .set('Content-Type', 'application/json') + .send(JSON.stringify(loginDto)) + .expect(201); + }); + }); + + describe('DELETE /auth/logout', () => { + it('works', async () => { + config.get('authConfig').local.enableLogin = true; + const loginDto: LoginDto = { + password: password, + username: username, + }; + const response = await request(app.getHttpServer()) + .post('/auth/local/login') + .set('Content-Type', 'application/json') + .send(JSON.stringify(loginDto)) + .expect(201); + const cookie = response.get('Set-Cookie')[0]; + await request(app.getHttpServer()) + .delete('/auth/logout') + .set('Cookie', cookie) + .expect(200); + }); + }); +});