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

Learning of FK-constraint on Postgres fails for non-owner of the relation #1879

Open
nbenn opened this issue Apr 29, 2023 · 7 comments
Open
Labels
bug Something isn't working
Milestone

Comments

@nbenn
Copy link
Member

nbenn commented Apr 29, 2023

@krlmlr Could you maybe comment on

WHERE pg_has_role(x.tblowner, 'USAGE'::text)

This requires the role issuing the query to have USAGE privileges for the role that owns the relation for the corresponding FK-constraint to be picked up by dm_meta_raw(). Admittedly, I'm fairly new to the topic of DB access privileges, but this seems unnecessarily restrictive. Couldn't you easily find yourself in the situation where the user/role owning theses objects is much more capable than the user/role trying to "learn" them?

@krlmlr
Copy link
Collaborator

krlmlr commented May 9, 2023

Does that help:

https://stackoverflow.com/q/16830740/946850

The file you linked to comes directly from Postgres, with small adaptations:

psql -c '\d+ information_schema.constraint_column_usage'

@nbenn
Copy link
Member Author

nbenn commented May 9, 2023

According to relevant docs, the view generated by the code that serves as basis for this query is defined such that

Only those columns are shown that are contained in a table owned by a currently enabled role.

Given my current understanding, this does not mean that the offending WHERE clause cannot safely be excluded.

@nbenn
Copy link
Member Author

nbenn commented Jun 23, 2023

@krlmlr Any update here? I believe there is a mismatch in semantics.

Calling dm_from_con(con, learn_keys = TRUE) my expectation is (and the docs do not point in any other direction) that PKs, FKs, etc. are returned for all tables that the current user has access to (and not is owner of).

You are using query code, which is specifically restricted to tables that are owned by the current user (as explicitly stated in the docs).

In a setting where the con user has access to tables but does not own these tables this then yields "wrong" results.

@krlmlr
Copy link
Collaborator

krlmlr commented Jun 23, 2023

We found the code that needs adoption. Would you like to take a stab, do you need help?

@krlmlr
Copy link
Collaborator

krlmlr commented Jun 27, 2023

We also need a reprex.

@nbenn
Copy link
Member Author

nbenn commented Aug 6, 2023

The reprex, as requested. I'm sorry for the delay.

General set up:

CREATE DATABASE fk_vis_reprex;

CREATE USER fk_vis_owner WITH PASSWORD 'pwd123';
CREATE USER fk_vis_reader WITH PASSWORD 'pwd345';

GRANT CONNECT, TEMPORARY, CREATE ON DATABASE fk_vis_reprex TO fk_vis_owner;
GRANT CONNECT, TEMPORARY ON DATABASE fk_vis_reprex TO fk_vis_reader;

then, with a connection of our user fk_vis_owner, some more set up (as psql -d fk_vis_reprex -U fk_vis_owner ):

CREATE SCHEMA fk_vis;

GRANT USAGE ON SCHEMA fk_vis TO fk_vis_reader;
ALTER DEFAULT PRIVILEGES IN SCHEMA fk_vis GRANT SELECT ON TABLES TO fk_vis_reader;

CREATE TABLE fk_vis.item (
  item INT GENERATED ALWAYS AS IDENTITY,
  name VARCHAR(255) NOT NULL,
  PRIMARY KEY(item)
);

CREATE TABLE fk_vis.customer (
  customer INT GENERATED ALWAYS AS IDENTITY,
  name VARCHAR(255) NOT NULL,
  PRIMARY KEY(customer)
);

CREATE TABLE fk_vis.sale (
  sale INT GENERATED ALWAYS AS IDENTITY,
  item INT,
  customer INT,
  PRIMARY KEY(sale),
  CONSTRAINT fk_item
    FOREIGN KEY(item)
    REFERENCES fk_vis.item(item),
  CONSTRAINT fk_customer
    FOREIGN KEY(customer)
    REFERENCES fk_vis.customer(customer)
);

With this, dm::dm_from_con(., learn_keys = TRUE) gives us:

  • for the owner user fk_vis_owner:

    own_con <- DBI::dbConnect(
      RPostgres::Postgres(),
      user = "fk_vis_owner",
      password = "pwd123",
      dbname = "fk_vis_reprex"
    )
    
    dm::dm_from_con(own_con, learn_keys = TRUE, schema = "fk_vis")
    
    #> ── Table source ─────────────────────────────────────────────────────────
    #> src:  postgres  [fk_vis_owner@/tmp:5432/fk_vis_reprex]
    #> ── Metadata ─────────────────────────────────────────────────────────────
    #> Tables: `customer`, `item`, `sale`
    #> Columns: 7
    #> Primary keys: 3
    #> Foreign keys: 2
  • and for the reader user fk_vis_reader:

    read_con <- DBI::dbConnect(
      RPostgres::Postgres(),
      user = "fk_vis_reader",
      password = "pwd345",
      dbname = "fk_vis_reprex"
    )
    
    dm::dm_from_con(read_con, learn_keys = TRUE, schema = "fk_vis")
    #> ── Table source ─────────────────────────────────────────────────────────
    #> src:  postgres  [fk_vis_reader@/tmp:5432/fk_vis_reprex]
    #> ── Metadata ─────────────────────────────────────────────────────────────
    #> Tables: `customer`, `item`, `sale`
    #> Columns: 7
    #> Primary keys: 0
    #> Foreign keys: 0

From what I understand, there is no restriction for getting this information as the "reader" user (as psql -d fk_vis_reprex -U fk_vis_reader), for the example of FK constraints:

SELECT conrelid::regclass AS table_name,
       conname AS foreign_key,
       pg_get_constraintdef(oid)
FROM   pg_constraint
WHERE  contype = 'f'
AND    connamespace = 'fk_vis'::regnamespace
ORDER  BY conrelid::regclass::text, contype DESC;
#>  table_name  | foreign_key |                    pg_get_constraintdef
#>  -------------+-------------+-------------------------------------------------------------
#>  fk_vis.sale | fk_item     | FOREIGN KEY (item) REFERENCES fk_vis.item(item)
#>  fk_vis.sale | fk_customer | FOREIGN KEY (customer) REFERENCES fk_vis.customer(customer)

@krlmlr krlmlr added this to the learn milestone Aug 20, 2023
@salim-b
Copy link
Contributor

salim-b commented May 20, 2024

Just ran into the same issue. My (PG)SQL knowledge is very limited, but I noticed that NocoDB suffers from the same issue and its proposed solution might serve as inspiration here (see the PGSQL query changes).

@krlmlr krlmlr added the bug Something isn't working label Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

3 participants