Skip to content

[TASK-7429] fix authentication #571

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

Merged
merged 12 commits into from
Dec 17, 2024
Merged

[TASK-7429] fix authentication #571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
151e57b
refactor: use base url
jjramirezn Dec 10, 2024
926d10a
fix: ask for user auth when getting kyc status
jjramirezn Dec 11, 2024
6798dc5
fix: correctly get cookie
jjramirezn Dec 11, 2024
1814717
refactor: remove extra get-user route
jjramirezn Dec 12, 2024
fd1b44f
Merge pull request #564 from peanutprotocol/fix/get-kyc-link
Hugo0 Dec 12, 2024
e3f88ba
fix(kyc): update bridge customer id on link creation
jjramirezn Dec 13, 2024
dff936b
Merge pull request #568 from peanutprotocol/fix/get-kyc-statu
Hugo0 Dec 13, 2024
8716d7b
fix(kyc): store kyc status in db
jjramirezn Dec 13, 2024
a1709ea
Merge pull request #569 from peanutprotocol/fix/get-kyc-statu
jjramirezn Dec 14, 2024
6f084f4
fix: hide points estimation on consumer pages
kushagrasarathe Dec 17, 2024
860a582
fix: code formatting
kushagrasarathe Dec 17, 2024
16eb9e8
Merge pull request #573 from peanutprotocol/fix/points
jjramirezn Dec 17, 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
2 changes: 1 addition & 1 deletion docs/high_level_overview.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ There are three major contexts provided in the `/context` folder:
const {
user,
setUser,
updateBridgeCustomerId,
updateBridgeCustomerData,
fetchUser,
updateUserName,
submitProfilePhoto,
Expand Down
27 changes: 24 additions & 3 deletions src/app/api/bridge/user/new/get-status/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,34 @@
import type { NextRequest } from 'next/server'
import { NextResponse } from 'next/server'
import { NextResponse, NextRequest } from 'next/server'
import * as interfaces from '@/interfaces'
import { GET as getUserFromCookie } from '@/app/api/peanut/user/get-user-from-cookie/route'
Copy link
Contributor

Choose a reason for hiding this comment

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

eww


export async function POST(request: NextRequest) {
try {
const { userId, type } = await request.json()
if (!process.env.BRIDGE_API_KEY) {
throw new Error('BRIDGE_API_KEY is not defined')
}

const getUserFromCookieRequest = new NextRequest(
Copy link
Contributor

Choose a reason for hiding this comment

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

this plus { GET as getUserFromCookie } ? oof

`${process.env.NEXT_PUBLIC_BASE_URL}/api/peanut/user/get-user-from-cookie`,
{
method: 'GET',
headers: {
cookie: request.headers.get('cookie') ?? '',
...request.headers,
Comment on lines +17 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid potential header conflicts when spreading headers

Spreading request.headers after explicitly setting the cookie header may lead to duplicate or conflicting headers, especially if cookie is already present in request.headers. To prevent this, remove the spread operator or ensure that headers are merged without duplication.

Apply this diff to adjust the headers:

headers: {
    cookie: request.headers.get('cookie') ?? '',
-   ...request.headers,
},
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cookie: request.headers.get('cookie') ?? '',
...request.headers,
cookie: request.headers.get('cookie') ?? '',

},
}
)
const getUserFromCookieResponse = await getUserFromCookie(getUserFromCookieRequest)
Copy link
Contributor

Choose a reason for hiding this comment

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

what's going on here, why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need the user data, of the calling user, so we need to ensure that the user is authenticated and get its data. We can't trust on input

if (!getUserFromCookieResponse.ok) {
return new NextResponse('Unauthorized', { status: 401 })
}
const { user } = await getUserFromCookieResponse.json()

if (userId !== user?.bridge_customer_id) {
return new NextResponse('Forbidden', { status: 403 })
}
Comment on lines +28 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure type consistency when comparing userId and bridge_customer_id

To prevent potential mismatches due to type differences, ensure that both userId and user.bridge_customer_id are strings before comparing. This avoids incorrect authorization failures if one is a number and the other is a string.

Apply this diff to enforce type consistency:

- if (userId !== user?.bridge_customer_id) {
+ if (String(userId) !== String(user?.bridge_customer_id)) {
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (userId !== user?.bridge_customer_id) {
return new NextResponse('Forbidden', { status: 403 })
}
if (String(userId) !== String(user?.bridge_customer_id)) {
return new NextResponse('Forbidden', { status: 403 })
}


const response = await fetch(`https://api.bridge.xyz/v0/kyc_links/${userId}`, {
method: 'GET',
headers: {
Expand Down Expand Up @@ -37,7 +58,7 @@ export async function POST(request: NextRequest) {
},
})
} else if (type === 'customer_id') {
return new NextResponse(JSON.stringify({ customer_id: data.customer_id }), {
return new NextResponse(JSON.stringify({ id: data.customer_id, kyc_status: data.kyc_status }), {
status: 200,
headers: {
'Content-Type': 'application/json',
Expand Down
15 changes: 7 additions & 8 deletions src/app/api/peanut/user/get-user-from-cookie/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
import { cookies } from 'next/headers'
import { NextRequest, NextResponse } from 'next/server'
import * as consts from '@/constants'

export async function GET(request: NextRequest) {
export async function GET(_request: NextRequest) {
const cookieStore = cookies()
const token = cookieStore.get('jwt-token')
const apiKey = process.env.PEANUT_API_KEY

if (!token) {
if (!token || !apiKey) {
return new NextResponse('Bad Request: missing required parameters', { status: 400 })
}
try {
const { protocol, hostname, port } = new URL(request.url)
const apiUrl = `${protocol}//${hostname}:${port}/api/peanut/user/get-user`

const response = await fetch(apiUrl, {
const response = await fetch(`${consts.PEANUT_API_URL}/get-user`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${token.value}`,
'api-key': apiKey,
},
body: JSON.stringify({ authToken: token.value }),
})

if (response.status !== 200) {
Expand Down
41 changes: 0 additions & 41 deletions src/app/api/peanut/user/get-user/route.ts
View file
Open in desktop

This file was deleted.

3 changes: 2 additions & 1 deletion src/app/api/peanut/user/update-user/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { NextRequest, NextResponse } from 'next/server'
import * as consts from '@/constants'

export async function POST(request: NextRequest) {
const { userId, username, bridge_customer_id } = await request.json()
const { userId, username, bridge_customer_id, kycStatus } = await request.json()
const apiKey = process.env.PEANUT_API_KEY
const cookieStore = cookies()
const token = cookieStore.get('jwt-token')
Expand All @@ -24,6 +24,7 @@ export async function POST(request: NextRequest) {
userId,
username,
bridge_customer_id,
kycStatus,
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Handle undefined kycStatus gracefully

If kycStatus is undefined or null, it could cause issues with the API request. Consider setting a default value or conditionally including it in the request body.

Apply this diff to include kycStatus conditionally:

body: JSON.stringify({
    userId,
    username,
    bridge_customer_id,
-   kycStatus,
+   ...(kycStatus && { kycStatus }),
}),

Committable suggestion skipped: line range outside the PR's diff.

}),
})

Expand Down
2 changes: 1 addition & 1 deletion src/components/Cashout/Components/Initial.view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export const InitialCashoutView = ({
recipient: bankAccountNumber,
password: '',
})
if (user?.user.kycStatus == 'verified') {
if (user?.user.kycStatus === 'approved') {
const account = user.accounts.find(
(account: any) =>
account.account_identifier.replaceAll(/\s/g, '').toLowerCase() ===
Expand Down
6 changes: 3 additions & 3 deletions src/components/Claim/Link/Initial.view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export const InitialClaimLinkView = ({
recipient: recipient.name ?? recipient.address,
password: '',
})
if (user?.user.kycStatus === 'verified') {
if (user?.user.kycStatus === 'approved') {
const account = user.accounts.find(
(account: any) =>
account.account_identifier.replaceAll(/\s/g, '').toLowerCase() ===
Expand Down Expand Up @@ -528,12 +528,12 @@ export const InitialClaimLinkView = ({
</span>
</div>

{/* TODO: correct points estimation
<div className="flex w-full flex-row items-center justify-between px-2 text-h8 text-gray-1">
<div className="flex w-max flex-row items-center justify-center gap-1">
<Icon name={'plus-circle'} className="h-4 fill-gray-1" />
<label className="font-bold">Points</label>
</div>
{/* TODO: correct points estimation
<span className="flex flex-row items-center justify-center gap-1 text-center text-sm font-normal leading-4">
{estimatedPoints < 0 ? estimatedPoints : `+${estimatedPoints}`}
<MoreInfo
Expand All @@ -546,8 +546,8 @@ export const InitialClaimLinkView = ({
}
/>
</span>
*/}
</div>
*/}
</div>
)}
</div>{' '}
Expand Down
4 changes: 2 additions & 2 deletions src/components/Claim/Link/Onchain/Confirm.view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,12 @@ export const ConfirmClaimLinkView = ({
</span>
</div>

{/* TODO: correct points estimation
<div className="flex w-full flex-row items-center justify-between px-2 text-h8 text-gray-1">
<div className="flex w-max flex-row items-center justify-center gap-1">
<Icon name={'plus-circle'} className="h-4 fill-gray-1" />
<label className="font-bold">Points</label>
</div>
{/* TODO: correct points estimation
<span className="flex flex-row items-center justify-center gap-1 text-center text-sm font-normal leading-4">
{estimatedPoints < 0 ? estimatedPoints : `+${estimatedPoints}`}
<MoreInfo
Expand All @@ -238,8 +238,8 @@ export const ConfirmClaimLinkView = ({
}
/>
</span>
*/}
</div>
*/}
</div>

<div className="flex w-full flex-col items-center justify-center gap-2">
Expand Down
4 changes: 2 additions & 2 deletions src/components/Create/Link/Confirm.view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,12 +315,12 @@ export const CreateLinkConfirmView = ({
</label>
</div>
)}
{/* TODO: correct points estimation
<div className="flex w-full flex-row items-center justify-between px-2 text-h8 text-gray-1">
<div className="flex w-max flex-row items-center justify-center gap-1">
<Icon name={'plus-circle'} className="h-4 fill-gray-1" />
<label className="font-bold">Points</label>
</div>
{/* TODO: correct points estimation
<span className="flex flex-row items-center justify-center gap-1 text-center text-sm font-normal leading-4">
{estimatedPoints && estimatedPoints < 0 ? estimatedPoints : `+${estimatedPoints}`}
<MoreInfo
Expand All @@ -335,8 +335,8 @@ export const CreateLinkConfirmView = ({
}
/>
</span>
*/}
</div>
*/}
</div>

<div className="flex w-full flex-col items-center justify-center gap-2">
Expand Down
2 changes: 1 addition & 1 deletion src/components/Global/Header/components/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const NavLink = ({
<Link
{...rest}
href={href}
className="flex h-full w-full items-center justify-start py-2 uppercase sm:w-max sm:justify-center px-8 lg:px-4 xl:px-8"
className="flex h-full w-full items-center justify-start px-8 py-2 uppercase sm:w-max sm:justify-center lg:px-4 xl:px-8"
>
{children}
</Link>
Expand Down
12 changes: 6 additions & 6 deletions src/components/Global/KYCComponent/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ export const GlobalKYCComponent = ({ intialStep, offrampForm, setOfframpForm, on

const [loadingState, setLoadingState] = useState<string>('Idle')
const isLoading = useMemo(() => loadingState !== 'Idle', [loadingState])
const { fetchUser, updateBridgeCustomerId } = useAuth()

const [kycStatus, setKycStatus] = useState<'completed' | 'under_review' | 'rejected' | 'approved'>('completed')
const { fetchUser, updateBridgeCustomerData } = useAuth()

const {
setStep: setActiveStep,
Expand Down Expand Up @@ -98,6 +96,7 @@ export const GlobalKYCComponent = ({ intialStep, offrampForm, setOfframpForm, on
}
} else {
let data = await utils.getUserLinks(inputFormData)
await updateBridgeCustomerData(data)
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Error handling is inconsistent across updateBridgeCustomerData calls

The verification reveals inconsistent error handling patterns:

  • In handleKYCStatus and the last call, the function is within a try-catch block with proper error logging and user feedback
  • However, the first two calls (in the else block and handleTOSStatus) lack explicit error handling, which could lead to unhandled promise rejections

Locations needing attention:

  • Line 99: Missing try-catch block in the else clause
  • Line 132: Missing try-catch in handleTOSStatus
  • Line 187: Has proper error handling (keep as is)
🔗 Analysis chain

Verify consistent error handling across all updateBridgeCustomerData calls.

The function is called in multiple places with potentially different data shapes. Ensure error handling is consistent across all calls.

Also applies to: 132-132, 187-187

Let me check the error handling implementation by looking for try-catch blocks around these calls.

Let me try a different approach to check the error handling context.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check error handling consistency across updateBridgeCustomerData calls.

# Search for all updateBridgeCustomerData calls and their surrounding error handling
rg -A 5 -B 5 'updateBridgeCustomerData\('

Length of output: 3838

Script:

#!/bin/bash
# Search for try-catch blocks containing updateBridgeCustomerData calls
ast-grep --pattern 'try {
  $$$
  updateBridgeCustomerData($$$)
  $$$
} catch ($$$) {
  $$$
}'

Length of output: 96

Script:

#!/bin/bash
# Get more context around the function calls to see error handling patterns
rg -A 15 -B 15 'updateBridgeCustomerData\(' src/components/Global/KYCComponent/index.tsx

Length of output: 4894

setCustomerObject(data)

let { tos_status: tosStatus, kyc_status: kycStatus } = data
Expand Down Expand Up @@ -130,6 +129,7 @@ export const GlobalKYCComponent = ({ intialStep, offrampForm, setOfframpForm, on
// @ts-ignore
if (!customerObject || customerObject.code === 'invalid_parameters') {
_customerObject = await utils.getUserLinks(_offrampForm)
await updateBridgeCustomerData(_customerObject)
setCustomerObject(_customerObject)
} else {
_customerObject = customerObject
Expand Down Expand Up @@ -184,6 +184,7 @@ export const GlobalKYCComponent = ({ intialStep, offrampForm, setOfframpForm, on
const _offrampForm = watchOfframp()
if (!customerObject) {
_customerObject = await utils.getUserLinks(_offrampForm)
await updateBridgeCustomerData(_customerObject)
setCustomerObject(_customerObject)
} else {
_customerObject = customerObject
Expand Down Expand Up @@ -258,10 +259,10 @@ export const GlobalKYCComponent = ({ intialStep, offrampForm, setOfframpForm, on

// Get customer ID
const customer = await utils.getStatus(_customerObject.id, 'customer_id')
setCustomerObject({ ..._customerObject, customer_id: customer.customer_id })
setCustomerObject({ ..._customerObject, customer_id: customer.id })

// Update peanut user with bridge customer id
const updatedUser = await updateBridgeCustomerId(customer.customer_id)
await updateBridgeCustomerData(customer)

// recipientType === 'us' && setAddressRequired(true)
setLoadingState('Idle')
Expand Down Expand Up @@ -307,7 +308,6 @@ export const GlobalKYCComponent = ({ intialStep, offrampForm, setOfframpForm, on
})

// Ensure the state updates happen
setKycStatus('completed')
if (onCompleted) {
onCompleted('KYC completed')
}
Expand Down
6 changes: 3 additions & 3 deletions src/components/Global/LinkAccountComponent/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export const GlobaLinkAccountComponent = ({ accountNumber, onCompleted }: IGloba
// Only need address for US accounts
let address
if (formData.type === 'us') {
if (user?.user?.kycStatus === 'verified') {
if (user?.user?.kycStatus === 'approved') {
console.log('User accounts:', user.accounts)

// Find account with valid address details
Expand Down Expand Up @@ -424,7 +424,7 @@ export const GlobaLinkAccountComponent = ({ accountNumber, onCompleted }: IGloba
<span className="text-h9 font-light">Your account number</span>
</div>

{(!user?.user?.kycStatus || user.user.kycStatus !== 'verified') && (
{(!user?.user?.kycStatus || user.user.kycStatus !== 'approved') && (
<div className="flex w-full flex-col gap-4 border-t border-gray-200 pt-4">
<span className="text-h8 font-medium">Your US address details</span>

Expand Down Expand Up @@ -545,7 +545,7 @@ export const GlobaLinkAccountComponent = ({ accountNumber, onCompleted }: IGloba
}
}

return user?.user?.kycStatus === 'verified' ? (
return user?.user?.kycStatus === 'approved' ? (
completedLinking ? (
<div className="flex w-full flex-col items-center justify-center gap-6 py-2 pb-20 text-center">
<p>You have successfully linked your account!</p>
Expand Down
2 changes: 1 addition & 1 deletion src/components/Kyc/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const KYCComponent = () => {
<span className="sr-only">Loading...</span>
</div>
</div>
) : user && user?.user?.kycStatus === 'verified' ? (
) : user && user?.user?.kycStatus === 'approved' ? (
<div className="flex flex-col items-center justify-center gap-4">
<p className="text-h4">Welcome back, {user?.user?.username ?? user?.user?.email}</p>
<p className="text-h8 font-light">You have already completed the KYC process!</p>
Expand Down
2 changes: 1 addition & 1 deletion src/components/LinkAccount/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const LinkAccountComponent = () => {
<span className="sr-only">Loading...</span>
</div>
</div>
) : user && user?.user?.kycStatus != 'verified' ? (
) : user && user?.user?.kycStatus !== 'approved' ? (
<div className="flex flex-col items-center justify-center gap-4">
<p className="text-h4">Welcome back, {user?.user?.username ?? user?.user?.email}</p>
<p className="text-h8 font-light">Before linking an account, you will have to complete KYC!</p>
Expand Down
4 changes: 2 additions & 2 deletions src/components/Profile/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,9 +396,9 @@ export const Profile = () => {
utils.printableAddress(user.accounts?.[0]?.account_identifier)}
<div className={`flex flex-row items-center justify-center `}>
<div
className={`kyc-badge select-none ${user?.user?.kycStatus === 'verified' ? 'bg-kyc-green px-2 py-1 text-black' : 'bg-gray-1 text-white hover:ring-2 hover:ring-gray-2'} w-max`}
className={`kyc-badge select-none ${user?.user?.kycStatus === 'approved' ? 'bg-kyc-green px-2 py-1 text-black' : 'bg-gray-1 text-white hover:ring-2 hover:ring-gray-2'} w-max`}
>
{user?.user?.kycStatus === 'verified' ? (
{user?.user?.kycStatus === 'approved' ? (
'KYC'
) : (
<Link className="px-2 py-1" href={'/kyc'}>
Expand Down
4 changes: 2 additions & 2 deletions src/components/Request/Pay/Views/Initial.view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,12 +433,12 @@ export const InitialView = ({
</div>
)}

{/* TODO: correct points estimation
<div className="flex w-full flex-row items-center justify-between px-2 text-h8 text-gray-1">
<div className="flex w-max flex-row items-center justify-center gap-1">
<Icon name={'plus-circle'} className="h-4 fill-gray-1" />
<label className="font-bold">Points</label>
</div>
{/* TODO: correct points estimation
<span className="flex flex-row items-center justify-center gap-1 text-center text-sm font-normal leading-4">
{estimatedPoints ? (
`${estimatedPoints > 0 ? '+' : ''}${estimatedPoints}`
Expand All @@ -455,8 +455,8 @@ export const InitialView = ({
}
/>
</span>
*/}
</div>
*/}
</>
)}
</div>
Expand Down
Loading
Loading