supabase / realtime

Broadcast, Presence, and Postgres Changes via WebSockets
https://supabase.com/realtime
Apache License 2.0
6.7k stars 299 forks source link

Bug: realtime RLS policy evaluation differs based on type of change (INSERT vs DELETE), DELETE broadcasting in violation of RLS policy #562

Open zachblume opened 1 year ago

zachblume commented 1 year ago

Bug report

System information

Details and reproduction

I am using Clerk.dev's supabase template to sign my Supabase JS client with a proper JWT. The JWT includes an organization id metadata, orgID:

const supabase = createClient(
            process.env.NEXT_PUBLIC_SUPABASE_URL,
            process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY,
            {
                global: {
                    headers: { Authorization: `Bearer ${supabaseAccessToken}` },
                },
            }
        );

RLS works fine with a custom function:

create or replace function requesting_org_id()
returns text
language sql stable
as $$
  select nullif((current_setting('request.jwt.claims', true)::json->>'user_metadata')::json->>'orgID', '')::text;
$$;

With a simple policy:

ALTER TABLE call_sessions ENABLE ROW LEVEL SECURITY;

ALTER TABLE call_sessions FORCE ROW LEVEL SECURITY;

DROP POLICY IF EXISTS call_sessions_org_id on call_sessions;

CREATE POLICY call_sessions_org_id ON call_sessions USING (organization_id = requesting_org_id());

This is where things start getting weird.

I subscribe to the table call_sessions realtime replication channel:

const channel = supabase
    .channel("call_sessions")
    .on(
        "postgres_changes",
        {
            event: "*",
            schema: "public",
            table: "call_sessions",
        },
        (result) => {
            console.log("Listen to changes:", { result });
        }
    )
    .subscribe();

When I insert or update rows through the authenticated client, the listener is not triggered (it should be). When I delete rows through the authenticated client, the listener is correctly triggered.

Same thing with a raw connection: When I insert or update rows through PSQL or the Supabase studio, the listener is not triggered. When I delete rows which previously contained the correct organization_id through PSQL or supabase studio, the listener is triggered (correctly).

When I delete rows which previously contained a nonsense organization_id (organization_id="id_that_shouldnt_match_any_RLS_client") through PSQL or supabase studio, the authenticated listener somehow is also triggered (incorrectly).

When I disable RLS, the client listener is triggered by UPDATE, INSERT and DELETE from both authenticated client and raw connections. So clearly the problem is that RLS is being evaluated in a nonstandard way, i.e. the execution context must differ between regular SQL and realtime production.

What is going on there? Eeek.

Can I somehow fix:

w3b6x9 commented 1 year ago

@zachblume can you take a look at these docs and see if that addresses your issue? https://supabase.com/docs/guides/realtime/postgres-changes#custom-tokens

zachblume commented 1 year ago

Giving that a try, I did:

const supabase = createClient(
      process.env.NEXT_PUBLIC_SUPABASE_URL,
      process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY,
      {
          global: {
              headers: { Authorization: `Bearer ${supabaseAccessToken}` },
          },
          realtime: {
              headers: {
                  apikey: process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY,
              },
              params: {
                  apikey: supabaseAccessToken,
              },
          },
      }
  );

And got the following console error:

browser.js?eccf:25 
WebSocket connection to 'ws://localhost:54321/realtime/v1/websocket?apikey=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhcHBfbWV0YWRhdGEiOnt9LCJhdWQiOiJhdXRoZW50aWNhdGVkIiwiYXpwIjoiaHR0cDovL2xvY2FsaG9zdDozMDAwIiwiZW1haWwiOiJ6YWNoYmx1bWVAZ21haWwuY29tIiwiZXhwIjoxNzc5NTgyNzk0LCJpYXQiOjE2Nzk1ODI3OTUsImlzcyI6Imh0dHBzOi8vY2xlcmsucHJvbXB0Lm1lZXJrYXQtODUubGNsLmRldiIsImp0aSI6IjdiNmViMjM5ZmQ3YmM3ODk4YmQ4IiwibmJmIjoxNjc5NTgyNzkwLCJyb2xlIjoiYXV0aGVudGljYXRlZCIsInN1YiI6InVzZXJfMkpBMURxUDJnRGcwN0tmOU81UGMwY3A5Z3llIiwidXNlcl9tZXRhZGF0YSI6eyJvcmdJRCI6Im9yZ18yTU4wb0hvQnFFeXVEWWZGQlg4VXZCNWJLRm8ifX0.bCWGrHCs5Xb2lF4Cuww0_ZP2C73Bmh_mr930RcV3ExI&vsn=1.0.0' failed: 

W3CWebSocket | @ | browser.js?eccf:25
-- | -- | --
  | connect | @ | RealtimeClient.js?6b6d:97
  | channel | @ | RealtimeClient.js?6b6d:187
  | channel | @ | SupabaseClient.js?3f72:127
  | TestRealtimePage | @ | test.realtime.js?844a:9
  | renderWithHooks | @ | react-dom.development.js?ac89:16305
  | mountIndeterminateComponent | @ | react-dom.development.js?ac89:20074
  | beginWork | @ | react-dom.development.js?ac89:21587
  | beginWork$1 | @ | react-dom.development.js?ac89:27426
  | performUnitOfWork | @ | react-dom.development.js?ac89:26557
  | workLoopSync | @ | react-dom.development.js?ac89:26466
  | renderRootSync | @ | react-dom.development.js?ac89:26434
  | performConcurrentWorkOnRoot | @ | react-dom.development.js?ac89:25738
  | workLoop | @ | scheduler.development.js?bcd2:266
  | flushWork | @ | scheduler.development.js?bcd2:239
  | performWorkUntilDeadline | @ | scheduler.development.js?bcd2:533

The error is only trigger if I make the supabase......subscribe() realtime call.

zachblume commented 1 year ago

The token I'm using from Clerk.dev:

eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9....

Using jwt.io looks like this:

{
  "alg": "HS256",
  "typ": "JWT"
}
PAYLOAD
{
  "app_metadata": {},
  "aud": "authenticated",
  "azp": "http://localhost:3000",
  "email": "zachblume@gmail.com",
  "exp": 1779583417,
  "iat": 1679583418,
  "iss": "https://clerk.prompt.meerkat-85.lcl.dev",
  "jti": "8edf6de02496da7d134c",
  "nbf": 1679583413,
  "role": "authenticated",
  "sub": "user_2JA1DqP2gDg07Kf9O5Pc0cp9gye",
  "user_metadata": {
    "orgID": "org_2MN0oHoBqEyuDYfFBX8UvB5bKFo"
  }
}
zachblume commented 1 year ago

If I switch out all the keys for the anon key:

eyJhb...
{
  "alg": "HS256",
  "typ": "JWT"
}
PAYLOAD
{
  "iss": "supabase-demo",
  "role": "anon",
  "exp": 1983812996
}

Then the websocket works (except for RLS of course).

Seems to me like token.role might be at issue since clerk.dev passes a token signed with role=authenticated? Or something else?

zachblume commented 1 year ago

Looks like the same problem here:

              @w3b6x9 Got it. Also https://supabase.com/docs/guides/realtime/postgres-changes#custom-tokens, this doesn't fix this. In fact, it throws a web-socket connection error when we use any key other than the anon key. Since `setAuth` is not exposed as an API, I cannot use my current workaround without modifying the superbase-js source code. Can we get this PR merged? Or do we have any work around here?

_Originally posted by @itsgouthamraj in https://github.com/supabase/supabase-js/issues/704#issuecomment-1418373969_
zachblume commented 1 year ago

I eventually found a workaround in a related thread using supabase.realtime.setAuth:

const supabase = createClient(
    process.env.NEXT_PUBLIC_SUPABASE_URL,
    process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY,
    {
        global: {
            headers: { Authorization: `Bearer ${supabaseAccessToken}` },
        },
        // realtime: {
        //     headers: {
        //         apikey: process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY,
        //     },
        //     params: {
        //         accessToken: process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY,
        //     },
        // },
    }
);
supabase.realtime.setAuth(supabaseAccessToken);

RLS process mostly correctly on self-hosted (after I re-made my development server and double checked replication and RLS policies).

However, the oustanding bug of ALL deletes being broadcasted still persists, ignoring the RLS policy.

zachblume commented 1 year ago

To summarize:

I'm interested in collaborating on a PR to fix these

das-monki commented 1 year ago

I can confirm all of the issues summarized above, and in addition, I was not able to fix them with the mentioned workaround.

Also, I'm using the hosted version.

@w3b6x9 tagging you, since you have been very helpful in solving issues with custom JWT 😄.

Ps.: Might help - I've added logging to the function I use in the RLS policy, and while the logs are there when getting data via select, they are completely missing for realtime, suggesting that changes on realtime-subscriptions never make it to the RLS policy.

Pss.: @zachblume logs can be added by using plpgsql and the raise log statement - the function I'm using:

create or replace function auth.order_id() returns uuid language plpgsql as $$
declare
  ret uuid;
begin
  ret := nullif(current_setting('request.jwt.claims', true)::json->>'order_id', '')::uuid;
  RAISE LOG 'JWT claims are: %', current_setting('request.jwt.claims', true)::json;
  RAISE LOG 'order_id is: %', ret;
  return ret;
end;
$$;
zachblume commented 1 year ago

das-monki

Thanks for the pointer to raise log. Do you know where Supabase points the pl/sql native logging by default? Regular postgres logging?

das-monki commented 1 year ago

Not sure which logs you mean with pl/sql native logging, but the logs from raise log end up as part of the regular postgres logs which are available in the dashboard.

das-monki commented 1 year ago

I've tried one more time to apply the workaround, where I create the supabase-client without any options, and then call supabase.realtime.setAuth(jwt). I can see that my token is sent in the payload of the websocket, but the response is

{"event":"phx_reply","payload":{"response":{"reason":"{:error, :error_generating_signer}"},"status":"error"},"ref":"1","topic":"realtime:order-changes"}

@w3b6x9 any pointers as to what the error error_generating_signer means?

For reference, the payload of my token is:

{
  "role": "authenticated",
  "order_id": "f7c5f9b0-d210-4c92-a613-faf22751e1d3",
  "sub": "I2Wu5KydSMTucmPFgGrPQN2",
  "aud": "authenticated",
  "iat": 1680175405,
  "exp": 1680189805
}

with header:

{
  "alg": "HS256"
}

This token works with RLS policy when getting data via select though, so it should be fine.

@zachblume I've just seen it mentioned in the docs, that DELETE is broadcast without RLS, though only the primary key is broadcast. https://supabase.com/docs/reference/javascript/subscribe

das-monki commented 1 year ago

I finally got the workaround working.. I saw that the header of the custom jwt has to contain "typ": "JWT", which was missing in my header.

Found it while looking at: https://github.com/supabase/realtime/blob/61f4930a0576ce2f3b67643a5a260418fbb75352/lib/realtime_web/channels/auth/jwt_verification.ex#L55

@zachblume sorry for freeriding on your ticket..