From c47520b09e841625323d0626358b2d150e65ad64 Mon Sep 17 00:00:00 2001 From: Dexter Chua Date: Tue, 23 Jun 2020 18:43:24 +0800 Subject: [PATCH] Centralize permission checking This makes it more convenient to modify the permission model, both for future modifications and for custom installations. This changes the `owner` property of NoteSession to `ownerId`, which is a more accurate description anyway. Signed-off-by: Dexter Chua --- src/lib/realtime.ts | 49 ++++++++----------------------------- src/lib/web/note/util.ts | 52 ++++++++++++++++++++++++++++++++++------ 2 files changed, 55 insertions(+), 46 deletions(-) diff --git a/src/lib/realtime.ts b/src/lib/realtime.ts index 9ba9c2626..8a2034282 100644 --- a/src/lib/realtime.ts +++ b/src/lib/realtime.ts @@ -14,6 +14,7 @@ import { NoteAuthorship } from './models/note' import { PhotoProfile } from './utils/PhotoProfile' import { EditorSocketIOServer } from './ot/editor-socketio-server' import { mapToObject } from './utils/functions' +import { getPermission, Permission } from './web/note/util' export type SocketWithNoteId = Socket & { noteId: string } @@ -102,7 +103,7 @@ class NoteSession { id: string alias: string title: string - owner: string + ownerId: string ownerprofile: PhotoProfile | null permission: string lastchangeuser: string | null @@ -377,7 +378,7 @@ function emitRefresh (socket: SocketWithNoteId): void { const out = { title: note.title, docmaxlength: config.documentMaxLength, - owner: note.owner, + owner: note.ownerId, ownerprofile: note.ownerprofile, lastchangeuser: note.lastchangeuser, lastchangeuserprofile: note.lastchangeuserprofile, @@ -442,16 +443,6 @@ function interruptConnection (socket: Socket, noteId: string, socketId): void { connectNextSocket() } -function checkViewPermission (req, note: NoteSession): boolean { - if (note.permission === 'private') { - return !!(req.user?.logged_in && req.user.id === note.owner) - } else if (note.permission === 'limited' || note.permission === 'protected') { - return !!(req.user?.logged_in) - } else { - return true - } -} - function finishConnection (socket: SocketWithNoteId, noteId: string, socketId: string): void { // if no valid info provided will drop the client if (!socket || !notes.get(noteId) || !users.get(socketId)) { @@ -460,7 +451,7 @@ function finishConnection (socket: SocketWithNoteId, noteId: string, socketId: s // check view permission const note = notes.get(noteId) if (!note) return - if (!checkViewPermission(socket.request, note)) { + if (getPermission(socket.request.user, note) === Permission.None) { interruptConnection(socket, noteId, socketId) return failConnection(403, 'connection forbidden', socket) } @@ -513,27 +504,7 @@ function ifMayEdit (socket: SocketWithNoteId, originIsOperation: boolean, callba if (!noteId) return const note = notes.get(noteId) if (!note) return - let mayEdit = true - switch (note.permission) { - case 'freely': - // not blocking anyone - break - case 'editable': - case 'limited': - // only login user can change - if (!socket.request.user || !socket.request.user.logged_in) { - mayEdit = false - } - break - case 'locked': - case 'private': - case 'protected': - // only owner can change - if (!note.owner || note.owner !== socket.request.user.id) { - mayEdit = false - } - break - } + const mayEdit = (getPermission(socket.request.user, note) >= Permission.Write) // if user may edit and this is a text operation if (originIsOperation && mayEdit) { // save for the last change user id @@ -625,7 +596,7 @@ function startConnection (socket: SocketWithNoteId): void { if (!note) { return failConnection(404, 'note not found', socket) } - const owner = note.ownerId + const ownerId = note.ownerId const ownerprofile = note.owner ? PhotoProfile.fromUser(note.owner) : null const lastchangeuser = note.lastchangeuserId @@ -653,7 +624,7 @@ function startConnection (socket: SocketWithNoteId): void { id: noteId, alias: note.alias, title: note.title, - owner: owner, + ownerId: ownerId, ownerprofile: ownerprofile, permission: note.permission, lastchangeuser: lastchangeuser, @@ -863,7 +834,7 @@ function connection (socket: SocketWithNoteId): void { const note = notes.get(noteId) if (!note) return // Only owner can change permission - if (note.owner && note.owner === socket.request.user.id) { + if (getPermission(socket.request.user, note) === Permission.Owner) { if (permission === 'freely' && !config.allowAnonymous && !config.allowAnonymousEdits) return note.permission = permission Note.update({ @@ -884,7 +855,7 @@ function connection (socket: SocketWithNoteId): void { const sock = note.socks[i] if (typeof sock !== 'undefined' && sock) { // check view permission - if (!checkViewPermission(sock.request, note)) { + if (getPermission(sock.request.user, note) === Permission.None) { sock.emit('info', { code: 403 }) @@ -910,7 +881,7 @@ function connection (socket: SocketWithNoteId): void { const note = notes.get(noteId) if (!note) return // Only owner can delete note - if (note.owner && note.owner === socket.request.user.id) { + if (getPermission(socket.request.user, note) === Permission.Owner) { Note.destroy({ where: { id: noteId diff --git a/src/lib/web/note/util.ts b/src/lib/web/note/util.ts index ea52f08af..7e470cd5a 100644 --- a/src/lib/web/note/util.ts +++ b/src/lib/web/note/util.ts @@ -33,13 +33,51 @@ export function newNote (req, res: Response, body: string | null): void { }) } -export function checkViewPermission (req, note: Note): boolean { - if (note.permission === 'private') { - return req.isAuthenticated() && note.ownerId === req.user.id - } else if (note.permission === 'limited' || note.permission === 'protected') { - return req.isAuthenticated() +export enum Permission { + None, + Read, + Write, + Owner +} + +interface NoteObject { + ownerId?: string; + permission: string; +} + +export function getPermission (user, note: NoteObject): Permission { + // There are two possible User objects we get passed. One is from socket.io + // and the other is from passport directly. The former sets the logged_in + // parameter to either true or false, whereas for the latter, the logged_in + // parameter is always undefined, and the existence of user itself means the + // user is logged in. + if (!user || user.logged_in === false) { + // Anonymous + switch (note.permission) { + case 'freely': + return Permission.Write + case 'editable': + case 'locked': + return Permission.Read + default: + return Permission.None + } + } else if (note.ownerId === user.id) { + // Owner + return Permission.Owner } else { - return true + // Registered user + switch (note.permission) { + case 'editable': + case 'limited': + case 'freely': + return Permission.Write + case 'locked': + case 'protected': + return Permission.Read + default: + return Permission.None + } } } @@ -58,7 +96,7 @@ export function findNoteOrCreate (req: Request, res: Response, callback: (note: if (!note) { return newNote(req, res, '') } - if (!checkViewPermission(req, note)) { + if (getPermission(req.user, note) === Permission.None) { return errors.errorForbidden(res) } else { return callback(note)