From 0e7979585630be256c1a796afe8678018ec65ab1 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 5 Sep 2022 10:22:17 +0200 Subject: [PATCH 1/5] fix: Cannot download export result, closes #4059 --- server/routes/api/fileOperations.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/server/routes/api/fileOperations.ts b/server/routes/api/fileOperations.ts index b04f6a762..e790577c7 100644 --- a/server/routes/api/fileOperations.ts +++ b/server/routes/api/fileOperations.ts @@ -7,6 +7,7 @@ import { FileOperation, Team } from "@server/models"; import { FileOperationType } from "@server/models/FileOperation"; import { authorize } from "@server/policies"; import { presentFileOperation } from "@server/presenters"; +import { ContextWithState } from "@server/types"; import { getSignedUrl } from "@server/utils/s3"; import { assertIn, assertSort, assertUuid } from "@server/validation"; import pagination from "./middlewares/pagination"; @@ -68,8 +69,8 @@ router.post( } ); -router.post("fileOperations.redirect", auth({ admin: true }), async (ctx) => { - const { id } = ctx.body; +const handleFileOperationsRedirect = async (ctx: ContextWithState) => { + const { id } = ctx.body as { id?: string }; assertUuid(id, "id is required"); const { user } = ctx.state; @@ -84,7 +85,18 @@ router.post("fileOperations.redirect", auth({ admin: true }), async (ctx) => { const accessUrl = await getSignedUrl(fileOperation.key); ctx.redirect(accessUrl); -}); +}; + +router.get( + "fileOperations.redirect", + auth({ admin: true }), + handleFileOperationsRedirect +); +router.post( + "fileOperations.redirect", + auth({ admin: true }), + handleFileOperationsRedirect +); router.post("fileOperations.delete", auth({ admin: true }), async (ctx) => { const { id } = ctx.body; From 98e44f528fda36d6be9df692578a87cc20ba45fe Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Wed, 21 Sep 2022 09:05:39 -0400 Subject: [PATCH 2/5] 0.66.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 10f3071cd..e55e7f040 100644 --- a/package.json +++ b/package.json @@ -348,5 +348,5 @@ "js-yaml": "^3.14.1", "jpeg-js": "0.4.4" }, - "version": "0.66.0" + "version": "0.66.1" } From 49d53ccfc284b9a1e18fabad96d33c7045591d6a Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 15 Oct 2022 22:11:09 -0400 Subject: [PATCH 3/5] fix: Disallow adding self to collection (#4299) * api * ui * update collection permissions --- .../AddPeopleToCollection.tsx | 4 +- server/commands/userDemoter.test.ts | 41 +++++++++++++++++++ server/models/User.ts | 16 +++++++- .../__snapshots__/collections.test.ts.snap | 8 ++++ server/routes/api/collections.test.ts | 18 ++++++++ server/routes/api/collections.ts | 6 ++- 6 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 server/commands/userDemoter.test.ts diff --git a/app/scenes/CollectionPermissions/AddPeopleToCollection.tsx b/app/scenes/CollectionPermissions/AddPeopleToCollection.tsx index 5cefd0932..2836b4449 100644 --- a/app/scenes/CollectionPermissions/AddPeopleToCollection.tsx +++ b/app/scenes/CollectionPermissions/AddPeopleToCollection.tsx @@ -109,7 +109,9 @@ class AddPeopleToCollection extends React.Component { {t("No people left to add")} ) } - items={users.notInCollection(collection.id, this.query)} + items={users + .notInCollection(collection.id, this.query) + .filter((member) => member.id !== user.id)} fetch={this.query ? undefined : users.fetchPage} renderItem={(item: User) => ( { + const ip = "127.0.0.1"; + + it("should change role and associated collection permissions", async () => { + const admin = await buildAdmin(); + const user = await buildUser({ teamId: admin.teamId }); + const collection = await buildCollection({ teamId: admin.teamId }); + + const membership = await CollectionUser.create({ + createdById: admin.id, + userId: user.id, + collectionId: collection.id, + permission: CollectionPermission.ReadWrite, + }); + + await userDemoter({ + user, + actorId: admin.id, + to: UserRole.Viewer, + ip, + }); + + expect(user.isViewer).toEqual(true); + + await membership.reload(); + expect(membership.permission).toEqual(CollectionPermission.Read); + }); +}); diff --git a/server/models/User.ts b/server/models/User.ts index 404571b54..464fe616f 100644 --- a/server/models/User.ts +++ b/server/models/User.ts @@ -28,6 +28,7 @@ import env from "@server/env"; import { ValidationError } from "../errors"; import ApiKey from "./ApiKey"; import Collection from "./Collection"; +import CollectionUser from "./CollectionUser"; import NotificationSetting from "./NotificationSetting"; import Star from "./Star"; import Team from "./Team"; @@ -419,7 +420,7 @@ class User extends ParanoidModel { if (res.count >= 1) { if (to === "member") { - return this.update( + await this.update( { isAdmin: false, isViewer: false, @@ -427,13 +428,24 @@ class User extends ParanoidModel { options ); } else if (to === "viewer") { - return this.update( + await this.update( { isAdmin: false, isViewer: true, }, options ); + await CollectionUser.update( + { + permission: CollectionPermission.Read, + }, + { + ...options, + where: { + userId: this.id, + }, + } + ); } return undefined; diff --git a/server/routes/api/__snapshots__/collections.test.ts.snap b/server/routes/api/__snapshots__/collections.test.ts.snap index 9738e34b7..dbf330937 100644 --- a/server/routes/api/__snapshots__/collections.test.ts.snap +++ b/server/routes/api/__snapshots__/collections.test.ts.snap @@ -8,6 +8,14 @@ Object { } `; +exports[`#collections.add_user should not allow add self 1`] = ` +Object { + "error": "authorization_error", + "message": "Authorization error", + "ok": false, +} +`; + exports[`#collections.add_user should require user in team 1`] = ` Object { "error": "authorization_error", diff --git a/server/routes/api/collections.test.ts b/server/routes/api/collections.test.ts index 9c8e7b9a2..2c404a2d4 100644 --- a/server/routes/api/collections.test.ts +++ b/server/routes/api/collections.test.ts @@ -422,6 +422,24 @@ describe("#collections.add_user", () => { expect(users.length).toEqual(2); }); + it("should not allow add self", async () => { + const user = await buildUser(); + const collection = await buildCollection({ + teamId: user.teamId, + permission: null, + }); + const res = await server.post("/api/collections.add_user", { + body: { + token: user.getJwtToken(), + id: collection.id, + userId: user.id, + }, + }); + const body = await res.json(); + expect(res.status).toEqual(403); + expect(body).toMatchSnapshot(); + }); + it("should require user in team", async () => { const user = await buildUser(); const collection = await buildCollection({ diff --git a/server/routes/api/collections.ts b/server/routes/api/collections.ts index 684d51683..1560cd5c1 100644 --- a/server/routes/api/collections.ts +++ b/server/routes/api/collections.ts @@ -9,7 +9,7 @@ import { RateLimiterStrategy } from "@server/RateLimiter"; import collectionExporter from "@server/commands/collectionExporter"; import teamUpdater from "@server/commands/teamUpdater"; import { sequelize } from "@server/database/sequelize"; -import { ValidationError } from "@server/errors"; +import { AuthorizationError, ValidationError } from "@server/errors"; import auth from "@server/middlewares/authentication"; import { rateLimiter } from "@server/middlewares/rateLimiter"; import { @@ -362,6 +362,10 @@ router.post("collections.add_user", auth(), async (ctx) => { }, }); + if (userId === ctx.state.user.id) { + throw AuthorizationError("You cannot add yourself to a collection"); + } + if (permission) { assertCollectionPermission(permission); } From f277d08982d525e6b1ebc16fae314d26bcd1ce6a Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 22 Oct 2022 11:17:31 -0400 Subject: [PATCH 4/5] fix: Actor on users.delete --- server/routes/api/users.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/routes/api/users.ts b/server/routes/api/users.ts index b408821ad..e5e50cb55 100644 --- a/server/routes/api/users.ts +++ b/server/routes/api/users.ts @@ -402,6 +402,7 @@ router.post( rateLimiter(RateLimiterStrategy.TenPerHour), async (ctx) => { const { id, code = "" } = ctx.body; + const actor = ctx.state.user; let user: User; if (id) { @@ -410,13 +411,13 @@ router.post( rejectOnEmpty: true, }); } else { - user = ctx.state.user; + user = actor; } - authorize(user, "delete", user); + authorize(actor, "delete", user); // If we're attempting to delete our own account then a confirmation code // is required. This acts as CSRF protection. - if (!id || id === ctx.state.user.id) { + if (!id || id === actor.id) { const deleteConfirmationCode = user.deleteConfirmationCode; if ( @@ -433,7 +434,7 @@ router.post( await userDestroyer({ user, - actor: user, + actor, ip: ctx.request.ip, }); From c0a86753bd9ab01477b6fb2d36d4e89fbe7efc80 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 22 Oct 2022 11:24:04 -0400 Subject: [PATCH 5/5] 0.66.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e55e7f040..5e6e48a10 100644 --- a/package.json +++ b/package.json @@ -348,5 +348,5 @@ "js-yaml": "^3.14.1", "jpeg-js": "0.4.4" }, - "version": "0.66.1" + "version": "0.66.2" }