Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(schema, api-client, api, cli): Add api-key schemas and types; Fix schema inconsistencies; Minor fix for CLI build errors #557

Merged
merged 8 commits into from
Nov 27, 2024

Conversation

muntaxir4
Copy link
Contributor

@muntaxir4 muntaxir4 commented Nov 25, 2024

User description

Description

@keyshade/schema

  • Add schemas and types for api-key.
  • Add tests for api-key schemas.
  • Export api-key schemas from index and its types from index.types .
  • Fixed inconsistencies between schemas and actual types in API.
  • Changed date properties to z.string().datetime() for stricter validation.

@keyshade/api-client

  • Implement the api-key Controller and its tests.

api

  • Implement pagination response for getApiKeysOfUser.

cli

  • Fix some build errors caused due to schemas.

Related to #519

Future Improvements

CLI build errors could be fixed for secrets and variables.

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

If changes are made in the code:

  • I have followed the coding guidelines
  • My changes in code generate no new warnings
  • My changes are breaking another fix/feature of the project
  • I have added test cases to show that my feature works
  • I have added relevant screenshots in my PR
  • There are no UI/UX issues

Documentation Update

  • This PR requires an update to the documentation at docs.keyshade.xyz
  • I have made the necessary updates to the documentation, or no documentation changes are required.

PR Type

Enhancement, Tests


Description

  • Added comprehensive schemas for API key operations using zod, including request and response schemas.
  • Defined and exported TypeScript types inferred from the API key schemas.
  • Updated the main index files to export the new API key schemas and types.
  • Implemented extensive tests for the API key schemas, covering both valid and invalid scenarios.
  • Adjusted the tsconfig.json to correct the base path extension.

Changes walkthrough 📝

Relevant files
Enhancement
index.ts
Add comprehensive API key schemas and transformations       

packages/schema/src/api-key/index.ts

  • Added comprehensive API key schemas using zod.
  • Defined request and response schemas for API key operations.
  • Included transformations for date fields.
  • +64/-0   
    index.types.ts
    Define and export API key types                                                   

    packages/schema/src/api-key/index.types.ts

  • Defined TypeScript types inferred from API key schemas.
  • Exported types for API key operations.
  • +50/-0   
    index.ts
    Export API key schemas from index                                               

    packages/schema/src/index.ts

    • Exported API key schemas from the main index file.
    +2/-1     
    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.
    +1/-4     
    Tests
    api-key.spec.ts
    Add tests for API key schemas                                                       

    packages/schema/tests/api-key.spec.ts

  • Added tests for API key schemas.
  • Validated both valid and invalid cases for each schema.
  • +292/-8 
    Configuration changes
    tsconfig.json
    Update tsconfig base path                                                               

    packages/schema/tsconfig.json

    • Modified the path for extending base tsconfig.
    +1/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    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

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    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 minimum
    length of 32 characters is recommended for API keys.

    packages/schema/src/api-key/index.ts [9]

    -value: z.string(),
    +value: z.string().min(32),
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: 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 regex
    pattern.

    packages/schema/src/api-key/index.ts [8]

    -slug: z.string(),
    +slug: z.string().regex(/^[a-z0-9]+(?:-[a-z0-9]+)*$/),
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: 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 length
    limits.

    packages/schema/src/api-key/index.ts [7]

    -name: z.string(),
    +name: z.string().min(1).max(100),
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: 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

    @muntaxir4
    Copy link
    Contributor Author

    I will try to modify the date types for previously implemented schemas and also update the docs for integration in this PR.

    Copy link
    Member

    @rajdip-b rajdip-b left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Looks good, can you also include the API client changes?

    @muntaxir4
    Copy link
    Contributor Author

    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.

    @rajdip-b
    Copy link
    Member

    Actually the parent issue is a centralized one, so i was thinking it would be best to stash both of the changes in here

    @muntaxir4 muntaxir4 requested a review from rajdip-b November 25, 2024 20:18
    @muntaxir4
    Copy link
    Contributor Author

    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.

    packages/schema/src/api-key/index.ts Show resolved Hide resolved
    packages/schema/src/api-key/index.ts Outdated Show resolved Hide resolved
    @muntaxir4 muntaxir4 requested a review from rajdip-b November 26, 2024 12:08
    @muntaxir4 muntaxir4 changed the title feat(schema): Add api-key schemas and types feat(schema, api-client, api, cli): Add api-key schemas and types; Fix schema inconsistencies; Minor fix for CLI build errors Nov 26, 2024
    @rajdip-b rajdip-b merged commit 126d024 into keyshade-xyz:develop Nov 27, 2024
    7 checks passed
    @muntaxir4 muntaxir4 deleted the apiKey-schemas branch November 30, 2024 17:25
    rajdip-b pushed a commit that referenced this pull request Dec 3, 2024
    ## [2.8.0](v2.7.0...v2.8.0) (2024-12-03)
    
    ### 🚀 Features
    
    * **api:** Add workspace removal notification email template ([#476](#476)) ([40b754f](40b754f))
    * **cli:** Store `metrics_enabled` key in profile config ([#536](#536)) ([9283b22](9283b22))
    * **package, api, cli:** Add api-key schemas and types; Fix schema inconsistencies; Minor fix for CLI build errors  ([#557](#557)) ([126d024](126d024))
    * **platform:** Added screen for CREATE NEW PROJECT ([#540](#540)) ([b644633](b644633))
    * **platform:** Updated the empty state of dashboard ([#522](#522)) ([28739d9](28739d9))
    * **schema, api-client:** Migrate auth types to @keyshade/schema ([#532](#532)) ([d880098](d880098))
    * **schema, api-client:** Migrate event schemas and types to @keyshade/schema ([#546](#546)) ([a3267de](a3267de))
    * **schema, api-client:** Migrate integration schemas and types to @keyshade/schema ([#547](#547)) ([08868c3](08868c3))
    * **schema, api-client:** Migrate project schemas and environment schemas along with their types to @keyshade/schema ([#538](#538)) ([c468af0](c468af0))
    * **schema, api-client:** Migrate [secure] types and schemas to @keyshade/schema ([#539](#539)) ([bc3100b](bc3100b))
    * **schema, api-client:** Migrate user types and schemas to @keyshade/schema ([#535](#535)) ([c24695e](c24695e))
    * **schema, api-client:** Migrate variable schemas and types to @keyshade/schema ([#545](#545)) ([0ee8f9a](0ee8f9a))
    * **schema, api-client:** Migrate workspace-membership schemas and types to @keyshade/schema ([#569](#569)) ([4398969](4398969))
    * **schema, api-client:** Migrate workspace-role schemas and types to @keyshade/schema ([#568](#568)) ([9efbf2d](9efbf2d))
    * **schema:** Add User type inference from UserSchema ([#574](#574)) ([84c1db5](84c1db5))
    
    ### 🐛 Bug Fixes
    
    * **api:** Incorrect oauth redirect url ([58d96e5](58d96e5))
    * **platform:** Resolve loading SVG blocking input field interaction ([#571](#571)) ([30f4f65](30f4f65))
    
    ### 📚 Documentation
    
    * Add pictures to Bruno setup ([#541](#541)) ([210c0fd](210c0fd))
    * Migrate to Bruno ([#525](#525)) ([1793d92](1793d92))
    
    ### 🔧 Miscellaneous Chores
    
    * **ci:** Add script to validate schema package ([59e4280](59e4280))
    * Fixed codecov client version ([a998ae4](a998ae4))
    * **package:** Fixed tests and did housekeeping ([#544](#544)) ([40008e3](40008e3))
    * Update test coverage settings ([5b27e32](5b27e32))
    * Update Turbo to 2.3.1 ([#564](#564)) ([3a63823](3a63823))
    * **web:** Update dockerfile ([10d9cc5](10d9cc5))
    
    ### 🔨 Code Refactoring
    
    * **api-client, schema:** Add workspace's schemas and types in @keyshade/schema ([#520](#520)) ([7c8ee5d](7c8ee5d))
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants