This repository has been archived by the owner on Jul 2, 2024. It is now read-only.
Critical Authentication Hot fixes (Urgent) #149
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…an optimized production build
…robust for all account uses cases from Clerk
ColinLefter
changed the title
Hocng7
approved these changes
Mar 26, 2024
| @@ -88,7 +88,7 @@ export function FriendsTab(props: TextInputProps) { | |||
|
|
|||
| if (activeChat && user?.id) { // Ensure both activeChat and user.id are defined | |||
| return <Chat | |||
| sender={user.id} | |||
| sender={user.username as any} | |||
There was a problem hiding this comment.
Great change to using username so that it fits with our current database field name
| @@ -8,12 +8,19 @@ export default authMiddleware({ | |||
| publicRoutes: [ | |||
| '/', | |||
| '/api/webhooks/clerk-mongodb-sync', | |||
| '/api/registration', | |||
| '/api/update-user-data', | |||
| '/api/ably-auth', | |||
There was a problem hiding this comment.
Good inclusion of the path to all the APIs that are integrated so far.
| @@ -2,7 +2,7 @@ import { NextApiRequest, NextApiResponse } from 'next'; | |||
| import Ably from 'ably/promises'; | |||
|
|
|||
| export default async function handler(req: NextApiRequest, res: NextApiResponse) { | |||
| if (req.method === 'GET') { | |||
| if (req.method === 'POST') { | |||
There was a problem hiding this comment.
- Ensure robust error handling, especially around asynchronous operations like network requests. While you have a try-catch block to handle errors from Ably, consider adding specific error handling for different types of errors to provide more informative responses to clients.
| @@ -5,7 +5,7 @@ import { getMongoDbUri } from '@/lib/dbConfig'; | |||
|
|
|||
| export default async function handler(req: NextApiRequest, res: NextApiResponse) { | |||
| if (req.method === 'POST') { | |||
| const { id } = JSON.parse(req.body); // Assuming the email is sent in the body | |||
| const { id } = req.body; // Directly destructuring `id` from `req.body` because it's already an object | |||
There was a problem hiding this comment.
- Great code refactoring to reduce redundancy.
|
|
||
| try { | ||
| client = new MongoClient(getMongoDbUri()); | ||
| await client.connect(); | ||
| const db = client.db('Accord'); | ||
|
|
||
| const accountsCollection = db.collection("Accounts"); | ||
| // Querying the database by the userName we received | ||
|
|
||
| // First, check if an account with the same 'id' or 'userName' already exists |
There was a problem hiding this comment.
- Great change to ensure that there is no duplication in username and ensuring that an account for that particular id has not been created before.
| } catch (error) { | ||
| console.error(error); | ||
| console.error('Failed to register user:', error); |
There was a problem hiding this comment.
- Enhance error handling to provide more informative error messages to clients and aid in troubleshooting. Instead of a generic "Internal server error" message, consider providing more context about the specific error that occurred.
| }); | ||
|
|
||
| if (!response.ok) { |
There was a problem hiding this comment.
- Enhance error handling to provide more informative error messages and to handle different error scenarios appropriately. For example, in the user.deleted case, ensure that errors from the API call are properly handled, and consider providing more context about the nature of the error.
| @@ -1,3 +1,3 @@ | |||
| export function MyAccount() { | |||
| export default function MyAccount() { | |||
There was a problem hiding this comment.
- This can be good code-refactoring. However, if we need to export multiple items or want to be more explicit about what you're exporting in the future, we can use named exports (without the default) instead.
There was a problem hiding this comment.
react complain if not default. i make default. react happy. i happy.
This was
linked to
issues
Mar 26, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of Changes
clerk-mongodb-sync.tsnow checks if the account already exists in our database)delete-user.ts)npm run accordcommand inpackage.jsonwas launching the wrongngrokserver (the randomly generated one instead of the static domain, hence we were effectively creating 2 servers each time if you would run thengrokcommand for the static server I created before doingnpm run accord).