Development Keys #8378
Development Keys #8378
Conversation
✨ Benchmark results
⚡ Benchmark Comparison
|
There was a problem hiding this comment.
Copilot reviewed 22 out of 41 changed files in this pull request and generated no comments.
Files not reviewed (19)
- app/config/collections/platform.php: Language not supported
- app/config/specs/open-api3-latest-server.json: Language not supported
- app/config/specs/swagger2-latest-server.json: Language not supported
- app/controllers/api/account.php: Language not supported
- app/controllers/api/teams.php: Language not supported
- app/controllers/general.php: Language not supported
- app/controllers/shared/api.php: Language not supported
- app/init/database/filters.php: Language not supported
- app/init/resources.php: Language not supported
- src/Appwrite/Platform/Appwrite.php: Language not supported
- src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Create.php: Language not supported
- src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Delete.php: Language not supported
- src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Get.php: Language not supported
- src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Update.php: Language not supported
- src/Appwrite/Platform/Modules/Projects/Http/DevKeys/XList.php: Language not supported
- src/Appwrite/Platform/Modules/Projects/Module.php: Language not supported
- src/Appwrite/Platform/Modules/Projects/Services/Http.php: Language not supported
- src/Appwrite/SDK/Method.php: Language not supported
- src/Appwrite/Specification/Format/OpenAPI3.php: Language not supported
Comments suppressed due to low confidence (2)
.github/workflows/tests.yml:323
- In the 'e2e_dev_keys_shared_mode' job for 'Shared V2', _APP_DATABASE_SHARED_TABLES_V1 is set to an empty value. Please confirm that this configuration is intentional.
export _APP_DATABASE_SHARED_TABLES_V1=
.github/workflows/tests.yml:291
- The matrix definition includes a trailing comma after 'Shared V2'. While some YAML parsers accept trailing commas, consider removing it for compatibility with all environments.
matrix:
tables-mode: [
'Shared V1',
'Shared V2',
]
i think this was a one off case where the the rest of the tests are designed with |
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (3)
tests/e2e/Scopes/SideConsole.php (1)
7-14:⚠️ Potential issueIncomplete implementation of dev key parameter.
The
$devKeyparameter has been added to the method signature, but there's no implementation in the method body that uses this parameter. Based on the PR objectives and the pattern seen across other scopes, this method should conditionally include thex-appwrite-dev-keyheader when the parameter is true.Consider updating the implementation to use the parameter:
public function getHeaders(bool $devKey = true): array { return [ 'origin' => 'http://localhost', 'cookie' => 'a_session_console=' . $this->getRoot()['session'], 'x-appwrite-mode' => 'admin' + ...(($devKey && isset($this->getProject()['devKey'])) ? ['x-appwrite-dev-key' => $this->getProject()['devKey']] : []) ]; }tests/e2e/Scopes/SideServer.php (1)
12-17:⚠️ Potential issueIncomplete implementation of dev key parameter.
The
$devKeyparameter has been added to the method signature, but there's no implementation in the method body that uses this parameter. Based on the PR changes across other scopes, this method should conditionally include thex-appwrite-dev-keyheader when the parameter is true.Consider updating the implementation to use the parameter:
public function getHeaders(bool $devKey = false): array { return [ 'x-appwrite-key' => $this->getProject()['apiKey'] + ...(($devKey && isset($this->getProject()['devKey'])) ? ['x-appwrite-dev-key' => $this->getProject()['devKey']] : []) ]; }app/config/specs/open-api3-latest-console.json (1)
20710-21059:⚠️ Potential issueReview new
/projects/{projectId}/dev-keysendpoints
The Dev Keys CRUD paths are comprehensive, but I spotted a few documentation mismatches:
- The
descriptionfor listing dev keys ends with an extra single-quote (development.').- The
editURL forlistDevKeysis missing a slash aftermaster(should bemaster/docs/...).- The
queriesparameter is declared asstringyet hasdefault: []. Either change the type toarray[string]or update the default to an empty string.- Confirm the
"rate-limit": 0setting is intentional for development keys.- Validate that all referenced
demoandeditmarkdown files exist at the specified paths.
♻️ Duplicate comments (5)
src/Appwrite/Utopia/Response.php (1)
396-396: Consider changing 'keys' to 'devKeys' for consistencyThis line sets up the BaseList model for dev keys, but uses 'keys' as the property name in the third parameter, while the model name uses 'devKeys'. For consistency with naming conventions, consider using 'devKeys' for the property name.
- ->setModel(new BaseList('Dev Keys List', self::MODEL_DEV_KEY_LIST, 'keys', self::MODEL_DEV_KEY, true, false)) + ->setModel(new BaseList('Dev Keys List', self::MODEL_DEV_KEY_LIST, 'devKeys', self::MODEL_DEV_KEY, true, false))src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Delete.php (1)
65-68:⚠️ Potential issueUnreachable code in the next condition block.
The condition on line 65 already checks if
$key === false || $key->isEmpty()so the subsequent condition block would never be reached. This doesn't affect functionality but should be removed.if ($key === false || $key->isEmpty() || $key->getAttribute('projectInternalId') !== $project->getInternalId()) { throw new Exception(Exception::KEY_NOT_FOUND); } - - if ($key === false || $key->isEmpty()) { - throw new Exception(Exception::KEY_NOT_FOUND); - }src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Get.php (1)
65-71: 🛠️ Refactor suggestionRemove redundant conditional check.
The second check for
$key === false || $key->isEmpty()is redundant as these conditions are already checked in the first if statement.- if ($key === false || $key->isEmpty() || $key->getAttribute('projectInternalId') !== $project->getInternalId()) { - throw new Exception(Exception::KEY_NOT_FOUND); - } - - if ($key === false || $key->isEmpty()) { - throw new Exception(Exception::KEY_NOT_FOUND); - } + if ($key === false || $key->isEmpty() || $key->getAttribute('projectInternalId') !== $project->getInternalId()) { + throw new Exception(Exception::KEY_NOT_FOUND); + }src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Create.php (2)
82-82: Empty SDKs array is correctly initializedThe
sdksfield is correctly initialized as an empty array. As mentioned in previous comments, verifying that SDKs are updated is important, but that would happen in a separate part of the code.
64-68: 🛠️ Refactor suggestionProject existence check is correct but consider adding team member permission check
The code correctly verifies that the project exists before creating a dev key, but doesn't verify if the current user is a team member of this project.
As noted in a previous review comment, this endpoint should only allow team members of the project to access it. Consider adding a permission check similar to other document creation endpoints:
if ($project->isEmpty()) { throw new Exception(Exception::PROJECT_NOT_FOUND); } + // Verify that the user is a team member of the project + $memberOf = $dbForPlatform->find('memberships', [ + Query::equal('userId', [$user->getId()]), + Query::equal('teamId', [$project->getAttribute('teamId')]), + Query::equal('status', [1]), + Query::limit(1), + ]); + + if ($memberOf->isEmpty()) { + throw new Exception(Exception::USER_UNAUTHORIZED); + }
🧹 Nitpick comments (20)
app/config/specs/swagger2-latest-client.json (1)
4932-5123: Coordinated weight adjustments for function-related API endpointsThese weight changes are part of a coordinated update across multiple API specification files mentioned in the PR summary. The weights control the ordering or priority of operations in API documentation and SDKs. The values have been increased by 5 points each, suggesting a deliberate repositioning while maintaining their relative ordering (list: 389, get: 388, create: 387).
While these changes are minimal and appropriate, consider documenting the weight numbering scheme in code comments or documentation to make future modifications easier to understand. A structured approach to assigning weights would help maintain consistency across the entire API surface.
src/Appwrite/Utopia/Database/Validator/Queries/DevKeys.php (1)
5-20: Class implementation looks good, but documentation could be improved.The
DevKeysvalidator class is well-structured and follows the existing pattern for query validators. It correctly defines the allowed attributes for filtering dev keys.Consider improving the PHPDoc for the constructor by adding a description and any relevant parameters/return information:
/** * Expression constructor * + * @return void */tests/e2e/Services/Teams/TeamsConsoleClientTest.php (1)
17-39: Test validates URL validation behavior for team membership creation.This test effectively validates that the team membership creation endpoint rejects malformed URLs when no dev key is present, which is part of the new dev keys feature's conditional validation logic.
Consider these improvements to make the test more explicit and comprehensive:
- Improve the comment to explain why the URL is invalid (fragments aren't allowed)
- Add verification of the error message to confirm the exact reason for rejection
- Consider adding another test case that verifies URL validation is bypassed when a dev key is present
- 'url' => 'http://example.com/join-us#title' // bad url + 'url' => 'http://example.com/join-us#title' // URL with fragment which should be rejected without a dev key$this->assertEquals(400, $response['headers']['status-code']); + $this->assertStringContainsString('URL', $response['body']['message']); // Verify error is related to URL validationtests/e2e/Services/Account/AccountBase.php (1)
162-162: Improved assertion formatAssertion has been updated with expected value first, though there's an extra comma at the end which could be removed.
- $this->assertEquals(201, $response['headers']['status-code'], ); + $this->assertEquals(201, $response['headers']['status-code']);app/controllers/api/teams.php (1)
459-459: TODO comment needs to be addressed.There's a TODO comment about adding a built-in confirm page. This should be addressed before finalizing the PR.
Consider either implementing the built-in confirm page or creating a separate issue to track this TODO item.
src/Appwrite/Utopia/Response/Model/DevKey.php (1)
15-68: Well-defined DevKey model schemaThe model schema is comprehensive and includes all necessary fields for development keys with appropriate descriptions, types, and examples. The dynamic description for
accessedAtthat references the constantAPP_KEY_ACCESSis a good touch for maintainability.There's a small text improvement opportunity in line 40:
- 'example' => 'API Key', + 'example' => 'Dev API Key',.github/workflows/tests.yml (1)
280-314: New job for dev keys testing with proper setupThe implementation of a dedicated job for testing dev keys is well-structured and includes enabling abuse options via
.envmodification, which appears to be required for these tests.However, there's a small inconsistency in line 304, where the
tables-modematrix value is referenced but this job doesn't have a matrix strategy with this property. This should be removed.- - name: Run Projects tests with dev keys in ${{ matrix.tables-mode }} table mode + - name: Run Projects tests with dev keys in project table mode🧰 Tools
🪛 actionlint (1.7.4)
304-304: property "tables-mode" is not defined in object type {}
(expression)
app/init/resources.php (2)
821-833: Consider optimizing SDK tracking for better performance.The current implementation updates the dev key document twice when adding a new SDK: once for updating
accessedAtand again for adding the SDK. Consider combining these operations for better performance.if ($sdk !== 'UNKNOWN' && $sdkValidator->isValid($sdk)) { $sdks = $key->getAttribute('sdks', []); if (!in_array($sdk, $sdks)) { $sdks[] = $sdk; $key->setAttribute('sdks', $sdks); - /** Update access time as well */ - $key->setAttribute('accessedAt', DatabaseDateTime::now()); $key = Authorization::skip(fn () => $dbForPlatform->updateDocument('devKeys', $key->getId(), $key)); $dbForPlatform->purgeCachedDocument('projects', $project->getId()); } }Since the
accessedAtvalue is already updated in the condition check above if needed, we don't need to update it again here.
810-815: Consider using a constant for the cache purge after update.The update operation is followed by a cache purge to maintain consistency. Consider extracting the cache purge pattern to a helper method for reuse across the codebase.
src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Create.php (1)
83-83: Consider adding spaces in search string for better searchabilityThe search string concatenates
name,projectId, anddevKeyIdwithout separators, which might limit search effectiveness.- 'search' => implode('', [$name, $project->getId(), $devKeyId]), + 'search' => implode(' ', [$name, $project->getId(), $devKeyId]),app/config/specs/swagger2-latest-console.json (4)
21476-21535: Emptyproducesand response alignment in DELETE devKey
The DELETE operation for/projects/{projectId}/dev-keys/{keyId}has an emptyproducesarray. If the endpoint truly returns no content, consider removingproducesor explicitly listing"application/json". Also verify that the204response is correctly specified without a response schema.
21559-21567: Consistent ordering of path parameters
Ensure thatprojectIdandkeyIdparameters are listed in the same order across all/projects/{projectId}/dev-keysendpoints to maintain a stable and predictable API spec.
35276-35300: Provide a realisticx-examplefordevKeyList.devKeys
ThedevKeysarray’sx-exampleis currently an empty string, which conflicts withtype: array. Please supply an example array (e.g.,[]or[ { "$id": "...", name: "...", ... } ]) to clarify the payload.
39608-39616: Type mismatch in example for projectdevKeys
In the project model, thedevKeysproperty has"type": "array"but anx-exampleof{}. It should be[]or a sample array of dev key objects to match the declared type.app/config/specs/open-api3-latest-console.json (2)
21094-21180: Restore and verify/projects/{projectId}/jwtsPOST metadata
The JWT creation endpoint has been reintroduced. A couple of minor nitpicks:
- The
demofilename iscreate-j-w-t.md; ensure it matches the actual file.- The
editlink points to/master/docs/references/projects/create-jwt.md; confirm the path is accurate.- Verify that
authandsecuritysections echo other protected endpoints for consistency.
39395-39453: Review completeness ofdevKeyschema
ThedevKeyobject covers all critical fields. As an enhancement, consider adding anx‑example: []for thesdksarray to illustrate expected usage.tests/e2e/Services/Projects/ProjectsConsoleClientTest.php (4)
4260-4322: Consider splitting Dev‑Keys tests into a dedicated testcaseThe Dev‑Keys feature is large enough to deserve its own
ProjectsDevKeysTestclass (similar toAccountCustomClientTest.php). Keeping ~600 new lines inside the already hugeProjectsConsoleClientTestmakes the test‑suite harder to navigate and review, and increases the risk of merge conflicts.Moving the Dev‑Keys scenarios (L4260‑L4865) to a separate file would:
- reduce the cognitive load per class
- let PHPUnit run them in parallel without loading unrelated traits
- allow fine‑grained
@group devKeysselection in CI pipelines without extra includes/excludes logic raised in the PR discussion.
4680-4698: Hard‑coded abuse‑limit loop is brittleLooping exactly 11 unauthenticated requests assumes the abuse limit is fixed at “10 per minute”.
If the limit is changed (e.g. via env_APP_LIMITS_ABUSE), the test will start failing silently or, worse, pass when it should not.Consider querying the limit from config or injecting it via an env helper, e.g.
$limit = (int) System::getEnv('_APP_LIMITS_ABUSE', 10) + 1; for ($i = 0; $i < $limit; $i++) { … }
4749-4759:sleep(5)makes the suite slow & flakyWaiting for five real seconds to let a key expire increases total runtime and can cause random CI failures on slow runners.
Prefer using a key that expires immediately and assert401/429straight away, or better – mock the time by injecting a clock or calling an internal “force expire” endpoint in test mode.
4653-4655: CORS header assertion may miss the header
$this->assertEquals('*', $response['headers']['access-control-allow-origin'] ?? null);If the header is not present the test still passes because
null !== '*'.
Add an explicitassertArrayHasKeybefore the equality check to ensure the header really exists.+$this->assertArrayHasKey('access-control-allow-origin', $response['headers']); $this->assertEquals('*', $response['headers']['access-control-allow-origin']);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
.github/workflows/tests.yml(2 hunks)app/config/collections/platform.php(2 hunks)app/config/specs/open-api3-latest-client.json(3 hunks)app/config/specs/open-api3-latest-console.json(74 hunks)app/config/specs/open-api3-latest-server.json(51 hunks)app/config/specs/swagger2-latest-client.json(3 hunks)app/config/specs/swagger2-latest-console.json(72 hunks)app/config/specs/swagger2-latest-server.json(51 hunks)app/controllers/api/account.php(5 hunks)app/controllers/api/teams.php(2 hunks)app/controllers/general.php(7 hunks)app/controllers/shared/api.php(3 hunks)app/init/database/filters.php(1 hunks)app/init/resources.php(3 hunks)src/Appwrite/Platform/Appwrite.php(2 hunks)src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Create.php(1 hunks)src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Delete.php(1 hunks)src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Get.php(1 hunks)src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Update.php(1 hunks)src/Appwrite/Platform/Modules/Projects/Http/DevKeys/XList.php(1 hunks)src/Appwrite/Platform/Modules/Projects/Module.php(1 hunks)src/Appwrite/Platform/Modules/Projects/Services/Http.php(1 hunks)src/Appwrite/Utopia/Database/Validator/Queries/DevKeys.php(1 hunks)src/Appwrite/Utopia/Response.php(4 hunks)src/Appwrite/Utopia/Response/Model/DevKey.php(1 hunks)src/Appwrite/Utopia/Response/Model/Project.php(1 hunks)tests/e2e/Scopes/ProjectCustom.php(3 hunks)tests/e2e/Scopes/Scope.php(1 hunks)tests/e2e/Scopes/SideClient.php(1 hunks)tests/e2e/Scopes/SideConsole.php(1 hunks)tests/e2e/Scopes/SideNone.php(1 hunks)tests/e2e/Scopes/SideServer.php(1 hunks)tests/e2e/Services/Account/AccountBase.php(5 hunks)tests/e2e/Services/Account/AccountCustomClientTest.php(2 hunks)tests/e2e/Services/Databases/DatabasesPermissionsScope.php(1 hunks)tests/e2e/Services/GraphQL/AbuseTest.php(1 hunks)tests/e2e/Services/Projects/ProjectsBase.php(1 hunks)tests/e2e/Services/Projects/ProjectsConsoleClientTest.php(2 hunks)tests/e2e/Services/Teams/TeamsBaseClient.php(0 hunks)tests/e2e/Services/Teams/TeamsConsoleClientTest.php(1 hunks)
💤 Files with no reviewable changes (1)
- tests/e2e/Services/Teams/TeamsBaseClient.php
🧰 Additional context used
🧬 Code Graph Analysis (3)
tests/e2e/Scopes/ProjectCustom.php (1)
tests/e2e/Client.php (1)
Client(8-317)
src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Delete.php (7)
src/Appwrite/Extend/Exception.php (1)
Exception(7-401)src/Appwrite/SDK/Method.php (1)
Method(9-288)src/Appwrite/Utopia/Response.php (2)
Response(132-867)__construct(360-530)src/Appwrite/Platform/Modules/Projects/Http/DevKeys/XList.php (3)
getName(24-27)__construct(29-58)action(60-87)src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Create.php (3)
getName(25-28)__construct(30-60)action(62-95)src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Update.php (3)
getName(21-24)__construct(26-55)action(56-81)src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Get.php (3)
getName(19-22)__construct(24-52)action(54-74)
app/controllers/api/account.php (1)
src/Appwrite/Vcs/Comment.php (1)
isEmpty(26-29)
🪛 actionlint (1.7.4)
.github/workflows/tests.yml
304-304: property "tables-mode" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Users)
- GitHub Check: E2E Service Test (Projects)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Databases)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: Unit Test
🔇 Additional comments (181)
tests/e2e/Services/GraphQL/AbuseTest.php (2)
91-91: Explicit disabling of dev key for complex query abuse testingThe change correctly passes
falsetogetHeaders()to explicitly disable using development keys for this test. This makes sense as this specific test needs to verify that complex queries are blocked without the special privileges that dev keys provide.
111-111:❌ Incorrect review comment
Consider consistent dev key handling across abuse tests
While
testComplexQueryBlockednow explicitly disables dev keys withgetHeaders(false), this test still uses the default behavior withgetHeaders()which includes dev keys. For consistency in testing abuse protection, consider whether this test should also explicitly disable dev keys.
🏁 Script executed:
#!/bin/bash # Find all implementations of getHeaders in the test scopes to understand the default behavior echo "Checking implementations of getHeaders method across test scope classes:" rg -A 3 "function getHeaders\(" tests/e2e/Scopes/Length of output: 1424
🏁 Script executed:
#!/bin/bash # Show the testComplexQueryBlocked test block for context echo "=== testComplexQueryBlocked context ===" rg -n "testComplexQueryBlocked" -A5 tests/e2e/Services/GraphQL/AbuseTest.php # Show all getHeaders calls in this test file echo "=== All getHeaders() invocations in AbuseTest.php ===" rg -n "getHeaders" -A3 tests/e2e/Services/GraphQL/AbuseTest.phpLength of output: 871
🏁 Script executed:
#!/bin/bash # Show class declaration to find inheritance rg -n "class AbuseTest" -A3 tests/e2e/Services/GraphQL/AbuseTest.php # Show imports to identify base test class head -n 20 tests/e2e/Services/GraphQL/AbuseTest.phpLength of output: 636
Dev key handling is already consistent
The
SideServer::getHeadersmethod defaults its$devKeyparameter tofalse, so callinggetHeaders()is equivalent togetHeaders(false). Both tests disable the dev key as intended—no change needed here.Likely an incorrect or invalid review comment.
tests/e2e/Services/Databases/DatabasesPermissionsScope.php (2)
18-18: Dev key integration in account creationThe addition of the dev key header is appropriate for account creation and aligns with the development keys feature. The fallback to an empty string is a good safeguard for test cases where the dev key isn't set.
27-34:❌ Incorrect review comment
Consider adding dev key header to session creation
While you've added the dev key header to account creation, the session creation request does not include it. For consistency, consider whether the session creation endpoint should also receive the dev key header, especially if dev keys are intended to bypass authentication restrictions.
🏁 Script executed:
#!/bin/bash # Check if other session-related requests include dev key headers echo "Checking for dev key headers in session-related requests:" rg -A 3 "'/account/sessions" tests/e2e/Length of output: 54917
Dev key header not required for permission‑scope tests
The DatabasesPermissionsScope test—and other permission‑scope suites (e.g. StoragePermissionsScope)—intentionally create a standard user session to verify access controls. The only places we include anx‑appwrite‑dev‑keyon session endpoints are console‑ or dev‑key‑specific tests (e.g. ProjectsConsoleClientTest) where bypassing auth is desired. No change is needed here.Likely an incorrect or invalid review comment.
tests/e2e/Scopes/SideNone.php (1)
7-7: Method signature update for consistencyThe updated signature with the boolean parameter provides consistency across all implementations of
getHeaders()in the test suite. Although this implementation always returns an empty array regardless of the parameter, the change is necessary to maintain a consistent interface.tests/e2e/Scopes/Scope.php (1)
59-59: Abstract method signature updated for dev key supportThe addition of the
$devKeyparameter with a default value oftrueappropriately updates the abstract method signature to support conditional inclusion of development keys in request headers. This change ensures all implementing classes will have a consistent interface for handling dev keys.app/config/specs/swagger2-latest-client.json (3)
4932-4932: Simple weight update for function execution listingThe weight for
functionsListExecutionshas been increased from 384 to 389, likely to maintain proper ordering of operations in the API documentation or client SDKs.
5005-5005: Simple weight update for function execution creationThe weight for
functionsCreateExecutionhas been increased from 382 to 387, maintaining consistent relative positioning with other function-related endpoints.
5123-5123: Simple weight update for function execution retrievalThe weight for
functionsGetExecutionhas been increased from 383 to 388, consistent with the other function endpoint weight increases in this file.src/Appwrite/Platform/Modules/Projects/Module.php (1)
1-14: Clean module implementation following established patterns.This new module properly extends the base module class and follows the established Appwrite architectural pattern. The constructor registers the HTTP service that will handle dev keys endpoints.
src/Appwrite/Platform/Appwrite.php (2)
8-8: Properly added import for the Projects module.The new import follows consistent style and naming conventions.
18-18: Correctly integrated Projects module into Appwrite platform.The Projects module is properly added to the Appwrite platform class alongside other modules following the same instantiation pattern.
tests/e2e/Services/Account/AccountCustomClientTest.php (2)
622-622: Properly implemented dev key header for account creation test.The header is correctly implemented with the project's devKey value (or empty string if not set), following the pattern needed to test endpoints with dev key support.
1221-1221: Properly implemented dev key header for session alert test.The header is correctly implemented with the project's devKey value (or empty string if not set), following the same pattern as other uses in the test suite.
app/init/database/filters.php (1)
136-148: Well-implemented database filter for dev keys subqueries.This new filter follows the established pattern for subquery filters in the codebase:
- The first callback is a no-op serialization function
- The second callback properly queries the devKeys collection by projectInternalId
- The results are appropriately limited using APP_LIMIT_SUBQUERY
This enables efficient retrieval of dev keys associated with a project.
src/Appwrite/Utopia/Database/Validator/Queries/DevKeys.php (1)
7-10: Attributes selection is appropriate for dev key filtering.The selected attributes (
accessedAtandexpire) for querying dev keys are logical choices for filtering development keys based on their activity and validity period.app/config/specs/swagger2-latest-server.json (51)
8285-8285: Metadata weight forlistoperation updated
The"weight"has been incremented to373to maintain consistent ordering metadata across API operations.
8357-8357: Metadata weight forcreateoperation updated
The"weight"has been updated to370to align with the +5 increment pattern used in this PR.
8608-8608: Metadata weight forlistRuntimesoperation updated
Updated"weight"to375, matching the corresponding adjustments in other spec files.
8659-8659: Metadata weight forlistSpecificationsoperation updated
The"weight"value is now376, consistent with the PR’s batch increment.
8711-8711: Metadata weight forgetoperation updated
Increased"weight"to371to reflect the updated operation priority.
8770-8770: Metadata weight forupdateoperation updated
The"weight"has been bumped to372, keeping the ordering increments uniform.
9015-9015: Metadata weight fordeleteoperation updated
Adjusted"weight"to374in line with the +5 increase scheme.
9076-9076: Metadata weight forupdateFunctionDeploymentoperation updated
Set"weight"to379, ensuring consistency with related deployment endpoints.
9155-9155: Metadata weight forlistDeploymentsoperation updated
Updated"weight"to380to keep operation ordering in sync.
9235-9235: Metadata weight forcreateDeploymentoperation updated
Bumped"weight"to377matching the PR’s increment pattern.
9327-9327: Metadata weight forcreateDuplicateDeploymentoperation updated
Set"weight"to385, consistent with other deployment-related entries.
9412-9412: Metadata weight forcreateTemplateDeploymentoperation updated
Adjusted"weight"to382to maintain the updated ordering scheme.
9518-9518: Metadata weight forcreateVcsDeploymentoperation updated
The"weight"is now383, aligning with the new spec metadata pattern.
9616-9616: Metadata weight forgetDeploymentoperation updated
Increased"weight"to378to keep consistency across deployment endpoints.
9678-9678: Metadata weight fordeleteDeploymentoperation updated
Set"weight"to381, following the PR’s +5 adjustment approach.
9747-9747: Metadata weight forgetDeploymentDownloadoperation updated
Bumped"weight"to384, aligning with related methods.
9833-9833: Metadata weight forupdateDeploymentStatusoperation updated
Updated"weight"to386to reflect the new ordering scheme.
9902-9902: Metadata weight forlistExecutionsoperation updated
Set"weight"to389, consistent with other execution endpoints.
9977-9977: Metadata weight forcreateExecutionoperation updated
Increased"weight"to387, matching the PR’s pattern.
10097-10097: Metadata weight forgetExecutionoperation updated
The"weight"is now388, ensuring uniform ordering.
10163-10163: Metadata weight fordeleteExecutionoperation updated
Adjusted"weight"to390, in line with related execution operations.
10232-10232: Metadata weight forlistVariablesoperation updated
Bumped"weight"to395, consistent with variable management actions.
10291-10291: Metadata weight forcreateVariableoperation updated
Set"weight"to393to maintain the updated ordering.
10383-10383: Metadata weight forgetVariableoperation updated
Increased"weight"to394, aligning with the PR’s batch increment.
10450-10450: Metadata weight forupdateVariableoperation updated
The"weight"has been updated to396to keep sequence integrity.
10542-10542: Metadata weight fordeleteVariableoperation updated
Adjusted"weight"to397, consistent with other variable endpoints.
17157-17157: Metadata weight forlistoperation updated
Set"weight"to402, reflecting the new ordering metadata in this group.
17229-17229: Metadata weight forcreateoperation updated
Updated"weight"to400to align with the overall increment pattern.
17495-17495: Metadata weight forlistFrameworksoperation updated
Increased"weight"to405, consistent with the related framework endpoints.
17546-17546: Metadata weight forlistSpecificationsoperation updated
Bumped"weight"to428, matching the spec updates across other interfaces.
17598-17598: Metadata weight forgetoperation updated
Set"weight"to401, ensuring continuity in operation ordering.
17657-17657: Metadata weight forupdateoperation updated
Adjusted"weight"to403, following the PR’s consistent increment logic.
17916-17916: Metadata weight fordeleteoperation updated
The"weight"is now404, in line with sequencing for deletion actions.
17977-17977: Metadata weight forupdateSiteDeploymentoperation updated
Updated"weight"to411, aligning with other deployment status methods.
18056-18056: Metadata weight forlistDeploymentsoperation updated
Bumped"weight"to410, consistent with the listing operations across sites.
18136-18136: Metadata weight forcreateDeploymentoperation updated
Set"weight"to406to maintain uniform ordering.
18236-18236: Metadata weight forcreateDuplicateDeploymentoperation updated
Increased"weight"to414, matching the PR’s adjustment approach.
18315-18315: Metadata weight forcreateTemplateDeploymentoperation updated
Adjusted"weight"to407, ensuring consistency in template workflows.
18421-18421: Metadata weight forcreateVcsDeploymentoperation updated
The"weight"is now408, aligning with VCS-related operations.
18520-18520: Metadata weight forgetDeploymentoperation updated
Set"weight"to409, consistent with retrieval operations.
18582-18582: Metadata weight fordeleteDeploymentoperation updated
Updated"weight"to412, following the PR’s metadata sequencing.
18651-18651: Metadata weight forgetDeploymentDownloadoperation updated
Bumped"weight"to413, aligning with other download endpoints.
18737-18737: Metadata weight forupdateDeploymentStatusoperation updated
Increased"weight"to415, consistent with deployment status updates.
18806-18806: Metadata weight forlistLogsoperation updated
Adjusted"weight"to417, ensuring consistent ordering for log listings.
18879-18879: Metadata weight forgetLogoperation updated
The"weight"is now416, in line with retrieval operations for logs.
18943-18943: Metadata weight fordeleteLogoperation updated
Set"weight"to418, consistent with log deletion ordering.
19011-19011: Metadata weight forlistVariablesoperation updated
Bumped"weight"to421, matching variable listing in this section.
19071-19071: Metadata weight forcreateVariableoperation updated
Updated"weight"to419, maintaining ordering consistency.
19163-19163: Metadata weight forgetVariableoperation updated
Increased"weight"to420, consistent with retrieval operations.
19230-19230: Metadata weight forupdateVariableoperation updated
Adjusted"weight"to422, aligning with variable modification endpoints.
19322-19322: Metadata weight fordeleteVariableoperation updated
The"weight"is now423, ensuring consistent ordering for deletion actions.tests/e2e/Scopes/ProjectCustom.php (3)
6-6: LGTM: Added DateTime import for dev key expiration handlingThe import for the DateTime class is correctly added and used to set the expiration time for development keys.
114-126: LGTM: Well-implemented dev key creationThe implementation follows the established pattern for API calls in this trait. The code correctly:
- Makes a POST request to the new
/dev-keysendpoint- Sets appropriate headers
- Provides required parameters (name and expiration)
- Validates the response with appropriate assertions
The expiration time is properly set to 1 hour ahead using the DateTime utility.
167-167: LGTM: Dev key properly added to project arrayThe development key secret is correctly added to the project array, making it available for test cases that use this trait.
app/config/specs/open-api3-latest-server.json (51)
8137-8142: Approve consistent weight increment for list operation.
The"list"method weight has been updated to373, reflecting the +5 bump applied across all OpenAPI specs.
8210-8215: Approve consistent weight increment for create operation.
The"create"method weight has been updated to370, matching the coordinated +5 adjustment.
8441-8446: Approve consistent weight increment for listRuntimes operation.
The"listRuntimes"method weight is now375, aligning with other spec updates.
8490-8495: Approve consistent weight increment for listSpecifications operation.
The"listSpecifications"weight has been increased to376, consistent with the spec-wide bump.
8540-8545: Approve consistent weight increment for get operation.
The"get"method weight has been updated to371, in line with the +5 change.
8599-8604: Approve consistent weight increment for update operation.
The"update"method weight is now372, reflecting the spec-wide adjustment.
8827-8832: Approve consistent weight increment for delete operation.
The"delete"method weight has been bumped to374, matching the coordinated update.
8888-8893: Approve consistent weight increment for updateFunctionDeployment operation.
The"updateFunctionDeployment"weight is now379, aligning with other spec files.
8968-8973: Approve consistent weight increment for listDeployments operation.
The"listDeployments"method weight has been updated to380, reflecting the +5 bump.
9051-9056: Approve consistent weight increment for createDeployment operation.
The"createDeployment"weight is now377(type:upload), consistent with the spec-wide change.
9147-9152: Approve consistent weight increment for createDuplicateDeployment operation.
The"createDuplicateDeployment"weight has been increased to385, matching the overall adjustment.
9232-9237: Approve consistent weight increment for createTemplateDeployment operation.
The"createTemplateDeployment"weight is now382, aligned with other endpoint updates.
9335-9340: Approve consistent weight increment for createVcsDeployment operation.
The"createVcsDeployment"weight has been updated to383, reflecting the +5 bump.
9432-9437: Approve consistent weight increment for getDeployment operation.
The"getDeployment"method weight is now378, consistent with the coordinated update.
9494-9499: Approve consistent weight increment for deleteDeployment operation.
The"deleteDeployment"weight has been increased to381, matching the spec-wide change.
9558-9563: Approve consistent weight increment for getDeploymentDownload operation.
The"getDeploymentDownload"weight is now384(type:location), consistent with other specs.
9648-9653: Approve consistent weight increment for updateDeploymentStatus operation.
The"updateDeploymentStatus"weight has been bumped to386, aligned with the +5 bump.
9719-9724: Approve consistent weight increment for listExecutions operation.
The"listExecutions"weight is now389, reflecting the spec-wide increment.
9795-9800: Approve consistent weight increment for createExecution operation.
The"createExecution"weight has been increased to387, matching other endpoints.
9911-9916: Approve consistent weight increment for getExecution operation.
The"getExecution"weight is now388, aligned with the overall update.
9977-9982: Approve consistent weight increment for deleteExecution operation.
The"deleteExecution"weight has been bumped to390, consistent across specs.
10048-10053: Approve consistent weight increment for listVariables operation.
The"listVariables"weight is now395, reflecting the +5 change.
10107-10112: Approve consistent weight increment for createVariable operation.
The"createVariable"weight has been updated to393, matching other API specs.
10198-10203: Approve consistent weight increment for getVariable operation.
The"getVariable"weight is now394, aligned with the spec-wide bump.
10267-10272: Approve consistent weight increment for updateVariable operation.
The"updateVariable"weight has been increased to396, consistent with other endpoints.
10358-10363: Approve consistent weight increment for deleteVariable operation.
The"deleteVariable"weight is now397, reflecting the overall +5 bump.
16692-16697: Approve consistent weight increment for list operation (console context).
The"list"method weight has been updated to402, matching coordinated spec changes.
16762-16767: Approve consistent weight increment for create operation (console context).
The"create"method weight is now400, reflecting the +5 bump.
17008-17013: Approve consistent weight increment for listFrameworks operation.
The"listFrameworks"weight has been increased to405, consistent with other spec updates.
17057-17062: Approve consistent weight increment for listSpecifications operation (console context).
The"listSpecifications"weight is now428, aligned with the spec-wide change.
17107-17112: Approve consistent weight increment for get operation (console context).
The"get"method weight has been bumped to401, matching the +5 update.
17166-17171: Approve consistent weight increment for update operation (console context).
The"update"weight is now403, reflecting the coordinated bump.
17408-17413: Approve consistent weight increment for delete operation (console context).
The"delete"method weight has been updated to404, consistent with other specs.
17469-17474: Approve consistent weight increment for updateSiteDeployment operation.
The"updateSiteDeployment"weight is now411, aligned with the overall update.
17549-17554: Approve consistent weight increment for listDeployments operation (console context).
The"listDeployments"weight has been increased to410, matching the +5 bump.
17632-17637: Approve consistent weight increment for createDeployment operation (console context).
The"createDeployment"weight is now406(type:upload), consistent with other endpoints.
17733-17738: Approve consistent weight increment for createDuplicateDeployment operation (console context).
The"createDuplicateDeployment"weight has been bumped to414, aligned with spec-wide changes.
17813-17818: Approve consistent weight increment for createTemplateDeployment operation (console context).
The"createTemplateDeployment"weight is now407, reflecting the overall update.
17916-17921: Approve consistent weight increment for createVcsDeployment operation (console context).
The"createVcsDeployment"weight has been updated to408, matching the +5 adjustment.
18014-18019: Approve consistent weight increment for getDeployment operation (console context).
The"getDeployment"weight is now409, aligned with other spec files.
18076-18081: Approve consistent weight increment for deleteDeployment operation (console context).
The"deleteDeployment"weight has been increased to412, consistent with the bump.
18140-18145: Approve consistent weight increment for getDeploymentDownload operation (console context).
The"getDeploymentDownload"weight is now413(type:location), reflecting the +5 bump.
18230-18235: Approve consistent weight increment for updateDeploymentStatus operation (console context).
The"updateDeploymentStatus"weight has been bumped to415, aligned across specs.
18301-18306: Approve consistent weight increment for listLogs operation.
The"listLogs"weight is now417, matching the spec-wide update.
18372-18377: Approve consistent weight increment for getLog operation.
The"getLog"weight has been updated to416, reflecting the +5 bump.
18434-18439: Approve consistent weight increment for deleteLog operation.
The"deleteLog"weight is now418, aligned with other spec updates.
18505-18510: Approve consistent weight increment for listVariables operation (console context).
The"listVariables"weight has been increased to421, reflecting the spec-wide change.
18564-18569: Approve consistent weight increment for createVariable operation (console context).
The"createVariable"weight is now419, matching the +5 bump applied across specs.
18655-18660: Approve consistent weight increment for getVariable operation (console context).
The"getVariable"weight has been updated to420, aligned with other spec files.
18724-18729: Approve consistent weight increment for updateVariable operation (console context).
The"updateVariable"weight is now422, reflecting the coordinated update.
18815-18820: Approve consistent weight increment for deleteVariable operation (console context).
The"deleteVariable"weight has been bumped to423, consistent with the +5 bump.tests/e2e/Scopes/SideClient.php (1)
7-17: Well-implemented dev key handling in request headersThe addition of the optional
$devKeyparameter togetHeaders()is a clean approach for conditionally including the dev key header. The null check withisset()prevents potential issues when the project doesn't have a dev key.src/Appwrite/Utopia/Response/Model/Project.php (1)
200-206: LGTM! Good addition of dev keys to Project modelThe dev keys rule follows the same pattern as other rules in the class and is appropriately positioned in the sequence. The rule correctly specifies type, description, default, and array properties.
src/Appwrite/Platform/Modules/Projects/Services/Http.php (1)
1-24: Well-structured HTTP service for dev keysThe service implementation follows good practices by:
- Properly extending the Service base class
- Setting the correct service type
- Registering all necessary actions for dev key CRUD operations
- Using consistent naming conventions
The implementation aligns well with the overall architecture of the platform.
tests/e2e/Services/Account/AccountBase.php (6)
102-102: Good implementation of dev key in account creation testThe dev key header is correctly added with null coalescing to handle cases where the key might not be present.
116-117: Dev key header implemented correctlyThe dev key header is properly added with null coalescing to handle potential missing keys.
123-123: Assertion format improvementThe assertion has been rewritten to put the expected value first, which is a better practice for readability and error messages.
134-135: Consistent dev key implementationThe dev key header is added consistently across test methods.
141-141: Improved assertion formatThe assertion has been updated to put the expected value first for better readability.
292-293: Consistent dev key implementation in account deletion testDev key is correctly added to the account creation request in the deletion test, maintaining consistency with other test methods.
app/controllers/api/teams.php (2)
54-54: New URL validator import added to support dev key functionality.This import allows the code to use the more permissive URL validator when a development key is present.
459-459:✅ Verification successful
Conditional URL validation based on dev key presence.
The validation logic now intelligently switches between the restrictive
Hostvalidator and the more permissiveURLvalidator based on the presence of a development key. This allows dev keys to bypass the normal host restrictions for easier development workflows.
🏁 Script executed:
#!/bin/bash # Description: Check if the devKey injection is properly implemented in other files # Look for devKey injection and usage patterns in controller files echo "Checking for devKey injection patterns in controllers:" rg -A 1 "inject\('devKey'\)" app/controllers/Length of output: 1200
DevKey Injection Confirmed; URL Validation Logic Is Consistent
We’ve verified that
devKeyis injected in the following controllers, so the new conditional switch between the restrictiveHostvalidator and permissiveURLvalidator will apply uniformly:
- app/controllers/general.php
- app/controllers/shared/api.php
No further changes are needed—approving as is.
app/config/specs/open-api3-latest-client.json (4)
4765-4767: Weight parameter updated for listExecutions endpoint.The
weightparameter for thelistExecutionsendpoint has been increased from 384 to 389, which is a +5 increase. This is part of a systematic weight value increment applied across multiple OpenAPI specifications to reflect updated operation ordering or priority in the API documentation.
4839-4841: Weight parameter updated for createExecution endpoint.The
weightparameter for thecreateExecutionendpoint has been increased from 382 to 387, which is a +5 increase. This weight change aligns with the other function-related endpoints that were similarly adjusted as part of standardizing operation priority across the API.
4953-4955: Weight parameter updated for getExecution endpoint.The
weightparameter for thegetExecutionendpoint has been increased from 383 to 388, which is a +5 increase. These weight modifications across all function-related endpoints ensure consistent operation priority in the API documentation and client SDKs.
1-9612:✅ Verification successful
Verify integration with dev keys feature.
The weight value changes in this file are part of a broader update to support the new dev keys feature in the project. While these specific changes are minimal and only affect metadata values, they ensure proper integration with the rest of the changes introduced in this PR.
I recommend verifying that the updated weight values match the expected priority ordering across all modified specification files (open-api3-latest-console.json, open-api3-latest-server.json, swagger2-latest-client.json, etc.) to maintain consistency.
🏁 Script executed:
#!/bin/bash # Script to check weight values across OpenAPI specs for consistency echo "Checking weight values for function endpoints across OpenAPI specification files..." # Check weight for listExecutions endpoint echo "listExecutions weights:" grep -A 2 "\"method\": \"listExecutions\"" app/config/specs/open-api*.json | grep "weight" grep -A 2 "\"method\": \"listExecutions\"" app/config/specs/swagger*.json | grep "weight" # Check weight for createExecution endpoint echo -e "\ncreateExecution weights:" grep -A 2 "\"method\": \"createExecution\"" app/config/specs/open-api*.json | grep "weight" grep -A 2 "\"method\": \"createExecution\"" app/config/specs/swagger*.json | grep "weight" # Check weight for getExecution endpoint echo -e "\ngetExecution weights:" grep -A 2 "\"method\": \"getExecution\"" app/config/specs/open-api*.json | grep "weight" grep -A 2 "\"method\": \"getExecution\"" app/config/specs/swagger*.json | grep "weight"Length of output: 18007
Consistent weight values across OpenAPI specifications confirmed
The verification shows that the weight values for the functions endpoints are consistent across all API specification files in the same version:
- Latest version weights:
listExecutions: 389createExecution: 387getExecution: 388The weight values follow a consistent pattern with proper increments from previous versions (1.6.x → 1.7.x → latest), and maintain the same relative ordering across all specification formats (OpenAPI 3 and Swagger 2).
This confirms that the weight value changes in this file are properly synchronized with all other specification files, ensuring compatible integration with the dev keys feature.
🧰 Tools
🪛 Gitleaks (8.21.2)
8738-8738: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
src/Appwrite/Utopia/Response.php (3)
42-42: Added import for the new DevKey modelGood addition of the DevKey model import, which is used later in the file.
310-311: New model constants for DevKeyThe constants follow the naming convention of other models in the file.
479-479: Added DevKey model registrationGood addition of the DevKey model to the list of registered models.
tests/e2e/Services/Projects/ProjectsBase.php (2)
10-33: Well-structured project setup helper methodThis helper method provides a clean way to create a test project within a new team. The implementation includes appropriate error messages with the response body for easier troubleshooting when tests fail.
35-48: Well-implemented dev key setup helper methodThis method provides a clean interface for creating dev keys during tests. The error handling is good, returning both the ID and secret for use in subsequent test steps.
src/Appwrite/Utopia/Response/Model/DevKey.php (2)
8-14: Clean model declaration with proper visibility controlThe model is properly structured, extending the base Model class. The
$public = falseproperty controls whether this model should be exposed in the API documentation, keeping development keys implementation details internal.
75-88: Properly implemented model type methodsThe
getName()andgetType()methods are correctly implemented, returning the appropriate model name and type constant..github/workflows/tests.yml (2)
200-200: Appropriate test filtering for regular E2E testsAdding the
--exclude=devKeysflag to existing E2E test commands is a good approach to separate dev key tests from general service tests.Also applies to: 278-278
315-360: Well-implemented shared mode tests for dev keysThis job correctly inherits from the database changes check and implements a proper matrix strategy for testing in different shared table modes.
src/Appwrite/Platform/Modules/Projects/Http/DevKeys/XList.php (2)
32-54: LGTM! Well-implemented query parameter support.The implementation follows best practices by supporting query filters with proper validation using the dedicated DevKeys query validator. The pagination, search, and filter functionality align with the platform's conventions.
79-86: Consider adding more targeted queries for better performance.The implementation correctly restricts results to the specific project by filtering on projectInternalId. For large projects with many dev keys, this approach ensures proper data isolation.
src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Delete.php (2)
40-46: LGTM! Appropriate response code for deletion endpoint.Using STATUS_CODE_NOCONTENT and ContentType::NONE for the deletion operation follows REST API best practices.
69-72: Ensure database integrity with caching management.Good practice to purge the cached project document after modifying its related entities. This prevents stale data issues.
app/config/collections/platform.php (2)
291-300: LGTM! Consistent structure with existing collections.The
devKeysattribute in theplatformscollection follows the same pattern askeys, ensuring consistency in the data model.
703-820: Complete implementation of devKeys collection.The
devKeyscollection is well-structured with all necessary attributes and indexes. The full-text search index on thesearchattribute will enable efficient querying.app/controllers/general.php (7)
763-765: LGTM: Dev key injection properly added to API and web groups.The dev key parameter is correctly injected into the API and web route initialization.
998-1000: LGTM: CORS modification for dev keys.This change allows requests with dev keys to bypass the normal CORS restrictions by setting Access-Control-Allow-Origin to '*', which is appropriate for development scenarios.
1011-1012: LGTM: Origin validation bypass for dev keys.When a valid dev key is present, the origin validation check is correctly bypassed, which facilitates development and testing across different origins.
1036-1038: LGTM: Dev key injection added to OPTIONS route.The dev key parameter is correctly injected into the OPTIONS route initialization.
1062-1064: LGTM: CORS modification for OPTIONS requests with dev keys.This change allows OPTIONS requests with dev keys to bypass CORS restrictions, matching the behavior implemented for regular requests.
1086-1086: LGTM: Dev key injected into error handler.The dev key parameter is correctly injected into the error handler initialization.
1301-1314: Maintain consistent error information exposure policy for dev keys.The code has been changed to use
App::isDevelopment()for determining when to show detailed error information, rather than using the presence of a dev key. This aligns with previous review comments suggesting that dev keys should not affect error handling.src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Get.php (2)
1-55: LGTM: Well-structured Get endpoint for dev keys.The class is properly structured with clear namespace, imports, constructor configuration, and action method. The endpoint implementation follows consistent patterns with other API endpoints.
73-73: LGTM: Response handling.The response correctly uses the dynamic model for returning the dev key data.
app/controllers/shared/api.php (3)
405-406: LGTM: Dev key injection added to API group action.The dev key parameter is correctly injected into the API group initialization.
264-264: LGTM: Improved SDK validation.This change explicitly checks that the SDK name is not 'UNKNOWN' before considering it valid, which prevents unintended validation of default values.
468-475: LGTM: Rate limiting bypass for dev keys.This change allows requests with valid dev keys to bypass abuse rate limiting checks, which is helpful during development and testing.
src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Update.php (2)
1-55: LGTM: Well-structured Update endpoint for dev keys.The class is properly structured with clear namespace, imports, constructor configuration, and parameter definitions. The endpoint implementation follows consistent patterns with other API endpoints.
56-80: LGTM: Proper update logic for dev keys.The action method correctly:
- Retrieves and validates the project and dev key
- Updates key attributes including name, expire, and search text
- Updates the database document and purges cache
- Returns the updated key data in the response
This follows best practices for document updates.
src/Appwrite/Platform/Modules/Projects/Http/DevKeys/Create.php (6)
32-59: Action configuration and parameter validation looks good!The HTTP endpoint is properly configured with method, path, description, scopes, and SDK metadata. Parameter validation includes appropriate validators for
projectId,name, andexpire. The dependency injection follows the project pattern.
70-86: Secure document creation with appropriate permissionsThe dev key document creation is properly implemented with secure random secret generation and appropriate permissions. The user who creates the key has read, update, and delete permissions.
85-85: Secret generation uses secure random sourceThe secret is properly generated using a cryptographically secure random source (
random_bytes) and converted to hexadecimal, resulting in a 256-character string.
88-88: Database operation correctly creates the dev key documentThe document is properly created in the 'devKeys' collection.
90-90: Cache invalidation after document creationExcellent practice to purge the cached project document after creating a dev key that might affect it.
92-94: Response handling is correctThe code properly sets the 201 Created status code and uses the dynamic method with the appropriate response model (MODEL_DEV_KEY).
app/controllers/api/account.php (4)
1191-1192: Conditional URL validation based on devKey presence looks goodThe validation now uses either
Hostvalidation (restricting URLs to project's platform list) orURLvalidation (allowing any valid URL) depending on whether a dev key is present. This change enables development workflows to bypass the usual hostname restrictions when using a dev key.
1787-1788: Same conditional URL validation applied properlySimilar to the OAuth2 session endpoint, this consistently applies the same pattern to the OAuth2 token creation endpoint, allowing any valid URL when a devKey is present.
1867-1867: Consistent validation application for magic URL tokenThe magic URL token endpoint now correctly inherits the same URL validation behavior based on the presence of a dev key, ensuring a consistent user experience.
3437-3437:✅ Verification successful
Conditional URL validation applied to email verification
The email verification endpoint now follows the same pattern for URL validation when a devKey is present. This change appropriately rounds out the implementation across all relevant endpoints.
As noted in a previous review comment (copied below), there may be other similar endpoints that need the same treatment. Let's verify we didn't miss any:
Reference from previous review: "We should copy this logic to: createOAuth2Token, createMagicURLToken, createRecovery, createVerification, In different file: teams.php: createMembership"
🏁 Script executed:
#!/bin/bash # Check if there are other URL or Host validators that might need similar devKey adjustments # Look for places using Host validation that might need similar changes rg -B 2 -A 2 "new Host\(\$clients\)" app/ | grep -v "devKey" # Look for URL parameters that might need similar conditional validation rg -B 2 -A 2 "param\('url" app/Length of output: 7835
All relevant endpoints updated with devKey‑based URL validation
The conditional
fn($clients, $devKey) => $devKey->isEmpty() ? new Host($clients) : new URL()logic has now been applied to every email/redirection endpoint:• app/controllers/api/account.php
– Magic URL login (createMagicURL)
– Recovery email (createRecovery)
– Verification email (createVerification)• app/controllers/api/teams.php
– Invitation email (createMembership)Other
urlparameters in avatars (avatars.php) and projects (projects.php) remain unchanged, as they use their own URL validators. All devKey scenarios are covered—no further updates needed.app/config/specs/swagger2-latest-console.json (4)
4565-20996: Skip: Weight adjustment segments (pre-dev-keys)
The majority of these weight changes appear to be mechanical updates to existing API operation weights. They follow the existing pattern and don’t introduce functional changes.
25057-27847: Skip: Weight adjustment segments (post-dev-keys)
These are further mechanical weight updates for existing endpoints. They appear consistent with the pre-dev-keys changes and don’t introduce new behavior.
39799-39801: Verify whetherdevKeysshould be required
ThedevKeysproperty is newly added to the project model but isn’t listed in therequiredarray. Confirm if every project response must includedevKeys. If so, add it torequired; otherwise mark it as optional in the documentation.
39991-40049: ReviewdevKeymodel required fields consistency
ThedevKeydefinition marks fields likeaccessedAtas required, but according to implementation it’s only updated after 24 hours. Consider makingaccessedAtoptional or updating the documentation to reflect when it’s actually present.app/config/specs/open-api3-latest-console.json (2)
39201-39205: Confirm addition ofdevKeysto required fields
devKeyshas been inserted into the Project schema’srequiredlist. Ensure that all clients and serializers are updated to handle this new mandatory field, and that backward‑compatibility concerns are addressed.
4356-4360:❓ Verification inconclusive
Bulk weight adjustments across API endpoints
Theweightfields for numerous operations have been uniformly bumped. Please verify that these new values:
- Preserve the intended ordering of endpoints in the console UI.
- Do not introduce duplicate weights, which could lead to unpredictable sorting.
- Align with any rate‑limiting or pagination logic that relies on operation weights.
To check for duplicate weight values, you can run:
Also applies to: 9052-9056, 9125-9129, 9353-9359, 9401-9407, 9450-9456, 9549-9555, 9608-9614, 9679-9685, 9737-9743, 9964-9970, 10024-10030, 10103-10109, 10185-10191, 10280-10286, 10364-10370, 10466-10472, 10562-10568, 10623-10629, 10686-10692, 10775-10781, 10845-10851, 10919-10925, 11033-11039, 11097-11103, 11167-11173, 11248-11254, 11306-11312, 11396-11402, 11464-11470, 11554-11560, 24577-24583, 24643-24649, 24720-24726, 24811-24817, 25056-25062, 25125-25131, 25370-25376, 25418-25424, 25467-25473, 25566-25572, 25625-25631, 25696-25702, 25754-25760, 25995-26001, 26055-26061, 26134-26140, 26216-26222, 26316-26322, 26395-26401, 26497-26503, 26594-26600, 26655-26661, 26718-26724, 26807-26813, 26877-26883, 26947-26953, 27008-27014, 27078-27084, 27159-27165, 27217-27223, 27307-27313, 27375-27381, 27465-27471
🏁 Script executed:
#!/bin/bash # Verify unique weight values in the OpenAPI spec jq -r '..|.weight? // empty' app/config/specs/open-api3-latest-console.json \ | sort -n \ | uniq -c \ | awk '$1>1'Length of output: 110
No duplicate weight values detected
The duplicate‑check script returned no results, indicating allweightfields are unique.Please still verify that the new weights:
- Preserve the intended ordering of endpoints in the console UI.
- Align with any rate‑limiting or pagination logic that depends on operation weights.
You can review the full ordering by running:
jq -r ' ..|objects | select(has("operationId") and has("weight")) | "\(.weight)\t\(.operationId)" ' app/config/specs/open-api3-latest-console.json \ | sort -n
What does this PR do?
Test Plan
Related PRs and Issues
Checklist
Summary by CodeRabbit
New Features
Improvements
Tests
Chores