supabase / ssr

Supabase clients for use in server-side rendering frameworks.
MIT License
54 stars 6 forks source link

cookieOptions and config.toml config for cookie MaxAge are ignored #40

Open Rudolf-Dudarev opened 2 months ago

Rudolf-Dudarev commented 2 months ago

Bug report

Describe the bug

The supabase/ssr method createServerClient ignores cookieOptions property maxAge. Not only that but the config.toml property jwt_expiry is also ignored when setting the auth cookie expiry. So we get cookies set with Expires / Max-Age of about 1 year. I have a Next.js app where this is observed.

To Reproduce

  1. Open config.toml, set the jwt_expiry to a low value, e.g., 300 (5min)
  2. Initiate a supabase server client with vanilla config found in docs: https://supabase.com/docs/guides/auth/server-side/creating-a-client
  3. Add cokieOptions object with the maxAge property in the server client config object with a low value, e.g., 300..
  4. From client side initiate a request to an endpoint that handles server side auth. Use one of the sign-in options like signInAnonymousley() or signInWithPassword() on the server side.
  5. Inspect set auth cookie in your browsers dev tools - the Max Age / Expiry of the cookie is set to be around 1 year, which does not match configuration in the cookieOptions or the config.toml.

I observed this issue in my supabase server client utility function:

import { createServerClient, type CookieOptions } from '@supabase/ssr';
import { Database } from '@/lib/types/supabase';
import { cookies } from 'next/headers';

export default function supabaseServerClient() {
  const cookieStore = cookies();
  return createServerClient<Database>(
    process.env.NEXT_PUBLIC_SUPABASE_API_URL!,
    process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY!,
    {
      cookieOptions: {
        // The maxAge property I would expect to be used in setting the cookie expiry and overriding the config.toml config value.
        maxAge: 300,
      },
      cookies: {
        getAll() {
          return cookieStore.getAll();
        },
        setAll(cookiesToSet) {
          try {
            cookiesToSet.forEach(({ name, value, options }) => {
              console.log('_@ OPTIONS', options);

              // The maxAge is not matching the maxAge set in cookieOptions. Nor is it matching the value in the config.toml  

              // _@ OPTIONS {
              //   path: '/',
              //   sameSite: 'lax',
              //   httpOnly: false,
              //   maxAge: 31536000000,
              //   expires: 2024-07-14T17:11:04.968Z
              // }

              return cookieStore.set(name, value, options);
            });
          } catch {}
        },
      },
    },
  );
}

Expected behavior

The set cookie has an expiry matching thejwt_expiry field in config.toml or is overriden and matches the maxAge property set in the createServerClient config cookieOptions.

Screenshots

Screenshot 2024-07-14 at 21 13 43

System information

Additional context

Using Next.js 14.

j4w8n commented 2 months ago

I'm pretty sure I know what's going on here.

The ssr library is overwriting the maxAge, set via options, to their default of 1000 years. However, Chrome is now adhering to RFC6265bis and lowering it to 400 days when setting the cookie.

There is a PR from a community member to get the default lowered to 365 or 400, but has not gotten any traction in the past.

Nonetheless, there is also an issue of the ssr library overriding what devs are setting. I assume this is by design, but I'm unsure why.

dmitriyrusnak1 commented 1 month ago

Is there any update when this issue might be handled? It's quite concerning that we can't set a shorter expiry time than 1 year for the access token.

Rudolf-Dudarev commented 1 month ago

@j4w8n @encima Sorry to ping you, but it does seem like a meaningful security issue that the JWT doesn't expire for a year. I read Supabase docs and everything related to JWTs, it seems that leaving that cookie for a year in client's browser makes it very vulnerable to token theft. As this post hasn't received much attention, I am curious if it could be just an issue with our setup. It's especially concerning for our company as we migrated our e-commerce backend to Supabase and are about to go live. I see that it's not just a local development issue as the setting from our staging Supabase instance also sets the expiry of the cookie to 1 year, overriding the settings.

j4w8n commented 1 month ago

@Rudolf-Dudarev I think the auth team has been pretty busy with some high-priority features, and I'm guessing they've had to triage other items like this one. Hopefully they will chime-in by early September, if not sooner; but I have no idea.

J0 commented 1 week ago

Thanks @j4w8n for jumping in here.

Here's more context around the fixed value and lack of configurability in the library

We recently updated it to 400 days

Main goal of the long time is to ensure Supabase Auth session expires before the max-age is reached, so as to guard against random logouts.