Disallow bypass_conditional_access on iOS/iPadOS - #51555
Conversation
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent iOS/iPadOS devices from using the bypass_conditional_access device endpoint, aligning the enterprise service behavior with the expectation that iOS/iPadOS devices don’t participate in conditional access policies.
Changes:
- Added an authentication-method check to deny conditional access bypass for device-URL-authenticated requests.
- Introduced a new import for the authz context authn-method constants.
Suppressed comments (2)
ee/server/service/devices.go:115
- This blocks bypass only for AuthnDeviceURL, but iOS/iPadOS device self-service can also authenticate via client certificate (AuthnDeviceCertificate). That means iOS/iPadOS could still call this endpoint successfully when certificate-authenticated, which contradicts the intent to disallow bypass on iOS/iPadOS.
// iOS/iPadOS devices authenticate by UUID in the URL and don't participate in
// conditional access policies, so they can't bypass it.
if svc.authz.IsAuthenticatedWith(ctx, authz_ctx.AuthnDeviceURL) {
return fleet.NewUserMessageError(errors.New("conditional access bypass is not supported on this device"), http.StatusForbidden)
}
ee/server/service/devices.go:115
- This change introduces new iOS/iPadOS-specific behavior (403 for bypass) but there doesn’t appear to be test coverage ensuring iOS/iPadOS cannot bypass conditional access. Consider adding an integration test subcase (e.g., in TestConditionalAccessBypass) that creates an iOS/ipados host and asserts this endpoint returns 403.
// iOS/iPadOS devices authenticate by UUID in the URL and don't participate in
// conditional access policies, so they can't bypass it.
if svc.authz.IsAuthenticatedWith(ctx, authz_ctx.AuthnDeviceURL) {
return fleet.NewUserMessageError(errors.New("conditional access bypass is not supported on this device"), http.StatusForbidden)
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
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 selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughThe service imports authorization-context support. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #51555 +/- ##
==========================================
+ Coverage 68.64% 68.83% +0.19%
==========================================
Files 3993 4008 +15
Lines 257444 259930 +2486
Branches 13647 13647
==========================================
+ Hits 176724 178926 +2202
- Misses 65006 65137 +131
- Partials 15714 15867 +153
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:
|
Related issue: Resolves https://github.com/fleetdm/security/issues/15
Checklist for submitter
If some of the following don't apply, delete the relevant line.
No actual user-facing change as this endpoint was not used by the user-facing iOS/iPadOS self service page
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit