Skip to content

Database Automatic User Provisioning support for Redshift#33307

Merged
greedy52 merged 6 commits intomasterfrom
STeve/27323_redshift
Oct 23, 2023
Merged

Database Automatic User Provisioning support for Redshift#33307
greedy52 merged 6 commits intomasterfrom
STeve/27323_redshift

Conversation

@greedy52
Copy link
Copy Markdown
Contributor

@greedy52 greedy52 commented Oct 11, 2023

Part of:

Related:

Summary

  • The overall logic of the stored procedures for Redshift follows the same logic as Postgres.
  • Different tables/views are used for Redshift.
  • Some syntax differences for Redshift
    • Redshift has no array so have to use JSON text
    • Slightly different syntax for execute
    • Have to raise exception to generate errors
  • Used connection limit to lock/unlock the account (client sees FATAL: too many connections for user "STeve" if attempt to login)
  • Added logic to detect User has active connections and roles have changed
Manual testing setup example

1. Configure Redshift

Login in as a superuser, create teleport-admin:

CREATE USER "teleport-admin" WITH PASSWORD DISABLE;
GRANT ROLE "sys:superuser" TO "teleport-admin";

Generate a few roles for testing:

CREATE ROLE role1;
CREATE ROLE role2;
CREATE ROLE role3;
GRANT ALL ON DATABASE test TO ROLE "role1";

2. Configure Teleport

Create a Teleport role for auto-user and assign it to your Teleport user, ex:

kind: role
version: v6
metadata:
  name: redshift-auto-user
spec:
  options:
    create_db_user: true
  allow:
    db_labels:
      "Owner": "STeve"
      "teleport.dev/db-admin": "teleport-admin"
    db_names:
    - "*"
    db_roles:
    - "role1"
    - "role3"

Use auto-discovery to register the Redshift database. Make sure teleport.dev/db-admin resource tag is set to teleport-admin.

3. Connect

  • tsh login
  • tsh db connect --db-user <teleport-user> --db-name test steve-redshift
  • select * from svv_user_grants

@greedy52 greedy52 added database-access Database access related issues and PRs changelog labels Oct 11, 2023
@greedy52 greedy52 self-assigned this Oct 11, 2023
@greedy52 greedy52 force-pushed the STeve/27323_redshift branch from 0ba8343 to af74b13 Compare October 11, 2023 17:18
Comment thread lib/srv/db/postgres/redshift-activate-user.sql Outdated
Comment thread lib/srv/db/postgres/redshift-activate-user.sql Outdated
Comment thread lib/srv/db/postgres/users.go Outdated
Comment thread lib/srv/db/postgres/redshift-activate-user.sql Outdated
@greedy52 greedy52 requested a review from Tener October 16, 2023 16:04
Copy link
Copy Markdown
Contributor

@Tener Tener left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed this one; uppercase scripts were a pure nit, so my intent was to approve prior to this change.

Comment thread lib/srv/db/postgres/redshift-activate-user.sql Outdated
Comment thread lib/srv/db/postgres/users.go
@@ -0,0 +1,40 @@
CREATE OR REPLACE PROCEDURE teleport_activate_user(username varchar, roles text)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can me move the sql scripts to a separate directory postgres/assets? ( not necessary in this PR)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I planned to suggest the same, except I had postgres/sql in mind.

-- Otherwise reactivate the user, but first strip it of all roles to
-- account for scenarios with left-over roles if database agent crashed
-- and failed to cleanup upon session termination.
CALL teleport_deactivate_user(username);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got following error from teleport_activate_user procedure call in fresh redshift setup during testing:

tsh db connect --db-user=marek --db-name=dev marek-redshift-cluster-1
psql: error: connection to server at "localhost" (::1), port 54170 failed: Connection refused
	Is the server running on that host and accepting TCP/IP connections?
connection to server at "localhost" (127.0.0.1), port 54170 failed: your Teleport role requires automatic database user provisioning but an attempt to activate database user "marek" failed due to the following error: ERROR: user "marek" does not exist (SQLSTATE 42704)

Also when I tries to call teleport_activate_user manually:
Screenshot 2023-10-20 at 13 14 26

dev=# select * from pg_user;
    usename     | usesysid | usecreatedb | usesuper | usecatupd |  passwd  | valuntil | useconfig
----------------+----------+-------------+----------+-----------+----------+----------+-----------
 rdsdb          |        1 | t           | t        | t         | ******** | infinity |
 admin          |      100 | t           | t        | f         | ******** |          |
 teleport-admin |      101 | f           | f        | f         | ******** |          |
dev=# select * from SVV_ROLES;
 role_id |     role_name      |   role_owner   | external_id
---------+--------------------+----------------+-------------
  105587 | sys:operator       | rdsdb          |
  105588 | sys:monitor        | rdsdb          |
  105589 | sys:dba            | rdsdb          |
  105590 | sys:secadmin       | rdsdb          |
  105591 | sys:superuser      | rdsdb          |
  106245 | role1              | admin          |
  106246 | role2              | admin          |
  106247 | role3              | admin          |
  106248 | teleport-auto-user | teleport-admin |

@greedy52 I'm missing something ?

Copy link
Copy Markdown
Contributor Author

@greedy52 greedy52 Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems caused by missing spaces. fixed in dc1cdc4.

Could you give it a try?

This is so weird that it worked for my user STeve. I think because QUOTE_IDENT has no effect on username with only lower cases. Even then, we are supposed to see CREATE USER failed first before the grant. I played around a little bit more but so far seems just Redshift weirdness. Doc didn't say much either.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, After dc1cdc4 my setup works. Thanks for the alignment.

@greedy52 greedy52 added this pull request to the merge queue Oct 23, 2023
Merged via the queue into master with commit 4dce835 Oct 23, 2023
@greedy52 greedy52 deleted the STeve/27323_redshift branch October 23, 2023 19:04
@public-teleport-github-review-bot
Copy link
Copy Markdown

@greedy52 See the table below for backport results.

Branch Result
branch/v14 Failed

@public-teleport-github-review-bot
Copy link
Copy Markdown

@greedy52 See the table below for backport results.

Branch Result
branch/v14 Failed

greedy52 added a commit that referenced this pull request Nov 1, 2023
* Database Automatic User Provisioning support for Redshift

* capitalize sql script

* DeleteUser to fallback to deactivate.

* add TPxxx code to RAISE messages

* fix missing space in scripts
github-merge-queue Bot pushed a commit that referenced this pull request Nov 6, 2023
* Database Automatic User Provisioning support for Redshift (#33307)

* Database Automatic User Provisioning support for Redshift

* capitalize sql script

* DeleteUser to fallback to deactivate.

* add TPxxx code to RAISE messages

* fix missing space in scripts

* feat(postgres): support auto-provisioned user deletion Redshift (#34006)

---------

Co-authored-by: Gabriel Corado <gabriel.oliveira@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

database-access Database access related issues and PRs size/md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants