diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a517eb..80f8b4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,13 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). `username` column, its uniqueness constraint and its registration validation — AuthKit does not own the users table and ships no migration for it. See `docs/login.md`. Closes [#65](https://github.com/laranail/authkit/issues/65). +- **Explicit, secure identity linking.** `IdentityLinkingInterface` lets a provider package attach a + sign-in method to an already authenticated user without that path ever being able to sign anyone + in. A link attempt requires a recently confirmed credential, carries a single-use token bound to + the initiating user, guard, provider and authentication context, and refuses an external identity + owned by another account — never switching the current user and never transferring the identity. + The surface accepts no email address at all, so a matching address cannot become authorisation. + See `docs/identity-linking.md`. Closes [#64](https://github.com/laranail/authkit/issues/64). - The two-factor middleware is registered under the vendor-scoped alias `laranail-authkit-two-factor` (`AuthKitServiceProvider::TWO_FACTOR_MIDDLEWARE`). `NamingConventionTest` asserts it, and that no other bare alias is registered, against the live router. diff --git a/README.md b/README.md index 3fffb0b..90a5ad0 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,7 @@ Full documentation: ['email'], ], + 'identity_linking' => [ + /* + * How long a link attempt stays valid. + * + * It spans one redirect to an external provider and back, so minutes rather than hours. + * The attempt is single-use regardless; this bounds how long an unused one lingers. + */ + 'intent_ttl_minutes' => (int) env(key: 'AUTHKIT_LINK_INTENT_TTL_MINUTES', default: 10), + ], + 'rate_limit' => [ 'max_attempts' => (int) env(key: 'AUTHKIT_RATE_LIMIT_MAX_ATTEMPTS', default: 5), 'decay_minutes' => (int) env(key: 'AUTHKIT_RATE_LIMIT_DECAY_MINUTES', default: 1), diff --git a/docs/configuration.md b/docs/configuration.md index 12d2948..f9e46d0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -9,6 +9,7 @@ php artisan vendor:publish --tag=laranail::authkit-config Set `AUTHKIT_GUARD` to the application's intended guard. The `laranail.authkit.fortify.features` list enables Fortify capabilities individually: `reset-passwords`, `update-profile-information`, `update-passwords`, `email-verification`, and `passkeys`. Removing an item prevents Auth Kit from enabling that capability; remove corresponding application routes and UI too. The `laranail.authkit.login` block chooses how a user identifies themselves. `field` is the request input the identifier arrives in (`email` by default, `AUTHKIT_LOGIN_FIELD`), and `identifiers` lists the user columns it is matched against, in order (`['email']` by default). See [Login](login.md#signing-in-with-a-username). +`laranail.authkit.identity_linking.intent_ttl_minutes` bounds how long an unstarted link attempt lingers (`AUTHKIT_LINK_INTENT_TTL_MINUTES`, ten minutes by default). See [Identity linking](identity-linking.md). Credential throttling is controlled by `AUTHKIT_RATE_LIMIT_MAX_ATTEMPTS` and `AUTHKIT_RATE_LIMIT_DECAY_MINUTES`, defaulting to five attempts per minute. Social credentials, callbacks, and enabled providers belong to [`laranail/authkit-social-login`](https://github.com/laranail/authkit-social-login); see its [social login guide](https://github.com/laranail/authkit-social-login/blob/main/docs/social-login.md). diff --git a/docs/identity-linking.md b/docs/identity-linking.md new file mode 100644 index 0000000..36e2aeb --- /dev/null +++ b/docs/identity-linking.md @@ -0,0 +1,128 @@ +# Identity linking + +Adding a sign-in method to an account that is already signed in, without that ever becoming a way +to sign in. + +`Simtabi\Laranail\AuthKit\Contracts\IdentityLinkingInterface` is the mechanism, bound to +`Services\IdentityLinking`. A provider package — `laranail/authkit-social-login` and its siblings — +owns where links are stored and which external identity it just saw. This owns whether attaching +one is allowed. + +## Why it is separate from signing in + +Resolving an account from a verified address is how someone *signs in* with a provider. It is not +good enough to *attach a new permanent credential* to an account, because the two have different +blast radii: a wrong sign-in ends with a session, and a wrong link ends with a standing way back +in that the account's owner never chose. + +So the two paths are kept apart, structurally rather than by convention: + +- Nothing here can authenticate anybody. The service has no access to a guard's login, and every + method returns a value object or nothing. +- Nothing here accepts an email address. There is no parameter for one and no field on + `LinkIntent`, so a matching address cannot become authorisation even by mistake. A test asserts + that shape rather than the behaviour. + +## The flow + +```php +use Simtabi\Laranail\AuthKit\Contracts\IdentityLinkingInterface; +use Simtabi\Laranail\AuthKit\Exceptions\IdentityLinkException; + +// 1. The user asks to connect a provider. Refused unless a credential was confirmed recently. +$token = app(IdentityLinkingInterface::class)->issueIntent( + request: $request, + user: $request->user(), + guard: 'web', + provider: 'github', +); + +// 2. Carry $token through the provider redirect as your own state, and on the way back: +$linking = app(IdentityLinkingInterface::class); + +try { + $linking->consumeIntent($request, $token, $request->user(), 'web', 'github'); + $linking->assertLinkable($request->user(), $ownerOfThisExternalIdentity); +} catch (IdentityLinkException $exception) { + return back()->withErrors(['connection' => $exception->getMessage()]); +} + +// 3. Only now does your package write the link. +``` + +`$ownerOfThisExternalIdentity` is whoever already holds the identity, or `null` when nobody does. +The provider package looks that up, because it owns the table. + +## What each step guarantees + +| Step | Refuses | +|---|---| +| `issueIntent` | No credential confirmed within `auth.password_timeout` | +| `consumeIntent` | A token already spent, unknown, or expired | +| `consumeIntent` | A different user, guard, provider or authentication context than the one that started it | +| `assertLinkable` | An identity owned by a different account | + +`assertLinkable` is idempotent for an identity the same user already owns, so a double submission +is harmless. It never switches the current user and never transfers an identity between accounts: +the only outcomes are "allowed" and an exception. + +## Recent reauthentication + +A browser session answers through Laravel's own `auth.password_confirmed_at`, checked against +`auth.password_timeout` — exactly what the framework's `password.confirm` middleware maintains, so +an application that already confirms passwords needs nothing new. Put `password.confirm` on the +route that starts a link and this is satisfied before the service is reached. + +A token-authenticated client has no session to carry that, so it proves the credential on the +request that starts the link: + +```php +POST /api/auth/connections/github { "password": "..." } +``` + +The password is checked through the guard's own provider, so an application that has replaced its +user provider keeps its own rules. + +## Why the attempt is single-use and bound + +The token is 256 bits of randomness, and only its SHA-256 is stored — the token itself exists only +in the response that returned it. The record binds the user, their model class, the guard, the +provider and a hash of the session or access token the attempt began from, and all of them are +compared again on the way back. + +Spending is a read-and-delete taken under an atomic lock where the cache store offers one. A +replayed callback therefore finds nothing, which is the refusal that matters: without it, one +intercepted callback URL could be replayed to attach the same identity repeatedly, or attach it +after the user believed the attempt had failed. + +A client presenting neither a session nor an access token binds on the user alone. That is weaker, +but it still cannot cross accounts, because the user is compared too. + +## Failure handling + +`IdentityLinkException::$failure` is a closed `IdentityLinkFailure` enum, so a caller branches on +the reason rather than parsing a message — a web flow wants to redirect to password confirmation +where an API wants a 403, and both are the same refusal. + +| Case | Meaning | +|---|---| +| `RecentAuthenticationRequired` | Confirm a password, then start again | +| `IntentInvalid` | Unknown, expired, or already spent | +| `IntentMismatch` | Finished by a different user, guard, provider or context | +| `IdentityOwnedByAnotherUser` | Already connected elsewhere; nothing was changed | + +## Configuration + +```php +// config/laranail/authkit.php +'identity_linking' => [ + 'intent_ttl_minutes' => 10, +], +``` + +The attempt spans one redirect out to a provider and back, so minutes rather than hours. It is +single-use regardless; this only bounds how long an unused one lingers. + +--- + +[← Docs index](../README.md#documentation) diff --git a/src/Contracts/IdentityLinkingInterface.php b/src/Contracts/IdentityLinkingInterface.php new file mode 100644 index 0000000..a36ac48 --- /dev/null +++ b/src/Contracts/IdentityLinkingInterface.php @@ -0,0 +1,52 @@ + 'Confirm your password before adding a sign-in method.', + self::IntentInvalid, + self::IntentMismatch => 'This linking request is no longer valid. Start again.', + self::IdentityOwnedByAnotherUser => 'That account is already connected to a different user.', + }; + } +} diff --git a/src/Exceptions/IdentityLinkException.php b/src/Exceptions/IdentityLinkException.php new file mode 100644 index 0000000..be552d4 --- /dev/null +++ b/src/Exceptions/IdentityLinkException.php @@ -0,0 +1,28 @@ +message()); + } + + public static function because(IdentityLinkFailure $failure): self + { + return new self($failure); + } +} diff --git a/src/Providers/AuthKitServiceProvider.php b/src/Providers/AuthKitServiceProvider.php index 4470c82..11e072a 100644 --- a/src/Providers/AuthKitServiceProvider.php +++ b/src/Providers/AuthKitServiceProvider.php @@ -54,6 +54,7 @@ public function packageRegistered(): void $this->app->bind(abstract: Contracts\CheckEmailExistsInterface::class, concrete: Actions\CheckEmailExists::class); $this->app->bind(abstract: Contracts\FindUserByEmailInterface::class, concrete: Actions\FindUserByEmail::class); $this->app->bind(abstract: Contracts\LoginUserInterface::class, concrete: Actions\LoginUser::class); + $this->app->bind(abstract: Contracts\IdentityLinkingInterface::class, concrete: Services\IdentityLinking::class); $this->app->bind(abstract: Contracts\LogoutUserInterface::class, concrete: Actions\LogoutUser::class); $this->app->bind(abstract: Contracts\IssueTokenForUserInterface::class, concrete: Actions\IssueTokenForUser::class); $this->app->singleton(Contracts\TokenIssuerRegistryInterface::class, function (): Services\TokenIssuerRegistry { diff --git a/src/Services/IdentityLinking.php b/src/Services/IdentityLinking.php new file mode 100644 index 0000000..edbb53b --- /dev/null +++ b/src/Services/IdentityLinking.php @@ -0,0 +1,201 @@ +recentlyAuthenticated(request: $request, user: $user, guard: $guard)) { + throw IdentityLinkException::because(IdentityLinkFailure::RecentAuthenticationRequired); + } + + $token = bin2hex(random_bytes(32)); + + $this->cache->put( + key: $this->key($token), + value: $this->describe(request: $request, user: $user, guard: $guard, provider: $provider)->toArray(), + ttl: now()->addMinutes($this->ttlMinutes()), + ); + + return $token; + } + + public function consumeIntent(Request $request, string $token, Authenticatable $user, string $guard, string $provider): LinkIntent + { + $record = $this->spend($token); + + if (! is_array($record)) { + throw IdentityLinkException::because(IdentityLinkFailure::IntentInvalid); + } + + $stored = LinkIntent::fromArray($record); + + if (! $stored->matches($this->describe(request: $request, user: $user, guard: $guard, provider: $provider))) { + throw IdentityLinkException::because(IdentityLinkFailure::IntentMismatch); + } + + return $stored; + } + + public function assertLinkable(Authenticatable $user, ?Authenticatable $currentOwner): void + { + if ($currentOwner === null) { + return; + } + + $sameAccount = $currentOwner::class === $user::class + && hash_equals((string) $currentOwner->getAuthIdentifier(), (string) $user->getAuthIdentifier()); + + if ($sameAccount) { + return; + } + + throw IdentityLinkException::because(IdentityLinkFailure::IdentityOwnedByAnotherUser); + } + + /** + * Read the record and delete it in the same breath, so a token spends exactly once. + * + * `Repository::pull()` is a get followed by a forget, which leaves a window two simultaneous + * callbacks could both read. Where the store offers atomic locks the spend is taken under one; + * where it does not, the window stands and is smaller than the round trip that would exploit + * it. Both are single-use for any sequential replay, which is the attack this defends. + * + * @return array|null + */ + private function spend(string $token): ?array + { + $key = $this->key($token); + $store = $this->cache->getStore(); + + if (! $store instanceof LockProvider) { + $record = $this->cache->pull($key); + + return is_array($record) ? $record : null; + } + + $lock = $store->lock($key . ':spend', 5); + + return $lock->block(3, function () use ($key): ?array { + $record = $this->cache->pull($key); + + return is_array($record) ? $record : null; + }); + } + + /** + * Has this user proved an account credential recently enough to add another one? + * + * A browser session answers through Laravel's own `auth.password_confirmed_at` against + * `auth.password_timeout`, which is exactly what the framework's `password.confirm` + * middleware maintains -- so an application that already confirms passwords needs nothing new. + * + * A token-authenticated client has no session to carry that, so it proves the credential on + * the request itself, checked through the guard's own provider. + */ + private function recentlyAuthenticated(Request $request, Authenticatable $user, string $guard): bool + { + $timeout = (int) config(key: 'auth.password_timeout', default: 10800); + + if ($request->hasSession()) { + $confirmedAt = (int) $request->session()->get('auth.password_confirmed_at', 0); + + if ($confirmedAt > 0 && (time() - $confirmedAt) < $timeout) { + return true; + } + } + + $password = (string) $request->input('password', ''); + + if ($password === '') { + return false; + } + + if (! $this->auth->guard($guard)->getProvider()->validateCredentials($user, ['password' => $password])) { + return false; + } + + if ($request->hasSession()) { + $request->session()->put('auth.password_confirmed_at', time()); + } + + return true; + } + + /** + * Everything the attempt is bound to, recomputed on the way in and on the way out. + * + * The authentication context is the session or the access token the attempt was started from, + * hashed. It is what stops a callback landing on a different session from completing somebody + * else's link. A client presenting neither binds on the user alone, which is weaker but cannot + * cross accounts, since the user is compared too. + */ + private function describe(Request $request, Authenticatable $user, string $guard, string $provider): LinkIntent + { + return new LinkIntent( + userId: (string) $user->getAuthIdentifier(), + userClass: $user::class, + guard: $guard, + provider: $provider, + context: $this->context(request: $request, user: $user), + ); + } + + private function context(Request $request, Authenticatable $user): string + { + if ($request->hasSession()) { + return 'session:' . hash('sha256', $request->session()->getId()); + } + + $token = method_exists($user, 'currentAccessToken') ? $user->currentAccessToken() : null; + + if ($token !== null && method_exists($token, 'getKey')) { + return 'token:' . hash('sha256', (string) $token->getKey()); + } + + return 'none'; + } + + private function key(string $token): string + { + return self::PREFIX . hash('sha256', $token); + } + + private function ttlMinutes(): int + { + $minutes = (int) config(key: 'laranail.authkit.identity_linking.intent_ttl_minutes', default: 10); + + return $minutes > 0 ? $minutes : 10; + } +} diff --git a/src/Support/LinkIntent.php b/src/Support/LinkIntent.php new file mode 100644 index 0000000..78d2dfb --- /dev/null +++ b/src/Support/LinkIntent.php @@ -0,0 +1,59 @@ + $record */ + public static function fromArray(array $record): self + { + return new self( + userId: (string) ($record['user_id'] ?? ''), + userClass: (string) ($record['user_class'] ?? ''), + guard: (string) ($record['guard'] ?? ''), + provider: (string) ($record['provider'] ?? ''), + context: (string) ($record['context'] ?? ''), + ); + } + + /** @return array{user_id: string, user_class: string, guard: string, provider: string, context: string} */ + public function toArray(): array + { + return [ + 'user_id' => $this->userId, + 'user_class' => $this->userClass, + 'guard' => $this->guard, + 'provider' => $this->provider, + 'context' => $this->context, + ]; + } + + public function matches(self $other): bool + { + return hash_equals($this->userId, $other->userId) + && hash_equals($this->userClass, $other->userClass) + && hash_equals($this->guard, $other->guard) + && hash_equals($this->provider, $other->provider) + && hash_equals($this->context, $other->context); + } +} diff --git a/tests/Feature/IdentityLinkingTest.php b/tests/Feature/IdentityLinkingTest.php new file mode 100644 index 0000000..de89390 --- /dev/null +++ b/tests/Feature/IdentityLinkingTest.php @@ -0,0 +1,267 @@ +start(); + + if ($confirmed) { + $store->put('auth.password_confirmed_at', time()); + } + + return $store; +} + +function linkRequest(?Store $session = null, array $params = []): Request +{ + $request = Request::create(uri: '/account/connections', method: 'POST', parameters: $params); + + if ($session !== null) { + $request->setLaravelSession($session); + } + + return $request; +} + +function linking(): IdentityLinking +{ + return app(IdentityLinking::class); +} + +function failure(callable $callback): IdentityLinkFailure +{ + try { + $callback(); + } catch (IdentityLinkException $exception) { + return $exception->failure; + } + + throw new RuntimeException('Expected the attempt to be refused, and it was not.'); +} + +it('binds the contract to the service', function (): void { + expect(app(IdentityLinkingInterface::class))->toBeInstanceOf(IdentityLinking::class); +}); + +/* ---------------------------------------------------------------- reauthentication */ + +it('refuses to start a link without a recently confirmed credential', function (): void { + $user = User::factory()->create(); + + $reason = failure(fn () => linking()->issueIntent( + request: linkRequest(linkSession(confirmed: false)), + user: $user, + guard: 'web', + provider: 'github', + )); + + expect($reason)->toBe(IdentityLinkFailure::RecentAuthenticationRequired); +}); + +it('refuses to start a link once the confirmation has aged out', function (): void { + config()->set('auth.password_timeout', 60); + $user = User::factory()->create(); + + $session = linkSession(confirmed: false); + $session->put('auth.password_confirmed_at', time() - 120); + + $reason = failure(fn () => linking()->issueIntent( + request: linkRequest($session), + user: $user, + guard: 'web', + provider: 'github', + )); + + expect($reason)->toBe(IdentityLinkFailure::RecentAuthenticationRequired); +}); + +it('starts a link for a session that has confirmed a password', function (): void { + $user = User::factory()->create(); + + $token = linking()->issueIntent( + request: linkRequest(linkSession()), + user: $user, + guard: 'web', + provider: 'github', + ); + + expect($token)->toBeString()->and(strlen($token))->toBe(64); +}); + +it('lets a client with no session prove its password on the request', function (): void { + $password = Str::random(16); + $user = User::factory()->create(['password' => bcrypt($password)]); + + $token = linking()->issueIntent( + request: linkRequest(params: ['password' => $password]), + user: $user, + guard: 'web', + provider: 'github', + ); + + expect($token)->toBeString()->and(strlen($token))->toBe(64); +}); + +it('refuses a client with no session and the wrong password', function (): void { + $password = Str::random(16); + expect(TestCredentials::password('wrong'))->not->toBe($password); + + $user = User::factory()->create(['password' => bcrypt($password)]); + + $reason = failure(fn () => linking()->issueIntent( + request: linkRequest(params: ['password' => TestCredentials::password('wrong')]), + user: $user, + guard: 'web', + provider: 'github', + )); + + expect($reason)->toBe(IdentityLinkFailure::RecentAuthenticationRequired); +}); + +/* ------------------------------------------------------------------- the intent */ + +it('completes a link for the user and context that started it', function (): void { + $user = User::factory()->create(); + $session = linkSession(); + + $token = linking()->issueIntent(linkRequest($session), $user, 'web', 'github'); + + $intent = linking()->consumeIntent(linkRequest($session), $token, $user, 'web', 'github'); + + expect($intent->userId)->toBe((string) $user->getAuthIdentifier()) + ->and($intent->provider)->toBe('github') + ->and($intent->guard)->toBe('web'); +}); + +it('never signs anyone in while completing a link', function (): void { + $user = User::factory()->create(); + $session = linkSession(); + + $token = linking()->issueIntent(linkRequest($session), $user, 'web', 'github'); + linking()->consumeIntent(linkRequest($session), $token, $user, 'web', 'github'); + + expect(auth()->guard('web')->check())->toBeFalse(); +}); + +it('spends the intent exactly once', function (): void { + $user = User::factory()->create(); + $session = linkSession(); + + $token = linking()->issueIntent(linkRequest($session), $user, 'web', 'github'); + linking()->consumeIntent(linkRequest($session), $token, $user, 'web', 'github'); + + $reason = failure(fn () => linking()->consumeIntent(linkRequest($session), $token, $user, 'web', 'github')); + + expect($reason)->toBe(IdentityLinkFailure::IntentInvalid); +}); + +it('rejects a token that was never issued', function (): void { + $user = User::factory()->create(); + + $reason = failure(fn () => linking()->consumeIntent( + linkRequest(linkSession()), + bin2hex(random_bytes(32)), + $user, + 'web', + 'github', + )); + + expect($reason)->toBe(IdentityLinkFailure::IntentInvalid); +}); + +it('refuses an intent presented by a different user', function (): void { + $initiator = User::factory()->create(); + $attacker = User::factory()->create(); + $session = linkSession(); + + $token = linking()->issueIntent(linkRequest($session), $initiator, 'web', 'github'); + + $reason = failure(fn () => linking()->consumeIntent(linkRequest($session), $token, $attacker, 'web', 'github')); + + expect($reason)->toBe(IdentityLinkFailure::IntentMismatch); +}); + +it('refuses a callback that lands on a different session', function (): void { + $user = User::factory()->create(); + + $token = linking()->issueIntent(linkRequest(linkSession()), $user, 'web', 'github'); + + $reason = failure(fn () => linking()->consumeIntent(linkRequest(linkSession()), $token, $user, 'web', 'github')); + + expect($reason)->toBe(IdentityLinkFailure::IntentMismatch); +}); + +it('refuses an intent completed against a different provider', function (): void { + $user = User::factory()->create(); + $session = linkSession(); + + $token = linking()->issueIntent(linkRequest($session), $user, 'web', 'github'); + + $reason = failure(fn () => linking()->consumeIntent(linkRequest($session), $token, $user, 'web', 'gitlab')); + + expect($reason)->toBe(IdentityLinkFailure::IntentMismatch); +}); + +/* ------------------------------------------------------------------- ownership */ + +it('allows an identity nobody owns', function (): void { + $user = User::factory()->create(); + + linking()->assertLinkable($user, null); +})->throwsNoExceptions(); + +it('is idempotent for an identity this user already owns', function (): void { + $user = User::factory()->create(); + + linking()->assertLinkable($user, $user); +})->throwsNoExceptions(); + +it('refuses an identity owned by another user, and transfers nothing', function (): void { + $user = User::factory()->create(); + $owner = User::factory()->create(); + + $reason = failure(fn () => linking()->assertLinkable($user, $owner)); + + expect($reason)->toBe(IdentityLinkFailure::IdentityOwnedByAnotherUser) + ->and($owner->fresh()->getAuthIdentifier())->toBe($owner->getAuthIdentifier()); +}); + +/* -------------------------------------------------------------------- structure */ + +/* + * "Do not treat a matching email as authorization to link accounts" is a property of the shape, + * not of a branch somebody has to remember. The surface has nowhere to put an address, so this + * asserts the shape rather than the behaviour -- a parameter named for one would fail here before + * any logic could use it. + */ +it('offers no way to pass an email address into a link', function (): void { + $reflection = new ReflectionClass(IdentityLinking::class); + $checked = 0; + + foreach ($reflection->getMethods() as $method) { + foreach ($method->getParameters() as $parameter) { + $checked++; + expect(Str::lower($parameter->getName()))->not->toContain('email'); + } + } + + foreach ((new ReflectionClass(Simtabi\Laranail\AuthKit\Support\LinkIntent::class))->getProperties() as $property) { + $checked++; + expect(Str::lower($property->getName()))->not->toContain('email'); + } + + expect($checked)->toBeGreaterThan(15); +});