macos password sync feature branch#47422
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #47422 +/- ##
==========================================
- Coverage 68.11% 67.99% -0.12%
==========================================
Files 3732 3701 -31
Lines 235793 236922 +1129
Branches 12520 12447 -73
==========================================
+ Hits 160602 161092 +490
- Misses 60771 61260 +489
- Partials 14420 14570 +150
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:
|
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #47105 Also fixes a small clock skew bug I found while testing. My mac was 1 second ahead of my server and that caused JWT verification to fail and need to be retried. Added a 1 second allowance on it Going into feature branch so some test failures are expected/fine and will be fixed on the feature branch in followup PRs # Checklist for submitter No changes file but the overall feature will add one If some of the following don't apply, delete the relevant line. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually ## Database migrations - [x] Checked schema for all modified table for columns that will auto-update timestamps during migration. - [x] Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects. - [x] Ensured the correct collation is explicitly set for character columns (`COLLATE utf8mb4_unicode_ci`).
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #46942 Moves the API endpoints to their final locations, refactor's them to use the standard handlers where possible and cleans up a bit more of the POC code. No changes file as the overall feature will have one # 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](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #47127 # Checklist for submitter If some of the following don't apply, delete the relevant line. No changes file - overall feature will have one - [ ] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually ## New Fleet configuration settings - [ ] Setting(s) is/are explicitly excluded from GitOps If you didn't check the box above, follow this checklist for GitOps-enabled settings: - [x] Verified that the setting is exported via `fleetctl generate-gitops` - [x] Verified the setting is documented in a separate PR to [the GitOps documentation](https://github.com/fleetdm/fleet/blob/main/docs/Configuration/yaml-files.md#L485) - [x] Verified that the setting is cleared on the server if it is not supplied in a YAML file (or that it is documented as being optional) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Apple Account Provisioning configuration support in MDM settings with OAuth/OIDC parameters for macOS local account provisioning * Enabled GitOps-based management of provisioning settings (global configuration only) * Implemented secure storage and handling of OAuth client credentials * **Bug Fixes** * Improved conditional loading of OAuth credentials to prevent configuration validation errors on endpoints that don't require secrets * **Tests** * Added test coverage for Apple Account Provisioning configuration parsing and validation <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #47122 Cleanup of key, CA and certificate minting versus the POC where they were were more or less ad-hoc # 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](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **Bug Fixes** * Strengthened Apple Platform SSO security by enforcing ECDSA algorithm verification for device JWTs and validating device binding metadata during key exchanges. * **Refactor** * Improved Platform SSO key management by persisting cryptographic assets and loading them during initialization, replacing on-demand generation for better consistency. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #46939 This PR is stacked on the work from #47801 Moves the SSO extension under the fleet desktop app. Adds the proper entitlements so it signs and can be distributed. Updates Fleet's AASA handling so the CI-generated SSO extension can establish trust by default Tested CI build end-to-end on a mac which produced the findigns about codesign/notarization shockingly not flagging the mismatched profile/entitlements and the app failing at runtime. The entire old `apple-sso-extension` directory is deleted - it was mostly just a wrapper app for the extension anyways No changes file as the base branch will have one # 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](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Enhanced macOS build workflow with improved signing and notarization process for Platform SSO extension to ensure secure integration. * Updated bundle identifiers and team configuration for better SSO extension integration and authentication support. * Streamlined build process to properly embed and sign Platform SSO components with the host application. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Resolved conflicts in README.md (kept PSSO extension section + main's Learn more link), server/fleet/app.go (kept both GoogleWorkspace masking and Apple account provisioning secret masking), and regenerated schema.sql. Bumped CreateApplePSSOTables migration 20260611140639 -> 20260701173047 so it orders after main's latest migration (20260626120000).
090bbac to
591ea90
Compare
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves # # 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](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves # # 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](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added secure password encryption support for Platform SSO logins on macOS Fleet desktop. * PSSO JWKS endpoint now publishes encryption public key alongside signing key. * **Tests** * Added coverage for password encryption and decryption flows. * **Chores** * Implemented automated encryption key generation and management for Platform SSO. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
…assword-sync # Conflicts: # server/datastore/mysql/schema.sql
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
This PR introduces Apple Platform SSO (PSSO) / macOS account provisioning support across Fleet server, datastore, GitOps, and the macOS Fleet Desktop app (including an embedded Platform SSO extension), enabling host-scoped registration tokens and new PSSO protocol endpoints.
Changes:
- Adds PSSO HTTP endpoints (including
.well-known/apple-app-site-association), Redis-backed nonce storage, device/key persistence, and host-secret expansion for per-host registration tokens. - Adds
mdm.apple_account_provisioningAppConfig/GitOps support with the IdP client secret stored inmdm_config_assets(masked in API responses) plus activity logging. - Updates Fleet Desktop macOS build/signing pipeline to bundle and ship a PSSO app extension, plus supporting entitlements/provisioning-profile handling in CI.
Reviewed changes
Copilot reviewed 74 out of 75 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/cloner-check/generated_files/appconfig.txt | Updates generated AppConfig field inventory for new MDM config fields. |
| server/service/testing_utils_test.go | Adjusts test service wiring for new EE service arg and MDM protocol registration signature. |
| server/service/svctest/service.go | Adjusts svctest EE service wiring for new PSSO nonce store arg. |
| server/service/svctest/server.go | Updates MDM protocol services registration call with svc. |
| server/service/handler.go | Registers PSSO endpoints and serves AASA from root mux; updates protocol service signature. |
| server/service/client.go | Ensures GitOps clears apple_account_provisioning when omitted. |
| server/service/apple_psso.go | Adds core (non-EE) PSSO endpoints + asset bootstrap + license stubs. |
| server/service/apple_psso_test.go | Tests core PSSO request decoding and asset bootstrap behavior. |
| server/service/apple_mdm.go | Adds new Fleet var and validates safe placement of PSSO registration token var in profiles. |
| server/service/apple_mdm_test.go | Adds tests for PSSO registration token variable validation + licensing. |
| server/service/appconfig.go | Adds apple account provisioning config validation + secret persistence in config assets + activity. |
| server/service/appconfig_test.go | Adds tests covering secret storage/masking/clearing semantics and activity emission. |
| server/mock/service/service_mock.go | Extends mock fleet.Service with PSSO method hooks. |
| server/mock/datastore_mock.go | Extends mock datastore with PSSO device/key methods. |
| server/mdm/psso/providers.go | Adds public constructor to wire Redis nonce store without importing internal package. |
| server/mdm/psso/internal/redis_nonces_store/redis_nonces_store.go | Implements Redis-backed PSSO nonce store with TTL + consume semantics. |
| server/mdm/psso/internal/redis_nonces_store/redis_nonces_store_test.go | Tests nonce store behavior in standalone and cluster redis test setups. |
| server/mdm/nanomdm/service/nanomdm/service.go | Expands host-scoped secrets at command delivery time for secret profiles. |
| server/mdm/apple/psso/regtoken/regtoken.go | Implements mint/validate logic for Fleet-signed device registration JWTs. |
| server/mdm/apple/psso/regtoken/regtoken_test.go | Tests regtoken mint/validate, expiry, audience/method pinning, and PEM minting parity. |
| server/mdm/apple/profile_processor.go | Rewrites new PSSO fleet var to host-secret placeholder and avoids per-host fanout when possible. |
| server/mdm/apple/profile_processor_test.go | Tests PSSO placeholder behavior and fanout short-circuiting. |
| server/fleet/service.go | Extends fleet.Service interface with PSSO methods. |
| server/fleet/secrets.go | Adds new host secret type for PSSO device registration token. |
| server/fleet/mdm.go | Adds PSSO fleet var regex and new mdm_config_assets names. |
| server/fleet/datastore.go | Extends Datastore interface with PSSO device/key persistence methods. |
| server/fleet/apple_psso.go | Adds fleet domain types for PSSO devices/keys/settings and AppleAccountProvisioning config. |
| server/fleet/apple_profiles.go | Treats host-scoped secrets as “profiles with secrets” for secret-command path. |
| server/fleet/app.go | Adds AppleAccountProvisioning to AppConfig MDM + masks secret in Obfuscate(). |
| server/fleet/activities.go | Adds new activity type for edited account provisioning settings. |
| server/datastore/mysql/secret_variables.go | Adds host secret expansion path to mint PSSO registration token at delivery time. |
| server/datastore/mysql/secret_variables_test.go | Tests token minting failure without signing key and successful mint/validation. |
| server/datastore/mysql/migrations/tables/20260708190008_AddPSSODeviceRegistrationTokenFleetVar.go | Adds DB migration to insert new fleet variable definition. |
| server/datastore/mysql/migrations/tables/20260708190008_AddPSSODeviceRegistrationTokenFleetVar_test.go | Tests fleet variable row insertion migration. |
| server/datastore/mysql/migrations/tables/20260708185958_CreateApplePSSOTables.go | Adds PSSO device/key tables with FK cascade. |
| server/datastore/mysql/migrations/tables/20260708185958_CreateApplePSSOTables_test.go | Tests PSSO tables constraints, timestamps, and cascade behavior. |
| server/datastore/mysql/hosts.go | Documents intentional preservation of PSSO device/key rows across host deletion. |
| server/datastore/mysql/apple_psso.go | Implements MySQL PSSO device/key CRUD/upsert operations. |
| server/datastore/mysql/apple_psso_test.go | Tests MySQL PSSO upsert/get/list/delete semantics and re-registration behavior. |
| server/datastore/mysql/apple_mdm.go | Clears PSSO registration on ADE re-enrollment reset. |
| server/datastore/mysql/apple_mdm_test.go | Extends reset-on-reenrollment test to cover PSSO device/key cleanup. |
| pkg/spec/gitops.go | Adds GitOps controls parsing + global-only enforcement for apple_account_provisioning. |
| pkg/spec/gitops_test.go | Adds tests for apple_account_provisioning parsing and team-file rejection. |
| go.mod | Promotes go-jose v3 to direct dependency. |
| ee/server/service/service.go | Wires nonce store into EE service and adds PSSO service state fields. |
| ee/server/service/mdm_external_test.go | Updates EE service constructor invocation for new arg. |
| ee/server/service/apple_psso_test.go | Adds EE tests for configuration gating, nonce behavior, JWKS, and registration validation. |
| ee/server/service/apple_psso_idp_oidc_ropg.go | Adds generic OIDC ROPG IdP client for password validation and claim extraction. |
| ee/server/service/apple_psso_crypto_test.go | Adds extensive crypto/JWT/JWE tests for PSSO wire format and key derivations. |
| cmd/fleetctl/fleetctl/testdata/macosSetupExpectedAppConfigSet.yml | Updates expected appconfig output to include apple_account_provisioning fields. |
| cmd/fleetctl/fleetctl/testdata/macosSetupExpectedAppConfigEmpty.yml | Updates expected appconfig output to include apple_account_provisioning fields. |
| cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigYaml.yml | Updates expected get-config yaml to include apple_account_provisioning. |
| cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigJson.json | Updates expected get-config json to include apple_account_provisioning. |
| cmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigYaml.yml | Updates expected app-config yaml to include apple_account_provisioning. |
| cmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigTeamMaintainerYaml.yml | Updates expected team-maintainer yaml to include apple_account_provisioning. |
| cmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigTeamMaintainerJson.json | Updates expected team-maintainer json to include apple_account_provisioning. |
| cmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigJson.json | Updates expected app-config json to include apple_account_provisioning. |
| cmd/fleetctl/fleetctl/generate_gitops.go | Emits apple_account_provisioning controls with TODO secret placeholder during generate-gitops. |
| cmd/fleetctl/fleetctl/generate_gitops_test.go | Adds test for generate-gitops output for apple_account_provisioning. |
| cmd/fleet/serve.go | Wires Redis nonce store into EE service and passes svc to Apple MDM protocol registration. |
| apps/fleet-desktop-macos/README.md | Documents embedded Platform SSO extension, endpoints, entitlements, and signing requirements. |
| apps/fleet-desktop-macos/FleetPSSOExtension/Info.plist | Adds appex metadata for Platform SSO extension. |
| apps/fleet-desktop-macos/FleetPSSOExtension/FleetPSSOExtension.entitlements | Adds extension entitlements (sandbox, network, managed associated domains). |
| apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Shared.swift | Adds shared helpers for registration payload, key IDs, UUID lookup, and login configuration. |
| apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+PSSO.swift | Implements Platform SSO v2 Password-mode registration handler. |
| apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Networking.swift | Adds URLSession networking + minimal JWKS parsing for encryption public key. |
| apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController.swift | Adds principal class to satisfy extension load requirements. |
| apps/fleet-desktop-macos/FleetDesktop/FleetDesktop.entitlements | Adds host app entitlements for managed associated domains. |
| apps/fleet-desktop-macos/fleet-sso-extension-example.mobileconfig | Adds example profile showing BaseURL and RegistrationToken variable usage. |
| apps/fleet-desktop-macos/build.sh | Builds and embeds the Platform SSO extension into the app bundle. |
| .github/workflows/fleet-desktop-macos-build.yml | Adds provisioning-profile embedding + inside-out signing for app + appex. |
| docs/Contributing/research/mdm/psso.md | Content excluded from automated review (policy-restricted). |
Files excluded by content exclusion policy (1)
- docs/Contributing/research/mdm/psso.md
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func keyID(_ key: SecKey) -> String { | ||
| let digest = SHA256.hash(data: derRepresentation(of: key)) | ||
| return Data(digest).base64URLEncodedString() | ||
| } | ||
|
|
||
| func derRepresentation(of key: SecKey) -> Data { | ||
| guard let pub = SecKeyCopyPublicKey(key), | ||
| let data = SecKeyCopyExternalRepresentation(pub, nil) as Data? else { | ||
| return Data() | ||
| } | ||
| return data | ||
| } | ||
|
|
||
| func pemRepresentation(of key: SecKey) -> String { | ||
| let der = derRepresentation(of: key) | ||
| let b64 = der.base64EncodedString(options: [.lineLength64Characters, | ||
| .endLineWithLineFeed]) | ||
| return "-----BEGIN PUBLIC KEY-----\n\(b64)\n-----END PUBLIC KEY-----" | ||
| } |
There was a problem hiding this comment.
Sounds valid enough, but I'm not quite sure of the Swift language to fully validate
There was a problem hiding this comment.
I think this is actually valid, I will fix
| let payload = self.registrationPayload( | ||
| signing: signKey, | ||
| encryption: encKey, | ||
| registrationToken: registrationToken) | ||
| let ok = await self.postDeviceRegistration(payload: payload) | ||
| completion(ok ? .success : .failed) |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds Apple Platform SSO support end to end: an embedded macOS PSSO extension and build/signing updates, server-side PSSO endpoints and crypto handling, Redis nonce storage, MySQL persistence and migrations, registration-token minting and validation, Apple profile validation and secret expansion, and AppConfig/GitOps support for Apple account provisioning. It also adds matching tests and fixtures across the affected areas. Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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: 6
🧹 Nitpick comments (3)
apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Networking.swift (1)
25-25: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse an explicit short-lived URLSession timeout for Setup Assistant calls.
URLSession.sharedcan wait much longer than this registration flow should block. A dedicated ephemeral session with a boundedtimeoutIntervalForRequestkeeps registration/JWKS failures predictable.Also applies to: 49-49
🤖 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 `@apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController`+Networking.swift at line 25, The JWKS fetch in AuthenticationViewController+Networking uses URLSession.shared, which can block longer than the registration flow should allow. Update the networking calls in this file to use a dedicated ephemeral URLSession with a short timeoutIntervalForRequest instead of the shared session, and apply the same timeout-bound session to the other matching network call referenced in this controller so Setup Assistant failures surface quickly and predictably.server/mdm/apple/psso/regtoken/regtoken.go (1)
29-34: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftConsider a shorter-lived, revocable registration token.
A 5-year token with no
jti/revocation tracking means a leaked registration token can't be individually revoked — the only remediation is rotating Fleet's PSSO signing key, which invalidates every host's token. DefaultValidity is the token lifetime. A long lifetime lets a device reuse the same token across re-registrations without resending the profile. SinceValidatealways checks against the currently-active signing key (per thePSSORegisterDeviceflow), a per-token revocation list (keyed by ajticlaim) or a considerably shorter TTL with re-issuance-on-profile-refresh would reduce blast radius if a profile leaks.Also applies to: 79-117
🤖 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/mdm/apple/psso/regtoken/regtoken.go` around lines 29 - 34, Shorten the registration token lifetime and add per-token revocation support in the PSSO registration token flow. Update the token issuance/validation logic in regtoken.go (especially DefaultValidity, Generate, and Validate) to include a unique jti claim and check it against a revocation list, or otherwise reduce the default TTL and reissue tokens during profile refresh so a leaked token can be invalidated without rotating the global signing key.server/service/handler.go (1)
1094-1105: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider rate-limiting the unauthenticated PSSO endpoints.
pssoNoncePath/pssoRegistrationPath/pssoTokenPathare registered on the no-auth endpointer with no rate limiter, unlike other unauthenticated endpoints in this function (login, sso, sso/callback, mdm/sso, forgot_password all get a dedicatedthrottledbucket). The nonce endpoint in particular mints and stores a value on every call with no authentication, which could be flooded to exhaust the nonce store.💡 Example: reuse the existing rate-limit pattern
+ pssoLimiter := limiter.Limit("psso", throttled.RateQuota{MaxRate: throttled.PerMin(60), MaxBurst: 20}) - ne.POST(pssoNoncePath, pssoNonceEndpoint, pssoNonceRequest{}) - ne.POST(pssoRegistrationPath, pssoRegistrationEndpoint, pssoRegistrationRequest{}) - ne.POST(pssoTokenPath, pssoTokenEndpoint, pssoTokenRequest{}) + ne.WithCustomMiddleware(pssoLimiter).POST(pssoNoncePath, pssoNonceEndpoint, pssoNonceRequest{}) + ne.WithCustomMiddleware(pssoLimiter).POST(pssoRegistrationPath, pssoRegistrationEndpoint, pssoRegistrationRequest{}) + ne.WithCustomMiddleware(pssoLimiter).POST(pssoTokenPath, pssoTokenEndpoint, pssoTokenRequest{}) ne.GET(pssoJWKSPath, pssoJWKSEndpoint, pssoJWKSRequest{})Note:
limiteris defined later in this function (Line 1176); the limiter setup would need to move earlier or a separate limiter instantiated near Line 1102.🤖 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/handler.go` around lines 1094 - 1105, The unauthenticated PSSO routes registered in registerPSSO via ne.POST/ne.GET are missing the same throttling used by other public endpoints in handler.go. Add a dedicated rate limit bucket for pssoNonceEndpoint, pssoRegistrationEndpoint, and pssoTokenEndpoint (especially pssoNonceEndpoint, which writes a nonce each call), and wire it into the no-auth endpointer registration instead of leaving these paths unthrottled. If needed, move or recreate the limiter near the registerPSSO setup so these endpoints can share the existing throttled pattern.
🤖 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
`@apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController`+Networking.swift:
- Around line 24-36: The `loginRequestEncryptionKey(jwksURL:)` flow currently
returns `nil` on fetch/decode/key-selection failures, which lets registration
continue without a `loginRequestEncryptionPublicKey`. Update the caller path in
`AuthenticationViewController+Networking` to treat a missing Fleet encryption
JWK as a registration failure instead of silently falling back, and ensure the
`loginRequestEncryptionKey(jwksURL:)` result is validated so absence of the
`enc` key or any load/parse error stops saving the login configuration.
In
`@apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController`+Shared.swift:
- Around line 76-79: The BaseURL parsing in AuthenticationViewController+Shared
currently accepts any valid URL scheme, so update the validation to reject
non-HTTPS values before continuing. In the shared configuration path where
baseString is converted to URL and host is extracted, require the parsed URL
scheme to be HTTPS (alongside a non-nil host) and throw the existing FleetPSSO
error if it is anything else. Keep the change localized to the BaseURL guard so
all downstream registration, nonce, token, and JWKS endpoint construction only
proceeds from secure HTTPS configuration.
- Around line 37-50: The PEM conversion in derRepresentation(of:) /
pemRepresentation(of:) is wrapping raw SecKeyCopyExternalRepresentation output,
which is not valid for a PUBLIC KEY PEM. Update
AuthenticationViewController+Shared.swift so the key bytes are first encoded as
SPKI/PKIX SubjectPublicKeyInfo DER before base64-wrapping, especially for the
P-256 path, and then have pemRepresentation(of:) build the PEM from that SPKI
DER rather than the raw X9.63 bytes.
In `@ee/server/service/apple_psso.go`:
- Around line 398-432: The PSSO registration flow currently canonicalizes and
stores caller-supplied key IDs without verifying they belong to the submitted
public keys, which can let a valid registration overwrite another key row. In
`registerPSSODevice`, compute the expected device KID from each parsed public
key (using the existing `parseECPublicKeyPEM` results, e.g. a helper like
`computePSSODeviceKID`) and compare it to `req.SigningKeyID` and
`req.EncryptionKeyID` before building the `fleet.PSSOKey` list. Reject the
request with a bad request error if either KID does not match its corresponding
key, then proceed to `SetOrUpdatePSSODevice` only after both checks pass.
In `@server/service/appconfig_test.go`:
- Around line 2014-2016: The assertion in the test for AppleAccountProvisioning
secret masking only checks the masked case, so it can miss regressions where an
unmasked response still contains a secret. Update the test around
modified.MDM.AppleAccountProvisioning.OAuthIdPClientSecret.Value to also assert
the value is empty whenever tc.want.wantMasked is false, using the same test
block and response object so both masked and unmasked outcomes are validated.
In `@server/service/appconfig.go`:
- Around line 1142-1163: The provisioning flow in appconfig currently updates
`mdm_config_assets` via `persistAppleAccountProvisioningSecret` and
`bootstrapPSSOAssets` before `SaveAppConfig`, so a failed save can leave assets
out of sync with the stored AppConfig. Refactor the
`svc.persistAppleAccountProvisioningSecret`, `bootstrapPSSOAssets`, and
`svc.ds.SaveAppConfig` sequence into one atomic datastore transaction or a
single helper on the datastore/service layer so the secret/asset mutations and
AppConfig persistence succeed or fail together.
---
Nitpick comments:
In
`@apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController`+Networking.swift:
- Line 25: The JWKS fetch in AuthenticationViewController+Networking uses
URLSession.shared, which can block longer than the registration flow should
allow. Update the networking calls in this file to use a dedicated ephemeral
URLSession with a short timeoutIntervalForRequest instead of the shared session,
and apply the same timeout-bound session to the other matching network call
referenced in this controller so Setup Assistant failures surface quickly and
predictably.
In `@server/mdm/apple/psso/regtoken/regtoken.go`:
- Around line 29-34: Shorten the registration token lifetime and add per-token
revocation support in the PSSO registration token flow. Update the token
issuance/validation logic in regtoken.go (especially DefaultValidity, Generate,
and Validate) to include a unique jti claim and check it against a revocation
list, or otherwise reduce the default TTL and reissue tokens during profile
refresh so a leaked token can be invalidated without rotating the global signing
key.
In `@server/service/handler.go`:
- Around line 1094-1105: The unauthenticated PSSO routes registered in
registerPSSO via ne.POST/ne.GET are missing the same throttling used by other
public endpoints in handler.go. Add a dedicated rate limit bucket for
pssoNonceEndpoint, pssoRegistrationEndpoint, and pssoTokenEndpoint (especially
pssoNonceEndpoint, which writes a nonce each call), and wire it into the no-auth
endpointer registration instead of leaving these paths unthrottled. If needed,
move or recreate the limiter near the registerPSSO setup so these endpoints can
share the existing throttled pattern.
🪄 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: a60b58d9-b9ad-4928-8160-112523525db0
⛔ Files ignored due to path filters (2)
apps/fleet-desktop-macos/README.mdis excluded by!**/*.mddocs/Contributing/research/mdm/psso.mdis excluded by!**/*.md
📒 Files selected for processing (73)
.github/workflows/fleet-desktop-macos-build.ymlapps/fleet-desktop-macos/FleetDesktop/FleetDesktop.entitlementsapps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Networking.swiftapps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+PSSO.swiftapps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Shared.swiftapps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController.swiftapps/fleet-desktop-macos/FleetPSSOExtension/FleetPSSOExtension.entitlementsapps/fleet-desktop-macos/FleetPSSOExtension/Info.plistapps/fleet-desktop-macos/build.shapps/fleet-desktop-macos/fleet-sso-extension-example.mobileconfigcmd/fleet/serve.gocmd/fleetctl/fleetctl/generate_gitops.gocmd/fleetctl/fleetctl/generate_gitops_test.gocmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigJson.jsoncmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigTeamMaintainerJson.jsoncmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigTeamMaintainerYaml.ymlcmd/fleetctl/fleetctl/testdata/expectedGetConfigAppConfigYaml.ymlcmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigJson.jsoncmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigYaml.ymlcmd/fleetctl/fleetctl/testdata/macosSetupExpectedAppConfigEmpty.ymlcmd/fleetctl/fleetctl/testdata/macosSetupExpectedAppConfigSet.ymlee/server/service/apple_psso.goee/server/service/apple_psso_crypto.goee/server/service/apple_psso_crypto_test.goee/server/service/apple_psso_idp_oidc_ropg.goee/server/service/apple_psso_test.goee/server/service/mdm_external_test.goee/server/service/service.gogo.modpkg/spec/gitops.gopkg/spec/gitops_test.goserver/datastore/mysql/apple_mdm.goserver/datastore/mysql/apple_mdm_test.goserver/datastore/mysql/apple_psso.goserver/datastore/mysql/apple_psso_test.goserver/datastore/mysql/hosts.goserver/datastore/mysql/migrations/tables/20260708185958_CreateApplePSSOTables.goserver/datastore/mysql/migrations/tables/20260708185958_CreateApplePSSOTables_test.goserver/datastore/mysql/migrations/tables/20260708190008_AddPSSODeviceRegistrationTokenFleetVar.goserver/datastore/mysql/migrations/tables/20260708190008_AddPSSODeviceRegistrationTokenFleetVar_test.goserver/datastore/mysql/schema.sqlserver/datastore/mysql/secret_variables.goserver/datastore/mysql/secret_variables_test.goserver/fleet/activities.goserver/fleet/app.goserver/fleet/apple_profiles.goserver/fleet/apple_psso.goserver/fleet/datastore.goserver/fleet/mdm.goserver/fleet/secrets.goserver/fleet/service.goserver/mdm/apple/profile_processor.goserver/mdm/apple/profile_processor_test.goserver/mdm/apple/psso/regtoken/regtoken.goserver/mdm/apple/psso/regtoken/regtoken_test.goserver/mdm/nanomdm/service/nanomdm/service.goserver/mdm/psso/internal/redis_nonces_store/redis_nonces_store.goserver/mdm/psso/internal/redis_nonces_store/redis_nonces_store_test.goserver/mdm/psso/providers.goserver/mock/datastore_mock.goserver/mock/service/service_mock.goserver/service/appconfig.goserver/service/appconfig_test.goserver/service/apple_mdm.goserver/service/apple_mdm_test.goserver/service/apple_psso.goserver/service/apple_psso_test.goserver/service/client.goserver/service/handler.goserver/service/svctest/server.goserver/service/svctest/service.goserver/service/testing_utils_test.gotools/cloner-check/generated_files/appconfig.txt
…for TokenToUserMapping(e.g. setting custom full name/account name) (#48161) Resolves: #47173 Basically anything mapped into the IdP profile prefixed with account(e.g. accountFullName, accountUserName) can now be referenced by the profile's TokenToUserMapping and mapped to user attributes like FullName or Username(short name) # 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](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved Apple PSSO login handling for shared devices and user registration, including better validation before completing sign-in. * Custom identity-provider account claims can now be carried through sign-in more reliably, while keeping system-controlled login fields protected. * The login flow now consistently targets the correct PSSO endpoint for key handling. * **Bug Fixes** * Fixed cases where user registration could proceed without a valid username or fail to save login configuration cleanly. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ee/server/service/apple_psso_token_mapping_test.go (1)
67-106: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExpand reserved-claim smuggling coverage in
TestBuildPSSOIDTokenClaims.The test only smuggles
issandexpthroughExtra(lines 81–82). SincebuildPSSOIDTokenClaimsalso setssub,aud,nonce, andiatas Fleet-controlled values, adding these to theExtramap would verify that all reserved claims are overridden — not just two of them.♻️ Proposed addition to the Extra map
// An IdP that tries to smuggle reserved claims must not win. "iss": "https://evil.example.com", "exp": int64(1), + "sub": "evil-sub", + "aud": "evil-client", + "nonce": "evil-nonce", + "iat": int64(0),🤖 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 `@ee/server/service/apple_psso_token_mapping_test.go` around lines 67 - 106, `TestBuildPSSOIDTokenClaims` only verifies reserved-claim overriding for `iss` and `exp`; expand the `idpClaims.Extra` setup in `buildPSSOIDTokenClaims` coverage to also include smuggled `sub`, `aud`, `nonce`, and `iat` values. Then assert those keys still resolve to the Fleet-controlled values returned by `buildPSSOIDTokenClaims`, ensuring every reserved claim is overridden, not just `iss` and `exp`.
🤖 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 `@ee/server/service/apple_psso_token_mapping_test.go`:
- Around line 67-106: `TestBuildPSSOIDTokenClaims` only verifies reserved-claim
overriding for `iss` and `exp`; expand the `idpClaims.Extra` setup in
`buildPSSOIDTokenClaims` coverage to also include smuggled `sub`, `aud`,
`nonce`, and `iat` values. Then assert those keys still resolve to the
Fleet-controlled values returned by `buildPSSOIDTokenClaims`, ensuring every
reserved claim is overridden, not just `iss` and `exp`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 67500167-86f0-4456-8d0b-d9f4d8a2a642
⛔ Files ignored due to path filters (1)
docs/Contributing/research/mdm/psso.mdis excluded by!**/*.md
📒 Files selected for processing (5)
apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+PSSO.swiftapps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Shared.swiftee/server/service/apple_psso.goee/server/service/apple_psso_idp_oidc_ropg.goee/server/service/apple_psso_token_mapping_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- ee/server/service/apple_psso_idp_oidc_ropg.go
- apps/fleet-desktop-macos/FleetPSSOExtension/AuthenticationViewController+Shared.swift
- ee/server/service/apple_psso.go
MagnusHJensen
left a comment
There was a problem hiding this comment.
Overall code looks good to me, flow as well. Tested on my local mac and it worked just fine.
Take a look at the AI comments, some I think seem valid enough. Ping me for a re-review once addressed
…assword-sync # Conflicts: # server/datastore/mysql/schema.sql
…assword-sync # Conflicts: # server/fleet/service.go # server/mock/service/service_mock.go
Related issue: Resolves #45524
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/oree/fleetd-chrome/changes.See Changes files for more information.
Input data is properly validated,
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Timeouts are implemented and retries are limited to avoid infinite loops
If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes
Testing
Added/updated automated tests
Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)
QA'd all new/changed functionality manually
For unreleased bug fixes in a release candidate, one of:
Database migrations
COLLATE utf8mb4_unicode_ci).New Fleet configuration settings
If you didn't check the box above, follow this checklist for GitOps-enabled settings:
fleetctl generate-gitopsSummary by CodeRabbit