-
-
Notifications
You must be signed in to change notification settings - Fork 13.5k
♻️ Refactor: Make LobeNextAuthDBAdapter Edge Compatible #8188
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
base: main
Are you sure you want to change the base?
Conversation
@cy948 is attempting to deploy a commit to the LobeHub Community Team on Vercel. A member of the Team first needs to authorize it. |
Reviewer's GuideRefactors the NextAuth DB adapter to be edge runtime compatible by moving ORM operations behind a secure backend API endpoint, centralizing database logic in NextAuthUserService, and updating NextAuth initialization, contexts, and configuration to use the new fetch-based adapter. Sequence diagram for NextAuth adapter DB operation via backend APIsequenceDiagram
participant NextAuthAdapter as NextAuth Adapter (Edge)
participant BackendAPI as /api/auth/adapter (Backend API)
participant NextAuthUserService as NextAuthUserService
participant DB as Database
NextAuthAdapter->>BackendAPI: POST /api/auth/adapter {action, data}
BackendAPI->>NextAuthUserService: Call method based on action
NextAuthUserService->>DB: Perform ORM/database operation
DB-->>NextAuthUserService: Return result
NextAuthUserService-->>BackendAPI: Return result
BackendAPI-->>NextAuthAdapter: {success, data}
Class diagram for NextAuthUserService and Adapter refactorclassDiagram
class NextAuthUserService {
+safeUpdateUser()
+createAuthenticator()
+createSession()
+createUser()
+createVerificationToken()
+deleteSession()
+deleteUser()
+getAccount()
+getAuthenticator()
+getSessionAndUser()
+getUser()
+getUserByAccount()
+getUserByEmail()
+linkAccount()
+listAuthenticatorsByUserId()
+unlinkAccount()
+updateAuthenticatorCounter()
+updateSession()
+updateUser()
+useVerificationToken()
}
class LobeNextAuthDbAdapter {
+createAuthenticator()
+createSession()
+createUser()
+createVerificationToken()
+deleteSession()
+deleteUser()
+getAccount()
+getAuthenticator()
+getSessionAndUser()
+getUser()
+getUserByAccount()
+getUserByEmail()
+linkAccount()
+listAuthenticatorsByUserId()
+unlinkAccount()
+updateAuthenticatorCounter()
+updateSession()
+updateUser()
+useVerificationToken()
-fetcher()
-postProcessor()
}
LobeNextAuthDbAdapter <.. NextAuthUserService : fetches via API
NextAuthUserService --> "1" Database : uses
NextAuthUserService --> "1" UserModel : uses
NextAuthUserService --> "1" AgentService : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
👍 @cy948 Thank you for raising your pull request and contributing to our Community |
349e47c
to
d39361f
Compare
d39361f
to
d42fd6c
Compare
TestGru AssignmentSummary
Files
Tip You can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @cy948 - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Unresolved merge conflict markers present in code. (link)
General comments:
- There are unresolved Git conflict markers in AuthSignInBox.tsx (<<<<<<< HEAD / >>>>>>>); please remove them and finalize the intended callbackUrl logic.
- Enhance the fetcher in LobeNextAuthDbAdapter to check response.ok and throw or handle HTTP errors before parsing JSON to avoid unexpected failures.
- Add request validation (e.g. with Zod) in the /api/auth/adapter route to enforce correct action names and payload shapes and prevent malformed requests.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are unresolved Git conflict markers in AuthSignInBox.tsx (<<<<<<< HEAD / >>>>>>>); please remove them and finalize the intended callbackUrl logic.
- Enhance the fetcher in LobeNextAuthDbAdapter to check response.ok and throw or handle HTTP errors before parsing JSON to avoid unexpected failures.
- Add request validation (e.g. with Zod) in the /api/auth/adapter route to enforce correct action names and payload shapes and prevent malformed requests.
## Individual Comments
### Comment 1
<location> `src/libs/next-auth/adapter/index.ts:42` </location>
<code_context>
+ throw new Error('LobeNextAuthDbAdapter: KEY_VAULTS_SECRET is not set in environment variables');
+ }
+
+ const fetcher = (action: string, data: any) => fetch(interactionUrl, {
+ method: 'POST',
+ headers: {
</code_context>
<issue_to_address>
No retry or error handling for network failures in fetcher.
Add try/catch and handle network errors, timeouts, and non-JSON responses to prevent unhandled promise rejections and improve error clarity.
</issue_to_address>
### Comment 2
<location> `src/libs/next-auth/adapter/index.ts:79` </location>
<code_context>
- .delete(nextauthSessions)
- .where(eq(nextauthSessions.sessionToken, sessionToken));
+ const result = await fetcher('deleteSession', sessionToken);
+ await postProcessor(result);
return;
},
</code_context>
<issue_to_address>
deleteSession and deleteUser do not return deleted objects as per Adapter contract.
The current implementation always returns undefined, which may break consumers relying on the deleted object. Please return the deleted object or null if not found, as required by the Adapter contract.
</issue_to_address>
### Comment 3
<location> `src/libs/next-auth/adapter/index.ts:145` </location>
<code_context>
- eq(nextauthAccounts.providerAccountId, account.providerAccountId),
- ),
- );
+ const result = await fetcher('unlinkAccount', account);
+ await postProcessor(result);
+ return
</code_context>
<issue_to_address>
unlinkAccount does not return AdapterAccount as per Adapter contract.
Currently, unlinkAccount always returns undefined. To align with the Adapter contract and avoid issues for consumers expecting the deleted AdapterAccount, return the deleted account when available.
</issue_to_address>
### Comment 4
<location> `src/app/(backend)/api/auth/adapter/route.ts:93` </location>
<code_context>
+ }
+ return NextResponse.json({ success: true, data: result });
+ } catch (error) {
+ return NextResponse.json({ success: false, error }, { status: 400 });
+ }
+}
</code_context>
<issue_to_address>
Error responses may leak internal error objects.
Avoid returning raw error objects; instead, return sanitized error messages to prevent exposing sensitive information.
</issue_to_address>
### Comment 5
<location> `src/app/[variants]/(auth)/next-auth/signin/AuthSignInBox.tsx:78` </location>
<code_context>
const searchParams = useSearchParams();
+<<<<<<< HEAD
// Redirect back to the page url, fallback to '/' if failed
+=======
+ // Redirect back to the page url
+>>>>>>> ecf695d9b (:bug: fix: default callbackUrl)
const callbackUrl = searchParams.get('callbackUrl') ?? '/';
</code_context>
<issue_to_address>
Unresolved merge conflict markers present in code.
Please resolve the merge conflict markers (<<<<<<<, =======, >>>>>>>) to prevent syntax errors before merging.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/libs/next-auth/adapter/index.ts
Outdated
throw new Error('LobeNextAuthDbAdapter: KEY_VAULTS_SECRET is not set in environment variables'); | ||
} | ||
|
||
const fetcher = (action: string, data: any) => fetch(interactionUrl, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): No retry or error handling for network failures in fetcher.
Add try/catch and handle network errors, timeouts, and non-JSON responses to prevent unhandled promise rejections and improve error clarity.
.delete(nextauthSessions) | ||
.where(eq(nextauthSessions.sessionToken, sessionToken)); | ||
const result = await fetcher('deleteSession', sessionToken); | ||
await postProcessor(result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): deleteSession and deleteUser do not return deleted objects as per Adapter contract.
The current implementation always returns undefined, which may break consumers relying on the deleted object. Please return the deleted object or null if not found, as required by the Adapter contract.
eq(nextauthAccounts.providerAccountId, account.providerAccountId), | ||
), | ||
); | ||
const result = await fetcher('unlinkAccount', account); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): unlinkAccount does not return AdapterAccount as per Adapter contract.
Currently, unlinkAccount always returns undefined. To align with the Adapter contract and avoid issues for consumers expecting the deleted AdapterAccount, return the deleted account when available.
} | ||
return NextResponse.json({ success: true, data: result }); | ||
} catch (error) { | ||
return NextResponse.json({ success: false, error }, { status: 400 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 issue (security): Error responses may leak internal error objects.
Avoid returning raw error objects; instead, return sanitized error messages to prevent exposing sensitive information.
<<<<<<< HEAD | ||
// Redirect back to the page url, fallback to '/' if failed | ||
======= | ||
// Redirect back to the page url | ||
>>>>>>> ecf695d9b (:bug: fix: default callbackUrl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Unresolved merge conflict markers present in code.
Please resolve the merge conflict markers (<<<<<<<, =======, >>>>>>>) to prevent syntax errors before merging.
src/libs/next-auth/adapter/index.ts
Outdated
const data = await postProcessor(result); | ||
return data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable
)
const data = await postProcessor(result); | |
return data; | |
return await postProcessor(result); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
src/libs/next-auth/adapter/index.ts
Outdated
const session = await postProcessor(result); | ||
return session; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable
)
const session = await postProcessor(result); | |
return session; | |
return await postProcessor(result); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
src/libs/next-auth/adapter/index.ts
Outdated
const data = await postProcessor(result); | ||
return data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable
)
const data = await postProcessor(result); | |
return data; | |
return await postProcessor(result); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
src/libs/next-auth/adapter/index.ts
Outdated
const data = await postProcessor(result); | ||
return data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable
)
const data = await postProcessor(result); | |
return data; | |
return await postProcessor(result); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
const adapterUser = mapLobeUserToAdapterUser(existingUser); | ||
return adapterUser; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable
)
const adapterUser = mapLobeUserToAdapterUser(existingUser); | |
return adapterUser; | |
return mapLobeUserToAdapterUser(existingUser); | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
The latest updates on your projects. Learn more about Vercel for GitHub.
|
d3424a9
to
5ce892b
Compare
ac1ab0b
to
8b51ea6
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8188 +/- ##
==========================================
- Coverage 84.04% 83.95% -0.09%
==========================================
Files 870 869 -1
Lines 70571 70590 +19
Branches 4889 6261 +1372
==========================================
- Hits 59309 59264 -45
- Misses 11256 11320 +64
Partials 6 6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ef87b70
to
1d22501
Compare
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
Main Changes:
src/libs/next-auth/adapter/index.ts
: 改为call后端api endpoint完成 database 调用。src/app/(backend)/api/auth/adapter/route.ts
: 调用 service。src/server/services/nextAuthUser/index.ts
: 放置原 adapter 的 ORM 操作。Cascade Changes:
src/libs/next-auth/edge.ts
📝 补充信息 | Additional Information
Summary by Sourcery
Refactor NextAuth database adapter to be edge-compatible by routing all ORM operations through a secured backend API endpoint and update related configurations and initializations
Enhancements: