Base Orbit enrollment end user auth on server policy - #49675
Conversation
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughOrbit enrollment end-user authentication gating now derives from server-side policy rather than client-advertised capabilities. The new Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/orbit.go`:
- Around line 288-294: Update the warning log in the unauthenticated enrollment
path to dereference secret.TeamID before passing it as the "team_id" attribute.
Handle a nil TeamID safely while ensuring non-nil values log the actual numeric
team ID rather than the pointer address.
🪄 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: 921eacdf-4c53-489c-be2c-1b0f52b20121
📥 Commits
Reviewing files that changed from the base of the PR and between d5d1b11 and dab070a8c8dcdf269d3dfe10fce898ae40f1ed3d.
⛔ Files ignored due to path filters (1)
docs/Configuration/fleet-server-configuration.mdis excluded by!**/*.md
📒 Files selected for processing (5)
changes/16798-orbit-enroll-end-user-authserver/config/config.goserver/service/integration_enterprise_test.goserver/service/orbit.goserver/service/orbit_test.go
dab070a to
0e42153
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #49675 +/- ##
=======================================
Coverage 68.08% 68.08%
=======================================
Files 3936 3936
Lines 250647 250661 +14
Branches 13275 13275
=======================================
+ Hits 170642 170664 +22
+ Misses 64693 64685 -8
Partials 15312 15312
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
0e42153 to
166102b
Compare
|
Note The previously reviewed commits are no longer reachable (likely due to a force-push or rebase), so CodeRabbit is performing a full review instead of an incremental one. This review may take a little longer. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/service/orbit.go (1)
278-281: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a lower log level for the expected bypass path.
This branch fires on every enrollment where the (default-enabled) bypass allows a non-EUA-advertising client onto a team requiring EUA — likely the common case for many fleets. Logging this at
Warnon every such enrollment may drown out genuine issues (e.g., the "blocking enrollment" warning below). ConsiderInfoContextorDebugContextinstead, consistent with the "allowing re-enrollment" log at line 305 which usesInfoContextfor a similarly benign case.✏️ Proposed log-level tweak
- svc.logger.WarnContext(ctx, "allowing enrollment without end-user authentication: end-user auth bypass is enabled and the client does not support end-user auth", - "host_uuid", hostInfo.HardwareUUID) + svc.logger.InfoContext(ctx, "allowing enrollment without end-user authentication: end-user auth bypass is enabled and the client does not support end-user auth", + "host_uuid", hostInfo.HardwareUUID)🤖 Prompt for 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. In `@server/service/orbit.go` around lines 278 - 281, Lower the log level for the expected enrollment bypass branch in the `AllowOrbitEndUserAuthBypass && !clientSupportsEUA` case from `WarnContext` to `InfoContext` (or the established equivalent benign-path level), while preserving the message and `host_uuid` context.
🤖 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.
Nitpick comments:
In `@server/service/orbit.go`:
- Around line 278-281: Lower the log level for the expected enrollment bypass
branch in the `AllowOrbitEndUserAuthBypass && !clientSupportsEUA` case from
`WarnContext` to `InfoContext` (or the established equivalent benign-path
level), while preserving the message and `host_uuid` context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b540eece-b2ed-4daa-bbfb-c1c7728d2de7
⛔ Files ignored due to path filters (1)
docs/Configuration/fleet-server-configuration.mdis excluded by!**/*.md
📒 Files selected for processing (5)
changes/16798-orbit-enroll-end-user-authserver/config/config.goserver/service/integration_enterprise_test.goserver/service/orbit.goserver/service/orbit_test.go
EnrollOrbit now determines end user authentication requirements from server policy rather than the client-supplied X-Fleet-Capabilities header, which is treated as an informational hint. Adds the mdm.allow_orbit_end_user_auth_bypass setting (enabled by default) controlling whether hosts that do not complete end user authentication may enroll into a team that requires it; set it to false to strictly enforce end user auth. The setting also governs installers built with fleetctl package --bypass-end-user-auth.
…tor related tests
166102b to
1512ce5
Compare
EnrollOrbit now determines end user authentication requirements from server policy rather than the client-supplied X-Fleet-Capabilities header, which is treated as an informational hint.
Adds the mdm.allow_orbit_end_user_auth_bypass setting (enabled by default) controlling whether hosts that do not complete end user authentication may enroll into a team that requires it; set it to false to strictly enforce end user auth. The setting also governs installers built with fleetctl package --bypass-end-user-auth.
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests