Feat/web3auth wallet #1679
Feat/web3auth wallet #1679
Conversation
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe changes introduce several new features and modifications across various files in the application. Notably, new environment variables for GraphQL API connections are added, and the authentication logic is refactored to utilize a context provider. Several new utility functions for user management are introduced, enhancing the application's interaction with a GraphQL backend. Additionally, modifications to existing components streamline logic and improve code organization. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant AtlasProvider
participant GraphQLAPI
User->>App: Initiates authentication
App->>AtlasProvider: Calls authoriseUser
AtlasProvider->>GraphQLAPI: Sends authentication request
GraphQLAPI-->>AtlasProvider: Returns access token
AtlasProvider-->>App: Provides user data
App-->>User: Displays user information
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
Code Climate has analyzed commit f1a7943 and detected 21 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (6)
web/src/utils/atlas/fetchUser.ts (1)
24-46: LGTM with suggestions!The
fetchUserfunction is correctly implemented with proper error handling and logging. However, consider the following improvements:
Use a proper logging library (e.g.,
winstonorpino) for error logging instead ofconsole.log. This will provide more flexibility and configurability for logging.Simplify the error message construction using optional chaining and nullish coalescing operators. For example:
-const errorMessage = Array.isArray(errors?.response?.errors) - ? errors.response.errors[0]?.message - : "Error user nonce"; +const errorMessage = errors?.response?.errors?.[0]?.message ?? "Error user nonce";This will make the code more concise and readable.
web/src/utils/atlas/getNonce.ts (2)
16-42: Improve the error handling.Consider the following suggestions to improve the error handling:
- Remove the console log at line 27 as it can leak sensitive information in production.
- Use a more descriptive error message that includes the error code and message from the API response.
Apply this diff to improve the error handling:
- // eslint-disable-next-line no-console - console.log("Error fetching nonce for address:", address, { errors }); const errorMessage = Array.isArray(errors?.response?.errors) - ? errors.response.errors[0]?.message + ? `${errors.response.errors[0]?.extensions?.code}: ${errors.response.errors[0]?.message}` : "Error fetching nonce";
16-42: Add a type annotation to the returned promise.Consider adding a type annotation to the returned promise to improve the type safety of the code.
Apply this diff to add a type annotation to the returned promise:
-export function getNonce(client: GraphQLClient, address: string): Promise<string> { +export function getNonce(client: GraphQLClient, address: string): Promise<string | undefined> {web/src/utils/atlas/addUser.ts (1)
20-49: Improve error handling by providing more specific error messages.The error handling in the
addUserfunction could be improved by providing more specific error messages based on the error type. For example, you could check for specific error codes or messages returned by the GraphQL API and display a corresponding error message in the toast notification.Here's an example of how you could improve the error handling:
.catch((errors) => { // eslint-disable-next-line no-console console.log("Add User error:", { errors }); + if (errors?.response?.errors?.[0]?.extensions?.code === "DUPLICATE_EMAIL") { + throw new Error("A user with this email already exists."); + } + const errorMessage = Array.isArray(errors?.response?.errors) ? errors.response.errors[0]?.message : "Unknown error"; throw new Error(errorMessage); }),In this example, we check for a specific error code returned by the GraphQL API (
DUPLICATE_EMAIL) and display a corresponding error message. You can add more checks for other error types as needed.web/src/utils/atlas/loginUser.ts (1)
23-53: Improve error handling and avoid usingeslint-disable-next-linecomment.The
loginUserfunction is well-structured and follows best practices. However, consider the following suggestions for improvement:
Provide more specific error messages based on the type of error. For example, distinguish between network errors and authentication errors.
Avoid using
eslint-disable-next-linecomment if possible. If the console log is necessary for debugging, consider using a logger library that can be easily enabled or disabled based on the environment.Here's an example of how you can improve the error handling:
.catch((errors) => { - // eslint-disable-next-line no-console - console.log("Authorization error:", { errors }); + if (errors.response) { + // The request was made and the server responded with a status code + // that falls out of the range of 2xx + console.error("Authentication error:", errors.response.status, errors.response.data); + } else if (errors.request) { + // The request was made but no response was received + console.error("No response received:", errors.request); + } else { + // Something happened in setting up the request that triggered an Error + console.error("Error setting up the request:", errors.message); + } const errorMessage = Array.isArray(errors?.response?.errors) ? errors.response.errors[0]?.message : "Unknown error"; throw new Error(errorMessage); }),web/src/context/AtlasProvider.tsx (1)
124-127: Simplify the boolean assignment.The static analysis tool suggests simplifying the code by directly assigning the result without using a ternary operator. Instead of using
user.email ? true : false, you can simply return!!user.emailto achieve the same result.Apply this diff to simplify the code:
-return user.email ? true : false; +return !!user.email;Tools
Biome
[error] 126-126: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (28)
- web/.env.devnet-neo.public (1 hunks)
- web/.env.devnet-university.public (1 hunks)
- web/.env.devnet.public (1 hunks)
- web/.env.local.public (1 hunks)
- web/.env.mainnet-neo.public (1 hunks)
- web/.env.testnet.public (1 hunks)
- web/netlify/middleware/authMiddleware.ts (2 hunks)
- web/package.json (2 hunks)
- web/src/app.tsx (2 hunks)
- web/src/components/EnsureAuth.tsx (1 hunks)
- web/src/consts/index.ts (1 hunks)
- web/src/context/AtlasProvider.tsx (1 hunks)
- web/src/context/Web3Provider.tsx (2 hunks)
- web/src/hooks/useSessionStorage.ts (1 hunks)
- web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/FormContact.tsx (2 hunks)
- web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (4 hunks)
- web/src/layout/Header/navbar/Menu/Settings/Notifications/index.tsx (1 hunks)
- web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx (1 hunks)
- web/src/pages/Resolver/index.tsx (1 hunks)
- web/src/utils/Web3AuthInstance.tsx (1 hunks)
- web/src/utils/atlas/addUser.ts (1 hunks)
- web/src/utils/atlas/createMessage.ts (1 hunks)
- web/src/utils/atlas/fetchUser.ts (1 hunks)
- web/src/utils/atlas/getNonce.ts (1 hunks)
- web/src/utils/atlas/index.ts (1 hunks)
- web/src/utils/atlas/loginUser.ts (1 hunks)
- web/src/utils/atlas/updateUser.ts (1 hunks)
- web/src/utils/uploadFormDataToIPFS.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- web/.env.devnet-neo.public
- web/.env.mainnet-neo.public
- web/src/utils/atlas/index.ts
Additional context used
Gitleaks
web/src/utils/Web3AuthInstance.tsx
48-48: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Biome
web/src/context/AtlasProvider.tsx
[error] 126-126: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
Additional comments not posted (46)
web/.env.local.public (1)
5-5: LGTM! The new environment variable is correctly defined.The addition of the
REACT_APP_ATLAS_URIenvironment variable pointing to a local GraphQL endpoint is approved.Please ensure that the GraphQL endpoint at
http://localhost:3000/graphqlis properly implemented and secured. Consider running the following script to verify the endpoint:web/.env.testnet.public (1)
6-6: LGTM!The new environment variable
REACT_APP_ATLAS_URIis correctly defined and aligns with the PR objective of introducing Web3Auth as a new login method.web/src/hooks/useSessionStorage.ts (1)
6-7: LGTM!The code change is approved. The hook now correctly uses
sessionStorageinstead oflocalStorage.web/.env.devnet.public (1)
7-7: LGTM!The new environment variable
REACT_APP_ATLAS_URIis correctly defined and follows the naming convention of the other environment variables in the file. The URL suggests that the application is connecting to a local GraphQL API, which is appropriate for a development environment. There are no security concerns with this variable as it is not exposing any sensitive information.web/.env.devnet-university.public (1)
8-8: LGTM!The added environment variable
REACT_APP_ATLAS_URIis correctly defined and used for development purposes to configure the application to connect to the Atlas service's GraphQL API. There are no security concerns with the environment variable, as it does not contain any sensitive information.web/src/utils/atlas/createMessage.ts (1)
5-23: LGTM!The
createMessagefunction is correctly implemented and follows the SIWE standard. It uses thecreateSiweMessagefunction from the well-known and trustedviemlibrary to create the SIWE message. The function sets a reasonable expiration time of 10 minutes for the SIWE message and uses theDEFAULT_CHAINconstant for the defaultchainIdvalue, which is a good practice to avoid hardcoding values. The function also uses thewindow.locationobject to set thedomainandorigin, which ensures that the SIWE message is specific to the current domain and origin.The code changes are approved.
web/src/components/EnsureAuth.tsx (5)
1-1: LGTM!The code segment is approved.
3-3: LGTM!The code segment is approved.
7-7: LGTM!The code segment is approved.
14-30: LGTM!The refactored
EnsureAuthcomponent looks good:
- It uses the
useAtlasProvidercontext for authentication, which reduces the complexity of the component and consolidates the authentication logic in a single place.- It no longer manages local state for loading and authentication, which reduces the complexity of the component and eliminates the possibility of state inconsistencies.
- It no longer has the
handleSignInfunction, which reduces the complexity of the component and eliminates the possibility of inconsistencies between the component and the context.- It now directly calls the
authoriseUserfunction from the context, which reduces the complexity of the component and eliminates the possibility of inconsistencies between the component and the context.
30-30: LGTM!The code segment is approved.
web/netlify/middleware/authMiddleware.ts (1)
25-25: Verify the impact of removingissuerandaudienceparameters fromjwt.jwtVerify.The
issuerandaudienceparameters have been removed from thejwt.jwtVerifyfunction call. This change simplifies the function call but may affect the verification process if the JWT library defaults to different values than those previously specified.Please verify the JWT library defaults and configurations to ensure that the removal of these parameters does not introduce any unintended consequences or security vulnerabilities.
Run the following script to search for other occurrences of
jwt.jwtVerifyin the codebase and analyze how the function is being used:web/src/utils/uploadFormDataToIPFS.ts (1)
6-6: LGTM!The code changes are approved.
web/src/utils/atlas/fetchUser.ts (2)
1-14: LGTM!The imports and type definitions are correctly set up.
16-22: LGTM!The GraphQL query is correctly defined.
web/src/utils/atlas/getNonce.ts (2)
1-5: LGTM!The imports are correctly used in the code.
6-14: LGTM!The type and GraphQL mutation query are correctly defined.
web/src/utils/atlas/updateUser.ts (5)
1-4: LGTM!The code changes are approved.
6-10: LGTM!The code changes are approved.
12-19: LGTM!The code changes are approved.
21-47: LGTM!The code changes are approved.
28-36: LGTM!The code changes are approved.
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/FormContact.tsx (2)
41-41: LGTM!The code change is approved. It simplifies the condition and enhances clarity by removing the unnecessary ternary operator.
54-54: LGTM!The code change is approved. It updates the dependency array of the
useMemohook to includeisEditing, ensuring that thefieldVariantlogic is responsive to changes in the editing state.web/src/layout/Header/navbar/Menu/Settings/Notifications/index.tsx (1)
8-8: LGTM!The change from named import to default import for the
EnsureAuthcomponent is consistent with its usage in theNotificationSettingscomponent and does not introduce any issues.web/src/consts/index.ts (1)
24-25: The new email regex pattern is more comprehensive but complex.The new regex pattern accommodates a wider range of valid email formats, enhancing the email validation capability. However, the pattern is complex and difficult to read, which can impact maintainability.
Consider adding comments to explain the different parts of the regex pattern for better maintainability.
Verify the regex against ReDoS attacks.
The regex pattern is marked with an
eslint-disable-next-linecomment to suppress thesecurity/detect-unsafe-regexwarning. This indicates that the regex might be vulnerable to ReDoS attacks.Verify the regex against ReDoS attacks using the following script:
If the regex is found to be vulnerable, consider alternative solutions such as:
- Using a simpler regex pattern that covers most common email formats.
- Using a library like validator.js for email validation.
web/src/utils/Web3AuthInstance.tsx (2)
9-29: LGTM!The code changes are approved.
31-75: LGTM!The code changes are approved.
Tools
Gitleaks
48-48: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
web/src/context/Web3Provider.tsx (4)
6-6: LGTM!The code changes are approved.
13-13: LGTM!The code changes are approved.
62-62: LGTM!The code changes are approved.
67-67: LGTM!The code changes are approved.
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (2)
Line range hint
41-80: LGTM!The changes in this code segment reflect a significant shift in how user data is managed within the component, moving from a settings-based approach to a more direct user management strategy using the
useAtlasProviderhook.The simplified logic for determining whether the email is being edited and the updated
useEffecthook ensure that the component only attempts to access user data when it is available.The modified form submission logic enhances the functionality by allowing for both user creation and updates within the same form submission process.
Overall, these changes improve the component's interaction with the user management system and streamline the code.
110-113: LGTM!The button's disabled state has been updated to include additional conditions based on the user management state, ensuring that the button is only enabled when appropriate. This change is consistent with the overall shift towards the new user management system.
web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx (1)
14-14: Verify that theEnsureAuthcomponent is exported as a default export.The import statement of the
EnsureAuthcomponent has been changed from a named import to a default import, which is fine in this file.However, please ensure that the
EnsureAuthcomponent is indeed exported as a default export in its source file (components/EnsureAuth) to avoid potential issues.Run the following script to verify the export:
Verification successful
Verification successful:
EnsureAuthis correctly exported as a default export.The
EnsureAuthcomponent is indeed exported as a default export in its source file, which aligns with the import change inSubmitEvidenceModal.tsx. No issues are found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `EnsureAuth` component is exported as a default export. # Test: Search for the default export in the `EnsureAuth` component file. Expect: A default export. rg --type typescript -g 'components/EnsureAuth' -e $'export default'Length of output: 102
Script:
#!/bin/bash # Description: Verify that the `EnsureAuth` component is exported as a default export. # Test: Search for the default export in the `EnsureAuth` component file. Expect: A default export. rg --glob '**/components/EnsureAuth*.{ts,tsx}' -e 'export default'Length of output: 129
web/src/app.tsx (2)
7-7: LGTM!The import statement for the
AtlasProvideris correctly written and consistent with the summary.
33-99: LGTM!The component hierarchy has been restructured to include the
AtlasProvideras a new wrapper around the existing context providers. This change is consistent with the summary and does not introduce any new routes or alter existing ones.web/package.json (4)
91-91: LGTM!The addition of
@web3auth/ethereum-providerdependency at version^8.12.4is approved. It is essential for enhancing the application's capabilities in handling Ethereum-based authentication and interactions.
92-92: LGTM!The addition of
@web3auth/modaldependency at version^8.12.6is approved. It is essential for enhancing the application's capabilities in handling Web3 authentication.
93-93: LGTM!The addition of
@web3auth/web3auth-wagmi-connectordependency at version^6.0.0is approved. It is essential for enhancing the application's capabilities in handling Web3 authentication and Wagmi integration.
124-124: Verify the reason for the downgrade.The
viemdependency is downgraded from version^2.1.0to^2.17.3. Please verify the reason for the downgrade. Is it to address compatibility issues or bugs in the newer version?web/src/context/AtlasProvider.tsx (5)
1-22: Imports and type definitions are properly organized.The code imports necessary dependencies from React and other libraries. It also imports custom utility functions and types from the
utils/atlasmodule, which provide type safety for the managed data.
23-37: Interface and context creation are properly implemented.The
IAtlasProviderinterface defines the shape of the context value, providing a clear contract. TheContextis properly created usingcreateContextfrom React with the correct type.
43-235:AtlasProvidercomponent is properly implemented.The
AtlasProvidercomponent manages the authentication state and user data using various hooks and functions. It uses theuseQueryhook fromreact-queryfor data fetching and caching, which is a good practice.The component provides functions to authorize the user, add a new user, and update user settings. These functions are properly implemented with error handling.
The context value is memoized using
useMemoto avoid unnecessary re-renders.Overall, the component seems to be properly structured and implements the necessary functionality.
Tools
Biome
[error] 126-126: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
237-243:useAtlasProviderhook is properly implemented.The
useAtlasProviderhook is a custom hook that provides access to the context value. It uses theuseContexthook from React to retrieve the context value and throws an error if the context is not found.This custom hook is a good way to encapsulate the context access logic and provides a convenient way for components to access the context value. The error handling ensures that the context is properly set up.
1-245: Overall assessment: The code is well-implemented and ready to be merged.The
AtlasProvidercomponent is properly structured and implements the necessary functionality for user authentication and data management using a context provider. It uses various libraries and custom utility functions to handle the authentication flow and data management effectively.The code follows good practices such as using
react-queryfor data fetching and caching, and memoizing the context value to avoid unnecessary re-renders. It provides a clear and convenient way for components to access the user data through theuseAtlasProviderhook.The static analysis hint has been addressed, and the code has been reviewed for potential improvements.
Overall, the code is well-organized, follows best practices, and is ready to be merged.
Tools
Biome
[error] 126-126: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
| const privateKeyProvider = new EthereumPrivateKeyProvider({ config: { chainConfig } }); | ||
|
|
||
| const web3AuthInstance = new Web3Auth({ | ||
| clientId: "BL8AeNAyRT1eZuwyXy3DimD4Ez4Bedcx2f7BxgQQdMVRE5fnlqtQp__XK-rGec1C5vbdQCTIxvtb7nPtgrpmjHQ", |
There was a problem hiding this comment.
Do not hardcode sensitive information in the source code.
The client ID string is a sensitive piece of information that should not be hardcoded in the source code. Hardcoding sensitive information in the source code is a security risk, as it can be easily exposed if the source code is compromised.
Store the client ID string in a secure location, such as an environment variable or a secrets management system, and access it at runtime. For example:
-const web3AuthInstance = new Web3Auth({
- clientId: "BL8AeNAyRT1eZuwyXy3DimD4Ez4Bedcx2f7BxgQQdMVRE5fnlqtQp__XK-rGec1C5vbdQCTIxvtb7nPtgrpmjHQ",
- chainConfig,
- privateKeyProvider,
- uiConfig: {
- appName: name,
- loginMethodsOrder: ["github", "google"],
- defaultLanguage: "en",
- modalZIndex: "2147483647",
- logoLight: "https://web3auth.io/images/web3authlog.png",
- logoDark: "https://web3auth.io/images/web3authlogodark.png",
- uxMode: "popup",
- mode: "light",
- },
- web3AuthNetwork: WEB3AUTH_NETWORK.SAPPHIRE_DEVNET,
- enableLogging: true,
-});
+const web3AuthInstance = new Web3Auth({
+ clientId: process.env.WEB3AUTH_CLIENT_ID,
+ chainConfig,
+ privateKeyProvider,
+ uiConfig: {
+ appName: name,
+ loginMethodsOrder: ["github", "google"],
+ defaultLanguage: "en",
+ modalZIndex: "2147483647",
+ logoLight: "https://web3auth.io/images/web3authlog.png",
+ logoDark: "https://web3auth.io/images/web3authlogodark.png",
+ uxMode: "popup",
+ mode: "light",
+ },
+ web3AuthNetwork: WEB3AUTH_NETWORK.SAPPHIRE_DEVNET,
+ enableLogging: true,
+});Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| clientId: "BL8AeNAyRT1eZuwyXy3DimD4Ez4Bedcx2f7BxgQQdMVRE5fnlqtQp__XK-rGec1C5vbdQCTIxvtb7nPtgrpmjHQ", | |
| clientId: process.env.WEB3AUTH_CLIENT_ID, |
Tools
Gitleaks
48-48: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
|
Only 4 file changes after changing the base branch to feat/atlas-siwe :) |
This PR inclues Web3Auth as a login method to enable social logins.
This connector allow users to login with their email (or facebook, github, etc) to kleros court. Behind the scenes a wallet is created and all the txs signs are handled without interaction from the user.
I don't think that this is a great wallet considering safety, or transparency of the txs that you are making, but will be great to onboard new jurors without crypto knowleadge and remove some frictions in the onboarding. For example, when you perform a stake, or approval or whatever, there is no popup to sign or nothing like that, all the transactions are signed automatically. This creates a "web2"experience, and I think that is great for users without crypto knowleadge. We can include in the docs or somewhere some disclaimers abourt not having a lot of money on that wallet).
Web3Auth is not free, but in the free plan there are until 1000 monthly active users.
This is very draft and more a PoC than a PR to be merged. Opening here to be discussed.
PR-Codex overview
This PR focuses on restructuring API calls and environment variables for better functionality and security.
Detailed summary
Summary by CodeRabbit
New Features
REACT_APP_ATLAS_URIfor local GraphQL API connections across multiple configuration files.AtlasProviderfor enhanced user authentication and data interactions.Improvements
EnsureAuthcomponent.FormContactDetailscomponent.Bug Fixes
Documentation