Merged
Conversation
…le Slate DOM sync errors during page transitions
… for new OTP users
Reviewer's GuideImproves authentication and guest login flows by adding structured logging, tightening redirect behavior after auth (especially for magic link/OTP and guest invitations), handling workspace-load failures via context, and hardening the editor against transient DOM resolution errors. Sequence diagram for guest invitation magic link/OTP login flowsequenceDiagram
actor User
participant Browser
participant AsGuest
participant LoginPage
participant AuthService
participant GoTrue
participant AppFlowyCloudAPI as AppFlowyCloud
participant afterAuthFn as afterAuth
participant AuthContext
participant EventBus
participant Backend
User->>Browser: Open /app/accept-guest-invitation?workspaceId&code
Browser->>AsGuest: Render AsGuest
AsGuest->>AuthContext: useIsAuthenticatedOptional
AuthContext-->>AsGuest: isAuthenticated = false
AsGuest->>Browser: window.open(/login?redirectTo=invitationURL)
Browser->>LoginPage: Render login UI
User->>LoginPage: Request magic link / OTP
LoginPage->>GoTrue: signInWithMagicLink or OTP request
GoTrue-->>User: Email with magic link / OTP code
User->>Browser: Click magic link or submit OTP code
Browser->>AuthService: signInOTP(email, redirectTo, code, type)
AuthService->>GoTrue: POST /verify OTP or magic link
GoTrue-->>AuthService: access_token, refresh_token
AuthService->>Browser: saveGoTrueAuth(token)
AuthService->>AppFlowyCloudAPI: verifyToken(access_token)
AppFlowyCloudAPI-->>AuthService: is_new flag
alt verifyToken success
AuthService->>EventBus: emit SESSION_VALID
AuthService->>afterAuthFn: afterAuth()
afterAuthFn->>Browser: Read redirectTo from localStorage
alt redirectTo present and not user specific
afterAuthFn->>Browser: window.location.href = redirectTo
else redirectTo missing or user specific
afterAuthFn->>Browser: window.location.href = /app
end
else verifyToken failure
AuthService->>EventBus: emit SESSION_INVALID
AuthService-->>Browser: Reject with error
end
Browser->>AsGuest: Re-render AsGuest at invitation URL
AsGuest->>AuthContext: useIsAuthenticatedOptional
AuthContext-->>AsGuest: isAuthenticated = true
AsGuest->>AsGuest: loadInvitation()
AsGuest->>Backend: GET guest invitation info
Backend-->>AsGuest: Invitation details
AsGuest-->>User: Show shared workspace as guest
Sequence diagram for OAuth callback processing via loginAuth and signInWithUrlsequenceDiagram
actor User
participant Browser
participant OAuthProvider
participant AuthService
participant loginAuthFn as loginAuth
participant signInWithUrlFn as signInWithUrl
participant AppFlowyCloud as AppFlowyCloudAPI
participant GoTrue as GoTrueAPI
participant afterAuthFn as afterAuth
User->>Browser: Click Sign in with Google/GitHub/Apple/Discord
Browser->>GoTrueAPI: /authorize?provider=...
GoTrueAPI->>OAuthProvider: Redirect for consent
OAuthProvider-->>GoTrueAPI: Redirect back with tokens in hash or error
GoTrueAPI-->>Browser: Redirect to /login/auth#access_token&refresh_token
Browser->>LoginAuth: Render LoginAuth component
LoginAuth->>AuthService: login(window.location.href)
AuthService->>loginAuthFn: loginAuth(url)
loginAuthFn->>signInWithUrlFn: signInWithUrl(url)
signInWithUrlFn->>signInWithUrlFn: parseGoTrueErrorFromUrl(url)
alt GoTrue error in URL
signInWithUrlFn-->>loginAuthFn: Reject { code, message }
loginAuthFn->>EventBus: emit SESSION_INVALID
loginAuthFn-->>AuthService: Reject error
else No GoTrue error
signInWithUrlFn->>signInWithUrlFn: Extract access_token, refresh_token from hash
alt Tokens missing
signInWithUrlFn-->>loginAuthFn: Reject Missing tokens
loginAuthFn->>EventBus: emit SESSION_INVALID
else Tokens present
signInWithUrlFn->>Browser: localStorage.removeItem(token)
signInWithUrlFn->>AppFlowyCloudAPI: verifyToken(access_token)
alt verifyToken fails
AppFlowyCloudAPI-->>signInWithUrlFn: Error
signInWithUrlFn-->>loginAuthFn: Reject Verify token failed
loginAuthFn->>EventBus: emit SESSION_INVALID
else verifyToken succeeds
AppFlowyCloudAPI-->>signInWithUrlFn: OK
signInWithUrlFn->>GoTrueAPI: refreshToken(refresh_token)
alt refreshToken fails
GoTrueAPI-->>signInWithUrlFn: Error
signInWithUrlFn-->>loginAuthFn: Reject Refresh token failed
loginAuthFn->>EventBus: emit SESSION_INVALID
else refreshToken succeeds
GoTrueAPI-->>signInWithUrlFn: New tokens
signInWithUrlFn-->>loginAuthFn: Resolve
loginAuthFn->>EventBus: emit SESSION_VALID
loginAuthFn->>afterAuthFn: afterAuth()
afterAuthFn->>Browser: Redirect based on redirectTo or /app
end
end
end
end
Class diagram for updated auth context and workspace redirect handlingclassDiagram
class AuthInternalContextType {
+UserWorkspaceInfo userWorkspaceInfo
+string currentWorkspaceId
+boolean isAuthenticated
+boolean enablePageHistory
+Error workspaceInfoError
+function onChangeWorkspace(workspaceId: string) Promise~void~
+function retryLoadWorkspaceInfo() void
}
class AppAuthLayer {
+UserWorkspaceInfo userWorkspaceInfo
+Error workspaceInfoError
+boolean enablePageHistory
+string currentWorkspaceId
+boolean isAuthenticated
+function loadUserWorkspaceInfo() Promise~UserWorkspaceInfo~
+function onChangeWorkspace(workspaceId: string) Promise~void~
+function retryLoadWorkspaceInfo() void
}
class AppWorkspaceRedirect {
+UserWorkspaceInfo userWorkspaceInfo
+boolean hasError
+function useEffectRedirect() void
}
class AsGuest {
+boolean loading
+boolean isInvalid
+boolean isError
+string invalidMessage
+boolean isAuthenticated
+function loadInvitation() Promise~void~
}
AppAuthLayer ..> AuthInternalContextType : provides
AppWorkspaceRedirect ..> AuthInternalContextType : consumes
AsGuest ..> AuthInternalContextType : uses isAuthenticated
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The global monkey-patching of
ReactEditor.hasDOMNodeinCollaborativeEditor.tsxis a heavy module-level side effect; consider moving this into a dedicated initialization utility (guarded to run once) so it’s easier to reason about and avoids surprises during HMR or potential SSR. - In
AsGuest,useIsAuthenticatedOptional()is treated as a simple boolean and unauthenticated users are immediately redirected, which may also trigger while auth state is still loading; consider exposing and checking an explicit loading/initialized flag to avoid redirect flicker or premature redirects.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The global monkey-patching of `ReactEditor.hasDOMNode` in `CollaborativeEditor.tsx` is a heavy module-level side effect; consider moving this into a dedicated initialization utility (guarded to run once) so it’s easier to reason about and avoids surprises during HMR or potential SSR.
- In `AsGuest`, `useIsAuthenticatedOptional()` is treated as a simple boolean and unauthenticated users are immediately redirected, which may also trigger while auth state is still loading; consider exposing and checking an explicit loading/initialized flag to avoid redirect flicker or premature redirects.
## Individual Comments
### Comment 1
<location path="src/application/services/js-services/http/gotrue.ts" line_range="62-65" />
<code_context>
}
export async function signInWithPassword(params: { email: string; password: string; redirectTo: string }) {
+ Log.info('[Auth] signInWithPassword: starting', { email: params.email });
try {
const response = await axiosInstance?.post<{
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider avoiding logging raw email addresses and other PII in auth-related logs.
These new log statements include raw email addresses (e.g., in `signInWithPassword`, `signUpWithPassword`, `forgotPassword`), which can create privacy and compliance issues depending on where logs are stored and for how long. Prefer omitting the email or logging a redacted/derived value (e.g., `userId` or a stable hash of the email) while keeping useful non-PII context like flow name and error codes.
Suggested implementation:
```typescript
Log.info('[Auth] signInWithPassword: starting', { hasEmail: Boolean(params.email) });
```
```typescript
Log.info('[Auth] signUpWithPassword: starting', { hasEmail: Boolean(params.email) });
```
```typescript
Log.info('[Auth] forgotPassword: starting', { hasEmail: Boolean(params.email) });
```
If the exact log messages differ slightly from the `SEARCH` strings above (e.g., different log text or object shape), adjust the `SEARCH` strings to match your current code. The key change is to remove `email: params.email` from the logged object and, if desired, replace it with a non-PII flag like `hasEmail: Boolean(params.email)` or omit the second argument entirely.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+62
to
65
| Log.info('[Auth] signInWithPassword: starting', { email: params.email }); | ||
| try { | ||
| const response = await axiosInstance?.post<{ | ||
| access_token: string; |
There was a problem hiding this comment.
🚨 suggestion (security): Consider avoiding logging raw email addresses and other PII in auth-related logs.
These new log statements include raw email addresses (e.g., in signInWithPassword, signUpWithPassword, forgotPassword), which can create privacy and compliance issues depending on where logs are stored and for how long. Prefer omitting the email or logging a redacted/derived value (e.g., userId or a stable hash of the email) while keeping useful non-PII context like flow name and error codes.
Suggested implementation:
Log.info('[Auth] signInWithPassword: starting', { hasEmail: Boolean(params.email) }); Log.info('[Auth] signUpWithPassword: starting', { hasEmail: Boolean(params.email) }); Log.info('[Auth] forgotPassword: starting', { hasEmail: Boolean(params.email) });If the exact log messages differ slightly from the SEARCH strings above (e.g., different log text or object shape), adjust the SEARCH strings to match your current code. The key change is to remove email: params.email from the logged object and, if desired, replace it with a non-PII flag like hasEmail: Boolean(params.email) or omit the second argument entirely.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Checklist
General
Testing
Feature-Specific
Summary by Sourcery
Improve authentication flows, guest invitation handling, and workspace loading robustness while adding structured logging across auth-related flows.
New Features:
Bug Fixes:
Enhancements:
Chores: