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

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

Open
2 tasks done
zachblume opened this issue Mar 22, 2023 · 12 comments
Labels
bug Something isn't working

Comments

@zachblume
Copy link

zachblume commented Mar 22, 2023

Bug report

  • I confirm this is a bug with Supabase, not with my own application.
  • 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 zachblume added the bug Something isn't working label Mar 22, 2023
@zachblume zachblume changed the title Realtime: RLS execution context differs based on type of change being listened to Bug: realtime RLS policy evaluation differs based on type of change (INSERT vs DELETE), DELETE broadcasting in violation of RLS policy Mar 22, 2023
@w3b6x9
Copy link
Member

w3b6x9 commented Mar 23, 2023

@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
Copy link
Author

zachblume commented Mar 23, 2023

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
Copy link
Author

zachblume commented Mar 23, 2023

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
Copy link
Author

zachblume commented Mar 23, 2023

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
Copy link
Author

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
Copy link
Author

zachblume commented Mar 23, 2023

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
Copy link
Author

zachblume commented Mar 23, 2023

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

@das-monki
Copy link

das-monki commented Mar 29, 2023

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

@zachblume
Copy link
Author

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
Copy link

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
Copy link

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
Copy link

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:

defp generate_signer(%{"typ" => "JWT", "alg" => alg}, jwt_secret) when alg in @hs_algorithms do

@zachblume sorry for freeriding on your ticket..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants