Skip to content

Commit d43af72

Browse files
committed
project/collaborators: check the api in a similar way
1 parent caf1d2e commit d43af72

File tree

5 files changed

+429
-94
lines changed

5 files changed

+429
-94
lines changed

src/packages/frontend/projects/actions.ts

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -640,13 +640,14 @@ export class ProjectsActions extends Actions<ProjectsState> {
640640
): Promise<void> {
641641
const removed_name = redux.getStore("users").get_name(account_id);
642642
try {
643-
await this.redux
644-
.getProjectActions(project_id)
645-
.async_log({ event: "remove_collaborator", removed_name });
646643
await webapp_client.project_collaborators.remove({
647644
project_id,
648645
account_id,
649646
});
647+
// Log AFTER successful removal
648+
await this.redux
649+
.getProjectActions(project_id)
650+
.async_log({ event: "remove_collaborator", removed_name });
650651
} catch (err) {
651652
const message = `Error removing ${removed_name} from project ${project_id} -- ${err}`;
652653
alert_message({ type: "error", message });
@@ -693,11 +694,6 @@ export class ProjectsActions extends Actions<ProjectsState> {
693694
replyto?: string,
694695
replyto_name?: string,
695696
): Promise<void> {
696-
await this.redux.getProjectActions(project_id).async_log({
697-
event: "invite_user",
698-
invitee_account_id: account_id,
699-
});
700-
701697
const title = store.get_title(project_id);
702698
const link2proj = `https://${window.location.hostname}/projects/${project_id}/`;
703699
// convert body from markdown to html, which is what the backend expects
@@ -714,6 +710,11 @@ export class ProjectsActions extends Actions<ProjectsState> {
714710
email,
715711
subject,
716712
});
713+
// Log AFTER successful invite
714+
await this.redux.getProjectActions(project_id).async_log({
715+
event: "invite_user",
716+
invitee_account_id: account_id,
717+
});
717718
} catch (err) {
718719
if (!silent) {
719720
const message = `Error inviting collaborator ${account_id} from ${project_id} -- ${err}`;
@@ -732,11 +733,6 @@ export class ProjectsActions extends Actions<ProjectsState> {
732733
replyto: string | undefined,
733734
replyto_name: string | undefined,
734735
): Promise<void> {
735-
await this.redux.getProjectActions(project_id).async_log({
736-
event: "invite_nonuser",
737-
invitee_email: to,
738-
});
739-
740736
const title = store.get_title(project_id);
741737
if (body == null) {
742738
const name = this.redux.getStore("account").get_fullname();
@@ -756,6 +752,11 @@ export class ProjectsActions extends Actions<ProjectsState> {
756752
email,
757753
subject,
758754
});
755+
// Log AFTER successful invite
756+
await this.redux.getProjectActions(project_id).async_log({
757+
event: "invite_nonuser",
758+
invitee_email: to,
759+
});
759760
if (!silent) {
760761
alert_message({
761762
message: `Invited ${to} to collaborate on project.`,

src/packages/server/projects/collab.ts

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* This file is part of CoCalc: Copyright © 2020 Sagemath, Inc.
2+
* This file is part of CoCalc: Copyright © 2020 - 2025 Sagemath, Inc.
33
* License: MS-RSL – see LICENSE.md for details
44
*/
55

@@ -8,6 +8,10 @@ import { is_array, is_valid_uuid_string } from "@cocalc/util/misc";
88
import { callback2 } from "@cocalc/util/async-utils";
99
import isSandbox from "@cocalc/server/projects/is-sandbox";
1010
import idleSandboxUsers from "@cocalc/server/projects/idle-sandbox-users";
11+
import {
12+
ensureCanManageCollaborators,
13+
ensureCanRemoveUser,
14+
} from "./ownership-checks";
1115

1216
const GROUPS = ["owner", "collaborator"] as const;
1317

@@ -40,6 +44,18 @@ export async function add_collaborators_to_projects(
4044
Also, the input is uuid's, which typescript can't check. */
4145
verify_types(account_id, accounts, projects);
4246

47+
// Check strict_collaborator_management setting before database changes
48+
// Only check if not using tokens (tokens have their own permission system)
49+
if (!tokens) {
50+
for (const project_id of new Set(projects)) {
51+
if (!project_id) continue; // skip empty strings
52+
await ensureCanManageCollaborators({
53+
project_id,
54+
account_id,
55+
});
56+
}
57+
}
58+
4359
// We now know that account_id is allowed to add users to all of the projects,
4460
// *OR* at that there are valid tokens to permit adding users.
4561

@@ -71,7 +87,7 @@ export async function remove_collaborators_from_projects(
7187
db: PostgreSQL,
7288
account_id: string,
7389
accounts: string[],
74-
projects: string[], // can be empty strings if tokens specified (since they determine project_id)
90+
projects: string[],
7591
): Promise<void> {
7692
try {
7793
// Ensure user is allowed to modify project(s)
@@ -92,6 +108,31 @@ export async function remove_collaborators_from_projects(
92108
Also, the input is uuid's, which typescript can't check. */
93109
verify_types(account_id, accounts, projects);
94110

111+
// Check strict_collaborator_management setting before database changes
112+
// Skip check if the user is only removing themselves
113+
const is_self_remove_only =
114+
accounts.length === 1 && accounts[0] === account_id;
115+
if (!is_self_remove_only) {
116+
for (const project_id of new Set(projects)) {
117+
if (!project_id) continue; // skip empty strings
118+
await ensureCanManageCollaborators({
119+
project_id,
120+
account_id,
121+
});
122+
}
123+
}
124+
125+
// CRITICAL: Verify that no target users are owners
126+
// Owners must be demoted to collaborator first, then removed
127+
for (const i in projects) {
128+
const project_id: string = projects[i];
129+
const target_account_id: string = accounts[i];
130+
await ensureCanRemoveUser({
131+
project_id,
132+
target_account_id,
133+
});
134+
}
135+
95136
// Remove users from projects
96137
//
97138
for (const i in projects) {

0 commit comments

Comments
 (0)