From 89980b1f0ef6bab907f8eb333f8b7ed5362d5036 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 18 Dec 2025 15:46:03 +0400 Subject: [PATCH 1/6] Enforce email verification when linking OAuth2 --- app/controllers/api/account.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 3e2512c073..29fc387d58 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1639,9 +1639,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $failureRedirect(Exception::USER_UNAUTHORIZED, 'OAuth provider failed to return email.'); } - /** - * Is verified is not used yet, since we don't know after an account is created anymore if it was verified or not. - */ $isVerified = $oauth2->isEmailVerified($accessToken); $identity = $dbForProject->findOne('identities', [ @@ -1653,12 +1650,16 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') $user = $dbForProject->getDocument('users', $identity->getAttribute('userId')); } - // If user is not found, check if there is an identity with the same provider user ID + // If user is not found, check if there is a user with the same email + // Only allow connecting to existing account if OAuth provider verified the email if ($user === false || $user->isEmpty()) { $userWithEmail = $dbForProject->findOne('users', [ Query::equal('email', [$email]), ]); if (!$userWithEmail->isEmpty()) { + if (!$isVerified) { + $failureRedirect(Exception::USER_OAUTH2_BAD_REQUEST, 'OAuth provider did not verify the email address.'); + } $user->setAttributes($userWithEmail->getArrayCopy()); } } From 144e88452e63514dfbd0fa2f1cc9bc1f44101f8d Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 18 Dec 2025 15:49:59 +0400 Subject: [PATCH 2/6] Use general bad request for unverified OAuth email --- app/controllers/api/account.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 29fc387d58..65af15d1fe 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1658,7 +1658,7 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') ]); if (!$userWithEmail->isEmpty()) { if (!$isVerified) { - $failureRedirect(Exception::USER_OAUTH2_BAD_REQUEST, 'OAuth provider did not verify the email address.'); + $failureRedirect(Exception::GENERAL_BAD_REQUEST); } $user->setAttributes($userWithEmail->getArrayCopy()); } From bae194e866f37db4bd3a0a178311bb39e6d250ac Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 18 Dec 2025 15:59:33 +0400 Subject: [PATCH 3/6] Link account by email during OAuth --- app/controllers/api/account.php | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 65af15d1fe..0b279b5534 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1664,6 +1664,20 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') } } + // If user is not found, check if there is an identity with the same email + // Only allow connecting to existing account if OAuth provider verified the email + if ($user === false || $user->isEmpty()) { + $identityWithMatchingEmail = $dbForProject->findOne('identities', [ + Query::equal('providerEmail', [$email]), + ]); + if (!$identityWithMatchingEmail->isEmpty()) { + if (!$isVerified) { + $failureRedirect(Exception::GENERAL_BAD_REQUEST); + } + $user->setAttributes($dbForProject->getDocument('users', $identityWithMatchingEmail->getAttribute('userId'))->getArrayCopy()); + } + } + if ($user === false || $user->isEmpty()) { // Last option -> create the user $limit = $project->getAttribute('auths', [])['limit'] ?? 0; @@ -1675,14 +1689,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') } } - // Makes sure this email is not already used in another identity - $identityWithMatchingEmail = $dbForProject->findOne('identities', [ - Query::equal('providerEmail', [$email]), - ]); - if (!$identityWithMatchingEmail->isEmpty()) { - $failureRedirect(Exception::GENERAL_BAD_REQUEST); /** Return a generic bad request to prevent exposing existing accounts */ - } - try { $emailCanonical = new Email($email); } catch (Throwable) { @@ -1736,7 +1742,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') 'providerType' => MESSAGE_TYPE_EMAIL, 'identifier' => $email, ])); - } catch (Duplicate) { $failureRedirect(Exception::USER_ALREADY_EXISTS); } From d094d6a08140119973e624654522c909645ccbf1 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 18 Dec 2025 16:00:41 +0400 Subject: [PATCH 4/6] Remove OAuth email verification comments --- app/controllers/api/account.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 0b279b5534..b740cbe87d 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -1651,7 +1651,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') } // If user is not found, check if there is a user with the same email - // Only allow connecting to existing account if OAuth provider verified the email if ($user === false || $user->isEmpty()) { $userWithEmail = $dbForProject->findOne('users', [ Query::equal('email', [$email]), @@ -1665,7 +1664,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') } // If user is not found, check if there is an identity with the same email - // Only allow connecting to existing account if OAuth provider verified the email if ($user === false || $user->isEmpty()) { $identityWithMatchingEmail = $dbForProject->findOne('identities', [ Query::equal('providerEmail', [$email]), From 81b40659219397cbde290a12119d5af362056c1f Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 18 Dec 2025 16:44:04 +0400 Subject: [PATCH 5/6] Fix identity connecting - Add MockUnverified OAuth2 provider config - Add /v1/mock/tests/general/oauth2/user-unverified endpoint - Add MockUnverified class for unverified OAuth2 flow - Update Mock::isEmailVerified to respect user['verified'] flag - Add end-to-end tests for linking unverified and verified OAuth2 users - Enable stopOnFailure in phpunit.xml --- app/config/oAuthProviders.php | 11 ++ app/controllers/mock.php | 22 +++ phpunit.xml | 2 +- src/Appwrite/Auth/OAuth2/Mock.php | 4 +- src/Appwrite/Auth/OAuth2/MockUnverified.php | 30 ++++ .../Account/AccountCustomClientTest.php | 151 ++++++++++++++++++ 6 files changed, 218 insertions(+), 2 deletions(-) create mode 100644 src/Appwrite/Auth/OAuth2/MockUnverified.php diff --git a/app/config/oAuthProviders.php b/app/config/oAuthProviders.php index d8dfc807b1..e6acd08c54 100644 --- a/app/config/oAuthProviders.php +++ b/app/config/oAuthProviders.php @@ -462,4 +462,15 @@ return [ 'mock' => true, 'class' => 'Appwrite\\Auth\\OAuth2\\Mock', ], + 'mock-unverified' => [ + 'name' => 'MockUnverified', + 'developers' => 'https://appwrite.io', + 'icon' => 'icon-appwrite', + 'enabled' => true, + 'sandbox' => false, + 'form' => false, + 'beta' => false, + 'mock' => true, + 'class' => 'Appwrite\\Auth\\OAuth2\\MockUnverified', + ], ]; diff --git a/app/controllers/mock.php b/app/controllers/mock.php index d78bb61481..6f092a5d19 100644 --- a/app/controllers/mock.php +++ b/app/controllers/mock.php @@ -117,6 +117,28 @@ App::get('/v1/mock/tests/general/oauth2/user') 'id' => 1, 'name' => 'User Name', 'email' => 'useroauth@localhost.test', + 'verified' => true, + ]); + }); + +App::get('/v1/mock/tests/general/oauth2/user-unverified') + ->desc('OAuth2 User Unverified') + ->groups(['mock']) + ->label('scope', 'public') + ->label('docs', false) + ->param('token', '', new Text(100), 'OAuth2 Access Token.') + ->inject('response') + ->action(function (string $token, Response $response) { + + if ($token != '123456') { + throw new Exception(Exception::GENERAL_MOCK, 'Invalid token'); + } + + $response->json([ + 'id' => 2, + 'name' => 'User Name Unverified', + 'email' => 'useroauthunverified@localhost.test', + 'verified' => false, ]); }); diff --git a/phpunit.xml b/phpunit.xml index a8578995c1..215ee2d59d 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -6,7 +6,7 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="false" + stopOnFailure="true" > diff --git a/src/Appwrite/Auth/OAuth2/Mock.php b/src/Appwrite/Auth/OAuth2/Mock.php index 61ce41d1b7..8bbf3647a1 100644 --- a/src/Appwrite/Auth/OAuth2/Mock.php +++ b/src/Appwrite/Auth/OAuth2/Mock.php @@ -130,7 +130,9 @@ class Mock extends OAuth2 */ public function isEmailVerified(string $accessToken): bool { - return true; + $user = $this->getUser($accessToken); + + return $user['verified'] ?? true; } /** diff --git a/src/Appwrite/Auth/OAuth2/MockUnverified.php b/src/Appwrite/Auth/OAuth2/MockUnverified.php new file mode 100644 index 0000000000..93bea02416 --- /dev/null +++ b/src/Appwrite/Auth/OAuth2/MockUnverified.php @@ -0,0 +1,30 @@ +user)) { + $user = $this->request('GET', 'http://localhost/' . $this->version . '/mock/tests/general/oauth2/user-unverified?token=' . \urlencode($accessToken)); + + $this->user = \json_decode($user, true); + } + + return $this->user; + } +} diff --git a/tests/e2e/Services/Account/AccountCustomClientTest.php b/tests/e2e/Services/Account/AccountCustomClientTest.php index 457799f991..d3aa8a4845 100644 --- a/tests/e2e/Services/Account/AccountCustomClientTest.php +++ b/tests/e2e/Services/Account/AccountCustomClientTest.php @@ -2183,6 +2183,157 @@ class AccountCustomClientTest extends Scope $this->assertEquals('tuvwxyz', $response['body']['providerRefreshToken']); $this->assertNotEquals($initialExpiry, $response['body']['providerAccessTokenExpiry']); + // Clean up - delete the user + $response = $this->client->call(Client::METHOD_DELETE, '/users/' . $userId, array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ])); + + $this->assertEquals(204, $response['headers']['status-code']); + + return []; + } + + public function testOAuthUnverifiedEmailCannotLinkToExistingAccount() + { + $provider = 'mock-unverified'; + $appId = '1'; + $secret = '123456'; + + // First, create a user with the same email that the unverified OAuth will try to use + $email = 'useroauthunverified@localhost.test'; + $password = 'password'; + + $response = $this->client->call(Client::METHOD_POST, '/account', [ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], [ + 'userId' => ID::unique(), + 'email' => $email, + 'password' => $password, + ]); + + $this->assertEquals(201, $response['headers']['status-code']); + $existingUserId = $response['body']['$id']; + + // Enable the mock-unverified provider + $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $this->getProject()['$id'] . '/oauth2', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => 'console', + 'cookie' => 'a_session_console=' . $this->getRoot()['session'], + ]), [ + 'provider' => $provider, + 'appId' => $appId, + 'secret' => $secret, + 'enabled' => true, + ]); + + $this->assertEquals(200, $response['headers']['status-code']); + + // Attempt OAuth login with unverified email - should fail because existing user has same email + $response = $this->client->call(Client::METHOD_GET, '/account/sessions/oauth2/' . $provider, array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ]), [ + 'success' => 'http://localhost/v1/mock/tests/general/oauth2/success', + 'failure' => 'http://localhost/v1/mock/tests/general/oauth2/failure', + ]); + + $this->assertEquals(400, $response['headers']['status-code']); + $this->assertEquals('failure', $response['body']['result']); + + // Clean up - delete the user + $response = $this->client->call(Client::METHOD_DELETE, '/users/' . $existingUserId, array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ])); + + $this->assertEquals(204, $response['headers']['status-code']); + + return []; + } + + public function testOAuthVerifiedEmailCanLinkToExistingAccount() + { + $provider = 'mock'; + $appId = '1'; + $secret = '123456'; + $email = 'useroauth@localhost.test'; + + // Create a user with the same email that the verified OAuth will try to use + $response = $this->client->call(Client::METHOD_POST, '/account', [ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ], [ + 'userId' => ID::unique(), + 'email' => $email, + 'password' => 'password', + ]); + + $this->assertEquals(201, $response['headers']['status-code']); + $existingUserId = $response['body']['$id']; + + // Enable the mock provider + $response = $this->client->call(Client::METHOD_PATCH, '/projects/' . $this->getProject()['$id'] . '/oauth2', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => 'console', + 'cookie' => 'a_session_console=' . $this->getRoot()['session'], + ]), [ + 'provider' => $provider, + 'appId' => $appId, + 'secret' => $secret, + 'enabled' => true, + ]); + + $this->assertEquals(200, $response['headers']['status-code']); + + // Attempt OAuth login with verified email - should succeed and link to existing account + $response = $this->client->call(Client::METHOD_GET, '/account/sessions/oauth2/' . $provider, array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + ]), [ + 'success' => 'http://localhost/v1/mock/tests/general/oauth2/success', + 'failure' => 'http://localhost/v1/mock/tests/general/oauth2/failure', + ]); + + $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEquals('success', $response['body']['result']); + + // Verify the OAuth identity was linked to the existing user + $sessionCookieKey = 'a_session_' . $this->getProject()['$id']; + $session = $response['cookies'][$sessionCookieKey]; + + $response = $this->client->call(Client::METHOD_GET, '/account', array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'cookie' => 'a_session_' . $this->getProject()['$id'] . '=' . $session, + ])); + + $this->assertEquals(200, $response['headers']['status-code']); + $this->assertEquals($existingUserId, $response['body']['$id']); + $this->assertEquals($email, $response['body']['email']); + + // Clean up - delete the user + $response = $this->client->call(Client::METHOD_DELETE, '/users/' . $existingUserId, array_merge([ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $this->getProject()['$id'], + 'x-appwrite-key' => $this->getProject()['apiKey'], + ])); + + $this->assertEquals(204, $response['headers']['status-code']); + return []; } From e61f3c7a42365132cfafb360c04d1474d0b82785 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 18 Dec 2025 18:03:02 +0400 Subject: [PATCH 6/6] Update phpunit.xml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- phpunit.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit.xml b/phpunit.xml index 215ee2d59d..a8578995c1 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -6,7 +6,7 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="true" + stopOnFailure="false" >