diff --git a/.env b/.env index d52d8ce38c7..419f8a9aaac 100644 --- a/.env +++ b/.env @@ -183,7 +183,7 @@ _APP_VCS_GITHUB_APP_NAME= _APP_VCS_GITHUB_CLIENT_ID= _APP_VCS_GITHUB_CLIENT_SECRET= _APP_VCS_GITHUB_PRIVATE_KEY=disabled -_APP_VCS_GITHUB_WEBHOOK_SECRET= +_APP_VCS_GITHUB_WEBHOOK_SECRET=github-webhook-secret _APP_VCS_GITEA_ENDPOINT=http://gitea:3000 _APP_VCS_GITEA_BROWSER_ENDPOINT=http://localhost:9515 _APP_VCS_GITEA_CLIENT_ID= diff --git a/app/config/vcs.php b/app/config/vcs.php index 3b2805cd9c1..dd03da37d81 100644 --- a/app/config/vcs.php +++ b/app/config/vcs.php @@ -28,7 +28,7 @@ 'appId' => ['required' => true, 'envVariable' => '_APP_VCS_GITHUB_APP_ID'], 'clientId' => ['required' => true, 'envVariable' => '_APP_VCS_GITHUB_CLIENT_ID'], 'clientSecret' => ['required' => true, 'envVariable' => '_APP_VCS_GITHUB_CLIENT_SECRET'], - 'webhookSecret' => ['required' => false, 'envVariable' => '_APP_VCS_GITHUB_WEBHOOK_SECRET'], + 'webhookSecret' => ['required' => true, 'envVariable' => '_APP_VCS_GITHUB_WEBHOOK_SECRET'], ], ], 'gitea' => [ @@ -45,8 +45,6 @@ 'endpoint' => ['required' => true, 'envVariable' => '_APP_VCS_GITEA_ENDPOINT'], 'clientId' => ['required' => true, 'envVariable' => '_APP_VCS_GITEA_CLIENT_ID'], 'clientSecret' => ['required' => true, 'envVariable' => '_APP_VCS_GITEA_CLIENT_SECRET'], - // Unlike GitHub's legacy optional secret, Gitea webhooks must - // always have a shared secret because Appwrite creates them directly. 'webhookSecret' => ['required' => true, 'envVariable' => '_APP_VCS_GITEA_WEBHOOK_SECRET'], ], ], diff --git a/app/controllers/api/account.php b/app/controllers/api/account.php index 17f44142d8d..8128caafcda 100644 --- a/app/controllers/api/account.php +++ b/app/controllers/api/account.php @@ -3401,7 +3401,8 @@ ->inject('store') ->inject('proofForToken') ->inject('project') - ->action(function (int $duration, Request $request, Response $response, User $user, Store $store, ProofsToken $proofForToken, Document $project) { + ->inject('mode') + ->action(function (int $duration, Request $request, Response $response, User $user, Store $store, ProofsToken $proofForToken, Document $project, string $mode) { if (!empty($request->getHeaderLine('x-appwrite-jwt', ''))) { throw new Exception(Exception::USER_JWT_CREATION_DENIED); } @@ -3418,7 +3419,8 @@ ->setStatusCode(Response::STATUS_CODE_CREATED) ->dynamic(new Document([ 'jwt' => $jwt->encode([ - 'projectId' => $project->getId(), + // In admin mode the session is a console session, whatever project is being managed. + 'projectId' => $mode === APP_MODE_ADMIN ? 'console' : $project->getId(), 'userId' => $user->getId(), 'sessionId' => $sessionId, ]) diff --git a/app/init/mqtt/connection.php b/app/init/mqtt/connection.php index 13898991877..abff49fbe09 100644 --- a/app/init/mqtt/connection.php +++ b/app/init/mqtt/connection.php @@ -49,6 +49,15 @@ $userId = $payload['userId'] ?? ''; $sessionId = $payload['sessionId'] ?? ''; + // Same binding as the HTTP and realtime user resources: a JWT is only good for + // the project that minted it, and one minted before the projectId claim existed + // only when it names a session. + $jwtProjectId = $payload['projectId'] ?? ''; + $bound = $jwtProjectId !== '' ? $jwtProjectId === $project->getId() : $sessionId !== ''; + if (!$bound) { + return new User([]); + } + /** @var User $user */ $user = $dbForProject->getDocument('users', $userId); diff --git a/app/init/realtime/connection.php b/app/init/realtime/connection.php index 0c2da225d4c..50942bb8c1b 100644 --- a/app/init/realtime/connection.php +++ b/app/init/realtime/connection.php @@ -239,15 +239,15 @@ // signup, so a token is only good for the project that minted it. Tokens // minted before the projectId claim existed are accepted only when bound // to a session, whose ID the server generated and no other project holds. + // An unbound token authenticates nobody rather than failing the request: + // a function domain resolves to the console, and clients send their + // project's JWT there for the function to read. $jwtProjectId = $payload['projectId'] ?? ''; $expectedProjectId = $mode === APP_MODE_ADMIN ? $console->getId() : $project->getId(); $bound = $jwtProjectId !== '' ? $jwtProjectId === $expectedProjectId : !empty($payload['sessionId']); - if (!$bound) { - throw new Exception(Exception::USER_JWT_INVALID, 'JWT was not issued for this project.'); - } $jwtUserId = $payload['userId'] ?? ''; - if (!empty($jwtUserId)) { + if ($bound && !empty($jwtUserId)) { if ($mode === APP_MODE_ADMIN) { /** @var User $user */ $user = $dbForPlatform->getDocument('users', $jwtUserId); diff --git a/app/init/resources/request.php b/app/init/resources/request.php index 31a84a79884..791e3bf3f57 100644 --- a/app/init/resources/request.php +++ b/app/init/resources/request.php @@ -530,15 +530,15 @@ // signup, so a token is only good for the project that minted it. Tokens // minted before the projectId claim existed are accepted only when bound // to a session, whose ID the server generated and no other project holds. + // An unbound token authenticates nobody rather than failing the request: + // a function domain resolves to the console, and clients send their + // project's JWT there for the function to read. $jwtProjectId = $payload['projectId'] ?? ''; $expectedProjectId = $mode === APP_MODE_ADMIN ? $console->getId() : $project->getId(); $bound = $jwtProjectId !== '' ? $jwtProjectId === $expectedProjectId : ! empty($payload['sessionId']); - if (! $bound) { - throw new Exception(Exception::USER_JWT_INVALID, 'JWT was not issued for this project.'); - } $jwtUserId = $payload['userId'] ?? ''; - if (! empty($jwtUserId)) { + if ($bound && ! empty($jwtUserId)) { if ($mode === APP_MODE_ADMIN) { /** @var User $user */ $user = $dbForPlatform->getDocument('users', $jwtUserId); diff --git a/src/Appwrite/Platform/Modules/VCS/Http/GitHub/Events/Create.php b/src/Appwrite/Platform/Modules/VCS/Http/GitHub/Events/Create.php index 070b67f008e..d798178e27b 100644 --- a/src/Appwrite/Platform/Modules/VCS/Http/GitHub/Events/Create.php +++ b/src/Appwrite/Platform/Modules/VCS/Http/GitHub/Events/Create.php @@ -75,11 +75,11 @@ public function action( $signature = $request->getHeaderLine('x-hub-signature-256', ''); $secretKey = $vcsWebhookSecret('github'); - $valid = empty($secretKey) ? true : $vcs->validateWebhookEvent($payload, $signature, $secretKey); + $valid = !empty($secretKey) && $vcs->validateWebhookEvent($payload, $signature, $secretKey); Span::add('vcs.github.event.signature.valid', $valid); if (!$valid) { - throw new Exception(Exception::GENERAL_ACCESS_FORBIDDEN, "Invalid webhook payload signature. Please make sure the webhook secret has same value in your GitHub app and in the _APP_VCS_GITHUB_WEBHOOK_SECRET environment variable"); + throw new Exception(Exception::GENERAL_ACCESS_FORBIDDEN, 'Invalid webhook payload signature. Please make sure the webhook secret has same value in your GitHub app and in the _APP_VCS_GITHUB_WEBHOOK_SECRET environment variable'); } $parsedPayloads = $vcs->getEvents($event, $payload); diff --git a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php index 7f2e48ab9c2..304bdd6b52b 100644 --- a/tests/e2e/Services/Functions/FunctionsCustomServerTest.php +++ b/tests/e2e/Services/Functions/FunctionsCustomServerTest.php @@ -2982,6 +2982,58 @@ public function testCookieExecution() $this->cleanupFunction($functionId); } + /** + * A function domain resolves to the console, and clients send their own project's + * user JWT there for the function to read. That JWT authenticates nobody at the + * console, but it must not stop the request from reaching the function. + */ + public function testFunctionsDomainServesRequestCarryingAnotherProjectsJwt(): void + { + $functionId = $this->setupFunction([ + 'functionId' => ID::unique(), + 'name' => 'Domain with foreign JWT', + 'runtime' => 'node-22', + 'entrypoint' => 'index.js', + 'timeout' => 15, + 'execute' => ['any'], + ]); + $domain = $this->setupFunctionDomain($functionId); + $this->setupDeployment($functionId, [ + 'code' => $this->packageFunction('cookies'), + 'activate' => true, + ]); + + $otherProject = $this->getProject(true); + $user = $this->client->call(Client::METHOD_POST, '/users', [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $otherProject['$id'], + 'x-appwrite-key' => $otherProject['apiKey'], + ], [ + 'userId' => ID::unique(), + 'email' => 'foreign-jwt-' . ID::unique() . '@appwrite.io', + 'password' => 'password', + ]); + $this->assertEquals(201, $user['headers']['status-code']); + + $jwt = $this->client->call(Client::METHOD_POST, '/users/' . $user['body']['$id'] . '/jwts', [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $otherProject['$id'], + 'x-appwrite-key' => $otherProject['apiKey'], + ]); + $this->assertEquals(201, $jwt['headers']['status-code']); + + $proxyClient = new Client(); + $proxyClient->setEndpoint('http://' . $domain); + + $response = $proxyClient->call(Client::METHOD_GET, '/', [ + 'content-type' => 'application/json', + 'x-appwrite-jwt' => $jwt['body']['jwt'], + ]); + $this->assertEquals(200, $response['headers']['status-code']); + + $this->cleanupFunction($functionId); + } + public function testFunctionsDomain() { $functionId = $this->setupFunction([ diff --git a/tests/e2e/Services/Mqtt/MqttServerTest.php b/tests/e2e/Services/Mqtt/MqttServerTest.php index ff48fa68d5e..e2c7fcdefc7 100644 --- a/tests/e2e/Services/Mqtt/MqttServerTest.php +++ b/tests/e2e/Services/Mqtt/MqttServerTest.php @@ -125,6 +125,42 @@ public function testBlockedUserConnectRejected(): void $subscriber->disconnect(); } + public function testJwtFromAnotherProjectConnectRejected(): void + { + $projectId = $this->getProject()['$id']; + ['userId' => $userId] = $this->createUser(); + + // Another project holds a user with the same ID and mints a session-less JWT for it. + $otherProject = $this->getProject(true); + $user = $this->client->call(Client::METHOD_POST, '/users', [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $otherProject['$id'], + 'x-appwrite-key' => $otherProject['apiKey'], + ], [ + 'userId' => $userId, + 'email' => 'mqtt-other-' . $userId . '@appwrite.io', + 'password' => 'password', + ]); + $this->assertEquals(201, $user['headers']['status-code']); + + $jwt = $this->client->call(Client::METHOD_POST, '/users/' . $userId . '/jwts', [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $otherProject['$id'], + 'x-appwrite-key' => $otherProject['apiKey'], + ]); + $this->assertEquals(201, $jwt['headers']['status-code']); + + // Still good where it was minted. + $own = new MqttSubscriber(self::BROKER_HOST, self::BROKER_PORT); + $this->assertSame(0, $own->connect($otherProject['$id'], $jwt['body']['jwt'], 'e2e-jwt-own-' . $userId, cleanStart: true)); + $own->disconnect(); + + // Test for FAILURE: replayed against this project, it must not authenticate as this project's user. + $subscriber = new MqttSubscriber(self::BROKER_HOST, self::BROKER_PORT); + $this->assertSame(0x87, $subscriber->connect($projectId, $jwt['body']['jwt'], 'e2e-jwt-cross-' . $userId, cleanStart: true)); + $subscriber->disconnect(); + } + public function testSessionAuthConnect(): void { $projectId = $this->getProject()['$id']; diff --git a/tests/e2e/Services/Users/UsersCustomServerTest.php b/tests/e2e/Services/Users/UsersCustomServerTest.php index c78feb737fe..7a2121e4fb1 100644 --- a/tests/e2e/Services/Users/UsersCustomServerTest.php +++ b/tests/e2e/Services/Users/UsersCustomServerTest.php @@ -4,11 +4,13 @@ namespace Tests\E2E\Services\Users; +use Ahc\Jwt\JWT; use Tests\E2E\Client; use Tests\E2E\Scopes\ProjectCustom; use Tests\E2E\Scopes\Scope; use Tests\E2E\Scopes\SideServer; use Utopia\Database\Helpers\ID; +use Utopia\System\System; final class UsersCustomServerTest extends Scope { @@ -77,4 +79,103 @@ public function testUserJWTIsBoundToItsProject(): void $this->assertSame(401, $console['headers']['status-code']); $this->assertArrayNotHasKey('email', $console['body']); } + + /** + * JWTs minted before the projectId claim existed are still in flight at deploy. They are + * accepted only when they name a session, and only while that session lives in the project. + */ + public function testLegacyJWTWithoutProjectClaimIsBoundBySession(): void + { + $project = $this->getProject(); + $otherProject = $this->getProject(true); + $userId = ID::unique(); + + foreach ([$project, $otherProject] as $p) { + $user = $this->client->call(Client::METHOD_POST, '/users', [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $p['$id'], + 'x-appwrite-key' => $p['apiKey'], + ], [ + 'userId' => $userId, + 'email' => 'legacy-' . ID::unique() . '@appwrite.io', + 'password' => 'password', + ]); + $this->assertSame(201, $user['headers']['status-code']); + } + + $session = $this->client->call(Client::METHOD_POST, '/users/' . $userId . '/sessions', [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $project['$id'], + 'x-appwrite-key' => $project['apiKey'], + ]); + $this->assertSame(201, $session['headers']['status-code']); + $sessionId = $session['body']['$id']; + + // The payload shape every minter produced before this change. + $encoder = new JWT(System::getEnv('_APP_OPENSSL_KEY_V1'), 'HS256', 900, 0); + $withSession = $encoder->encode(['userId' => $userId, 'sessionId' => $sessionId]); + $withoutSession = $encoder->encode(['userId' => $userId, 'sessionId' => '']); + + $account = fn (string $projectId, string $jwt) => $this->client->call(Client::METHOD_GET, '/account', [ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $projectId, + 'x-appwrite-jwt' => $jwt, + ]); + + $own = $account($project['$id'], $withSession); + $this->assertSame(200, $own['headers']['status-code']); + $this->assertSame($userId, $own['body']['$id']); + + // The other project's user has the same ID but not the session. + $this->assertSame(401, $account($otherProject['$id'], $withSession)['headers']['status-code']); + + // Without a session nothing ties it to a project. + $this->assertSame(401, $account($project['$id'], $withoutSession)['headers']['status-code']); + $this->assertSame(401, $account($otherProject['$id'], $withoutSession)['headers']['status-code']); + + $deleted = $this->client->call(Client::METHOD_DELETE, '/users/' . $userId . '/sessions/' . $sessionId, [ + 'content-type' => 'application/json', + 'x-appwrite-project' => $project['$id'], + 'x-appwrite-key' => $project['apiKey'], + ]); + $this->assertSame(204, $deleted['headers']['status-code']); + + $this->assertSame(401, $account($project['$id'], $withSession)['headers']['status-code']); + } + + /** + * In admin mode the caller holds a console session, so a JWT created there belongs to the + * console and authenticates admin-mode requests for any project the console user manages. + */ + public function testAdminModeJWTIsBoundToConsole(): void + { + $project = $this->getProject(); + $admin = [ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $project['$id'], + 'x-appwrite-mode' => 'admin', + ]; + + $jwt = $this->client->call(Client::METHOD_POST, '/account/jwts', array_merge($admin, [ + 'cookie' => 'a_session_console=' . $this->getRoot()['session'], + ])); + $this->assertSame(201, $jwt['headers']['status-code']); + + $account = $this->client->call(Client::METHOD_GET, '/account', array_merge($admin, [ + 'x-appwrite-jwt' => $jwt['body']['jwt'], + ])); + $this->assertSame(200, $account['headers']['status-code']); + $this->assertSame($this->getRoot()['$id'], $account['body']['$id']); + + // Outside admin mode the same token would resolve the console user's ID in the project's own users. + $client = $this->client->call(Client::METHOD_GET, '/account', [ + 'origin' => 'http://localhost', + 'content-type' => 'application/json', + 'x-appwrite-project' => $project['$id'], + 'x-appwrite-jwt' => $jwt['body']['jwt'], + ]); + $this->assertSame(401, $client['headers']['status-code']); + } } diff --git a/tests/e2e/Services/VCSGitHub/VCSGitHubConsoleClientTest.php b/tests/e2e/Services/VCSGitHub/VCSGitHubConsoleClientTest.php index bc77dd5adb9..320c0d2bd67 100644 --- a/tests/e2e/Services/VCSGitHub/VCSGitHubConsoleClientTest.php +++ b/tests/e2e/Services/VCSGitHub/VCSGitHubConsoleClientTest.php @@ -667,15 +667,12 @@ private function sendPushEvent(): array $headers = [ 'content-type' => 'application/json', 'x-github-event' => 'push', - ]; - $secret = System::getEnv('_APP_VCS_GITHUB_WEBHOOK_SECRET', ''); - if (!empty($secret)) { - $headers['x-hub-signature-256'] = 'sha256=' . \hash_hmac( + 'x-hub-signature-256' => 'sha256=' . \hash_hmac( 'sha256', \json_encode($payload, JSON_THROW_ON_ERROR), - $secret, - ); - } + System::getEnv('_APP_VCS_GITHUB_WEBHOOK_SECRET', ''), + ), + ]; // GitHub webhooks are public and intentionally have no x-appwrite-project header. $event = $this->client->call(Client::METHOD_POST, '/vcs/github/events', $headers, $payload); @@ -683,6 +680,31 @@ private function sendPushEvent(): array return ['event' => $event, 'commit' => $commit]; } + public function testCreateEventWithInvalidSignature(): void + { + $payload = [ + 'action' => 'deleted', + 'installation' => ['id' => (int) $this->providerInstallationId], + ]; + + $event = $this->client->call(Client::METHOD_POST, '/vcs/github/events', [ + 'content-type' => 'application/json', + 'x-github-event' => 'installation', + ], $payload); + + $this->assertEquals(403, $event['headers']['status-code']); + $this->assertEquals('general_access_forbidden', $event['body']['type']); + + $event = $this->client->call(Client::METHOD_POST, '/vcs/github/events', [ + 'content-type' => 'application/json', + 'x-github-event' => 'installation', + 'x-hub-signature-256' => 'sha256=' . \hash_hmac('sha256', \json_encode($payload, JSON_THROW_ON_ERROR), 'wrong-secret'), + ], $payload); + + $this->assertEquals(403, $event['headers']['status-code']); + $this->assertEquals('general_access_forbidden', $event['body']['type']); + } + public function testGitHubPushCreatesFunctionDeploymentWithoutProjectHeader(): void { $data = $this->setupFunctionUsingVCS(); diff --git a/tests/unit/Vcs/FactoryTest.php b/tests/unit/Vcs/FactoryTest.php index 5629ebdf6db..7877fd1396c 100644 --- a/tests/unit/Vcs/FactoryTest.php +++ b/tests/unit/Vcs/FactoryTest.php @@ -192,13 +192,18 @@ public function testFromProviderForBrowserFallsBackToTheApiEndpoint(): void public function testGetWebhookSecret(): void { - $factory = new Factory($this->cache(), ['github' => $this->githubEntry()]); + $entry = [ + 'adapter' => GitHub::class, + 'variables' => [ + 'webhookSecret' => ['required' => true, 'envVariable' => '_APP_VCS_TEST_TOKEN'], + ], + ]; + $factory = new Factory($this->cache(), ['github' => $entry]); $this->assertSame('', $factory->getWebhookSecret('github')); - \putenv('_APP_VCS_GITHUB_WEBHOOK_SECRET=hunter2'); + \putenv('_APP_VCS_TEST_TOKEN=hunter2'); $this->assertSame('hunter2', $factory->getWebhookSecret('github')); - \putenv('_APP_VCS_GITHUB_WEBHOOK_SECRET'); } protected function cache(): Cache @@ -220,7 +225,7 @@ protected function githubEntry(): array 'appId' => ['required' => true, 'envVariable' => '_APP_VCS_GITHUB_APP_ID'], 'clientId' => ['required' => true, 'envVariable' => '_APP_VCS_GITHUB_CLIENT_ID'], 'clientSecret' => ['required' => true, 'envVariable' => '_APP_VCS_GITHUB_CLIENT_SECRET'], - 'webhookSecret' => ['required' => false, 'envVariable' => '_APP_VCS_GITHUB_WEBHOOK_SECRET'], + 'webhookSecret' => ['required' => true, 'envVariable' => '_APP_VCS_GITHUB_WEBHOOK_SECRET'], ], ]; }