Fix write only security

This commit is contained in:
Matej Bačo
2026-04-27 18:04:22 +02:00
parent 50d86c5b5d
commit 015aee087a
9 changed files with 37 additions and 118 deletions
@@ -158,20 +158,8 @@ class Update extends Base
$project = $this->persistCredentials($project, $dbForPlatform, $authorization, $serviceId, $encodedSecret, $enabled);
$oAuthProviders = $project->getAttribute('oAuthProviders', []);
$storedRaw = $oAuthProviders[$providerId . 'Secret'] ?? '';
$decoded = [];
if (!empty($storedRaw)) {
$decoded = \json_decode($storedRaw, true) ?: [];
}
$response->dynamic(new Document([
'$id' => $providerId,
'enabled' => $oAuthProviders[$providerId . 'Enabled'] ?? false,
static::getClientIdParamName() => $oAuthProviders[$providerId . 'Appid'] ?? '',
'keyId' => $decoded['keyID'] ?? '',
'teamId' => $decoded['teamID'] ?? '',
'p8File' => $decoded['p8'] ?? '',
]), static::getResponseModel());
// Reuse buildReadResponse to keep PATCH/GET shapes identical and
// guarantee keyId/teamId/p8File are write-only on every response path.
$response->dynamic($this->buildReadResponse($project), static::getResponseModel());
}
}
@@ -146,19 +146,8 @@ class Update extends Base
$project = $this->persistCredentials($project, $dbForPlatform, $authorization, $clientId, $encodedSecret, $enabled);
$oAuthProviders = $project->getAttribute('oAuthProviders', []);
$storedRaw = $oAuthProviders[$providerId . 'Secret'] ?? '';
$decoded = [];
if (!empty($storedRaw)) {
$decoded = \json_decode($storedRaw, true) ?: [];
}
$response->dynamic(new Document([
'$id' => $providerId,
'enabled' => $oAuthProviders[$providerId . 'Enabled'] ?? false,
static::getClientIdParamName() => $oAuthProviders[$providerId . 'Appid'] ?? '',
static::getClientSecretParamName() => $decoded['clientSecret'] ?? '',
'endpoint' => $decoded['auth0Domain'] ?? '',
]), static::getResponseModel());
// Reuse buildReadResponse to keep PATCH/GET shapes identical and
// guarantee the clientSecret is write-only on every response path.
$response->dynamic($this->buildReadResponse($project), static::getResponseModel());
}
}
@@ -143,19 +143,8 @@ class Update extends Base
$project = $this->persistCredentials($project, $dbForPlatform, $authorization, $clientId, $encodedSecret, $enabled);
$oAuthProviders = $project->getAttribute('oAuthProviders', []);
$storedRaw = $oAuthProviders[$providerId . 'Secret'] ?? '';
$decoded = [];
if (!empty($storedRaw)) {
$decoded = \json_decode($storedRaw, true) ?: [];
}
$response->dynamic(new Document([
'$id' => $providerId,
'enabled' => $oAuthProviders[$providerId . 'Enabled'] ?? false,
static::getClientIdParamName() => $oAuthProviders[$providerId . 'Appid'] ?? '',
static::getClientSecretParamName() => $decoded['clientSecret'] ?? '',
'endpoint' => $decoded['authentikDomain'] ?? '',
]), static::getResponseModel());
// Reuse buildReadResponse to keep PATCH/GET shapes identical and
// guarantee the clientSecret is write-only on every response path.
$response->dynamic($this->buildReadResponse($project), static::getResponseModel());
}
}
@@ -311,16 +311,10 @@ abstract class Base extends Action
): void {
$project = $this->persistCredentials($project, $dbForPlatform, $authorization, $clientId, $clientSecret, $enabled);
$providerId = static::getProviderId();
$oAuthProviders = $project->getAttribute('oAuthProviders', []);
$queueForEvents->setParam('providerId', static::getProviderId());
$queueForEvents->setParam('providerId', $providerId);
$response->dynamic(new Document([
'$id' => $providerId,
'enabled' => $oAuthProviders[$providerId . 'Enabled'] ?? false,
static::getClientIdParamName() => $oAuthProviders[$providerId . 'Appid'] ?? '',
static::getClientSecretParamName() => $oAuthProviders[$providerId . 'Secret'] ?? '',
]), static::getResponseModel());
// Reuse buildReadResponse to keep PATCH/GET shapes identical and
// guarantee the clientSecret is write-only on every response path.
$response->dynamic($this->buildReadResponse($project), static::getResponseModel());
}
}
@@ -157,19 +157,8 @@ class Update extends Base
$project = $this->persistCredentials($project, $dbForPlatform, $authorization, $applicationId, $encodedSecret, $enabled);
$oAuthProviders = $project->getAttribute('oAuthProviders', []);
$storedRaw = $oAuthProviders[$providerId . 'Secret'] ?? '';
$decoded = [];
if (!empty($storedRaw)) {
$decoded = \json_decode($storedRaw, true) ?: [];
}
$response->dynamic(new Document([
'$id' => $providerId,
'enabled' => $oAuthProviders[$providerId . 'Enabled'] ?? false,
static::getClientIdParamName() => $oAuthProviders[$providerId . 'Appid'] ?? '',
static::getClientSecretParamName() => $decoded['clientSecret'] ?? '',
'endpoint' => $decoded['endpoint'] ?? '',
]), static::getResponseModel());
// Reuse buildReadResponse to keep PATCH/GET shapes identical and
// guarantee the secret is write-only on every response path.
$response->dynamic($this->buildReadResponse($project), static::getResponseModel());
}
}
@@ -153,19 +153,8 @@ class Update extends Base
$project = $this->persistCredentials($project, $dbForPlatform, $authorization, $applicationId, $encodedSecret, $enabled);
$oAuthProviders = $project->getAttribute('oAuthProviders', []);
$storedRaw = $oAuthProviders[$providerId . 'Secret'] ?? '';
$decoded = [];
if (!empty($storedRaw)) {
$decoded = \json_decode($storedRaw, true) ?: [];
}
$response->dynamic(new Document([
'$id' => $providerId,
'enabled' => $oAuthProviders[$providerId . 'Enabled'] ?? false,
static::getClientIdParamName() => $oAuthProviders[$providerId . 'Appid'] ?? '',
static::getClientSecretParamName() => $decoded['clientSecret'] ?? '',
'tenant' => $decoded['tenantID'] ?? '',
]), static::getResponseModel());
// Reuse buildReadResponse to keep PATCH/GET shapes identical and
// guarantee the applicationSecret is write-only on every response path.
$response->dynamic($this->buildReadResponse($project), static::getResponseModel());
}
}
@@ -183,22 +183,8 @@ class Update extends Base
$project = $this->persistCredentials($project, $dbForPlatform, $authorization, $clientId, $encodedSecret, $enabled);
$oAuthProviders = $project->getAttribute('oAuthProviders', []);
$storedRaw = $oAuthProviders[$providerId . 'Secret'] ?? '';
$decoded = [];
if (!empty($storedRaw)) {
$decoded = \json_decode($storedRaw, true) ?: [];
}
$response->dynamic(new Document([
'$id' => $providerId,
'enabled' => $oAuthProviders[$providerId . 'Enabled'] ?? false,
static::getClientIdParamName() => $oAuthProviders[$providerId . 'Appid'] ?? '',
static::getClientSecretParamName() => $decoded['clientSecret'] ?? '',
'wellKnownURL' => $decoded['wellKnownEndpoint'] ?? '',
'authorizationURL' => $decoded['authorizationEndpoint'] ?? '',
'tokenUrl' => $decoded['tokenEndpoint'] ?? '',
'userInfoUrl' => $decoded['userInfoEndpoint'] ?? '',
]), static::getResponseModel());
// Reuse buildReadResponse to keep PATCH/GET shapes identical and
// guarantee the clientSecret is write-only on every response path.
$response->dynamic($this->buildReadResponse($project), static::getResponseModel());
}
}
@@ -163,20 +163,8 @@ class Update extends Base
$project = $this->persistCredentials($project, $dbForPlatform, $authorization, $clientId, $encodedSecret, $enabled);
$oAuthProviders = $project->getAttribute('oAuthProviders', []);
$storedRaw = $oAuthProviders[$providerId . 'Secret'] ?? '';
$decoded = [];
if (!empty($storedRaw)) {
$decoded = \json_decode($storedRaw, true) ?: [];
}
$response->dynamic(new Document([
'$id' => $providerId,
'enabled' => $oAuthProviders[$providerId . 'Enabled'] ?? false,
static::getClientIdParamName() => $oAuthProviders[$providerId . 'Appid'] ?? '',
static::getClientSecretParamName() => $decoded['clientSecret'] ?? '',
'domain' => $decoded['oktaDomain'] ?? '',
'authorizationServerId' => $decoded['authorizationServerId'] ?? '',
]), static::getResponseModel());
// Reuse buildReadResponse to keep PATCH/GET shapes identical and
// guarantee the clientSecret is write-only on every response path.
$response->dynamic($this->buildReadResponse($project), static::getResponseModel());
}
}
+12 -5
View File
@@ -476,8 +476,10 @@ trait OAuth2Base
$this->assertSame(200, $response['headers']['status-code']);
$this->assertSame('apple', $response['body']['$id']);
$this->assertSame('ip.appwrite.app.web', $response['body']['serviceId']);
$this->assertSame('P4000000N8', $response['body']['keyId']);
$this->assertSame('D4000000R6', $response['body']['teamId']);
// keyId / teamId / p8File are write-only — PATCH response must not echo them back.
$this->assertSame('', $response['body']['keyId']);
$this->assertSame('', $response['body']['teamId']);
$this->assertSame('', $response['body']['p8File']);
$this->assertSame(false, $response['body']['enabled']);
// Cleanup
@@ -507,9 +509,12 @@ trait OAuth2Base
]);
$this->assertSame(200, $response['headers']['status-code']);
$this->assertSame('KEYUPDATED', $response['body']['keyId']);
$this->assertSame('TEAMSEED01', $response['body']['teamId']);
// serviceId is the (non-secret) clientId; keyId/teamId are write-only
// and must not surface in the response. Persistence of the merged
// values is verified separately via the enable-after-merge tests.
$this->assertSame('ip.appwrite.app.seed', $response['body']['serviceId']);
$this->assertSame('', $response['body']['keyId']);
$this->assertSame('', $response['body']['teamId']);
// Cleanup
$this->updateOAuth2('apple', [
@@ -539,7 +544,9 @@ trait OAuth2Base
'teamId' => 'TEAMROTATED',
]);
$this->assertSame(200, $teamOnly['headers']['status-code']);
$this->assertSame('TEAMROTATED', $teamOnly['body']['teamId']);
// teamId is write-only; verify only the non-secret serviceId echo.
// The actual merge is validated by the enable-after-merge call below.
$this->assertSame('', $teamOnly['body']['teamId']);
$this->assertSame('ip.appwrite.app.merge', $teamOnly['body']['serviceId']);
// Patch only `serviceId` — keyId/teamId/p8File live in the JSON blob