Skip to content

fix: team conversion permissions checked incorrectly - #41206

Merged
julio-rocketchat merged 5 commits into
developfrom
fix-team-conversion
Jul 8, 2026
Merged

fix: team conversion permissions checked incorrectly#41206
julio-rocketchat merged 5 commits into
developfrom
fix-team-conversion

Conversation

@julio-rocketchat

@julio-rocketchat julio-rocketchat commented Jul 7, 2026

Copy link
Copy Markdown
Member

Proposed changes (including videos or screenshots)

Aligns channels.convertToTeam and teams.create with the permission-checking pattern already used by groups.convertToTeam - permissions are now validated against the resolved room in all cases, rather than only for some input variants. No functional changes for existing valid use cases.

Issue(s)

VLN-488

Steps to test or reproduce

Further comments

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved authorization when converting a channel to a team and when creating a team from an existing room, preventing unauthorized team creation and room takeover attempts.
    • Requests without the required permissions now consistently return an access-denied response.
  • Tests
    • Added end-to-end coverage for blocked channel-to-team conversion when permissions are missing.
    • Added end-to-end coverage ensuring creating a team from an existing room fails when the requester lacks ownership/rights, without linking team details.

@dionisio-bot

dionisio-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@changeset-bot

changeset-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 998aed1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The head commit changed during the review from 405bb7d to 998aed1.

Walkthrough

This PR enforces combined create-team and edit-room checks when converting a channel to a team or creating a team from an existing room. It also adds end-to-end coverage and a patch changeset.

Changes

Room permission enforcement

Layer / File(s) Summary
convertToTeam permission gate
apps/meteor/server/api/v1/channels.ts
Resolves the room first, then checks create-team and edit-room together on that room and returns API.v1.forbidden() when the check fails.
TeamService.create ownership guard
apps/meteor/server/services/team/service.ts
Adds an early room.id guard that throws error-no-owner-channel unless both permissions are present.
End-to-end tests and changeset
apps/meteor/tests/end-to-end/api/channels.ts, apps/meteor/tests/end-to-end/api/teams.ts, .changeset/quiet-teams-guard.md
Adds tests for the unauthorized conversion and room-hijack cases, plus a patch changeset entry.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels

type: bug

Suggested reviewers

KevLehman, juliajforesti

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main fix: correcting team conversion permission checks.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • VLN-488: Request failed with status code 401

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.19%. Comparing base (31249c3) to head (998aed1).
⚠️ Report is 3 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #41206      +/-   ##
===========================================
+ Coverage    69.17%   69.19%   +0.02%     
===========================================
  Files         3651     3651              
  Lines       142782   142784       +2     
  Branches     25612    25595      -17     
===========================================
+ Hits         98764    98800      +36     
+ Misses       39728    39693      -35     
- Partials      4290     4291       +1     
Flag Coverage Δ
e2e 59.37% <ø> (+0.02%) ⬆️
e2e-api 50.32% <100.00%> (+<0.01%) ⬆️
unit 70.28% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@julio-rocketchat
julio-rocketchat marked this pull request as ready for review July 7, 2026 11:45
@julio-rocketchat
julio-rocketchat requested a review from a team as a code owner July 7, 2026 11:45
@julio-rocketchat julio-rocketchat added this to the 8.7.0 milestone Jul 7, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 5 files

Re-trigger cubic

Comment thread apps/meteor/tests/end-to-end/api/channels.ts Outdated
Comment thread apps/meteor/server/services/team/service.ts Outdated
@julio-rocketchat julio-rocketchat added the stat: QA assured Means it has been tested and approved by a company insider label Jul 8, 2026
@dionisio-bot dionisio-bot Bot added the stat: ready to merge PR tested and approved waiting for merge label Jul 8, 2026
@dionisio-bot
dionisio-bot Bot added this pull request to the merge queue Jul 8, 2026
@julio-rocketchat
julio-rocketchat removed this pull request from the merge queue due to a manual request Jul 8, 2026
@julio-rocketchat
julio-rocketchat merged commit 175a19c into develop Jul 8, 2026
85 of 87 checks passed
@julio-rocketchat
julio-rocketchat deleted the fix-team-conversion branch July 8, 2026 06:00
@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 8.6.1

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Sorry, I couldn't do that backport because of conflicts. Could you please solve them?

you can do so by running the following commands:

git fetch
git checkout backport-8.6.1-41206
git cherry-pick 175a19c4151f41910499ef37df54f58022276d12
// solve the conflict
git push

after that just run /backport 8.6.1 again

@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 8.6.1

@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 8.5.2

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Pull request #41276 added to Project: "Patch 8.6.1"

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Pull request #41280 added to Project: "Patch 8.3.7"

@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 8.2.7

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Sorry, I couldn't do that backport because of conflicts. Could you please solve them?

you can do so by running the following commands:

git fetch
git checkout backport-8.2.7-41206
git cherry-pick 175a19c4151f41910499ef37df54f58022276d12
// solve the conflict
git push

after that just run /backport 8.2.7 again

@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 8.2.7

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Pull request #41288 added to Project: "Patch 8.2.7"

@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 8.1.7

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Sorry, I couldn't do that backport because of conflicts. Could you please solve them?

you can do so by running the following commands:

git fetch
git checkout backport-8.1.7-41206
git cherry-pick 175a19c4151f41910499ef37df54f58022276d12
// solve the conflict
git push

after that just run /backport 8.1.7 again

@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 8.1.7

@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 8.0.8

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Pull request #41289 added to Project: "Patch 8.1.7"

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Sorry, I couldn't do that backport because of conflicts. Could you please solve them?

you can do so by running the following commands:

git fetch
git checkout backport-8.0.8-41206
git cherry-pick 175a19c4151f41910499ef37df54f58022276d12
// solve the conflict
git push

after that just run /backport 8.0.8 again

@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 8.0.8

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Pull request #41290 added to Project: "Patch 8.0.8"

@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 7.10.14

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Sorry, I couldn't do that backport because of conflicts. Could you please solve them?

you can do so by running the following commands:

git fetch
git checkout backport-7.10.14-41206
git cherry-pick 175a19c4151f41910499ef37df54f58022276d12
// solve the conflict
git push

after that just run /backport 7.10.14 again

@julio-rocketchat

Copy link
Copy Markdown
Member Author

/backport 7.10.14

@dionisio-bot

dionisio-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Pull request #41291 added to Project: "Patch 7.10.14"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge type: bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants