Open muntaxir4 opened 14 hours ago
Here are some key observations to aid the review process:
**🎫 Ticket compliance analysis 🔶** **[519](https://github.com/keyshade-xyz/keyshade/issues/519) - Partially compliant** Compliant requirements: - Created api-key folder with index.ts and index.types.ts - Added zod schemas for API key base type, requests and responses - Added type exports - Exported schemas and types from main index files Non-compliant requirements: - No evidence of api-client package updates to use the new schema types |
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ |
🧪 PR contains tests |
🔒 No security concerns identified |
⚡ Recommended focus areas for review Data Validation The date transformations in ApiKeySchema may lose timezone information when converting to ISO string. Consider preserving timezone data. Code Design GetApiKeysOfUserResponseSchema does not implement pagination which may cause performance issues with large datasets |
Explore these optional code suggestions:
Category | Suggestion | Score |
Security |
Enforce minimum length requirement for API key values to maintain security standards___ **Add validation for minimum length of API key value to ensure security. A minimumlength of 32 characters is recommended for API keys.** [packages/schema/src/api-key/index.ts [9]](https://github.com/keyshade-xyz/keyshade/pull/557/files#diff-eb680e1da49b1fb5b8806fe44f2a7cafcbef3240be6a1ea689f865b58bb363ecR9-R9) ```diff -value: z.string(), +value: z.string().min(32), ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 9Why: Adding minimum length validation for API keys is crucial for security, as it helps prevent weak or easily guessable keys. This is a critical security enhancement. | 9 |
General |
Enforce URL-safe format for identifier fields___ **Add validation for slug field to ensure it follows URL-safe format using regexpattern.** [packages/schema/src/api-key/index.ts [8]](https://github.com/keyshade-xyz/keyshade/pull/557/files#diff-eb680e1da49b1fb5b8806fe44f2a7cafcbef3240be6a1ea689f865b58bb363ecR8-R8) ```diff -slug: z.string(), +slug: z.string().regex(/^[a-z0-9]+(?:-[a-z0-9]+)*$/), ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7Why: Adding regex validation for the slug field ensures consistent, URL-safe formatting, preventing potential issues in API endpoints and improving system reliability. | 7 |
Add length constraints to string fields to prevent invalid data___ **Add validation for name field to prevent empty strings and enforce reasonable lengthlimits.** [packages/schema/src/api-key/index.ts [7]](https://github.com/keyshade-xyz/keyshade/pull/557/files#diff-eb680e1da49b1fb5b8806fe44f2a7cafcbef3240be6a1ea689f865b58bb363ecR7-R7) ```diff -name: z.string(), +name: z.string().min(1).max(100), ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 6Why: Adding length constraints to the name field prevents empty or excessively long values, improving data quality and user experience. | 6 |
💡 Need additional feedback ? start a PR chat
I will try to modify the date types for previously implemented schemas and also update the docs for integration in this PR.
Looks good, can you also include the API client changes?
Implementation for API-KEY controller? I was thinking of creating a new PR for that.
Actually the parent issue is a centralized one, so i was thinking it would be best to stash both of the changes in here
Actually the parent issue is a centralized one, so i was thinking it would be best to stash both of the changes in here
Done with this.
User description
Description
@keyshade/schema
api-key
.api-key
schemas.index
and its types fromindex.types
.z.string().datetime()
for stricter validation.@keyshade/schema
api-key
Controller and its tests.Related to #519
Future Improvements
The
GetApiKeysOfUserResponseSchema
could be changed to extend PageResponse once the API returns paginated response for this.Developer's checklist
If changes are made in the code:
Documentation Update
PR Type
Enhancement, Tests
Description
Changes walkthrough 📝
index.ts
Add comprehensive API key schemas and transformations
packages/schema/src/api-key/index.ts
index.types.ts
Define and export API key types
packages/schema/src/api-key/index.types.ts
index.ts
Export API key schemas from index
packages/schema/src/index.ts - Exported API key schemas from the main index file.
index.types.ts
Export API key types from index.types
packages/schema/src/index.types.ts - Exported API key types from the main index types file.
api-key.spec.ts
Add tests for API key schemas
packages/schema/tests/api-key.spec.ts
tsconfig.json
Update tsconfig base path
packages/schema/tsconfig.json - Modified the path for extending base tsconfig.