[6.x] Persist OAuth connections in the database for the eloquent user repository#15070
Open
ChipNeedham wants to merge 3 commits into
Open
[6.x] Persist OAuth connections in the database for the eloquent user repository#15070ChipNeedham wants to merge 3 commits into
ChipNeedham wants to merge 3 commits into
Conversation
duncanmcclean
changed the title
…itory
Statamic\OAuth\Provider persists the mapping between a provider account
(e.g. a Google sub) and a Statamic user via getIds()/setIds(), which
read/write a plain PHP file at storage_path("statamic/oauth/*.php").
That's fine as a default for the file user repository, which has no
database to lean on, but it's the one piece of auth-related state still
stuck on local disk when the eloquent repository is active -- users,
roles, and groups already live in the database via
Statamic\Auth\Eloquent\User/UserRepository.
In practice storage_path() isn't durable on a lot of common deployment
environments for an Eloquent-driven app: ephemeral compute, multiple
instances behind a load balancer, deploy pipelines that don't preserve
arbitrary storage/ subpaths across releases. When that file is missing
or not visible to the instance handling a request, findUser() returns
null, handleProviderCallback() falls through to createUser(), discovers
the email already taken, throws OAuthEmailExistsException, and the user
is silently redirected to the unauthorized page -- no exception logged,
no visible error.
Adds Statamic\Auth\Eloquent\OAuthProvider (persists via a new
oauth_connections table instead of the file) and
Statamic\Auth\Eloquent\OAuthConnections (the storage primitive, mirroring
the existing Roles/Groups query-builder classes rather than a full
Eloquent model). Statamic\OAuth\Manager::providers() picks the provider
class based on config('statamic.users.repository'), checked live on each
call -- not swapped via a container binding decided at boot time, so
switching repositories at runtime (as some of Statamic's own tests
already do) keeps working.
Also removes Manager::provider()'s by-name static cache. It isn't needed
(providers() is cheap, no I/O) and was itself a footgun -- a first
resolution for a given provider name stuck around for the life of the
process/Octane worker regardless of later config changes, the same class
of staleness driving this whole change. It became a real, reproducible
bug once the provider *type* could vary: a stale eloquent-backed
instance from one test class leaked into an unrelated OAuthDisconnectTest
whose environment never migrated oauth_connections.
statamic:auth:migration now also generates the oauth_connections
migration, matching the existing pattern for role_user/group_user/webauthn.
Tests: tests/Auth/Eloquent/EloquentOAuthTest.php covers provider class
selection (including switching repositories at runtime), the persistence
round-trip, id lookup, disconnect, multi-user isolation, and a full
end-to-end guest-login HTTP flow. Full package suite (9970 tests) passes.
Removing Manager::provider()'s cache entirely (in the previous commit) was too aggressive: it broke withUserData()/withUser() configuration. Consuming apps commonly attach that once, e.g. in a service provider's boot(), expecting the same instance to be reused whenever that provider is resolved again later (such as during the actual OAuth callback request) -- a fresh instance every call means that configuration is silently lost, since userDataCallback lives on the instance, not the class. This is exactly what our own app's @laravel.com domain restriction relies on, and it broke silently until a test caught it. Restores caching, but keyed by provider name *and* the current statamic.users.repository value, rather than name alone. That keeps the "configure once, reuse" behavior intact for the common case (repository config doesn't change mid-request/mid-process in a real app), while still handing back a fresh instance the moment that config does change -- which is what avoids the stale-type leak between test classes this whole fix is about. Added a regression test for the withUserData() case specifically, since this is exactly the kind of behavior that silently breaks and isn't caught by any test asserting persistence or class selection alone.
ChipNeedham
force-pushed
the
pr/eloquent-oauth-connections
branch
from
July 23, 2026 15:27
6178a89 to
c9f9dbd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #15069.
Statamic\OAuth\Provider's default storage (storage_path("statamic/oauth/*.php")) isn't durable on ephemeral/multi-instance deployments. When theeloquentuser repository is active, this adds a database-backed alternative so OAuth connections persist the same way users/roles/groups already do.Changes
Statamic\Auth\Eloquent\OAuthConnections— the storage primitive, mirroring the existingRoles/Groupsquery-builder pattern (not a full Eloquent model).Statamic\Auth\Eloquent\OAuthProvider— extendsProvider, overridesgetIds()/setIds()to persist viaOAuthConnections.Statamic\OAuth\Manager::providers()— picks the provider class based onconfig('statamic.users.repository'), checked live on each call (not baked in via a container binding at boot), so switching repositories at runtime (as some existing tests already do) keeps working.statamic:auth:migrationnow also generates theoauth_connectionsmigration, matching the existing pattern forrole_user/group_user/webauthn.config/users.php+ the install config stub — newtables.oauth_connectionskey.Manager::provider()'s cache is now keyed by provider name and the current repository setting, rather than name alone. Caching by name alone meant the first-resolved provider for a name stuck around for the life of the process/Octane worker regardless of later config changes — the same class of staleness this whole PR is about — and it's what caused real cross-test pollution while building this (a stale eloquent-backed instance leaked into an unrelatedOAuthDisconnectTest). Caching nothing (an earlier version of this PR) turned out to be too aggressive in the other direction — it silently brokewithUserData()/withUser()configuration, since that's attached to a specific instance and expected to survive for the life of the request. Both are covered by regression tests.Test plan
tests/Auth/Eloquent/EloquentOAuthTest.php(8 tests): provider/manager selection, persistence round-trip, id lookup, disconnect, multi-user isolation, live repository-switching, thewithUserDataregression, and a full end-to-end guest-login HTTP flow.