Skip to content

Add OAuth providers migration#191

Open
premtsd-code wants to merge 20 commits into
add-email-templates-migrationfrom
add-oauth-providers-migration
Open

Add OAuth providers migration#191
premtsd-code wants to merge 20 commits into
add-email-templates-migrationfrom
add-oauth-providers-migration

Conversation

@premtsd-code
Copy link
Copy Markdown
Contributor

Summary

  • Adds TYPE_OAUTH_PROVIDERS to GROUP_AUTH_RESOURCES for migrating the project's OAuth2 provider configuration map.
  • Source (Sources/Appwrite) reads $project->oAuthProviders and emits one OAuthProviders singleton carrying key/enabled/appId for each provider.
  • Destination (Destinations/Appwrite) merges the entries into the project doc's oAuthProviders map as flat {key}Enabled / {key}Appid keys via dbForPlatform (mirrors the destination path used by auth-methods / policies).
  • {key}Secret is 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.
  • Grouped under GROUP_AUTH (not GROUP_INTEGRATIONS) — OAuth providers are auth methods that happen to use external identity providers; same group as TYPE_AUTH_METHODS and TYPE_POLICIES.

Test plan

  • CI lints + tests green
  • E2E testAppwriteMigrationOAuthProviders (in appwrite/appwrite) passes

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Greptile Summary

This PR introduces TYPE_OAUTH2_PROVIDER migration support for Appwrite-to-Appwrite transfers, covering all 42 supported OAuth2 providers. The source reads provider state via listOAuth2Providers() and emits only configured providers; the destination merges each provider's non-secret fields (clientId/serviceId/endpoint/tenant/prompt) into the project's oAuthProviders map, deliberately skipping secrets.

  • Source (Sources/Appwrite.php): exportOAuth2Providers() uses a class-registry pattern (OAUTH2_PROVIDER_CLASSES + oauth2ClassFor()) to dispatch API payloads to strongly-typed Resource subclasses; unknown future providers are reported as non-fatal errors rather than silently dropped.
  • Destination (Destinations/Appwrite.php): createOAuth2Provider() does a read-merge-write per provider with cache purge, using mergeJsonSecret() / mergeAppleSecret() helpers to overlay migrated fields onto existing credential blobs without destroying destination secrets.
  • Resource hierarchy: OAuth2Provider → StandardProvider → WithEndpointProvider covers the full provider set, with bespoke subclasses for Apple, Google, and Microsoft that carry their extra non-secret fields.

Confidence Score: 5/5

Safe to merge; the change is additive, secrets are correctly withheld, and the destination read-merge-write path is sound.

The logic is well-structured and the known edge cases (write-only secrets, the Apple credential blob, JSON-secret providers) are handled correctly. The only gap found is that WithEndpointProvider does not override isConfigured() to include the endpoint field, meaning a disabled OIDC/Keycloak/Gitlab provider that has an endpoint URL saved but no clientId would be skipped during export and the endpoint lost post-migration.

src/Migration/Resources/Auth/OAuth2/WithEndpointProvider.php — isConfigured() should also check the endpoint field so that partially-configured OIDC-family providers are not silently dropped.

Important Files Changed

Filename Overview
src/Migration/Resources/Auth/OAuth2/OAuth2Provider.php New abstract base class for all OAuth2 provider resources; sound design with shared TYPE_OAUTH2_PROVIDER and per-provider dispatch via instanceof.
src/Migration/Resources/Auth/OAuth2/StandardProvider.php Base for most providers; isConfigured() only checks clientId — WithEndpointProvider inherits this without adding an endpoint check, so endpoint-only configurations of OIDC-family providers are silently skipped.
src/Migration/Resources/Auth/OAuth2/WithEndpointProvider.php Correct fromArray() and endpoint getter; does not override isConfigured() to include endpoint, so a disabled provider with endpoint set but empty clientId is skipped during export.
src/Migration/Resources/Auth/OAuth2/Apple.php Correctly handles Apple's bespoke credential split (serviceId/keyId/teamId vs write-only p8); isConfigured() covers the enabled-but-no-serviceId case.
src/Migration/Resources/Auth/OAuth2/Google.php Extends StandardProvider with prompt array; fromArray and getPrompt implemented correctly.
src/Migration/Resources/Auth/OAuth2/Microsoft.php Adds tenant field on top of StandardProvider; clean and correct.
src/Migration/Sources/Appwrite.php Adds exportOAuth2Providers() and calculateResourcesReport() logic with proper null-safe iteration (providers ?? []) and unknown-provider error reporting.
src/Migration/Destinations/Appwrite.php createOAuth2Provider() correctly does a read-merge-write per provider with cache purge; mergeAppleSecret and mergeJsonSecret helpers handle partial credential preservation correctly.
src/Migration/Resource.php Adds TYPE_OAUTH2_PROVIDER constant and registers it in the public-resource array; no issues.
src/Migration/Transfer.php TYPE_OAUTH2_PROVIDER added to GROUP_AUTH_RESOURCES and ALL_PUBLIC_RESOURCES; correct placement.

Reviews (17): Last reviewed commit: "OAuth2 export: null-guard listOAuth2Prov..." | Re-trigger Greptile

Comment thread src/Migration/Sources/Appwrite.php
Comment thread src/Migration/Sources/Appwrite.php Outdated
Comment thread src/Migration/Destinations/Appwrite.php Outdated
- 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.
Comment thread src/Migration/Sources/Appwrite.php Outdated
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