fix: Login Error #390
fix: Login Error #390
Conversation
|
@vinayak-vohra is attempting to deploy a commit to the Sanchit Bajaj's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe recent updates involve changes to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginComponent
participant AuthAPI
User->>LoginComponent: Enters email and password
LoginComponent->>AuthAPI: Calls login(email, password)
AuthAPI->>AuthAPI: Handle session deletion, error handling
AuthAPI->>LoginComponent: Return { user, expires }
LoginComponent->>LoginComponent: Sets cookies with expiration
LoginComponent->>User: Returns login response
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
github-actions
bot
added
wait for reviewers
gssoc
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Great job, @vinayak-vohra! 🎉 Thank you for submitting your pull request. Your contribution is valuable and we appreciate your efforts to improve our project. Keep being the part of the community!
'We will promptly review your changes and offer feedback. Keep up the excellent work!
Kindly remember to check our Contributing Guidelines'
github-actions
bot
removed
the
title needs formatting
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/backend/auth.api.ts (1 hunks)
- src/components/pages/auth/login/index.tsx (2 hunks)
Additional context used
Biome
src/backend/auth.api.ts
[error] 120-120: Change to an optional chain. (lint/complexity/useOptionalChain)
Unsafe fix: Change to an optional chain.
src/components/pages/auth/login/index.tsx
[error] 194-200: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
[error] 269-275: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
[error] 280-286: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)
The default type of a button is submit, which causes the submission of a form when placed inside a
formelement. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
Additional comments not posted (2)
src/backend/auth.api.ts (1)
146-159: Ensure proper error handling and session management in the updatedloginfunction.The changes made to handle session-specific errors and manage session expiration are aligned with best practices for robust authentication systems. However, consider adding more specific logging for different types of errors to aid in debugging and maintenance.
src/components/pages/auth/login/index.tsx (1)
Line range hint
67-83: Update LoginComponent to handle new response structure from the login function.The modifications in LoginComponent to accommodate the
expiresfield in the cookie settings align with security best practices by synchronizing session expiration on both the server and client sides. This should help prevent session hijacking and other common security issues.
|
@Sanchitbajaj02 please verify. |
| const user = await getUserByAccountId(accountId); | ||
| return user; | ||
| return { user, expires: new Date(expire) }; |
There was a problem hiding this comment.
this will expire immediately, you need to add some buffer time. I prefer to add 2 days
There was a problem hiding this comment.
I would like to know why would this expire immediately?
As per the Appwrite docs, the session object contains an expire property which is a date string. This string is converted to a date object here because it is required as such by nookies.
Also, the cookie expire time is set to be in sync with the appwrite session validation (1 year by default), that can be changed.
Cookie expire time (matches appwrite session length):
Appwrite console to set session length (auth > security):
Changing this to 2 days would set the cookie expire time to 2 days for users.
There was a problem hiding this comment.
Let me test that out locally, and then finalize the PR
Sanchitbajaj02
added
🪲 bug
Related Issue
fixes: #353
Description
deleteSessions()todeleteSession("current")to delete only active user session, if any, before login.deleteSession()in a separate try-catch block to bypassgenerate_unauthorized_scopeerror, which means no active sessions were found (and hence can be ignored).Screenshots
Flow 1: Normal Login
Palettegram.-.Login.mp4
Flow 2: Session active in Appwrite, but not on browser (accidental cookie clear by user)
User.Dashboard._.Palettegram.mp4
Flow 3: Logged in user
Palettegram.mp4
Summary by CodeRabbit
New Features
Bug Fixes
Improvements