Skip to content

PSSO migrations for final schema shape#47431

Open
JordanMontgomery wants to merge 3 commits into
feature/fleet-macos-password-syncfrom
JM-47105-psso-migration
Open

PSSO migrations for final schema shape#47431
JordanMontgomery wants to merge 3 commits into
feature/fleet-macos-password-syncfrom
JM-47105-psso-migration

Conversation

@JordanMontgomery

@JordanMontgomery JordanMontgomery commented Jun 11, 2026

Copy link
Copy Markdown
Member

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.

  • 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

Database migrations

  • Checked schema for all modified table for columns that will auto-update timestamps during migration.
  • Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.
  • Ensured the correct collation is explicitly set for character columns (COLLATE utf8mb4_unicode_ci).

@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.04698% with 64 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.04%. Comparing base (ef54ff6) to head (cf5aac9).

Files with missing lines Patch % Lines
ee/server/service/apple_psso.go 0.00% 28 Missing ⚠️
ee/server/service/apple_psso_crypto.go 55.55% 17 Missing and 7 partials ⚠️
server/datastore/mysql/apple_psso.go 82.69% 5 Missing and 4 partials ⚠️
server/datastore/mysql/apple_mdm.go 0.00% 1 Missing and 1 partial ⚠️
...ons/tables/20260611140639_CreateApplePSSOTables.go 92.30% 1 Missing ⚠️
Additional details and impacted files
@@                          Coverage Diff                          @@
##           feature/fleet-macos-password-sync   #47431      +/-   ##
=====================================================================
+ Coverage                              67.00%   67.04%   +0.03%     
=====================================================================
  Files                                   3284     3284              
  Lines                                 228924   228904      -20     
  Branches                               11709    11709              
=====================================================================
+ Hits                                  153382   153459      +77     
+ Misses                                 61675    61572     -103     
- Partials                               13867    13873       +6     
Flag Coverage Δ
backend 68.64% <57.04%> (+0.04%) ⬆️

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.

@JordanMontgomery JordanMontgomery marked this pull request as ready for review June 11, 2026 20:19
@JordanMontgomery JordanMontgomery requested a review from a team as a code owner June 11, 2026 20:19
// the previously-established session key, validates it against the
// upstream IdP via the wired PSSOIdPClient, and returns the resulting
// claims as a JWT-inside-JWE.
func (svc *Service) handlePSSOPasswordRequest(ctx context.Context, device *fleet.PSSODevice, claims *pssoTokenClaims) ([]byte, error) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all unused PSSO v1 code being removed(we are only supporting v2)

if err != nil {
return nil, ctxerr.Wrap(ctx, err, "parse apv encryption key")
}
hostKeys, err := svc.ds.ListPSSOKeys(ctx, hostUUID)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is strictly needed but it feels worthwhile just in case we've missed something in our KID calculations

pssoNonceStore fleet.PSSONonceStore

// pssoIdPClient validates passwords for the PSSO password_request flow.
// pssoIdPClient validates passwords for the PSSO password login flow.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed because password_request was PSSO v1 and password login is V2

// SetOrUpdatePSSODevice upserts a host's PSSO registration: the device row
// plus the given key rows in a single transaction. Keys are upserted by kid;
// keys from earlier registrations are left in place so they keep working.
func (ds *Datastore) SetOrUpdatePSSODevice(ctx context.Context, hostUUID string, keys []fleet.PSSOKey) error {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for leaving old keys behind is to allow for rotation. We do clear keys on ADE re-enroll. The apple docs make it seem like rotation should be supported but it's not entirely clear when that happens vs just a total reset/reregistration

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.

1 participant