Skip to content

feat: turnkey migration - sync factor policy - #401

Merged
paolodamico merged 13 commits into
mainfrom
worktree-velvety-meandering-popcorn
Aug 7, 2026
Merged

feat: turnkey migration - sync factor policy #401
paolodamico merged 13 commits into
mainfrom
worktree-velvety-meandering-popcorn

Conversation

@paolodamico

@paolodamico paolodamico commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Changes

  1. Introduces the sync factor policy migration. Rationale and functionality is documented in the code but at a high level this ensures each sync factor has the right policy. Previously users where created with a different policy, the Turnkey team advised against using activity.type for these policies. All with its relevant unit and functional tests.
  2. Refactors the Turnkey API cache to more generically serve different resources vs. just users, and most importantly flag consistency issues if different sub-org IDs are used during a single migration process.
  3. Addresses the outstanding comments from feat: introduce turnkey migration engine and apple audiences migration #392
  4. Non-change. The apple audiences functional tests were moved to a new sub-module to have a clear separation between each test suite. No changes are introduced.

Real Testing

I tested all the changes in this PR with the real Turnkey API using the run_migrations_against_real_turnkey test:

  1. I ran the test with an account created from the iOS app. These were the results (as expected),
    running 1 test
    [bedrock][Debug] [Bedrock][TurnkeyManager] run_migrations start is_suborg_provided=false
    [bedrock][Info] [Bedrock][TurnkeyManager] apple_audience skipped: skip: all providers are already configured
    [bedrock][Info] [Bedrock][TurnkeyManager] turnkey.migration.applied migration=sync_factor_policy changes=[updated policy for sync_factor_user_yK3SxF]
    [bedrock][Debug] [Bedrock][TurnkeyManager] run_migrations completed successfully
    run_migrations outcome: Completed
    test backup::turnkey::test::integration_tests::run_migrations_against_real_turnkey ... ok
    
    • You can see the policy for my Sync Factor user before the migration
    • ....and after the migration:
    - furthermore, you can see the activity was properly executed in Turnkey
  2. Afterwards I ran the migration again and as expected, no more changes.
    running 1 test
    [bedrock][Debug] [Bedrock][TurnkeyManager] run_migrations start is_suborg_provided=false
    [bedrock][Info] [Bedrock][TurnkeyManager] apple_audience skipped: skip: all providers are already configured
    [bedrock][Info] [Bedrock][TurnkeyManager] sync_factor_policy skipped: all sync factors have the correct policy
    [bedrock][Debug] [Bedrock][TurnkeyManager] run_migrations completed successfully
    run_migrations outcome: Completed
    test backup::turnkey::test::integration_tests::run_migrations_against_real_turnkey ... ok
    

Note

High Risk
Changes live Turnkey authorization policies and can delete policies; mistakes could weaken deletion permissions or remove needed policies, though guards (exact consensus matching, skip multi-policy users, UUID validation) limit blast radius.

Overview
Adds MigrationSyncFactorPolicy to the Turnkey migration pipeline so each sync-factor user gets the canonical DELETE policy (resource + action, not legacy activity.type), with create/update when drifted and delete for policies bound to users no longer in the sub-org. Canonical policy text, user-role classification, and UUID-safe consensus strings live in policies.rs.

TurnkeyApiClient replaces the per-sub-org HashMap user cache with OrgCache (single sub-org per client, Arc shared reads, consistency error on sub-org mismatch), returns Arc<Vec<User>>, and adds cached get_policies plus create_policy / update_policy / delete_policy with cache invalidation on writes. apple_audience is adjusted for borrowed users and minor docs; functional tests are split by migration with shared read mocks for users + policies.

Reviewed by Cursor Bugbot for commit ea3d54b. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI 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.

Pull request overview

This PR adds a new Turnkey migration to reconcile sync-factor authorization policies to a canonical deletion-only policy shape, while refactoring the Turnkey API client caching so one client instance can serve multiple migrations efficiently and detect cross-sub-org consistency violations. It also reorganizes and expands functional tests to cover policy create/update/delete flows and cache behavior, and adjusts the existing Apple audience migration to work with the new cached-return types.

Changes:

  • Introduces sync_factor_policy migration to create/update/prune per-sync-factor Turnkey policies using resource + action conditions.
  • Refactors TurnkeyApiClient caching into a per-client, single-suborg OrgCache and adds cached get_policies plus policy write APIs.
  • Reorganizes and expands functional tests to cover policy reconciliation and cache consistency, and updates Apple audience migration planning to operate on slices / cached reads.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
bedrock/src/backup/turnkey/test.rs Reorganizes functional tests and adds policy-migration + cache behavior coverage via new read/write mocks.
bedrock/src/backup/turnkey/policies.rs Adds sync-factor policy condition/consensus/name helpers and user role classification used by migrations.
bedrock/src/backup/turnkey/migrations/sync_factor_policy.rs Implements new migration to reconcile sync-factor policies (create/update/delete) based on users + policies.
bedrock/src/backup/turnkey/migrations/mod.rs Registers the new migration in the ordered MIGRATIONS list.
bedrock/src/backup/turnkey/migrations/apple_audience.rs Adapts planning to accept slices (works with cached Arc<Vec<User>>) and clarifies rationale/comments.
bedrock/src/backup/turnkey/api.rs Adds OrgCache, returns cached Arc<Vec<_>> for reads, and introduces policy list/create/update/delete API methods with cache invalidation.

Comment thread bedrock/src/backup/turnkey/policies.rs Outdated
Comment thread bedrock/src/backup/turnkey/migrations/sync_factor_policy.rs Outdated
Comment thread bedrock/src/backup/turnkey/migrations/sync_factor_policy.rs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: feded97186

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread bedrock/src/backup/turnkey/migrations/sync_factor_policy.rs
@paolodamico
paolodamico requested a lite review from Copilot August 5, 2026 19:25

Copilot AI 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.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread bedrock/src/backup/turnkey/policies.rs
Comment thread bedrock/src/backup/turnkey/migrations/sync_factor_policy.rs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fcf1b86e70

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread bedrock/src/backup/turnkey/policies.rs
@paolodamico
paolodamico merged commit 66983ec into main Aug 7, 2026
20 checks passed
@paolodamico
paolodamico deleted the worktree-velvety-meandering-popcorn branch August 7, 2026 22:02
This was referenced Aug 7, 2026
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.

3 participants