Merge pull request #1284 from hedgedoc/publicId

docs/content/dev/db-schema.plantuml

@@ -5,6 +5,7 @@ skinparam nodesep 60
 entity "note" {
   *id : uuid <<generated>>
   --
+  publicId: text
   alias : text
   *viewCount : number
   *ownerId : uuid <<FK user>>

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",

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();
-    });
-  });
 });

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

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,

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);

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<NoteMetadataDto> {
     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,

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);

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);
     });

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/')

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);
     });

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"