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.

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

FEAT: Revoke Refresh Tokens on Session-Destroy #129

Merged

Conversation

haruelrovix
Copy link
Contributor

Closes (#94)

Description

Tested on Logout ✅

HTTP Requests
-------------

GET    /_next/static/media/TanyaAja.15bf8495.svg                      200 OK
GET    /_next/static/chunks/app/login/page.js                         200 OK
GET    /_next/static/webpack/webpack.22acd9070cb303c3.hot-update.js   200 OK
GET    /_next/static/webpack/22acd9070cb303c3.webpack.hot-update.json 200 OK
GET    /login                                                         200 OK
GET    /login                                                         200 OK
DELETE /api/private/user/session-destroy                              200 OK
GET    /api/private/user/by-uuid/***                                  200 OK

📝 I haven't checked the Delete User flow, but I guess we need to Revoke the Refresh token there as well.

References:

@vercel
Copy link

vercel bot commented Oct 24, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @mazipan on Vercel.

@mazipan first needs to authorize it.

@mazipan
Copy link
Owner

mazipan commented Oct 24, 2023

I don't think the destroy session is necessary in the current flow.

Can you check, how if use previous token after doing logout?

Since we already doing logout from Firebase client side, I assume that the old token can not be verified anymore.

If it's safe, then we can remove the destroy session function completely.

@mazipan
Copy link
Owner

mazipan commented Oct 24, 2023

I will help to check from my local.

@vercel
Copy link

vercel bot commented Oct 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
tanyaaja ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 28, 2024 2:21am

@haruelrovix
Copy link
Contributor Author

@mazipan I checked it again and I guess both the master and this branch still has the Security issue.

Scenario:

  1. User login successfully
  2. User get JWT token
  3. Now copy the JWT token
  4. Use it to access private endpoints
  5. All should be good
  6. Now LOGOUT the user
  7. Use the JWT token from step 3 to access private endpoints
  8. Ideally, the request should Fail

Turns out it's bigger topic than I expected. To make it ideal, need to introduce two things:

  • Firebase Realtime Database
  • Firebase Security Rules

The approach is like documented here: Revoke refresh tokens

  • Rules
image
  • Revocation sample
    image

Now when we retest common flow above, it will be in ideal state.

{{BASE_URL}}/api/private/question/by-uid/pagination/*** FirebaseAuthError: The Firebase ID token has been revoked.
    at /tanyaaja/node_modules/.pnpm/[email protected][email protected]/node_modules/firebase-admin/lib/auth/base-auth.js:973:27
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async GET (webpack-internal:///(rsc)/./src/app/api/private/question/by-uid/pagination/[uid]/route.ts:22:34)
    at async /tanyaaja/node_modules/.pnpm/[email protected][email protected][email protected]/node_modules/next/dist/compiled/next-server/app-route.runtime.dev.js:6:62361 {
  errorInfo: {
    code: 'auth/id-token-revoked',
    message: 'The Firebase ID token has been revoked.'
  },
  codePrefix: 'auth'
}

@haruelrovix
Copy link
Contributor Author

Fixed the issue 💪🏻

HTTP Requests
-------------

GET    /api/private/question/by-uid/pagination/***   500 Internal Server Error
DELETE /api/private/user/session-destroy.            200 OK
GET    /api/private/user/by-uuid/***                 200 OK
GET    /api/private/question/by-uid/pagination/***   200 OK

📝 Ideally, the response should be 401 Unauthorized instead of 500 Internal Server Error.

  } catch (error: any) {
    console.error(request.url, error)
    if (error.code === 'auth/id-token-revoked') {
      // Token has been revoked. Inform the user to reauthenticate or signOut() the user.
      return NextResponse.json(
        { message: 'Session has expired. Please log in again to continue.' },
        { status: 401 },
      )
    }

    return NextResponse.json(
      { message: 'Error while get question by uid' },
      { status: 500 },
    )
  }

But this will be a huge refactor, let's:

@mazipan mazipan mentioned this pull request Oct 27, 2023
@mazipan mazipan force-pushed the feat/revoke-firebase-refresh-token branch from a24a2ab to ef58e1f Compare October 28, 2024 01:20
@mazipan mazipan merged commit b259b88 into mazipan:master Oct 28, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants