From 8c564a37cf2f2f90036a4ea6a15e22d1309d15f9 Mon Sep 17 00:00:00 2001 From: Gabriel do Carmo Vieira <48625433+gvieira18@users.noreply.github.com> Date: Mon, 22 Jun 2026 00:23:15 -0300 Subject: [PATCH 1/3] fix(integration-github): fetch primary email from /user/emails GitHub's /user endpoint only exposes a public email, so accounts with a private email return null. OAuth login then can't match an existing user by email and forks a duplicate. Fall back to /user/emails (covered by the user:email scope) and use the single primary verified address. Closes #366 --- .../src/OAuth/GitHubOAuthClient.php | 28 ++++++ .../Requests/Users/GetUserEmails.php | 28 ++++++ .../tests/Feature/GitHubOAuthClientTest.php | 93 +++++++++++++++++++ 3 files changed, 149 insertions(+) create mode 100644 app-modules/integration-github/src/Transport/Requests/Users/GetUserEmails.php create mode 100644 app-modules/integration-github/tests/Feature/GitHubOAuthClientTest.php diff --git a/app-modules/integration-github/src/OAuth/GitHubOAuthClient.php b/app-modules/integration-github/src/OAuth/GitHubOAuthClient.php index 8c8986bc..141e08d6 100644 --- a/app-modules/integration-github/src/OAuth/GitHubOAuthClient.php +++ b/app-modules/integration-github/src/OAuth/GitHubOAuthClient.php @@ -14,6 +14,7 @@ use He4rt\IntegrationGithub\Transport\GitHubOAuthConnector; use He4rt\IntegrationGithub\Transport\Requests\OAuth\ExchangeCodeForToken; use He4rt\IntegrationGithub\Transport\Requests\Users\GetCurrentUser; +use He4rt\IntegrationGithub\Transport\Requests\Users\GetUserEmails; final readonly class GitHubOAuthClient implements OAuthClientContract { @@ -61,9 +62,36 @@ public function getAuthenticatedUser(OAuthAccessDTO $credentials): GitHubOAuthUs /** @var array $userPayload */ $userPayload = $response->json(); + // GitHub's /user only exposes a *public* email, so it is null when the + // user keeps their address private. Recover the primary verified email + // from /user/emails (granted by the user:email scope); otherwise login + // can't match an existing account by email and forks a duplicate user. + if (($userPayload['email'] ?? null) === null) { + $userPayload['email'] = $this->resolvePrimaryEmail($credentials->accessToken); + } + return GitHubOAuthUserDTO::make($credentials, $userPayload); } + private function resolvePrimaryEmail(string $accessToken): ?string + { + $response = $this->apiConnector->send(new GetUserEmails( + accessToken: $accessToken, + )); + + if (!$response->ok()) { + return null; + } + + // Only the single primary + verified address identifies the account. A + // GitHub user often has several verified emails (work, school, noreply), + // so "any verified" could match the wrong one — require both flags. + return $response->collect() + ->where('primary', operator: true) + ->where('verified', operator: true) + ->value('email'); + } + private function callbackUrl(): string { return mb_rtrim(config('app.url'), '/').'/auth/oauth/github'; diff --git a/app-modules/integration-github/src/Transport/Requests/Users/GetUserEmails.php b/app-modules/integration-github/src/Transport/Requests/Users/GetUserEmails.php new file mode 100644 index 00000000..fb245e38 --- /dev/null +++ b/app-modules/integration-github/src/Transport/Requests/Users/GetUserEmails.php @@ -0,0 +1,28 @@ +accessToken); + } +} diff --git a/app-modules/integration-github/tests/Feature/GitHubOAuthClientTest.php b/app-modules/integration-github/tests/Feature/GitHubOAuthClientTest.php new file mode 100644 index 00000000..a859f1e7 --- /dev/null +++ b/app-modules/integration-github/tests/Feature/GitHubOAuthClientTest.php @@ -0,0 +1,93 @@ + $responses + */ +function githubOAuthClient(array $responses): GitHubOAuthClient +{ + $api = tap( + new GitHubApiConnector(), + fn (GitHubApiConnector $connector) => $connector->withMockClient(new MockClient($responses)), + ); + + return new GitHubOAuthClient( + oauthConnector: new GitHubOAuthConnector(clientId: 'cid', clientSecret: 'secret'), + apiConnector: $api, + ); +} + +/** + * @param array $overrides + */ +function githubUser(array $overrides = []): MockResponse +{ + return MockResponse::make([ + 'id' => 48_625_433, + 'login' => 'gvieira18', + 'name' => 'Gabriel Vieira', + 'email' => null, + 'avatar_url' => 'https://avatars.githubusercontent.com/u/48625433', + ...$overrides, + ]); +} + +test('uses the public /user email when GitHub exposes one', function (): void { + $user = githubOAuthClient([ + GetCurrentUser::class => githubUser(['email' => 'public@example.com']), + ])->getAuthenticatedUser(GitHubOAuthAccessDTO::make(['access_token' => 'tok'])); + + expect($user->email)->toBe('public@example.com') + ->and($user->provider)->toBe(IdentityProvider::GitHub) + ->and($user->providerId)->toBe('48625433'); +}); + +test('falls back to the primary verified email when /user email is private', function (): void { + $user = githubOAuthClient([ + GetCurrentUser::class => githubUser(['email' => null]), + GetUserEmails::class => MockResponse::make([ + ['email' => 'secondary@example.com', 'primary' => false, 'verified' => true], + ['email' => 'primary@example.com', 'primary' => true, 'verified' => true], + ['email' => 'unverified@example.com', 'primary' => false, 'verified' => false], + ]), + ])->getAuthenticatedUser(GitHubOAuthAccessDTO::make(['access_token' => 'tok'])); + + expect($user->email)->toBe('primary@example.com'); +}); + +test('ignores verified emails that are not primary', function (): void { + // Several verified addresses (work, school, noreply) but none primary — + // none should be picked; only a primary+verified pair counts. + $user = githubOAuthClient([ + GetCurrentUser::class => githubUser(['email' => null]), + GetUserEmails::class => MockResponse::make([ + ['email' => 'noreply@example.com', 'primary' => false, 'verified' => true], + ['email' => 'school@example.com', 'primary' => false, 'verified' => true], + ]), + ])->getAuthenticatedUser(GitHubOAuthAccessDTO::make(['access_token' => 'tok'])); + + expect($user->email)->toBeNull(); +}); + +test('ignores a primary email that is not verified', function (): void { + $user = githubOAuthClient([ + GetCurrentUser::class => githubUser(['email' => null]), + GetUserEmails::class => MockResponse::make([ + ['email' => 'primary-unverified@example.com', 'primary' => true, 'verified' => false], + ['email' => 'verified@example.com', 'primary' => false, 'verified' => true], + ]), + ])->getAuthenticatedUser(GitHubOAuthAccessDTO::make(['access_token' => 'tok'])); + + expect($user->email)->toBeNull(); +}); From 60c2468c94c468cc42a8a4d173dbfb81230026e0 Mon Sep 17 00:00:00 2001 From: Gabriel do Carmo Vieira <48625433+gvieira18@users.noreply.github.com> Date: Wed, 24 Jun 2026 13:41:17 -0300 Subject: [PATCH 2/3] fix(integration-github): hard-fail on unresolved email When /user/emails yields no primary+verified address there is no trustworthy email to correlate on, so abort the login instead of silently forking a duplicate user. Refs #366 --- .../Auth/Exceptions/OAuthFlowException.php | 8 ++++++ .../src/OAuth/GitHubOAuthClient.php | 5 +++- .../tests/Feature/GitHubOAuthClientTest.php | 27 ++++++++++++------- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/app-modules/identity/src/Auth/Exceptions/OAuthFlowException.php b/app-modules/identity/src/Auth/Exceptions/OAuthFlowException.php index 5ea3d836..5aa830ad 100644 --- a/app-modules/identity/src/Auth/Exceptions/OAuthFlowException.php +++ b/app-modules/identity/src/Auth/Exceptions/OAuthFlowException.php @@ -33,4 +33,12 @@ public static function tokenExchangeFailed(string $provider, string $error): sel { return new self(sprintf('Token exchange failed for "%s": %s', $provider, $error)); } + + public static function emailUnavailable(string $provider): self + { + return new self(sprintf( + 'Could not resolve a primary verified email from "%s"; aborting login to avoid forking a duplicate user.', + $provider, + )); + } } diff --git a/app-modules/integration-github/src/OAuth/GitHubOAuthClient.php b/app-modules/integration-github/src/OAuth/GitHubOAuthClient.php index 141e08d6..67659331 100644 --- a/app-modules/integration-github/src/OAuth/GitHubOAuthClient.php +++ b/app-modules/integration-github/src/OAuth/GitHubOAuthClient.php @@ -66,8 +66,11 @@ public function getAuthenticatedUser(OAuthAccessDTO $credentials): GitHubOAuthUs // user keeps their address private. Recover the primary verified email // from /user/emails (granted by the user:email scope); otherwise login // can't match an existing account by email and forks a duplicate user. + // When even the fallback can't yield a primary+verified address there is + // nothing trustworthy to correlate on, so hard-fail instead of forking. if (($userPayload['email'] ?? null) === null) { - $userPayload['email'] = $this->resolvePrimaryEmail($credentials->accessToken); + $userPayload['email'] = $this->resolvePrimaryEmail($credentials->accessToken) + ?? throw OAuthFlowException::emailUnavailable('github'); } return GitHubOAuthUserDTO::make($credentials, $userPayload); diff --git a/app-modules/integration-github/tests/Feature/GitHubOAuthClientTest.php b/app-modules/integration-github/tests/Feature/GitHubOAuthClientTest.php index a859f1e7..35f2b5b3 100644 --- a/app-modules/integration-github/tests/Feature/GitHubOAuthClientTest.php +++ b/app-modules/integration-github/tests/Feature/GitHubOAuthClientTest.php @@ -2,6 +2,7 @@ declare(strict_types=1); +use He4rt\Identity\Auth\Exceptions\OAuthFlowException; use He4rt\Identity\ExternalIdentity\Enums\IdentityProvider; use He4rt\IntegrationGithub\OAuth\DTO\GitHubOAuthAccessDTO; use He4rt\IntegrationGithub\OAuth\GitHubOAuthClient; @@ -66,28 +67,34 @@ function githubUser(array $overrides = []): MockResponse expect($user->email)->toBe('primary@example.com'); }); -test('ignores verified emails that are not primary', function (): void { +test('hard-fails when no email is primary', function (): void { // Several verified addresses (work, school, noreply) but none primary — - // none should be picked; only a primary+verified pair counts. - $user = githubOAuthClient([ + // none can be picked, so login must abort instead of forking a duplicate. + expect(fn () => githubOAuthClient([ GetCurrentUser::class => githubUser(['email' => null]), GetUserEmails::class => MockResponse::make([ ['email' => 'noreply@example.com', 'primary' => false, 'verified' => true], ['email' => 'school@example.com', 'primary' => false, 'verified' => true], ]), - ])->getAuthenticatedUser(GitHubOAuthAccessDTO::make(['access_token' => 'tok'])); - - expect($user->email)->toBeNull(); + ])->getAuthenticatedUser(GitHubOAuthAccessDTO::make(['access_token' => 'tok']))) + ->toThrow(OAuthFlowException::class); }); -test('ignores a primary email that is not verified', function (): void { - $user = githubOAuthClient([ +test('hard-fails when the primary email is not verified', function (): void { + expect(fn () => githubOAuthClient([ GetCurrentUser::class => githubUser(['email' => null]), GetUserEmails::class => MockResponse::make([ ['email' => 'primary-unverified@example.com', 'primary' => true, 'verified' => false], ['email' => 'verified@example.com', 'primary' => false, 'verified' => true], ]), - ])->getAuthenticatedUser(GitHubOAuthAccessDTO::make(['access_token' => 'tok'])); + ])->getAuthenticatedUser(GitHubOAuthAccessDTO::make(['access_token' => 'tok']))) + ->toThrow(OAuthFlowException::class); +}); - expect($user->email)->toBeNull(); +test('hard-fails when /user/emails is empty', function (): void { + expect(fn () => githubOAuthClient([ + GetCurrentUser::class => githubUser(['email' => null]), + GetUserEmails::class => MockResponse::make([]), + ])->getAuthenticatedUser(GitHubOAuthAccessDTO::make(['access_token' => 'tok']))) + ->toThrow(OAuthFlowException::class); }); From fdec0c4d2439c4f20506693521bee210ba29a9d7 Mon Sep 17 00:00:00 2001 From: Gabriel do Carmo Vieira <48625433+gvieira18@users.noreply.github.com> Date: Wed, 24 Jun 2026 13:41:26 -0300 Subject: [PATCH 3/3] test(identity): cover oauth link and login fork flows Document the callback orchestrator: linking a second provider while logged in attaches to the same user (email mismatch is a non-event), while a logged-out login with a different email forks a new account. --- .../Auth/HandleOAuthCallbackActionTest.php | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 app-modules/identity/tests/Feature/Auth/HandleOAuthCallbackActionTest.php diff --git a/app-modules/identity/tests/Feature/Auth/HandleOAuthCallbackActionTest.php b/app-modules/identity/tests/Feature/Auth/HandleOAuthCallbackActionTest.php new file mode 100644 index 00000000..cd8c6fd6 --- /dev/null +++ b/app-modules/identity/tests/Feature/Auth/HandleOAuthCallbackActionTest.php @@ -0,0 +1,172 @@ +instance(GitHubOAuthClient::class, new readonly class($githubUser) implements OAuthClientContract + { + public function __construct(private OAuthUserDTO $githubUser) {} + + public function redirectUrl(?OAuthStateDTO $state = null): string + { + return 'https://github.test/oauth'; + } + + public function auth(string $code): OAuthAccessDTO + { + return fakeOAuthAccess(); + } + + public function getAuthenticatedUser(OAuthAccessDTO $credentials): OAuthUserDTO + { + return $this->githubUser; + } + }); +} + +test('links a second provider to the logged-in user even when the email differs', function (): void { + // A user who originally signed up via Discord (discord@discord.com)... + $tenant = Tenant::factory()->create(); + $discordUser = User::factory()->create(['email' => 'discord@discord.com']); + + ExternalIdentity::factory()->create([ + 'tenant_id' => $tenant->id, + 'model_type' => (new User)->getMorphClass(), + 'model_id' => $discordUser->id, + 'connected_by' => $discordUser->id, + 'provider' => IdentityProvider::Discord, + 'external_account_id' => 'discord-123', + ]); + + actingAs($discordUser); + + // ...now logs in with GitHub, whose account carries a *different* email. + bindFakeGithubClient(fakeGithubUser(email: 'github@github.com', providerId: 'gh-999')); + + $state = new OAuthStateDTO( + intent: OAuthIntent::Link, + provider: IdentityProvider::GitHub, + panel: 'app', + tenant: $tenant->slug, + returnUrl: 'https://he4rt.test/app', + ); + + $result = resolve(HandleOAuthCallbackAction::class) + ->execute($state, IdentityProvider::GitHub, 'auth-code'); + + // The flow links to the existing logged-in user — it does not fork a new + // one, and the differing email is a non-event (no merge conflict). + expect($result->intent)->toBe(OAuthIntent::Link) + ->and($result->user->id)->toBe($discordUser->id) + ->and($result->mergeConflict)->toBeNull(); + + // The current user keeps their original email column untouched. + expect($discordUser->fresh()->email)->toBe('discord@discord.com'); + + // No user is created from the GitHub email — nothing was forked. + expect(User::query()->where('email', 'github@github.com')->exists())->toBeFalse(); + + // The GitHub identity is attached to the Discord user, and the GitHub email + // lives only in the identity metadata. + $githubIdentity = ExternalIdentity::query() + ->where('provider', IdentityProvider::GitHub) + ->where('external_account_id', 'gh-999') + ->first(); + + expect($githubIdentity)->not->toBeNull() + ->and($githubIdentity->model_id)->toBe($discordUser->id) + ->and($githubIdentity->metadata['email'])->toBe('github@github.com'); +}); + +test('forks a separate account when a logged-out github login has a different email', function (): void { + // Same starting point — a Discord user (discord@discord.com). + $tenant = Tenant::factory()->create(); + $discordUser = User::factory()->create(['email' => 'discord@discord.com']); + + ExternalIdentity::factory()->create([ + 'tenant_id' => $tenant->id, + 'model_type' => (new User)->getMorphClass(), + 'model_id' => $discordUser->id, + 'connected_by' => $discordUser->id, + 'provider' => IdentityProvider::Discord, + 'external_account_id' => 'discord-123', + ]); + + // ...but this time they are LOGGED OUT (no actingAs) and sign in from the + // login page with GitHub, intent = Login. With no shared email and no prior + // GitHub link, there is nothing to correlate on, so a new account is forked + // (Option B: email is the only merge key on the Login path). + bindFakeGithubClient(fakeGithubUser(email: 'github@github.com', providerId: 'gh-999')); + + $state = new OAuthStateDTO( + intent: OAuthIntent::Login, + provider: IdentityProvider::GitHub, + panel: 'app', + tenant: $tenant->slug, + returnUrl: 'https://he4rt.test/app', + ); + + $result = resolve(HandleOAuthCallbackAction::class) + ->execute($state, IdentityProvider::GitHub, 'auth-code'); + + // A brand-new user is created — NOT the existing Discord user. + expect($result->intent)->toBe(OAuthIntent::Login) + ->and($result->user->id)->not->toBe($discordUser->id) + ->and($result->user->email)->toBe('github@github.com'); + + // The original Discord account is left completely untouched. + expect($discordUser->fresh()->email)->toBe('discord@discord.com'); + + // The GitHub identity is attached to the new forked user, not the Discord one. + $githubIdentity = ExternalIdentity::query() + ->where('provider', IdentityProvider::GitHub) + ->where('external_account_id', 'gh-999') + ->first(); + + expect($githubIdentity)->not->toBeNull() + ->and($githubIdentity->model_id)->toBe($result->user->id) + ->and($githubIdentity->model_id)->not->toBe($discordUser->id); + + assertDatabaseCount(User::class, 3); +});