From f4e3a4fddb09dd51e36204e7205e8628ccb481eb Mon Sep 17 00:00:00 2001 From: Gavin Frazar Date: Sat, 15 Feb 2025 18:01:58 -0800 Subject: [PATCH] Wait for fine-grain user permissions to be revoked (#52162) * use separate users for the RDS postgres super/nonsuper db admin tests * parallelize the super/nonsuper db admin tests --- e2e/aws/databases_test.go | 2 +- e2e/aws/rds_test.go | 73 ++++++++++++++++++++++++------------ lib/srv/db/postgres/users.go | 10 ++--- 3 files changed, 54 insertions(+), 31 deletions(-) diff --git a/e2e/aws/databases_test.go b/e2e/aws/databases_test.go index 63843654e77..6b2adb30f8f 100644 --- a/e2e/aws/databases_test.go +++ b/e2e/aws/databases_test.go @@ -338,7 +338,7 @@ func connectPostgres(t *testing.T, ctx context.Context, info dbUserLogin, dbName _ = conn.Close(ctx) }) return &pgConn{ - logger: utils.NewSlogLoggerForTests(), + logger: utils.NewSlogLoggerForTests().With("test_name", t.Name()), Conn: conn, } } diff --git a/e2e/aws/rds_test.go b/e2e/aws/rds_test.go index 22519849ff8..04cddb0a129 100644 --- a/e2e/aws/rds_test.go +++ b/e2e/aws/rds_test.go @@ -83,6 +83,9 @@ func testRDS(t *testing.T) { autoUserFineGrain := "auto_fine_grain_" + randASCII(t) autoUserKeep := "auto_keep_" + randASCII(t) autoUserDrop := "auto_drop_" + randASCII(t) + autoUserFineGrain2 := "auto_fine_grain2_" + randASCII(t) + autoUserKeep2 := "auto_keep2_" + randASCII(t) + autoUserDrop2 := "auto_drop2_" + randASCII(t) autoRole1 := "auto_granted_role1_" + randASCII(t) autoRole2 := "auto_granted_role2_" + randASCII(t) @@ -90,26 +93,30 @@ func testRDS(t *testing.T) { accessRole := mustGetEnv(t, rdsAccessRoleARNEnv) discoveryRole := mustGetEnv(t, rdsDiscoveryRoleARNEnv) + dbAutoUserFineGrainRole := makeAutoUserDBPermissions( + types.DatabasePermission{ + Permissions: []string{"SELECT"}, + Match: types.Labels{ + "object_kind": {"table"}, + "schema": {"public", testSchema, "information_schema"}, + }, + }, + types.DatabasePermission{ + Permissions: []string{"SELECT"}, + Match: types.Labels{ + "object_kind": {"table"}, + "schema": {"pg_catalog"}, + "name": {"pg_range", "pg_proc"}, + }, + }, + ) opts := []testOptionsFunc{ - withUserRole(t, autoUserFineGrain, "db-auto-user-fine-grain", makeAutoUserDBPermissions( - types.DatabasePermission{ - Permissions: []string{"SELECT"}, - Match: types.Labels{ - "object_kind": {"table"}, - "schema": {"public", testSchema, "information_schema"}, - }, - }, - types.DatabasePermission{ - Permissions: []string{"SELECT"}, - Match: types.Labels{ - "object_kind": {"table"}, - "schema": {"pg_catalog"}, - "name": {"pg_range", "pg_proc"}, - }, - }, - )), + withUserRole(t, autoUserFineGrain, "db-auto-user-fine-grain", dbAutoUserFineGrainRole), withUserRole(t, autoUserKeep, "db-auto-user-keeper", makeAutoUserKeepRoleSpec(autoRole1, autoRole2)), withUserRole(t, autoUserDrop, "db-auto-user-dropper", makeAutoUserDropRoleSpec(autoRole1, autoRole2)), + withUserRole(t, autoUserFineGrain2, "db-auto-user-fine-grain", dbAutoUserFineGrainRole), + withUserRole(t, autoUserKeep2, "db-auto-user-keeper", makeAutoUserKeepRoleSpec(autoRole1, autoRole2)), + withUserRole(t, autoUserDrop2, "db-auto-user-dropper", makeAutoUserDropRoleSpec(autoRole1, autoRole2)), } cluster := makeDBTestCluster(t, accessRole, discoveryRole, types.AWSMatcherRDS, opts...) @@ -148,6 +155,9 @@ func testRDS(t *testing.T) { cleanupDB(t, ctx, conn, fmt.Sprintf("DROP ROLE IF EXISTS %q", autoUserKeep)) cleanupDB(t, ctx, conn, fmt.Sprintf("DROP ROLE IF EXISTS %q", autoUserDrop)) cleanupDB(t, ctx, conn, fmt.Sprintf("DROP ROLE IF EXISTS %q", autoUserFineGrain)) + cleanupDB(t, ctx, conn, fmt.Sprintf("DROP ROLE IF EXISTS %q", autoUserKeep2)) + cleanupDB(t, ctx, conn, fmt.Sprintf("DROP ROLE IF EXISTS %q", autoUserDrop2)) + cleanupDB(t, ctx, conn, fmt.Sprintf("DROP ROLE IF EXISTS %q", autoUserFineGrain2)) // create the roles that Teleport will auto assign. for _, r := range [...]string{autoRole1, autoRole2} { @@ -184,20 +194,33 @@ func testRDS(t *testing.T) { autoRolesQuery := fmt.Sprintf("select 1 from %q.%q", testSchema, testTable) var pgxConnMu sync.Mutex for _, test := range []struct { - name string - db types.Database + name string + db types.Database + autoUserKeep string + autoUserDrop string + autoUserFineGrain string }{ { - name: "non superuser db admin", - db: db1, + name: "non superuser db admin", + db: db1, + autoUserKeep: autoUserKeep, + autoUserDrop: autoUserDrop, + autoUserFineGrain: autoUserFineGrain, }, { - name: "superuser db admin", - db: db2, + name: "superuser db admin", + db: db2, + autoUserKeep: autoUserKeep2, + autoUserDrop: autoUserDrop2, + autoUserFineGrain: autoUserFineGrain2, }, } { + autoUserKeep := test.autoUserKeep + autoUserDrop := test.autoUserDrop + autoUserFineGrain := test.autoUserFineGrain + db := test.db t.Run(test.name, func(t *testing.T) { - db := test.db + t.Parallel() for name, test := range map[string]struct { user string dbUser string @@ -245,7 +268,7 @@ func testRDS(t *testing.T) { afterConnTestFn: func(t *testing.T) { pgxConnMu.Lock() defer pgxConnMu.Unlock() - waitForPostgresAutoUserPermissionsRemoved(t, ctx, conn, autoUserDrop) + waitForPostgresAutoUserPermissionsRemoved(t, ctx, conn, autoUserFineGrain) }, }, } { diff --git a/lib/srv/db/postgres/users.go b/lib/srv/db/postgres/users.go index 0b81a00f1bc..20c446d6bbb 100644 --- a/lib/srv/db/postgres/users.go +++ b/lib/srv/db/postgres/users.go @@ -188,18 +188,19 @@ func (e *Engine) granularPermissionsEnabled(sessionCtx *common.Session) bool { } func (e *Engine) applyPermissions(ctx context.Context, sessionCtx *common.Session) error { + logger := e.Log.With("user", sessionCtx.DatabaseUser) allow, _, err := sessionCtx.Checker.GetDatabasePermissions(sessionCtx.Database) if err != nil { - e.Log.ErrorContext(e.Context, "Failed to calculate effective database permissions.", "error", err) + logger.ErrorContext(e.Context, "Failed to calculate effective database permissions.", "error", err) return trace.Wrap(err) } if len(allow) == 0 { - e.Log.InfoContext(e.Context, "Skipping applying fine-grained permissions: none to apply.") + logger.InfoContext(e.Context, "Skipping applying fine-grained permissions: none to apply.") return nil } if len(sessionCtx.DatabaseRoles) > 0 { - e.Log.ErrorContext(ctx, "Cannot apply fine-grained permissions: non-empty list of database roles.", "roles", sessionCtx.DatabaseRoles) + logger.ErrorContext(ctx, "Cannot apply fine-grained permissions: non-empty list of database roles.", "roles", sessionCtx.DatabaseRoles) return trace.BadParameter("fine-grained database permissions and database roles are mutually exclusive, yet both were provided.") } @@ -223,7 +224,7 @@ func (e *Engine) applyPermissions(ctx context.Context, sessionCtx *common.Sessio } summary, eventData := permissions.SummarizePermissions(permissionSet) - e.Log.InfoContext(ctx, "Calculated database permissions.", "summary", summary, "user", sessionCtx.DatabaseUser) + logger.InfoContext(ctx, "Calculated database permissions.", "summary", summary) e.auditUserPermissions(sessionCtx, eventData) perms, err := convertPermissions(permissionSet) @@ -240,7 +241,6 @@ func (e *Engine) applyPermissions(ctx context.Context, sessionCtx *common.Sessio // teleport_remove_permissions and teleport_update_permissions are created in pg_temp table of the session database. // teleport_remove_permissions gets called by teleport_update_permissions as needed. - logger := e.Log.With("user", sessionCtx.DatabaseUser) err = withRetry(ctx, logger, func() error { err := e.createProcedures(ctx, sessionCtx, conn, []string{removePermissionsProcName, updatePermissionsProcName}) return trace.Wrap(err)