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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Bug Report: JWT still valid after Session Timeout #8000

Open
2 tasks done
jschmidtww opened this issue Apr 22, 2024 · 8 comments
Open
2 tasks done

馃悰 Bug Report: JWT still valid after Session Timeout #8000

jschmidtww opened this issue Apr 22, 2024 · 8 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers product / auth Fixes and upgrades for the Appwrite Auth / Users / Teams services.

Comments

@jschmidtww
Copy link

jschmidtww commented Apr 22, 2024

馃憻 Reproduction steps

I use JWT to authenticate a user on my API server. I use the /account endpoint to verify the JWT. If the user is logged out because the session has expired, the JWT is still valid and I still get a successful response when I call /account with the user's JWT.

馃憤 Expected behavior

The JWT should be invalid after the user is logged out and calling /account with users JWT should throw an error.

馃憥 Actual Behavior

Calling /account with the users JWT gives a successful response.

Discord thread: https://discord.com/channels/564160730845151244/1221805690050445362

馃幉 Appwrite version

Version 1.4.x

馃捇 Operating system

Linux

馃П Your Environment

I use Self-Hosted Appwrite Version 1.4.13

馃憖 Have you spent some time to check if this issue has been raised before?

  • I checked and didn't find similar issue

馃彚 Have you read the Code of Conduct?

@jschmidtww jschmidtww added the bug Something isn't working label Apr 22, 2024
@stnguyen90 stnguyen90 added product / auth Fixes and upgrades for the Appwrite Auth / Users / Teams services. good first issue Good for newcomers labels Apr 22, 2024
@stnguyen90
Copy link
Contributor

@jschmidtww, thanks for creating this issue! 馃檹馃徏 It looks like we check it's a valid JWT token, and there's an associated session:

appwrite/app/init.php

Lines 1199 to 1218 in b4bd48c

if (!empty($authJWT) && !$project->isEmpty()) { // JWT authentication
$jwt = new JWT(System::getEnv('_APP_OPENSSL_KEY_V1'), 'HS256', 900, 10); // Instantiate with key, algo, maxAge and leeway.
try {
$payload = $jwt->decode($authJWT);
} catch (JWTException $error) {
throw new Exception(Exception::USER_JWT_INVALID, 'Failed to verify JWT. ' . $error->getMessage());
}
$jwtUserId = $payload['userId'] ?? '';
$jwtSessionId = $payload['sessionId'] ?? '';
if ($jwtUserId && $jwtSessionId) {
$user = $dbForProject->getDocument('users', $jwtUserId);
}
if (empty($user->find('$id', $jwtSessionId, 'sessions'))) { // Match JWT to active token
$user = new Document([]);
}
}

but we don't check the session ID to see if it's still valid like we do here:

|| !Auth::sessionVerify($user->getAttribute('sessions', []), Auth::$secret)

@IshmeetSingh06
Copy link

Hi @stnguyen90 can I pick this issue?

before that can you confirm these things for me please
-> From what I understood from the issue description and your comment we need to check wether the session_id associated with that JWT is valid or not, should we check this inside the line:1211 block after getting the user or keep it a separate condition?

@stnguyen90
Copy link
Contributor

@IshmeetSingh06, assigned! Thanks for your interest! You can do it in the block. Please also make sure to set Auth::unique like here:

Auth::$unique = $session['id'] ?? '';

@IshmeetSingh06
Copy link

Hello @stnguyen90 @jschmidtww I have made the changes but unfortunately I was not able to reproduce and verify the issue locally, can you list the reproduction steps it would help me a lot.

@stnguyen90
Copy link
Contributor

@IshmeetSingh06, you can reproduce by:

  1. setting the session length (Auth > Security > Session length) to some small length
  2. signing in
  3. creating a JWT
  4. verify calling account.get() with JWT works
  5. wait for the session timeout
  6. call account.get() again

@ShivanshCharak
Copy link

@IshmeetSingh06 are you working on the issue??

@IshmeetSingh06
Copy link

Hi @ShivanshCharak I've worked on a possible fix but was not able to test it thoroughly due to being busy at work you can pick this up if you want to

@stnguyen90
Copy link
Contributor

@IshmeetSingh06, thanks for the update! @ShivanshCharak, I've assigned this issue to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers product / auth Fixes and upgrades for the Appwrite Auth / Users / Teams services.
Projects
None yet
Development

No branches or pull requests

4 participants