realtime icon indicating copy to clipboard operation
realtime copied to clipboard

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

Open zachblume opened this issue 2 years ago • 14 comments

Bug report

  • [x] I confirm this is a bug with Supabase, not with my own application.
  • [x] I confirm I have searched the Docs, GitHub Discussions, and Discord.

System information

  • Version of supabase-js: "supabase": "^1.42.0"
  • Using the self-hosted realtime server

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:

  • The execution environment for the RLS policy?
  • Rewrite the RLS function to better handle the environment?
  • Where do I find debug logs for the RLS evaluation inside realtime, and how would I add a line to the requesting_organization_id() function to print-to-log to debug the environmental context further?

zachblume avatar Mar 22 '23 23:03 zachblume

@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

w3b6x9 avatar Mar 23 '23 06:03 w3b6x9

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 avatar Mar 23 '23 14:03 zachblume

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": "[email protected]",
  "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 avatar Mar 23 '23 14:03 zachblume

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 avatar Mar 23 '23 15:03 zachblume

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 avatar Mar 23 '23 15:03 zachblume

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 avatar Mar 23 '23 15:03 zachblume

To summarize:

  • DELETE broadcasting in violation of RLS policy,
  • Supabase js client authorization header is not passed to realtime module as one would expect,
  • current documentation to correctly authenticate realtime with js client produces a unexplained websocket error,
  • supabase.realtime.setAuth as workaround to websocket error with custom JWT is not documented

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

zachblume avatar Mar 23 '23 15:03 zachblume

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

  • DELETE is broadcast without checking the RLS policy,
  • current documentation to pass custom JWT results in an authentication error
  • realtime with custom JWT only works with RLS turned off

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;
$$;

das-monki avatar Mar 29 '23 15:03 das-monki

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?

zachblume avatar Mar 29 '23 21:03 zachblume

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 avatar Mar 30 '23 09:03 das-monki

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 avatar Mar 30 '23 11:03 das-monki

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..

das-monki avatar Mar 30 '23 12:03 das-monki