diff --git a/docs/content/dev/db-schema.plantuml b/docs/content/dev/db-schema.plantuml index b3765ef3e..80a86a6d5 100644 --- a/docs/content/dev/db-schema.plantuml +++ b/docs/content/dev/db-schema.plantuml @@ -5,6 +5,7 @@ skinparam nodesep 60 entity "note" { *id : uuid <> -- + publicId: text alias : text *viewCount : number *ownerId : uuid <> diff --git a/package.json b/package.json index fe2bc1969..7346e598f 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "@types/minio": "7.0.7", "@types/node-fetch": "2.5.10", "@types/passport-http-bearer": "1.0.36", + "base32-encode": "1.2.0", "bcrypt": "5.0.1", "class-transformer": "0.4.0", "class-validator": "0.13.1", diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index 695c36578..2676328e3 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -17,6 +17,7 @@ import { NotInDBError, TokenNotValidError } from '../errors/errors'; import { Repository } from 'typeorm'; import { ConfigModule } from '@nestjs/config'; import appConfigMock from '../config/mock/app.config.mock'; +import { randomBytes } from 'crypto'; describe('AuthService', () => { let service: AuthService; @@ -79,7 +80,7 @@ describe('AuthService', () => { .then((result) => expect(result).toBeTruthy()); }); it('fails, if secret is too short', async () => { - const secret = service.bufferToBase64Url(service.randomString(54)); + const secret = service.bufferToBase64Url(randomBytes(54)); const hash = await service.hashPassword(secret); await service .checkPassword(secret, hash) @@ -328,10 +329,4 @@ describe('AuthService', () => { ); }); }); - describe('randomString', () => { - it('throws on invalid lenght parameter', () => { - expect(() => service.randomString(0)).toThrow(); - expect(() => service.randomString(-1)).toThrow(); - }); - }); }); diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 1c4d50ae8..cb7a16535 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -16,12 +16,12 @@ import { TokenNotValidError, TooManyTokensError, } from '../errors/errors'; -import { randomBytes } from 'crypto'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { TimestampMillis } from '../utils/timestamp'; import { Cron, Timeout } from '@nestjs/schedule'; +import { randomBytes } from 'crypto'; @Injectable() export class AuthService { @@ -62,13 +62,6 @@ export class AuthService { return await compare(cleartext, password); } - randomString(length: number): Buffer { - if (length <= 0) { - throw new Error('randomString cannot have a length < 1'); - } - return randomBytes(length); - } - 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 @@ -93,8 +86,8 @@ export class AuthService { `User '${user.userName}' has already 200 tokens and can't have anymore`, ); } - const secret = this.bufferToBase64Url(this.randomString(54)); - const keyId = this.bufferToBase64Url(this.randomString(8)); + const secret = this.bufferToBase64Url(randomBytes(54)); + const keyId = this.bufferToBase64Url(randomBytes(8)); const accessToken = await this.hashPassword(secret); let token; // Tokens can only be valid for a maximum of 2 years diff --git a/src/notes/note.entity.ts b/src/notes/note.entity.ts index 825876b4c..30f06bb3b 100644 --- a/src/notes/note.entity.ts +++ b/src/notes/note.entity.ts @@ -26,6 +26,8 @@ import { MediaUpload } from '../media/media-upload.entity'; export class Note { @PrimaryGeneratedColumn('uuid') id: string; + @Column({ type: 'text' }) + publicId: string; @Column({ unique: true, nullable: true, diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index ab852a9e9..2d8350c71 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -684,7 +684,7 @@ describe('NotesService', () => { ]; revisions[0].createdAt = new Date(1549312452000); jest.spyOn(revisionRepo, 'findOne').mockResolvedValue(revisions[0]); - note.id = 'testId'; + note.publicId = 'testId'; note.alias = 'testAlias'; note.title = 'testTitle'; note.description = 'testDescription'; @@ -719,7 +719,7 @@ describe('NotesService', () => { ]; note.viewCount = 1337; const metadataDto = await service.toNoteMetadataDto(note); - expect(metadataDto.id).toEqual(note.id); + expect(metadataDto.id).toEqual(note.publicId); expect(metadataDto.alias).toEqual(note.alias); expect(metadataDto.title).toEqual(note.title); expect(metadataDto.createTime).toEqual(revisions[0].createdAt); @@ -778,7 +778,7 @@ describe('NotesService', () => { .spyOn(revisionRepo, 'findOne') .mockResolvedValue(revisions[0]) .mockResolvedValue(revisions[0]); - note.id = 'testId'; + note.publicId = 'testId'; note.alias = 'testAlias'; note.title = 'testTitle'; note.description = 'testDescription'; @@ -813,7 +813,7 @@ describe('NotesService', () => { ]; note.viewCount = 1337; const noteDto = await service.toNoteDto(note); - expect(noteDto.metadata.id).toEqual(note.id); + expect(noteDto.metadata.id).toEqual(note.publicId); expect(noteDto.metadata.alias).toEqual(note.alias); expect(noteDto.metadata.title).toEqual(note.title); expect(noteDto.metadata.createTime).toEqual(revisions[0].createdAt); diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 481700186..b667d4776 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -32,6 +32,8 @@ import { NoteGroupPermission } from '../permissions/note-group-permission.entity import { GroupsService } from '../groups/groups.service'; import { checkArrayForDuplicates } from '../utils/arrayDuplicatCheck'; import appConfiguration, { AppConfig } from '../config/app.config'; +import base32Encode from 'base32-encode'; +import { randomBytes } from 'crypto'; @Injectable() export class NotesService { @@ -92,6 +94,7 @@ export class NotesService { newNote.revisions = Promise.resolve([ Revision.create(noteContent, noteContent), ]); + newNote.publicId = this.generatePublicId(); if (alias) { newNote.alias = alias; this.checkNoteIdOrAlias(alias); @@ -164,7 +167,7 @@ export class NotesService { const note = await this.noteRepository.findOne({ where: [ { - id: noteIdOrAlias, + publicId: noteIdOrAlias, }, { alias: noteIdOrAlias, @@ -210,6 +213,15 @@ export class NotesService { } } + /** + * Generate publicId for a note. + * This is a randomly generated 128-bit value encoded with base32-encode using the crockford variant and converted to lowercase. + */ + generatePublicId(): string { + const randomId = randomBytes(128); + return base32Encode(randomId, 'Crockford').toLowerCase(); + } + /** * @async * Delete a note @@ -358,8 +370,7 @@ export class NotesService { async toNoteMetadataDto(note: Note): Promise { const updateUser = await this.calculateUpdateUser(note); return { - // TODO: Convert DB UUID to base64 - id: note.id, + id: note.publicId, alias: note.alias ?? null, title: note.title ?? '', createTime: (await this.getFirstRevision(note)).createdAt, diff --git a/test/private-api/me.e2e-spec.ts b/test/private-api/me.e2e-spec.ts index 79166f13b..f4943f2e8 100644 --- a/test/private-api/me.e2e-spec.ts +++ b/test/private-api/me.e2e-spec.ts @@ -111,10 +111,26 @@ describe('Me', () => { expect(responseBefore.body).toHaveLength(0); const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url0 = await mediaService.saveFile(testImage, 'hardcoded', note1.id); - const url1 = await mediaService.saveFile(testImage, 'hardcoded', note1.id); - const url2 = await mediaService.saveFile(testImage, 'hardcoded', note2.id); - const url3 = await mediaService.saveFile(testImage, 'hardcoded', note2.id); + const url0 = await mediaService.saveFile( + testImage, + 'hardcoded', + note1.publicId, + ); + const url1 = await mediaService.saveFile( + testImage, + 'hardcoded', + note1.publicId, + ); + const url2 = await mediaService.saveFile( + testImage, + 'hardcoded', + note2.alias ?? '', + ); + const url3 = await mediaService.saveFile( + testImage, + 'hardcoded', + note2.alias ?? '', + ); const response = await request(httpServer) .get('/me/media/') @@ -147,7 +163,11 @@ describe('Me', () => { it('DELETE /me', async () => { const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url0 = await mediaService.saveFile(testImage, 'hardcoded', note1.id); + const url0 = await mediaService.saveFile( + testImage, + 'hardcoded', + note1.publicId, + ); const dbUser = await userService.getUserByUsername('hardcoded'); expect(dbUser).toBeInstanceOf(User); const mediaUploads = await mediaService.listUploadsByUser(dbUser); diff --git a/test/private-api/notes.e2e-spec.ts b/test/private-api/notes.e2e-spec.ts index aca2f5a30..515c23afb 100644 --- a/test/private-api/notes.e2e-spec.ts +++ b/test/private-api/notes.e2e-spec.ts @@ -261,25 +261,27 @@ describe('Notes', () => { describe('GET /notes/{note}/media', () => { it('works', async () => { - const note = await notesService.createNote(content, 'test6', user); - const extraNote = await notesService.createNote(content, 'test7', user); + const alias = 'test6'; + const extraAlias = 'test7'; + await notesService.createNote(content, alias, user); + await notesService.createNote(content, extraAlias, user); const httpServer = app.getHttpServer(); const response = await request(httpServer) - .get(`/notes/${note.id}/media/`) + .get(`/notes/${alias}/media/`) .expect('Content-Type', /json/) .expect(200); expect(response.body).toHaveLength(0); const testImage = await fs.readFile('test/private-api/fixtures/test.png'); - const url0 = await mediaService.saveFile(testImage, 'hardcoded', note.id); + const url0 = await mediaService.saveFile(testImage, 'hardcoded', alias); const url1 = await mediaService.saveFile( testImage, 'hardcoded', - extraNote.id, + extraAlias, ); const responseAfter = await request(httpServer) - .get(`/notes/${note.id}/media/`) + .get(`/notes/${alias}/media/`) .expect('Content-Type', /json/) .expect(200); expect(responseAfter.body).toHaveLength(1); @@ -299,13 +301,10 @@ describe('Notes', () => { .expect(404); }); it("fails, when user can't read note", async () => { - const note = await notesService.createNote( - 'This is a test note.', - 'test11', - user2, - ); + const alias = 'test11'; + await notesService.createNote('This is a test note.', alias, user2); await request(app.getHttpServer()) - .get(`/notes/${note.id}/media/`) + .get(`/notes/${alias}/media/`) .expect('Content-Type', /json/) .expect(401); }); diff --git a/test/public-api/me.e2e-spec.ts b/test/public-api/me.e2e-spec.ts index 3d58d8b60..f23a68190 100644 --- a/test/public-api/me.e2e-spec.ts +++ b/test/public-api/me.e2e-spec.ts @@ -243,10 +243,26 @@ describe('Me', () => { expect(response1.body).toHaveLength(0); const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url0 = await mediaService.saveFile(testImage, 'hardcoded', note1.id); - const url1 = await mediaService.saveFile(testImage, 'hardcoded', note1.id); - const url2 = await mediaService.saveFile(testImage, 'hardcoded', note2.id); - const url3 = await mediaService.saveFile(testImage, 'hardcoded', note2.id); + const url0 = await mediaService.saveFile( + testImage, + 'hardcoded', + note1.publicId, + ); + const url1 = await mediaService.saveFile( + testImage, + 'hardcoded', + note1.publicId, + ); + const url2 = await mediaService.saveFile( + testImage, + 'hardcoded', + note2.publicId, + ); + const url3 = await mediaService.saveFile( + testImage, + 'hardcoded', + note2.publicId, + ); const response = await request(httpServer) .get('/me/media/') diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index 024c3d0de..6a12bd731 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/test/public-api/notes.e2e-spec.ts @@ -204,7 +204,9 @@ describe('Notes', () => { ]; updateNotePermission.sharedToGroups = []; await notesService.updateNotePermissions(note, updateNotePermission); - const updatedNote = await notesService.getNoteByIdOrAlias(note.alias); + const updatedNote = await notesService.getNoteByIdOrAlias( + note.alias ?? '', + ); expect(updatedNote.userPermissions).toHaveLength(1); expect(updatedNote.userPermissions[0].canEdit).toEqual( updateNotePermission.sharedToUsers[0].canEdit, @@ -391,25 +393,27 @@ describe('Notes', () => { describe('GET /notes/{note}/media', () => { it('works', async () => { - const note = await notesService.createNote(content, 'test9', user); - const extraNote = await notesService.createNote(content, 'test10', user); + const alias = 'test9'; + const extraAlias = 'test10'; + await notesService.createNote(content, alias, user); + await notesService.createNote(content, extraAlias, user); const httpServer = app.getHttpServer(); const response = await request(httpServer) - .get(`/notes/${note.id}/media/`) + .get(`/notes/${alias}/media/`) .expect('Content-Type', /json/) .expect(200); expect(response.body).toHaveLength(0); const testImage = await fs.readFile('test/public-api/fixtures/test.png'); - const url0 = await mediaService.saveFile(testImage, 'hardcoded', note.id); + const url0 = await mediaService.saveFile(testImage, 'hardcoded', alias); const url1 = await mediaService.saveFile( testImage, 'hardcoded', - extraNote.id, + extraAlias, ); const responseAfter = await request(httpServer) - .get(`/notes/${note.id}/media/`) + .get(`/notes/${alias}/media/`) .expect('Content-Type', /json/) .expect(200); expect(responseAfter.body).toHaveLength(1); @@ -429,13 +433,10 @@ describe('Notes', () => { .expect(404); }); it("fails, when user can't read note", async () => { - const note = await notesService.createNote( - 'This is a test note.', - 'test11', - user2, - ); + const alias = 'test11'; + await notesService.createNote('This is a test note.', alias, user2); await request(app.getHttpServer()) - .get(`/notes/${note.id}/media/`) + .get(`/notes/${alias}/media/`) .expect('Content-Type', /json/) .expect(401); }); diff --git a/yarn.lock b/yarn.lock index 98d3afb6b..bc3fac0d0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1895,6 +1895,13 @@ balanced-match@^1.0.0: resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.2.tgz#e83e3a7e3f300b34cb9d87f615fa0cbf357690ee" integrity sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw== +base32-encode@1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/base32-encode/-/base32-encode-1.2.0.tgz#e150573a5e431af0a998e32bdfde7045725ca453" + integrity sha512-cHFU8XeRyx0GgmoWi5qHMCVRiqU6J3MHWxVgun7jggCBUpVzm1Ir7M9dYr2whjSNc3tFeXfQ/oZjQu/4u55h9A== + dependencies: + to-data-view "^1.1.0" + base64-js@^1.3.1: version "1.5.1" resolved "https://registry.yarnpkg.com/base64-js/-/base64-js-1.5.1.tgz#1b1b440160a5bf7ad40b650f095963481903930a" @@ -7061,6 +7068,11 @@ tmpl@1.0.x: resolved "https://registry.yarnpkg.com/tmpl/-/tmpl-1.0.4.tgz#23640dd7b42d00433911140820e5cf440e521dd1" integrity sha1-I2QN17QtAEM5ERQIIOXPRA5SHdE= +to-data-view@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/to-data-view/-/to-data-view-1.1.0.tgz#08d6492b0b8deb9b29bdf1f61c23eadfa8994d00" + integrity sha512-1eAdufMg6mwgmlojAx3QeMnzB/BTVp7Tbndi3U7ftcT2zCZadjxkkmLmd97zmaxWi+sgGcgWrokmpEoy0Dn0vQ== + to-fast-properties@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/to-fast-properties/-/to-fast-properties-2.0.0.tgz#dc5e698cbd079265bc73e0377681a4e4e83f616e"