Skip to content

Harden user creation from invite#49521

Open
juan-fdz-hawa wants to merge 2 commits into
mainfrom
16783-sso-only-invite-can-be-accepted-as-password-authenticated-account
Open

Harden user creation from invite#49521
juan-fdz-hawa wants to merge 2 commits into
mainfrom
16783-sso-only-invite-can-be-accepted-as-password-authenticated-account

Conversation

@juan-fdz-hawa

@juan-fdz-hawa juan-fdz-hawa commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Derive the authentication mode from the invite record instead of client input during invite acceptance.

Checklist for submitter

If some of the following don't apply, delete the relevant line.

  • Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
    See Changes files for more information.

Testing

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

Summary by CodeRabbit

  • Bug Fixes
    • SSO-only invitations can no longer be accepted via a password-based flow, including when an explicitly empty password is provided.
    • Non-SSO invitations now require a non-empty password to create a user.
    • SSO invitation acceptance now requires the dedicated SSO flow using the invite token, preventing authentication policy bypasses.
  • Tests
    • Added integration coverage for SSO vs non-SSO invite user creation behavior via the users API.

Derive the authentication mode from the invite record instead of
client input during invite acceptance.
@juan-fdz-hawa

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

SSO-only invitations now derive authentication mode exclusively from invite settings. Password-based acceptance is rejected for SSO invites, including empty password fields, while non-SSO invites require a non-empty password. User creation propagates SSO, SSO invite, and MFA settings from the invitation. Integration tests cover rejected password flows and successful SSO acceptance. A changelog entry documents the fix.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: hardening invite-based user creation.
Description check ✅ Passed The description includes the change summary, changes file, and testing checklist items, matching the template at a basic level.
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.
✨ 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 16783-sso-only-invite-can-be-accepted-as-password-authenticated-account

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.

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/service/integration_core_test.go`:
- Around line 3106-3121: Add a test alongside “password invite rejects sso
payload without password” covering a non-SSO invite with SSOInvite set to true
and a weak password. Submit the user creation request and assert it is rejected
for password complexity, then verify no user was created for the invite email.

In `@server/service/users.go`:
- Around line 445-448: Update the non-SSO branch in VerifyInviteCreate to
validate the supplied password against the existing password-complexity
requirements after confirming it is present, reusing the established validation
helper and error behavior. Keep the missing-password check intact and ensure
weak passwords cannot proceed when SSOInvite was falsely set.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b011b0b0-b281-43c7-9893-1131fe4b9f36

📥 Commits

Reviewing files that changed from the base of the PR and between 4d07660 and bdf06e9.

📒 Files selected for processing (3)
  • changes/16783-sso-invite-password-acceptance
  • server/service/integration_core_test.go
  • server/service/users.go

Comment thread server/service/integration_core_test.go
Comment thread server/service/users.go Outdated
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.81%. Comparing base (471bfba) to head (0c7706b).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #49521      +/-   ##
==========================================
- Coverage   68.07%   67.81%   -0.27%     
==========================================
  Files        3882     3887       +5     
  Lines      246400   247510    +1110     
  Branches    13135    13135              
==========================================
+ Hits       167746   167853     +107     
- Misses      63514    64506     +992     
- Partials    15140    15151      +11     
Flag Coverage Δ
backend 69.22% <100.00%> (-0.32%) ⬇️

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

☔ View full report in Codecov by Harness.
📢 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants