Add OAuth providers migration#191
Conversation
…nto add-oauth-providers-migration
Greptile SummaryAdds OAuth2 provider configuration migration between Appwrite projects. The source reads the provider list via
Confidence Score: 5/5Safe to merge; all correctness and security concerns from prior review rounds appear resolved, and the new code faithfully copies provider state without exposing secrets. The allow-list design prevents secret leakage, the enabled/secret guard problems flagged in earlier rounds are gone, and the The per-provider read-write-purge loop in Important Files Changed
Reviews (23): Last reviewed commit: "Fix OAuth provider secret field mappings" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…oved oAuthProviders off Models\Project)
… broken sign-in flow)
… non-secret fields
…atus counter size limit)
- Destination: dispatch via explicit case Resource::TYPE_OAUTH2_PROVIDER instead of default + instanceof - Source: count in report() directly like sibling resources (drop try/catch), and move the in_array guard inside the export try - Harden mergeAppleSecret/mergeJsonSecret against non-array decoded JSON - Fix stale OAuth2Provider docblock (single shared TYPE, not per-subclass)
- mergeAppleSecret now delegates to mergeJsonSecret (one merge implementation) - exportOAuth2Providers surfaces providers with no Resource class as non-fatal errors instead of dropping them silently - report() counts only migratable providers; fix the misleading enabled comment - use elseif for the mutually-exclusive provider-shape branches - add AppwriteOAuth2SecretTest (secret-merge) and OAuth2ProviderTransferTest (transfer round-trip via MockSource/MockDestination)
…rage Other migration resources (auth methods, policies, …) ship no per-resource tests in this library; keep OAuth2 consistent with that baseline.
…these; were unmapped)
There was a problem hiding this comment.
Can we generalize and have 1 class for all? Much easier to maintain
Replace the per-provider class hierarchy (OAuth2Provider base + StandardProvider/WithEndpointProvider + ~40 one-line subclasses) with a single OAuth2Provider class driven by a `providerKey => non-secret fields` map. Addresses review feedback to have one class for all providers. - Source dispatch: drop the 42-entry class registry + oauth2ClassFor(); build providers via OAuth2Provider::fromArray($key, $payload). - Destination dispatch: replace the instanceof chain with a key check (Apple) + data-driven settings routing for endpoint/tenant/prompt. - The field map doubles as a secret allow-list: only declared non-secret fields are copied off the listOAuth2Providers payload, so a future upstream secret field cannot leak into the migration. Net 44 fewer files. Pint, PHPStan level 3, and the Unit suite all pass.
- Remove unused getSetting(); getDestinationAppId/SecretFields cover all callers. - getDestinationAppId() now returns ?string (null when unset), so the destination drops its duplicate isEmptyOAuth2Setting() helper and isConfigured() reads as `enabled || appId !== null`. - Document the PROVIDERS target/key routing so adding a provider is self-explanatory.
2 participants
Summary
TYPE_OAUTH_PROVIDERStoGROUP_AUTH_RESOURCESfor migrating the project's OAuth2 provider configuration map.Sources/Appwrite) reads$project->oAuthProvidersand emits oneOAuthProviderssingleton carryingkey/enabled/appIdfor each provider.Destinations/Appwrite) merges the entries into the project doc'soAuthProvidersmap as flat{key}Enabled/{key}Appidkeys viadbForPlatform(mirrors the destination path used byauth-methods/policies).{key}Secretis intentionally not migrated — the source API never exposes secrets and the destination user must re-enter them post-migration. Same caveat as the SMTP password handling.GROUP_AUTH(notGROUP_INTEGRATIONS) — OAuth providers are auth methods that happen to use external identity providers; same group asTYPE_AUTH_METHODSandTYPE_POLICIES.Test plan
testAppwriteMigrationOAuthProviders(in appwrite/appwrite) passes