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/api/account.php b/app/controllers/api/account.php index 3e2512c073..b740cbe87d 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,16 +1650,32 @@ 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 if ($user === false || $user->isEmpty()) { $userWithEmail = $dbForProject->findOne('users', [ Query::equal('email', [$email]), ]); if (!$userWithEmail->isEmpty()) { + if (!$isVerified) { + $failureRedirect(Exception::GENERAL_BAD_REQUEST); + } $user->setAttributes($userWithEmail->getArrayCopy()); } } + // If user is not found, check if there is an identity with the same 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; @@ -1674,14 +1687,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) { @@ -1735,7 +1740,6 @@ App::get('/v1/account/sessions/oauth2/:provider/redirect') 'providerType' => MESSAGE_TYPE_EMAIL, 'identifier' => $email, ])); - } catch (Duplicate) { $failureRedirect(Exception::USER_ALREADY_EXISTS); } 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/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 []; }