Skip to content

fix: add missing authorization check to fingerprint endpoint - #40706

Merged
julio-rocketchat merged 1 commit into
developfrom
fix/fingerprint-endpoint-missing-authorization
May 27, 2026
Merged

fix: add missing authorization check to fingerprint endpoint#40706
julio-rocketchat merged 1 commit into
developfrom
fix/fingerprint-endpoint-missing-authorization

Conversation

@ricardogarim

@ricardogarim ricardogarim commented May 26, 2026

Copy link
Copy Markdown
Member

Proposed changes (including videos or screenshots)

The POST /api/v1/fingerprint endpoint was protected by authRequired: true but had no permissionsRequired check. The endpoint is still actively used as part of the deployment fingerprint change flow.

Fix: add permissionsRequired: ['manage-cloud'] to the endpoint options, matching the authorization pattern used by every other cloud management endpoint in cloud.ts. The manage-cloud permission is assigned to the admin role by default, so the legitimate flow is unaffected.

The 403 response code is also added to the endpoint's response schema, which was missing.

Issue(s)

CORE-2238, VLN-398

Steps to test or reproduce

Before the fix:

  1. Call POST /api/v1/fingerprint as a user without the manage-cloud permission -> returns 200

After the fix:

  1. Call POST /api/v1/fingerprint as a user without the manage-cloud permission -> expect 403 Forbidden
  2. Call POST /api/v1/fingerprint as a user with the manage-cloud permission -> expect 200
  3. Trigger the fingerprint change modal as an admin (change Site_Url setting) -> confirm the modal still works end-to-end

Further comments

The manage-cloud permission was chosen because it is already used by all other cloud management endpoints (cloud.syncWorkspace, cloud.createRegistrationIntent, cloud.getRegistrationStatus, etc.) and is semantically correct for an action that modifies workspace cloud identity.

@dionisio-bot

dionisio-bot Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR enforces the manage-cloud permission on POST /api/v1/fingerprint, adds a documented 403 response, introduces end-to-end tests for unauthenticated/unauthorized/authorized cases, and includes a changeset marking a patch release for @rocket.chat/meteor.

Changes

Security Fix - /fingerprint Endpoint Authorization

Layer / File(s) Summary
Permission requirement and response schema
apps/meteor/app/api/server/v1/misc.ts
Import validateForbiddenErrorResponse and add permissionsRequired: ['manage-cloud'] to the /fingerprint endpoint; extend response schema to include 403: validateForbiddenErrorResponse.
Authorization testing and release documentation
.changeset/salty-suits-strive.md, apps/meteor/tests/end-to-end/api/miscellaneous.ts
Add end-to-end tests validating 401 (unauthenticated), 403 (user missing manage-cloud) for two setDeploymentAs values, and 200 (authorized user); include a changeset describing the fix and patch release.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: adding a missing authorization check to the fingerprint endpoint, which aligns with the primary objective across all three files modified.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • CORE-2238: 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 and usage tips.

@changeset-bot

changeset-bot Bot commented May 26, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 72e7458

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

@ricardogarim ricardogarim added this to the 8.6.0 milestone May 26, 2026
@ricardogarim

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ricardogarim
ricardogarim marked this pull request as ready for review May 26, 2026 23:00
@ricardogarim
ricardogarim requested a review from a team as a code owner May 26, 2026 23:00

@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 3 files

Re-trigger cubic

@ricardogarim
ricardogarim force-pushed the fix/fingerprint-endpoint-missing-authorization branch from 6225b98 to 72e7458 Compare May 26, 2026 23:09
@codecov

codecov Bot commented May 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.62%. Comparing base (b8ada2f) to head (72e7458).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #40706      +/-   ##
===========================================
- Coverage    69.66%   69.62%   -0.04%     
===========================================
  Files         3339     3339              
  Lines       123287   123287              
  Branches     21954    21950       -4     
===========================================
- Hits         85887    85844      -43     
- Misses       34048    34076      +28     
- Partials      3352     3367      +15     
Flag Coverage Δ
e2e 59.17% <ø> (-0.04%) ⬇️
e2e-api 45.93% <ø> (-0.02%) ⬇️
unit 70.45% <ø> (-0.05%) ⬇️

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.

@ricardogarim ricardogarim added the stat: QA assured Means it has been tested and approved by a company insider label May 27, 2026
@dionisio-bot dionisio-bot Bot added the stat: ready to merge PR tested and approved waiting for merge label May 27, 2026
@dionisio-bot
dionisio-bot Bot added this pull request to the merge queue May 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 27, 2026
@ricardogarim ricardogarim removed this from the 8.6.0 milestone May 27, 2026
@dionisio-bot dionisio-bot Bot removed the stat: ready to merge PR tested and approved waiting for merge label May 27, 2026
@ricardogarim ricardogarim added this to the 8.6.0 milestone May 27, 2026
@dionisio-bot dionisio-bot Bot added the stat: ready to merge PR tested and approved waiting for merge label May 27, 2026
@dionisio-bot
dionisio-bot Bot added this pull request to the merge queue May 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 27, 2026
@ricardogarim ricardogarim removed this from the 8.6.0 milestone May 27, 2026
@dionisio-bot dionisio-bot Bot removed the stat: ready to merge PR tested and approved waiting for merge label May 27, 2026
@ricardogarim ricardogarim added this to the 8.6.0 milestone May 27, 2026
@dionisio-bot dionisio-bot Bot added the stat: ready to merge PR tested and approved waiting for merge label May 27, 2026
@dionisio-bot
dionisio-bot Bot added this pull request to the merge queue May 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 27, 2026
@julio-rocketchat

Copy link
Copy Markdown
Member

/backport 8.2.6

@dionisio-bot

dionisio-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Pull request #40907 added to Project: "Patch 8.2.6"

@julio-rocketchat

Copy link
Copy Markdown
Member

/backport 8.1.6

@dionisio-bot

dionisio-bot Bot commented Jun 11, 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.6-40706
git cherry-pick 9dcadc107312c03c42104b94e7409d8c4da52850
// solve the conflict
git push

after that just run /backport 8.1.6 again

@julio-rocketchat

Copy link
Copy Markdown
Member

/backport 8.1.6

@dionisio-bot

dionisio-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Pull request #40908 added to Project: "Patch 8.1.6"

@julio-rocketchat

Copy link
Copy Markdown
Member

/backport 8.0.7

@dionisio-bot

dionisio-bot Bot commented Jun 11, 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.7-40706
git cherry-pick 9dcadc107312c03c42104b94e7409d8c4da52850
// solve the conflict
git push

after that just run /backport 8.0.7 again

@julio-rocketchat

Copy link
Copy Markdown
Member

/backport 8.0.7

@dionisio-bot

dionisio-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Pull request #40910 added to Project: "Patch 8.0.7"

@julio-rocketchat

Copy link
Copy Markdown
Member

/backport 7.13.9

@dionisio-bot

dionisio-bot Bot commented Jun 11, 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.13.9-40706
git cherry-pick 9dcadc107312c03c42104b94e7409d8c4da52850
// solve the conflict
git push

after that just run /backport 7.13.9 again

@julio-rocketchat

Copy link
Copy Markdown
Member

/backport 7.13.9

@dionisio-bot

dionisio-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Pull request #40911 added to Project: "Patch 7.13.9"

@julio-rocketchat

Copy link
Copy Markdown
Member

/backport 7.10.13

@dionisio-bot

dionisio-bot Bot commented Jun 11, 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.13-40706
git cherry-pick 9dcadc107312c03c42104b94e7409d8c4da52850
// solve the conflict
git push

after that just run /backport 7.10.13 again

@julio-rocketchat

Copy link
Copy Markdown
Member

/backport 7.10.13

@dionisio-bot

dionisio-bot Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Pull request #40912 added to Project: "Patch 7.10.13"

@coderabbitai coderabbitai Bot mentioned this pull request Jun 11, 2026
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.

3 participants