Skip to content
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.

Sign up for GitHub

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

fix: chat auth bug #109

Merged
merged 3 commits into from
Apr 8, 2024
Merged

fix: chat auth bug #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f127889
fix: chat auth bug
gaboesquivel Apr 8, 2024
7caa6e2
fix: chat auth bug
gaboesquivel Apr 8, 2024
5028b53
fix: chat auth bug
gaboesquivel Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 2 additions & 18 deletions apps/masterbots.ai/app/auth/callback/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { type CookieOptions, createServerClient } from '@supabase/ssr'
import { getToken, validateJwtSecret } from '@repo/mb-lib'
import { upsertUser } from '@/services/hasura'
import { nanoid } from '@/lib/utils'
import { createSupabaseServerClient } from '@/services/supabase'

export async function GET(request: Request) {
const { searchParams, origin } = new URL(request.url)
Expand All @@ -12,24 +13,7 @@ export async function GET(request: Request) {
// if "next" is in param, use it as the redirect URL
const next = searchParams.get('next') ?? '/'

const cookieStore = cookies()
const supabase = createServerClient(
process.env.NEXT_PUBLIC_SUPABASE_URL,
process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY,
{
cookies: {
get(name: string) {
return cookieStore.get(name).value
},
set(name: string, value: string, options: CookieOptions) {
cookieStore.set({ name, value, ...options })
},
remove(name: string, options: CookieOptions) {
cookieStore.delete({ name, ...options })
}
}
}
)
const supabase = await createSupabaseServerClient()

const {
data: { user },
Expand Down
7 changes: 4 additions & 3 deletions apps/masterbots.ai/app/c/[chatbot]/[threadId]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ export default async function ChatPage({ params }: ChatPageProps) {
const {
data: { user }
} = await supabase.auth.getUser()
if (!user || !user.email) throw new Error('user not found')

if (!user || !user.email)
redirect(`/auth/sign-in?next=/${params.chatbot}/${params.threadId}`)
Comment on lines +14 to +15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper handling of asynchronous redirect calls with specific parameters.

-  if (!user || !user.email)
-    redirect(`/auth/sign-in?next=/${params.chatbot}/${params.threadId}`)
+  if (!user || !user.email) return redirect(`/auth/sign-in?next=/${params.chatbot}/${params.threadId}`)

The redirect function is asynchronous and should be returned to ensure the redirection is properly handled and no further code is executed. Additionally, including specific parameters in the redirect URL is a good practice for improving user experience.

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.

Suggested change
if (!user || !user.email)
redirect(`/auth/sign-in?next=/${params.chatbot}/${params.threadId}`)
if (!user || !user.email) return redirect(`/auth/sign-in?next=/${params.chatbot}/${params.threadId}`)

const jwt = cookies().get('hasuraJwt')?.value || ''

console.log({ jwt, expired: isTokenExpired(jwt), user })
// NOTE: maybe we should use same expiration time
if (!jwt || isTokenExpired(jwt) || !user)
redirect(`/auth/sign-in?next=/${params.threadId}/${params.threadId}`)
redirect(`/auth/sign-in?next=/${params.chatbot}/${params.threadId}`)

Comment on lines +21 to +22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper handling of asynchronous redirect calls with specific parameters based on JWT and user conditions.

-  if (!jwt || isTokenExpired(jwt) || !user)
-    redirect(`/auth/sign-in?next=/${params.chatbot}/${params.threadId}`)
+  if (!jwt || isTokenExpired(jwt) || !user) return redirect(`/auth/sign-in?next=/${params.chatbot}/${params.threadId}`)

Similar to the previous comment, the redirect function should be returned to ensure proper execution flow. Including specific parameters in the redirect URL is commendable for guiding users appropriately.

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.

Suggested change
redirect(`/auth/sign-in?next=/${params.chatbot}/${params.threadId}`)
if (!jwt || isTokenExpired(jwt) || !user) return redirect(`/auth/sign-in?next=/${params.chatbot}/${params.threadId}`)

const thread = await getThread({
threadId: params.threadId
})
Expand Down
7 changes: 4 additions & 3 deletions apps/masterbots.ai/app/c/[chatbot]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,18 @@ export default async function BotThreadsPage({
const {
data: { user }
} = await supabase.auth.getUser()
if (!user || !user.email) throw new Error('user not found')
if (!user || !user.email) redirect(`/auth/sign-in?next=/${params.chatbot}`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper handling of asynchronous redirect calls with specific parameters.

-  if (!user || !user.email) redirect(`/auth/sign-in?next=/${params.chatbot}`)
+  if (!user || !user.email) return redirect(`/auth/sign-in?next=/${params.chatbot}`)

The redirect function is asynchronous and should be returned to ensure the redirection is properly handled and no further code is executed. Including specific parameters in the redirect URL is a good practice for improving user experience.

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.

Suggested change
if (!user || !user.email) redirect(`/auth/sign-in?next=/${params.chatbot}`)
if (!user || !user.email) return redirect(`/auth/sign-in?next=/${params.chatbot}`)

const userProfile = await getUser({
email: user.email,
adminSecret: process.env.HASURA_GRAPHQL_ADMIN_SECRET || ''
})

if (!userProfile) throw new Error('user not found')
if (!userProfile) redirect(`/auth/sign-in?next=/${params.chatbot}`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper handling of asynchronous redirect calls when the user profile is not found.

-  if (!userProfile) redirect(`/auth/sign-in?next=/${params.chatbot}`)
+  if (!userProfile) return redirect(`/auth/sign-in?next=/${params.chatbot}`)

Similar to the previous comments, the redirect function should be returned to ensure proper execution flow. Including specific parameters in the redirect URL is commendable for guiding users appropriately.

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.

Suggested change
if (!userProfile) redirect(`/auth/sign-in?next=/${params.chatbot}`)
if (!userProfile) return redirect(`/auth/sign-in?next=/${params.chatbot}`)

const jwt = cookies().get('hasuraJwt')?.value || ''

// NOTE: maybe we should use same expiration time
if (!jwt || isTokenExpired(jwt) || !user) redirect(`/auth/sign-in?next=/c`)
if (!jwt || isTokenExpired(jwt))
redirect(`/auth/sign-in?next=/${params.chatbot}`)
Comment on lines +33 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper handling of asynchronous redirect calls with specific parameters based on JWT conditions.

-  if (!jwt || isTokenExpired(jwt))
-    redirect(`/auth/sign-in?next=/${params.chatbot}`)
+  if (!jwt || isTokenExpired(jwt)) return redirect(`/auth/sign-in?next=/${params.chatbot}`)

The redirect function is asynchronous and should be returned to ensure the redirection is properly handled and no further code is executed. Including specific parameters in the redirect URL is commendable for guiding users appropriately.

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.

Suggested change
if (!jwt || isTokenExpired(jwt))
redirect(`/auth/sign-in?next=/${params.chatbot}`)
if (!jwt || isTokenExpired(jwt)) return redirect(`/auth/sign-in?next=/${params.chatbot}`)

const chatbot = await getChatbot({
chatbotName: botNames.get(params.chatbot),
jwt
Expand Down
5 changes: 3 additions & 2 deletions apps/masterbots.ai/app/c/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ export default async function IndexPage() {
const {
data: { user }
} = await supabase.auth.getUser()
if (!user || !user.email) throw new Error('user not found')
if (!user || !user.email) redirect(`/auth/sign-in`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper handling of asynchronous redirect calls.

-  if (!user || !user.email) redirect(`/auth/sign-in`)
+  if (!user || !user.email) return redirect(`/auth/sign-in`)

The redirect function is asynchronous and should be returned to ensure the redirection is properly handled and no further code is executed.

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.

Suggested change
if (!user || !user.email) redirect(`/auth/sign-in`)
if (!user || !user.email) return redirect(`/auth/sign-in`)


const dbUserProfile = await getUser({
email: user.email,
adminSecret: process.env.HASURA_GRAPHQL_ADMIN_SECRET || ''
})

if (!dbUserProfile) throw new Error('user not found')
if (!dbUserProfile) redirect(`/auth/sign-in`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure proper handling of asynchronous redirect calls when the user profile is not found.

-  if (!dbUserProfile) redirect(`/auth/sign-in`)
+  if (!dbUserProfile) return redirect(`/auth/sign-in`)

Similar to the previous comment, the redirect function should be returned to ensure proper execution flow.

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.

Suggested change
if (!dbUserProfile) redirect(`/auth/sign-in`)
if (!dbUserProfile) return redirect(`/auth/sign-in`)


const jwt = cookies().get('hasuraJwt').value || ''

Expand Down
Loading