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): Migrate auth types to @keyshade/schema #532

Merged
merged 6 commits into from
Nov 13, 2024

Conversation

muntaxir4
Copy link
Contributor

@muntaxir4 muntaxir4 commented Nov 11, 2024

User description

Description

@keyshade/schema:

  • Add schemas for auth.
  • Update workspace's deleteResponse schema to have void type and add a test for it. Also add id's to GlobalSearchRespone Schema and update corresponding tests.
  • Export types for auth, workspace and pagination from index.types.ts.
  • Migrate existing index.types.d.ts to index.types.ts due to typescript ignoring .d.ts.

@keyshade/api-client:

  • Import new auth schema types from @keyshade/schema
  • Change imports for existing workspace and pagination types to @keyshade/schema
  • Remove existing index.types and auth.types from api-client'
  • Modify tsconfig for external module imports.

Related to #519

Dependencies

Mention any dependencies/packages used

Future Improvements

Mention any improvements to be done in future related to any file/feature

Mentions

Mention and tag the people

Screenshots of relevant screens

Add screenshots of relevant screens

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

  • Migrated auth types to @keyshade/schema and updated all relevant imports in the api-client package.
  • Added ResendOTPRequestSchema and ResendOTPResponseSchema in the schema package using zod.
  • Removed deprecated type definitions from api-client and replaced them with imports from @keyshade/schema.
  • Added tests for new auth schemas to ensure validation correctness.
  • Updated workspace schema to use z.void() for DeleteWorkspaceResponseSchema and added id fields to various response schemas.

Changes walkthrough 📝

Relevant files
Enhancement
19 files
auth.ts
Update auth controller to use schema package                         

packages/api-client/src/controllers/auth.ts

  • Updated imports to use @keyshade/schema for ResendOTPRequest and
    ResendOTPResponse.
  • Removed deprecated imports from @api-client/types.
  • +2/-5     
    environment.ts
    Update environment controller to use schema package           

    packages/api-client/src/controllers/environment.ts

    • Updated import for ClientResponse to use @keyshade/schema.
    +1/-1     
    event.ts
    Update event controller to use schema package                       

    packages/api-client/src/controllers/event.ts

    • Updated import for ClientResponse to use @keyshade/schema.
    +1/-1     
    integration.ts
    Update integration controller to use schema package           

    packages/api-client/src/controllers/integration.ts

    • Updated import for ClientResponse to use @keyshade/schema.
    +1/-1     
    project.ts
    Update project controller to use schema package                   

    packages/api-client/src/controllers/project.ts

    • Updated import for ClientResponse to use @keyshade/schema.
    +1/-1     
    secret.ts
    Update secret controller to use schema package                     

    packages/api-client/src/controllers/secret.ts

    • Updated import for ClientResponse to use @keyshade/schema.
    +1/-1     
    user.ts
    Update user controller to use schema package                         

    packages/api-client/src/controllers/user.ts

    • Updated import for ClientResponse to use @keyshade/schema.
    +1/-1     
    variable.ts
    Update variable controller to use schema package                 

    packages/api-client/src/controllers/variable.ts

    • Updated import for ClientResponse to use @keyshade/schema.
    +1/-1     
    workspace-membership.ts
    Update workspace membership controller to use schema package

    packages/api-client/src/controllers/workspace-membership.ts

    • Updated import for ClientResponse to use @keyshade/schema.
    +1/-1     
    workspace-role.ts
    Update workspace role controller to use schema package     

    packages/api-client/src/controllers/workspace-role.ts

    • Updated import for ClientResponse to use @keyshade/schema.
    +1/-1     
    pagination-parser.ts
    Update pagination parser to use schema package                     

    packages/api-client/src/core/pagination-parser.ts

    • Updated import for PageRequest to use @keyshade/schema.
    +1/-1     
    response-parser.ts
    Update response parser to use schema package                         

    packages/api-client/src/core/response-parser.ts

  • Updated imports for ClientResponse and ResponseError to use
    @keyshade/schema.
  • +1/-1     
    auth.types.ts
    Remove deprecated auth types                                                         

    packages/api-client/src/types/auth.types.ts

    • Removed ResendOTPRequest and ResendOTPResponse interfaces.
    +0/-4     
    index.types.ts
    Remove deprecated index types                                                       

    packages/api-client/src/types/index.types.ts

  • Removed all type definitions, now imported from @keyshade/schema.
  • +0/-36   
    auth.ts
    Add auth schemas using zod                                                             

    packages/schema/src/auth/auth.ts

  • Added ResendOTPRequestSchema and ResendOTPResponseSchema using zod.
  • +5/-0     
    auth.types.ts
    Add auth type exports                                                                       

    packages/schema/src/auth/auth.types.ts

    • Added type exports for ResendOTPRequest and ResendOTPResponse.
    +6/-0     
    index.ts
    Export auth schemas and types                                                       

    packages/schema/src/index.ts

    • Exported auth schemas and types.
    +4/-0     
    index.types.d.ts
    Add T-prefixed type exports for auth and workspace             

    packages/schema/src/index.types.d.ts

    • Added T-prefixed type exports for auth and workspace types.
    +49/-7   
    workspace.ts
    Update workspace schemas and add id fields                             

    packages/schema/src/workspace/workspace.ts

  • Updated DeleteWorkspaceResponseSchema to use z.void().
  • Added id field to various response schemas.
  • +5/-1     
    Tests
    2 files
    auth.spec.ts
    Add tests for auth schemas                                                             

    packages/schema/tests/auth.spec.ts

  • Added tests for ResendOTPRequestSchema and ResendOTPResponseSchema.
  • +37/-0   
    workspace.spec.ts
    Add test for workspace delete response schema                       

    packages/schema/tests/workspace.spec.ts

    • Added test for DeleteWorkspaceResponseSchema.
    +6/-0     

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

    …tion tests
    
    - Add schemas for auth
    - Update workspace deleteResponse schema to have void type and add a test for it.
    - Export types with T prefixed for auth and workspace
    …precated index.types
    
    - Import new auth schema types from keyshade/schema
    - Import existing workspace and pagination types from @keyshade/schema
    - Remove existing index.types and auth.types from api-client
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis 🔶

    519 - Partially compliant

    Fully compliant requirements:

    • Created auth module folder with auth.ts and auth.types.ts
    • Moved auth types from api-client to schema package
    • Updated api-client to use types from schema package

    Not compliant requirements:

    • Still need to implement for remaining modules: User, Project, Environment, Secret, Variable, Event, Integration
    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Type Validation
    Verify that ResendOTPRequestSchema email validation is sufficient - currently only validates string type without email format check

    Schema Consistency
    Verify that adding id fields to response schemas aligns with API contract and other module schemas

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Nov 11, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    ✅ Add validation to ensure email addresses are properly formatted

    Add email format validation to ensure the userEmail is a valid email address.

    packages/schema/src/auth/auth.ts [3]

    -export const ResendOTPRequestSchema = z.object({ userEmail: z.string() })
    +export const ResendOTPRequestSchema = z.object({ userEmail: z.string().email() })

    [Suggestion has been applied]

    Suggestion importance[1-10]: 9

    Why: Email validation is essential for security and data integrity. Without proper validation, invalid email addresses could cause issues with OTP delivery and authentication.

    9
    Best practice
    Add input validation constraints to prevent performance issues from unbounded search queries

    Add validation for the search query length in GlobalSearchRequestSchema to prevent
    overly long search queries that could impact performance.

    packages/schema/src/workspace/workspace.ts [115-118]

     export const GlobalSearchRequestSchema = z.object({
       workspaceSlug: z.string(),
    -  search: z.string()
    +  search: z.string().min(1).max(100)
     })
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding length constraints to search queries is crucial for preventing performance degradation and potential DoS attacks. The suggestion helps protect the API from resource exhaustion.

    8

    💡 Need additional feedback ? start a PR chat

    packages/schema/tests/auth.spec.ts Show resolved Hide resolved
    packages/schema/src/index.types.d.ts Outdated Show resolved Hide resolved
    packages/schema/src/index.ts Outdated Show resolved Hide resolved
    @rajdip-b
    Copy link
    Member

    The test is failing. Did you try to implement the fix you talked about? Rest looks good.

    @muntaxir4
    Copy link
    Contributor Author

    The test is failing. Did you try to implement the fix you talked about? Rest looks good.

    Yeah, the fix is almost done.

    @muntaxir4 muntaxir4 requested a review from rajdip-b November 12, 2024 17:49
    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.

    All the changes LGTM

    packages/schema/package.json Outdated Show resolved Hide resolved
    @muntaxir4 muntaxir4 requested a review from rajdip-b November 13, 2024 05:52
    @rajdip-b rajdip-b merged commit d880098 into keyshade-xyz:develop Nov 13, 2024
    5 checks passed
    @muntaxir4 muntaxir4 deleted the auth-schema branch November 19, 2024 04:42
    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