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

Replace JWT authentication with database-backed sessions #9

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

rlk-stripe
Copy link
Contributor

Currently, when a user logs in to the application, the server provides them with a signed JSON web token asserting their identity. On subsequent requests, the user presents this token, and the server authenticates the user by verifying the signature on the token.

While the use of a signed objects enables a relatively lightweight authentication scheme, it requires several security tradeoffs compared to traditional database-backed sessions:

  • Tokens cannot be invalidated prior to their scheduled expiration.
  • An application cannot easily restrict multiple simultaneous logins.
  • If a token is used to store dynamic information, a malicious user may retain tokens with outdated information and present them in place of current ones.

In addition, the JWT standard in particular enables a number of options, specified in the token itself, that are not necessary or desirable for single-app authentication tokens, including public/private key signatures and the option to disable authentication entirely with {"alg": "none"}. While most mature JWT library implementations take care to validate the provided algorithm and other values, these features remains a source of unnecessary complexity and potential security bugs.

Consequently, this PR removes the use of JWTs as an authentication token, and replaces them with a random string (16 bytes from crypto.randomBytes, base64 encoded) that is used to look up the session information in a database table (sessions).

Sessions now expire after an hour of inactivity, rather than a flat hour after login. Additionally, an /auth/logout endpoint will invalidate a session immediately. For simplicity, when a user logs in, any other existing sessions they might have are also invalidated.

This PR also removes client-side validation of the token[1], and causes the "sign out" link to post to the logout endpoint in addition to discarding the token locally.

[1] Currently, when the application front-end loads, and a token exists in sessionStorage, the app checks whether the token is expired to determine whether to treat the user as logged in. In the absence of this check, the app attempts to retrieve /api/account and fails, ending up in the same UI state anyway.

@rlk-stripe
Copy link
Contributor Author

cc @richo-stripe

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.

1 participant