feat(js-sdk): save and access JWT token in Browser Cookies for Authentication

[v0eak]

Added an implementation for storing and accessing the generated JWT Token in browser storage as a secure HTTPOnly cookie in medusajs-sdk.
Implementation also added a config variable and environment variable for setting the Key used for storing tokens in local-, session- and cookie-storage.
Implementation removes the need of passing the token on every request as Bearer Token manually.

[changeset-bot]

⚠️ No Changeset found

Latest commit: f4c7204

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

@v0eak is attempting to deploy a commit to the medusajs Team on Vercel.

A member of the Team first needs to authorize it.

[sradevski]

Hey @v0eak thanks for contributing!

I actually don't understand what problem this PR solves, so maybe you can elaborate a bit on that (and not what it does)? I don't see why we would pass the JWT as a cookie instead of sending it in the header, which is common practice. I also don't see why we need the redirect when doing a login? You would just have an Google for the Google login in your page, it won't really go through the SDK.

[v0eak]

Hi @sradevski,

well, initially, my problem was that once a customer logs in with their 3rd party Service, the token was passed as a query parameter, which imo is unsafe as it would allow a malicious actor to get a hold of the token through browser history. Which is why I have made my own custom callback route, which saves the token as a cookie. Unfortunately, the medusa-sdk is unable to automatically determine, that the token exists. This implementation makes the sdk aware that a token is saved in the cookies.

Essentially, this removes the need of the session.ts file as seen here (https://github.com/medusajs/medusa-eats/blob/main/frontend/src/lib/data/sessions.ts), where for each request, you first have to retrieve the cookie manually.

[sradevski]

Thanks for the explanation! There is nothing unsafe about having the oauth code in the query parameter, as there are other strategies that are applied on the BE to ensure this can't be abused, like the code expiring pretty quickly and passing a state argument. This is how Oauth works as a standard.

The code you linked is about managing the session within Nextjs, which then sends the token to Medusa, so it is not directly related to medusa in that case.

I'd say we can't merge the PR yet as it seems it's solving a specific problem. If we see a need for it later on we can revisit it.

[v0eak]

No you misunderstand, this is not related to the oauth code passed to the callback route, but the JWT Token generated afterwards, which is then passed as a query parameter.
In my own callback route, I have replaced

if (successRedirectUrl) {
  const url = new URL(successRedirectUrl!)
  url.searchParams.append("access_token", token)

  return res.redirect(url.toString())
}

with

if (successRedirectUrl) {
  res.cookie(jwtTokenStorageKey, token, {
      httpOnly: true,
      secure: true,
  })
  // TODO: Generate CSRF Token

  return res.redirect(successRedirectUrl)
}

This implementation refers to the authentication JWT Token generated after logging in with a 3rd Party or through emailpass authentication.

[sradevski]

@v0eak we are already doing some changes to how the authentication work, so can you please open a ticket for that? We'll have it included in the ongoing work, as the fix will probably be to just not include it (I'll need to see what exactly happens there and why we put it in the search params, as it seems it is not necessary)

[v0eak]

@sradevski, it actually is very necessary.
Well, the reason this is here, is because the js-sdk sets the generated JWT Token locally in the browser (

medusa/packages/core/js-sdk/src/auth/index.ts

Line 33 in 2f01669

this.client.setToken(token)

). The token is retrieved from the /auth/${actor}/${method} POST route, which gets transferred to the callback route. In both cases the POST route is referring to the GET route.
As a result of the successRedirect, the token cannot be returned as a JSON response, which is why the token is appended in the parameters (https://github.com/medusajs/medusa/blob/develop/packages/medusa/src/api/auth/%5Bactor_type%5D/%5Bauth_provider%5D/callback/route.ts#L74).
As explained earlier, imo, this is not safe, which is why I have added storage of the token in cookies, without exposing the token.

I have also noticed that CSRF Tokens are not generated, which would be the next step in securing authorization with JWT Tokens, will create a new pull Request for that as well.

[v0eak]

By passing the cookies automatically, I introduced CSRF attacks (which I guess is touched upon here (https://github.com/medusajs/medusa/blob/develop/packages/core/js-sdk/src/client.ts#L42). Which would need to be mitigated with CSRF Tokens, defeating the purpose of JWT Tokens, which are stateless, which is why I would only recommend using the jwtTokenStorageMethod "cookie", if you use authentication sessions instead of stateless jwt.

[sradevski]

Hey @v0eak thanks for all the work so far! We just had a discussion, and we would like to review the problem and potential solutions internally first, and then we will either try to get this PR to the ideal end-state or we will open a PR and ask for a review. We'll keep you updated!

[sradevski]

Hey @v0eak I've opened a PR that solves things a bit differently, please have a look: #8980

[v0eak]

@sradevski Looks great! I see, you have replaced successRedirectUrl, where location is now returned instead. Thank you very much!