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

postgresql_grant resource should allow empty list of privileges #16

Closed
gshutler opened this issue Dec 9, 2020 · 10 comments · Fixed by #27
Closed

postgresql_grant resource should allow empty list of privileges #16

gshutler opened this issue Dec 9, 2020 · 10 comments · Fixed by #27

Comments

@gshutler
Copy link

gshutler commented Dec 9, 2020

Replication of hashicorp/terraform-provider-postgresql#197


Being able to execute REVOKE ALL ON DATABASE XXX FROM PUBLIC is the one thing we're looking for if a separate, more targeted resource might be more appropriate.

@icterine
Copy link

That seems priority number #1. Many people suffers from not being REVOKE public access from database. Only one command is required for us to not use other provision tools.

@cyrilgdn
Copy link
Owner

@gshutler Thanks for (re)opening this issue.

When I created this resource I thought this would not be necessary but indeed we may need to revoke existing accesses without granting new one (e.g. the public one as mentioned by @icterine ).

I'll take a look.

@cyrilgdn
Copy link
Owner

cyrilgdn commented Jan 1, 2021

@gshutler @kamil-rogon-dragon

After digging a bit on this subject, we need 3 things:

The third one is required for @kamil-rogon-dragon 's example in hashicorp#197 :

This would allow to delete some default permission that postgresql creates (ex. for public schema).

Unless I'm wrong, REVOKE ALL ON DATABASE XXX FROM PUBLIC will not resolve this specific case.

This was referenced Jan 1, 2021
@cyrilgdn
Copy link
Owner

cyrilgdn commented Jan 3, 2021

After digging a bit on this subject, we need 3 things:

* Allow to set an empty list of privileges, should be done in #26

* Manage the `public` specific role, it should be ~quite easy~ doable (a bit less easy than expected after a quick look sweat_smile ).

* Allow to manage grant on schema (cf #12)

You can follow issue #12 for the third point.

@icterine
Copy link

icterine commented Jan 4, 2021

Here is the good example what needs to be done

https://aws.amazon.com/blogs/database/managing-postgresql-users-and-roles/

-- Revoke privileges from 'public' role
REVOKE CREATE ON SCHEMA public FROM PUBLIC;
REVOKE ALL ON DATABASE mydatabase FROM PUBLIC;

This is top level.

And then more granule access is managed by grant to database/schema like

CREATE ROLE readonly;
GRANT CONNECT ON DATABASE mydatabase TO readonly;
GRANT USAGE ON SCHEMA myschema TO readonly;

@cyrilgdn
Copy link
Owner

cyrilgdn commented Jan 4, 2021

@icterine Thanks for these clarifications 👍

All of these will be possible once #12 is addressed. I already started to work on it in #30. It already works but I just need to add the tests.

It should be released in the next version.

@icterine
Copy link

icterine commented Jan 7, 2021

@cyrilgdn sorry to being pedantic but my request was a little bit different

the issues you are working all deal with schema based access like

GRANT { { CREATE | USAGE } [, ...] | ALL [ PRIVILEGES ] }
ON SCHEMA schema_name [, ...]
TO role_specification [, ...] [ WITH GRANT OPTION ]

But we need also pure database privileges without schema like

GRANT { { CREATE | CONNECT | TEMPORARY | TEMP } [, ...] | ALL [ PRIVILEGES ] }
ON DATABASE database_name [, ...]
TO role_specification [, ...] [ WITH GRANT OPTION ]

I might be wrong and those one can be coveted is schema declaration will be optional in statement

resource "postgresql_grant" "readonly_tables" {
database = "test_db"
role = "test_role"
schema = "public"
object_type = "table"
privileges = ["SELECT"]
}

but it is mandatory

schema - (Required) The database schema to grant privileges on for this role. as for today.

@cyrilgdn
Copy link
Owner

cyrilgdn commented Jan 7, 2021

@icterine No problem, you're right I didn't answered fully to your question.

Actually what you want is already possible but the documentation is not up to date, I'll fix it. schema is not required anymore with object_type set to database

You can write:

resource postgresql_database "test" {
  name = "test"
}

resource postgresql_role "test_role" {
  name = "test_role"
}

resource postgresql_grant "test_role" {
  database    = postgresql_database.test.name
  role        = postgresql_role.test_role.name
  object_type = "database"
  privileges = [
    "CREATE",
    "CONNECT",
  ]
}

But with object_type set to table, schema is required, associated query is:

GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
    [, ...] | ALL [ PRIVILEGES ] }
    ON { [ TABLE ] table_name [, ...]
         | ALL TABLES IN SCHEMA schema_name [, ...] }
    TO role_specification [, ...] [ WITH GRANT OPTION ]

@icterine
Copy link

icterine commented Jan 7, 2021

Thanks @cyrilgdn .

Much appreciated for your help..

@cyrilgdn
Copy link
Owner

@gshutler @icterine

FYI, this has just been released in v1.11.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants