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: adding support for Passkeys #99

Merged
merged 3 commits into from
Jul 14, 2024
Merged

Conversation

aversini
Copy link
Collaborator

@aversini aversini commented Jul 14, 2024

PR Type

Enhancement, Dependencies


Description

  • Added support for Passkey registration and login in the AuthProvider component.
  • Implemented GraphQL queries and utility functions for Passkey operations.
  • Updated AuthContext and AuthContextProps to include new Passkey methods.
  • Added new buttons and handlers in the App component for Passkey registration and login.
  • Removed unnecessary console log from isAllowed decorator in Fastify server.
  • Updated dependencies to include @simplewebauthn/browser.

Changes walkthrough 📝

Relevant files
Enhancement
main.tsx
Add Passkey registration and login functionality                 

examples/code-flow/src/main.tsx

  • Added registeringForPasskey and loginWithPasskey methods to useAuth
    destructuring.
  • Implemented handleValidRegistration and handleValidLoginWithPasskey
    functions.
  • Added new buttons for Passkey registration and login.
  • +39/-2   
    types.d.ts
    Extend AuthContextProps with Passkey methods                         

    packages/auth-provider/src/common/types.d.ts

  • Added AUTH_TYPES import.
  • Updated LoginType to include AUTH_TYPES.CODE and AUTH_TYPES.PASSKEY.
  • Added registeringForPasskey and loginWithPasskey methods to
    AuthContextProps.
  • +4/-1     
    utilities.ts
    Add GraphQL queries and utility for Passkey support           

    packages/auth-provider/src/common/utilities.ts

  • Added GraphQL queries for Passkey registration and authentication.
  • Implemented graphQLCall function for making GraphQL requests.
  • +139/-0 
    AuthContext.ts
    Add Passkey methods stubs to AuthContext                                 

    packages/auth-provider/src/components/AuthProvider/AuthContext.ts

  • Added registeringForPasskey and loginWithPasskey stubs to AuthContext.

  • +2/-0     
    AuthProvider.tsx
    Implement Passkey registration and login in AuthProvider 

    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx

  • Imported startAuthentication and startRegistration from
    @simplewebauthn/browser.
  • Implemented registeringForPasskey and loginWithPasskey methods.
  • Updated AuthContext.Provider to include new methods.
  • +128/-1 
    Miscellaneous
    fastify.ts
    Remove console log from isAllowed decorator                           

    examples/fastify-server/src/common/fastify.ts

    • Removed console log statement from isAllowed decorator.
    +0/-1     
    Dependencies
    package.json
    Add @simplewebauthn/browser dependency                                     

    packages/auth-provider/package.json

    • Added @simplewebauthn/browser as a dependency.
    +1/-0     
    pnpm-lock.yaml
    Update lockfile for new dependencies                                         

    pnpm-lock.yaml

  • Updated lockfile to include @simplewebauthn/browser and related
    dependencies.
  • +35/-20 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Summary by CodeRabbit

    • New Features

      • Introduced passkey registration and login functionality in the app.
      • Added buttons for registering and logging in with a passkey to the UI.
    • Dependencies

      • Added @simplewebauthn/browser dependency for enhanced web authentication capabilities.
    • Enhancements

      • Improved registration and authentication processes within the AuthProvider component.

    Copy link

    coderabbitai bot commented Jul 14, 2024

    Walkthrough

    The changes introduce new functionality for passkey registration and authentication within the AuthProvider component. This includes adding new methods and event handlers in the App component and updating the useAuth hook. Additionally, GraphQL queries for registration and authentication are added, and a new dependency, @simplewebauthn/browser, is included for handling WebAuthn processes.

    Changes

    File(s) Change Summary
    examples/code-flow/src/main.tsx Added new event handlers and buttons for passkey registration and login, updated useAuth hook.
    examples/fastify-server/src/common/fastify.ts Removed a logging statement within the decorate method for isAllowed.
    packages/auth-provider/package.json Added @simplewebauthn/browser dependency version 10.0.0.
    packages/auth-provider/src/common/types.d.ts Modified LoginType to accept AUTH_TYPES, added new functions to AuthContextProps.
    packages/auth-provider/src/common/utilities.ts Added GraphQL queries and service types for registration and authentication, introduced graphQLCall function.
    packages/auth-provider/src/components/AuthProvider/AuthContext.ts Added new functions registeringForPasskey and loginWithPasskey to AuthContext.
    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx Integrated new functions for registration and authentication processes, updated AuthContext.Provider value assignment.

    Sequence Diagram(s)

    Passkey Registration Flow

    sequenceDiagram
        participant User
        participant App
        participant AuthProvider
        participant SimpleWebAuthn
        participant GraphQL API
    
        User ->> App: Click "Register with Passkey"
        App ->> AuthProvider: handleValidRegistration()
        AuthProvider ->> SimpleWebAuthn: startRegistration()
        SimpleWebAuthn ->> User: Display registration prompt
        User ->> SimpleWebAuthn: Complete registration
        SimpleWebAuthn ->> AuthProvider: Registration data
        AuthProvider ->> GraphQL API: Send registration data
        GraphQL API ->> AuthProvider: Confirmation
        AuthProvider ->> App: Update registration status
        App ->> User: Display success message
    
    Loading

    Passkey Login Flow

    sequenceDiagram
        participant User
        participant App
        participant AuthProvider
        participant SimpleWebAuthn
        participant GraphQL API
    
        User ->> App: Click "Login with Passkey"
        App ->> AuthProvider: handleValidLoginWithPasskey()
        AuthProvider ->> SimpleWebAuthn: startAuthentication()
        SimpleWebAuthn ->> User: Display login prompt
        User ->> SimpleWebAuthn: Complete login
        SimpleWebAuthn ->> AuthProvider: Authentication data
        AuthProvider ->> GraphQL API: Send authentication data
        GraphQL API ->> AuthProvider: Confirmation
        AuthProvider ->> App: Update login status
        App ->> User: Display success message
    
    Loading

    Tips

    Chat

    There are 3 ways to chat with CodeRabbit:

    • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
      • I pushed a fix in commit <commit_id>.
      • Generate unit testing code for this file.
      • Open a follow-up GitHub issue for this discussion.
    • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
      • @coderabbitai generate unit testing code for this file.
      • @coderabbitai modularize this function.
    • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
      • @coderabbitai generate interesting stats about this repository and render them as a table.
      • @coderabbitai show all the console.log statements in this repository.
      • @coderabbitai read src/utils.ts and generate unit testing code.
      • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
      • @coderabbitai help me debug CodeRabbit configuration file.

    Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

    CodeRabbit Commands (invoked as PR comments)

    • @coderabbitai pause to pause the reviews on a PR.
    • @coderabbitai resume to resume the paused reviews.
    • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
    • @coderabbitai full review to do a full review from scratch and review all the files again.
    • @coderabbitai summary to regenerate the summary of the PR.
    • @coderabbitai resolve resolve all the CodeRabbit review comments.
    • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
    • @coderabbitai help to get help.

    Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

    CodeRabbit Configration File (.coderabbit.yaml)

    • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
    • Please see the configuration documentation for more information.
    • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

    Documentation and Community

    • Visit our Documentation for detailed information on how to use CodeRabbit.
    • Join our Discord Community to get help, request features, and share feedback.
    • Follow us on X/Twitter for updates and announcements.

    @qodo-merge-pro qodo-merge-pro bot added enhancement New feature or request dependencies Pull requests that update a dependency file Review effort [1-5]: 4 labels Jul 14, 2024
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Error Handling
    The error handling in the registeringForPasskey and loginWithPasskey methods could be improved. Currently, it logs the error but does not provide feedback to the user or rethrow the error for further handling upstream. Consider adding user feedback or error propagation.

    GraphQL Query Structure
    The GraphQL queries are directly embedded within the utility functions. This could lead to maintenance challenges and makes the code less modular. Consider extracting these queries into a separate module or file to improve maintainability and separation of concerns.

    Button Disabled Logic
    The logic for disabling buttons in the UI based on authentication state might lead to unintended behavior. For instance, the 'Register for Passkey (valid)' button is disabled if the user is not authenticated, which might not align with the intended use case of registering new users. Review and adjust the logic as necessary.

    Copy link

    Bundle Size

    Status File Size (Gzip) Limits
    index.js 13.65 KB (+3.18 KB +30.32%) 15 kb

    Overall bundle size: 13.65 KB (+3.18 KB +30.32%)
    Overall status: ✅

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Securit
    Avoid exposing sensitive data in error logs to enhance security

    To avoid potential security risks, ensure that sensitive data such as accessToken is
    not exposed in error logs or in any part of the client-facing application. Consider
    removing or obfuscating sensitive data before logging or handling errors.

    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx [368]

    -console.error(_error);
    +console.error('An error occurred:', { message: _error.message });
     
    • Apply this suggestion
    Suggestion importance[1-10]: 10

    Why: This suggestion enhances security by ensuring that sensitive data is not exposed in error logs. It correctly modifies the logging to avoid printing sensitive information, which is a best practice for security.

    10
    Possible bug
    Add error handling for asynchronous calls to prevent crashes and allow graceful error handling

    It's recommended to handle potential exceptions when calling asynchronous functions
    like graphQLCall within the registeringForPasskey and loginWithPasskey methods. This
    ensures that any network or parsing errors do not cause the application to crash and
    allows for graceful error handling.

    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx [279-287]

    -let response = await graphQLCall({
    -    accessToken,
    -    clientId,
    -    type: SERVICE_TYPES.GET_REGISTRATION_OPTIONS,
    -    params: {
    +let response;
    +try {
    +    response = await graphQLCall({
    +        accessToken,
             clientId,
    -        id: user?.userId,
    -        username: user?.username,
    -    },
    -});
    +        type: SERVICE_TYPES.GET_REGISTRATION_OPTIONS,
    +        params: {
    +            clientId,
    +            id: user?.userId,
    +            username: user?.username,
    +        },
    +    });
    +} catch (error) {
    +    console.error('Failed to get registration options:', error);
    +    return false;
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding error handling for asynchronous calls is crucial to prevent application crashes and ensure graceful error handling. This suggestion correctly addresses a potential bug by wrapping the graphQLCall in a try-catch block.

    9
    Best practice
    Ensure consistent TypeScript version across all dependencies

    Consider using a consistent version of TypeScript across all dependencies to avoid
    potential compatibility issues. The version 5.5.3 is used in some dependencies,
    while 5.5.2 is used in others. This inconsistency can lead to unexpected behavior
    during development and production.

    pnpm-lock.yaml [12345]

    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Ensuring a consistent TypeScript version across all dependencies is crucial for avoiding compatibility issues and unexpected behavior during development and production. This suggestion correctly identifies and addresses the inconsistency.

    9
    Consistency
    Align TypeScript version in @vue/language-core with other dependencies

    Update the typescript version in the @vue/language-core dependency to 5.5.3 to align
    with other dependencies and avoid potential issues with mismatched versions.

    pnpm-lock.yaml [12534]

    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Aligning the TypeScript version in @vue/language-core with other dependencies helps maintain consistency and avoid potential issues with mismatched versions. This suggestion is accurate and beneficial.

    8
    Maintainability
    Standardize @microsoft/api-extractor version across all dependencies

    Update the @microsoft/api-extractor version to 7.43.0 in all dependencies where it
    is used to ensure consistency and compatibility with the latest TypeScript version.

    pnpm-lock.yaml [12342]

    -'@microsoft/[email protected](@types/[email protected])'
    +'@microsoft/[email protected]'
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Standardizing the version of @microsoft/api-extractor can help maintain compatibility and reduce potential issues. However, the suggestion does not provide a significant improvement over the existing code as the version is already consistent.

    7

    Copy link

    @coderabbitai coderabbitai bot left a comment

    Choose a reason for hiding this comment

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

    Actionable comments posted: 0

    Review details

    Configuration used: .coderabbit.yaml
    Review profile: CHILL

    Commits

    Files that changed from the base of the PR and between a421ddd and 7f7c45c.

    Files ignored due to path filters (1)
    • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
    Files selected for processing (7)
    • examples/code-flow/src/main.tsx (3 hunks)
    • examples/fastify-server/src/common/fastify.ts (1 hunks)
    • packages/auth-provider/package.json (1 hunks)
    • packages/auth-provider/src/common/types.d.ts (2 hunks)
    • packages/auth-provider/src/common/utilities.ts (1 hunks)
    • packages/auth-provider/src/components/AuthProvider/AuthContext.ts (1 hunks)
    • packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx (3 hunks)
    Files skipped from review due to trivial changes (1)
    • examples/fastify-server/src/common/fastify.ts
    Additional context used
    Learnings (1)
    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx (1)
    Learnt from: aversini
    PR: aversini/auth-client#21
    File: packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx:1-89
    Timestamp: 2024-06-24T18:39:55.099Z
    Learning: Avoid detailed error logging in security-sensitive components like authentication to prevent giving potential attackers insights into the system.
    
    Additional comments not posted (16)
    packages/auth-provider/src/components/AuthProvider/AuthContext.ts (2)

    16-16: LGTM!

    The addition of registeringForPasskey to AuthContext with a default value of stub is appropriate.


    17-17: LGTM!

    The addition of loginWithPasskey to AuthContext with a default value of stub is appropriate.

    packages/auth-provider/src/common/types.d.ts (3)

    1-1: LGTM!

    The import of AUTH_TYPES from @versini/auth-common is necessary for the new types being added.


    35-35: LGTM!

    The update to LoginType to accept AUTH_TYPES.CODE or AUTH_TYPES.PASSKEY is appropriate and necessary for supporting passkey authentication.


    43-44: LGTM!

    The addition of registeringForPasskey and loginWithPasskey to AuthContextProps is appropriate and necessary for supporting passkey authentication.

    packages/auth-provider/package.json (1)

    45-45: LGTM!

    The addition of @simplewebauthn/browser version 10.0.0 to the dependencies is appropriate and necessary for supporting passkey authentication.

    examples/code-flow/src/main.tsx (5)

    8-16: LGTM!

    The update to the useAuth hook to include registeringForPasskey and loginWithPasskey is appropriate and necessary for supporting passkey-based registration and login.


    78-84: LGTM!

    The addition of the handleValidRegistration function to handle passkey registration is appropriate and necessary.


    85-91: LGTM!

    The addition of the handleValidLoginWithPasskey function to handle passkey login is appropriate and necessary.


    110-117: LGTM!

    The addition of the button for passkey registration is appropriate and necessary.


    118-131: LGTM!

    The addition of the button for passkey login is appropriate and necessary.

    packages/auth-provider/src/common/utilities.ts (3)

    234-307: GraphQL Queries Addition Approved

    The GraphQL queries for passkey registration and authentication are well-defined and follow best practices.


    308-325: Service Types Addition Approved

    The service types mapping for the GraphQL queries is correctly defined and follows best practices.


    327-370: GraphQL Call Function Addition Approved

    The graphQLCall function is well-implemented, handling errors correctly and following best practices.

    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx (2)

    277-318: Passkey Registration Function Addition Approved

    The registeringForPasskey function is well-implemented, handling errors correctly and following best practices.


    320-388: Passkey Login Function Addition Approved

    The loginWithPasskey function is well-implemented, handling errors correctly and following best practices.

    @aversini aversini merged commit d005ae1 into main Jul 14, 2024
    4 checks passed
    @aversini aversini deleted the feat-adding-support-for-Passkeys branch July 14, 2024 22:41
    @aversini aversini mentioned this pull request Jul 14, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    dependencies Pull requests that update a dependency file enhancement New feature or request Review effort [1-5]: 4
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant