View recovery password: fix permissions

[mostlikelee]

Related issue: Resolves #41975

Existing permissions to view passwords are too restrictive

Testing

• Added/updated automated tests
• QA'd all new/changed functionality manually

For unreleased bug fixes in a release candidate, one of:

• Confirmed that the fix is not expected to adversely impact load test results

Summary by CodeRabbit

• Bug Fixes

  • Recovery lock password access permissions have been updated to align with standard host read permissions for improved consistency.

• Tests

  • Added comprehensive authorization tests for recovery lock password access, covering different user roles, team-scoped contexts, and access control validation.

[mostlikelee]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[Copilot]

Pull request overview

Adjusts server-side authorization for viewing macOS Recovery Lock passwords so it matches standard host read access, and adds tests to validate the updated permission behavior.

Changes:

• Update GetHostRecoveryLockPassword authorization from fleet.ActionWrite to fleet.ActionRead.
• Add an authorization matrix test covering allowed/disallowed users for global vs team hosts.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
server/service/hosts.go	Loosens/aligns the authz check for viewing recovery lock passwords to use host read permissions.
server/service/hosts_test.go	Adds tests to verify authorization behavior for viewing recovery lock passwords across user roles and host scoping.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Walkthrough

This pull request modifies the authorization check for GetHostRecoveryLockPassword from requiring ActionWrite to ActionRead permissions. The change aligns read access for recovery lock passwords with the existing read permissions for accessing hosts. Additionally, a new test function is added to validate authorization behavior across different host contexts and user roles, ensuring that the updated permission check is enforced correctly.

Possibly related PRs

• Recovery password: GET API #41436: Implements the GetHostRecoveryLockPassword API and service layer, which this PR now adjusts authorization requirements for and adds comprehensive authorization tests.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The PR description is incomplete and lacks required sections such as input validation, database migrations, and compatibility checks as specified in the template.	Complete the PR description by addressing all applicable checklist items from the template, including security validation, database migration checks, and any necessary backwards compatibility notes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: fixing overly restrictive permissions for viewing recovery passwords.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch recovery-pass-auth

📝 Coding Plan

• Generate coding plan for human review comments

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[mostlikelee]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[getvictor]

Nit. I think this is overkill for testing just to make sure we passed the right arguments to authz.Authorize

[mostlikelee]

you think it's better handled in integration tests?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.42%. Comparing base (35551f4) to head (d1bdda1).
⚠️ Report is 82 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #41951      +/-   ##
==========================================
+ Coverage   66.40%   66.42%   +0.01%     
==========================================
  Files        2510     2510              
  Lines      201184   201372     +188     
  Branches     9058     9058              
==========================================
+ Hits       133594   133756     +162     
- Misses      55517    55529      +12     
- Partials    12073    12087      +14     

Flag	Coverage Δ
backend	68.20% <ø> (+0.01%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.