Skip to content

Conversation

@haraldschilly
Copy link
Contributor

@haraldschilly haraldschilly commented Dec 3, 2025

  • ref improve project collaborator management #7718
  • as mentioned in the ticket, optionally tighten security via project setting
    • ⚠️ this requires updating the DB schema
  • above project setting can be enforce this via admin site settings for all projects
  • testing: I noticed more than 100 projects are started when I run my tests locally. Adding noFirstProject:true to many create account calls to avoid that. Also, in one tests was a delay with a cryptic note – copying that to several more placed, also account creation related. This makes more tests pass for me locally.

screenshots

The panels below are from the perspective of a collaborator, with and without owner-only management of collaborators (adding/removing). In all situations, though, someone is able to remove oneself.

Screenshot from 2025-12-04 16-25-11 Screenshot from 2025-12-04 16-24-48

site setting, enforcing this setting for all projects

Screenshot from 2025-12-04 16-25-34

changing role

Screenshot from 2025-12-04 16-25-52

if there is just one owner left, the owner can't demote oneself

Screenshot from 2025-12-04 16-26-11

@haraldschilly haraldschilly force-pushed the project-transfer-ownership-7718 branch from 6be9911 to b4795a6 Compare December 3, 2025 12:21
@haraldschilly haraldschilly changed the title project/collaborators: implement ownership management -- #7718 project/collaborators: implement ownership management Dec 3, 2025
@haraldschilly haraldschilly force-pushed the project-transfer-ownership-7718 branch 18 times, most recently from 1f9dee6 to 8a97abf Compare December 4, 2025 15:13
@haraldschilly haraldschilly force-pushed the project-transfer-ownership-7718 branch from 8a97abf to 2552dd7 Compare December 4, 2025 15:33
@haraldschilly haraldschilly added the PR-TODO-cocalc2 merge/migrate this PR into CoCalc2 in the future label Dec 4, 2025
@haraldschilly haraldschilly force-pushed the project-transfer-ownership-7718 branch from db46067 to 1e10ad7 Compare December 11, 2025 15:33
@haraldschilly
Copy link
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. Breezy!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@haraldschilly haraldschilly marked this pull request as ready for review December 12, 2025 14:48
@haraldschilly haraldschilly added PR-needs testing work done, only testing left to do and removed PR-needs review labels Dec 12, 2025
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

try {
// Ensure user is allowed to modify project(s)
//
await verify_write_access_to_projects(db, account_id, projects);
} catch (err) {

P1 Badge Owner-only removal bypassed via REST endpoint

The new manage_users_owner_only/strict_collaborator_management enforcement is only applied in server/projects/collaborators.ts, but the public REST endpoint /api/v2/projects/collaborators/remove still calls remove_collaborators_from_projects, which only checks generic write access and never consults the owner-only flag. A collaborator can therefore POST to that endpoint and remove other collaborators even when the project or site setting is supposed to limit collaborator management to owners. This leaves the new restriction ineffective for clients using the REST API.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@haraldschilly
Copy link
Contributor Author

@codex review latest change

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@haraldschilly haraldschilly force-pushed the project-transfer-ownership-7718 branch from 450c20f to 549595f Compare December 12, 2025 19:04
@haraldschilly
Copy link
Contributor Author

@codex do another review of the last commit

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

await this.redux
.getProjectActions(project_id)
.async_log({ event: "remove_collaborator", removed_name });
await webapp_client.project_collaborators.remove({
project_id,

P2 Badge Log collaborator removals only after backend succeeds

The project history entry for remove_collaborator is written before the backend call is made. With the new ownership checks (e.g., owner-only management or blocking owner removals), the server can now reject the removal, yet the log entry will already have been persisted, leaving an audit trail that claims a user was removed when nothing changed. Logging after a successful project_collaborators.remove call (as done for change_user_type below) would keep the history accurate.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@haraldschilly haraldschilly force-pushed the project-transfer-ownership-7718 branch from 549595f to e1570bd Compare December 12, 2025 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR-needs testing work done, only testing left to do PR-TODO-cocalc2 merge/migrate this PR into CoCalc2 in the future

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants