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

Using RLS with Slonik #441

Closed
victorlcm opened this issue Dec 20, 2022 · 6 comments
Closed

Using RLS with Slonik #441

victorlcm opened this issue Dec 20, 2022 · 6 comments
Labels

Comments

@victorlcm
Copy link

I'm evaluating using slonik for a multi-tenant application using postgres RLS. However, I did not find a good way to set postgres session variables with the tenant id before executing queries. Is there a way to do it with slonik?

Desired Behavior

A way to execute SET app.current_tenant =TENANT_ID before each query for postgres RLS to work properly

Motivation

Postgres RLS is gaining a lot of popularity and most major libraries/frameworks don't offer a good way of adding session variables per connection and resetting this info on release

Implementation

@kentare
Copy link

kentare commented Dec 22, 2022

I'm using slonik in a multi-tenant set up where I use RLS. I create a transaction, set the user (tenant id in your case) and do all relevant queries inside the transaction.

Here is an example:

export const set_current_user_id = async (
  transaction: DatabaseTransactionConnection | DatabasePoolConnection,
  user_id: number
) => {
  return await transaction.query(
    sql.unsafe`SET LOCAL app.current_user_id = ${sql.literalValue(`${user_id}`)};`
  )
}

async function query_whatever(
  connection: DatabasePoolConnection,
  user_id: number,
) {
  await connection.transaction(async (transaction) => {
    await set_current_user_id(transaction, user_id)
     // REST OF THE QUERIES HERE
    )
  })
}

The effects of SET LOCAL should last only till the end of the current transaction

@kentare
Copy link

kentare commented Jan 7, 2023

I'll also add some example of the sql of read access to a table if others are trying to do something similar.

CREATE FUNCTION current_app_user()
RETURNS INTEGER AS $$ SELECT
    NULLIF(
      current_setting(
        'app.current_user_id',
        TRUE
      ),
      ''
    )::INTEGER $$ LANGUAGE SQL SECURITY DEFINER;
    
CREATE FUNCTION has_access_to_whatever_table(_user_id INT, _access_level INT, _id INT)
RETURNS BOOLEAN AS $$
  -- The function returns true if the user has a matching
  -- record in the user_whatever_table_relation table with access_level > _access_level
 -- 0 = blocked, 1 = read, 2 = write, 3 = admin
  BEGIN
  RETURN (
    EXISTS (
      SELECT 1 FROM user_whatever_table_relation as r
      WHERE r.user_id = _user_id
        AND r.access_level >= _access_level
        AND r.whatever_table_id = _id
    )
  );
  END
$$ LANGUAGE plpgsql;   

ALTER TABLE whatever_table ENABLE ROW LEVEL SECURITY;


CREATE POLICY whatever_access ON whatever_table
FOR SELECT
USING (
  has_access_to_whatever_table(current_app_user(), 1, whatever_table_id) 
);

@victorlcm
Copy link
Author

we're currently doing something like that. However, the point of adding this to the library is to avoid the need of setting manually on each query, as this is very error-prone and the consequences of not adding this in a multi-tenant application might be catastrophic (i.e. fetching data from one tenant for another). If we had like an option to set this on every connection acquire from the pool, this would be much safer

@kentare
Copy link

kentare commented Jan 23, 2023

Also interested if there is a way to do this when acquiring a connection from a pool.
Maybe you can look at beforePoolConnection and afterPoolConnection interceptors?

For me this method works well. Every exposed function is wrapped in a wrapper which sets the user_id localized to the query and I get the tenant id from a signed jwt cookie.

@victorlampp
Copy link

I also choose this approach. I use afterPoolConnection and beforePoolConnectionRelease interceptors. This seems to do the job but let me know If I'm wrong

I get tenant information from an asyncLocalStorage (requestContext) like that :

import * as ctx from './RequestContext';

const interceptors: Interceptor[] = [
  {
    afterPoolConnection: async (
      connectionContext: ConnectionContext,
      connection: DatabasePoolConnection,
    ) => {
      const context = ctx.getContext();
      const tenant = context.tenantCustomerId;
      if (!tenant) {
        throw new NoRequestContextError(
          'The tenant is Empty in the async RequestContext object !!!!',
        );
      }

      logger.debug(
        { tenantContext: context, slonikContext: slonikContextFilter(connectionContext) },
        'After Pool Connection and before sql request -> set tenant information',
      );

      await connection.query(sql.typeAlias('void')`
      
            SET myDsn.tenant_id = ${sql.identifier([tenant])};
      `);
      return null;
    },

    beforePoolConnectionRelease: async (
      connectionContext: ConnectionContext,
      connection: DatabasePoolConnection,
    ) => {
      const context = ctx.getContext();
      logger.debug(
        { tenantContext: context, slonikContext: slonikContextFilter(connectionContext) },
        'Before Pool release and after sql request -> remove tenant information',
      );
      await connection.query(sql.typeAlias('void')`
            RESET ALL;`); 
      return null;
    },

@gajus
Copy link
Owner

gajus commented Mar 23, 2023

Sounds like a valid solution^

@gajus gajus closed this as completed Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants