Skip to content

Commit 768a2b6

Browse files
craig[bot]souravcrl
craig[bot]
and
souravcrl
committed
Merge #148532
148532: sql: update SHOW users/roles to support provisioned users r=rafiss a=souravcrl The SHOW users/roles view currently does not have a mechanism to filter users via the role options that are assigned to them. This support is needed as we will be adding `PROVISIONSRC` role option in #148200 and this needs to have filters support. Additionally, we will be populating the last login time(estimated) for the `system.users` table and this value will be piped here as well. Since this value is computed on a best effort basis; it is not guaranteed to capture every login event, and we will be adding a notice mentioning the same. informs #147602 fixes #147599 Epic CRDB-21590 Release note (enterprise change): The SHOW ROLES command now includes a column that shows the estimated time that the user last logged in. Additionally, the `options` column is now returned as an array of strings, rather than as a single comma-separated string. The data can be queried with a query such as: ``` root@localhost:26257/defaultdb> select * from [show roles] as r WHERE EXISTS (SELECT 1 FROM unnest(r.options) AS m(option) where option like 'SUBJECT=cn%'); username | options | member_of | estimated_last_login_time ------------+--------------------------------+-----------+---------------------------- testuser | {NOLOGIN,SUBJECT=cn=testuser} | {admin} | NULL (1 row) ``` Co-authored-by: souravcrl <[email protected]>
2 parents 265d36e + 9111c44 commit 768a2b6

File tree

11 files changed

+199
-156
lines changed

11 files changed

+199
-156
lines changed

‎pkg/backup/backup_test.go‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9830,13 +9830,13 @@ func TestBackupRestoreSystemUsers(t *testing.T) {
98309830
{"app_role", "app", "false"},
98319831
{"app_role", "test_role", "false"},
98329832
})
9833-
sqlDBRestore.CheckQueryResults(t, "SHOW USERS", [][]string{
9834-
{"admin", "", "{}"},
9835-
{"app", "", "{admin,app_role}"},
9836-
{"app_role", "NOLOGIN", "{}"},
9837-
{"root", "", "{admin}"},
9838-
{"test", "", "{}"},
9839-
{"test_role", "NOLOGIN", "{app_role}"},
9833+
sqlDBRestore.CheckQueryResults(t, "SELECT username, options, member_of from [SHOW USERS] ORDER BY username", [][]string{
9834+
{"admin", "{}", "{}"},
9835+
{"app", "{}", "{admin,app_role}"},
9836+
{"app_role", "{NOLOGIN}", "{}"},
9837+
{"root", "{}", "{admin}"},
9838+
{"test", "{}", "{}"},
9839+
{"test_role", "{NOLOGIN}", "{app_role}"},
98409840
})
98419841
})
98429842

@@ -9860,13 +9860,13 @@ func TestBackupRestoreSystemUsers(t *testing.T) {
98609860
{"test", "false"},
98619861
{"test_role", "true"},
98629862
})
9863-
sqlDBRestore1.CheckQueryResults(t, "SHOW USERS", [][]string{
9864-
{"admin", "", "{}"},
9865-
{"app", "", "{}"},
9866-
{"app_role", "", "{}"},
9867-
{"root", "", "{admin}"},
9868-
{"test", "", "{}"},
9869-
{"test_role", "", "{}"},
9863+
sqlDBRestore1.CheckQueryResults(t, "SELECT username, options, member_of from [SHOW USERS] ORDER BY username", [][]string{
9864+
{"admin", "{}", "{}"},
9865+
{"app", "{}", "{}"},
9866+
{"app_role", "{}", "{}"},
9867+
{"root", "{}", "{admin}"},
9868+
{"test", "{}", "{}"},
9869+
{"test_role", "{}", "{}"},
98709870
})
98719871
})
98729872
_, sqlDBRestore2, cleanupEmptyCluster2 := backupRestoreTestSetupEmpty(t, singleNode, tempDir, InitManualReplication, base.TestClusterArgs{})
@@ -9888,14 +9888,14 @@ func TestBackupRestoreSystemUsers(t *testing.T) {
98889888
{"test_role", "true"},
98899889
{"testuser", "false"},
98909890
})
9891-
sqlDBRestore2.CheckQueryResults(t, "SHOW USERS", [][]string{
9892-
{"admin", "", "{}"},
9893-
{"app", "", "{}"},
9894-
{"app_role", "", "{}"},
9895-
{"root", "", "{admin}"},
9896-
{"test", "", "{}"},
9897-
{"test_role", "", "{}"},
9898-
{"testuser", "", "{}"},
9891+
sqlDBRestore2.CheckQueryResults(t, "SELECT username, options, member_of from [SHOW USERS] ORDER BY username", [][]string{
9892+
{"admin", "{}", "{}"},
9893+
{"app", "{}", "{}"},
9894+
{"app_role", "{}", "{}"},
9895+
{"root", "{}", "{admin}"},
9896+
{"test", "{}", "{}"},
9897+
{"test_role", "{}", "{}"},
9898+
{"testuser", "{}", "{}"},
98999899
})
99009900
})
99019901
}

‎pkg/backup/testdata/backup-restore/system-users‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,14 @@ GRANT developer TO abbey;
1010
----
1111

1212
query-sql
13-
SHOW ROLES
13+
select username, options, member_of from [SHOW ROLES]
1414
----
15-
abbey {developer}
16-
admin {}
17-
developer CREATEDB, NOLOGIN {}
18-
root {admin}
19-
testuser NOLOGIN {}
20-
testuser2 CONTROLJOB, CREATEDB, NOLOGIN {}
15+
admin {} {}
16+
developer {CREATEDB,NOLOGIN} {}
17+
testuser {NOLOGIN} {}
18+
testuser2 {CONTROLJOB,CREATEDB,NOLOGIN} {}
19+
root {} {admin}
20+
abbey {} {developer}
2121

2222
query-sql
2323
SHOW GRANTS ON ROLE developer
@@ -38,14 +38,14 @@ RESTORE SYSTEM USERS FROM LATEST IN 'nodelocal://1/test/'
3838
----
3939

4040
query-sql cluster=s2
41-
SHOW ROLES
42-
----
43-
abbey {developer}
44-
admin {}
45-
developer CREATEDB, NOLOGIN {}
46-
root {admin}
47-
testuser NOLOGIN {}
48-
testuser2 CONTROLJOB, CREATEDB, NOLOGIN {}
41+
select username, options, member_of from [SHOW ROLES]
42+
----
43+
admin {} {}
44+
developer {CREATEDB,NOLOGIN} {}
45+
testuser {NOLOGIN} {}
46+
testuser2 {CONTROLJOB,CREATEDB,NOLOGIN} {}
47+
root {} {admin}
48+
abbey {} {developer}
4949

5050
query-sql cluster=s2
5151
SHOW GRANTS ON ROLE developer

‎pkg/ccl/testccl/authccl/testdata/ldap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ ok
435435
query_row
436436
SELECT options FROM [SHOW ROLES] AS r WHERE EXISTS (SELECT 1 FROM unnest(r.member_of) AS m(role_name) WHERE role_name = 'ldap-parent-synced')
437437
----
438-
PROVISIONSRC=ldap:localhost
438+
[PROVISIONSRC=ldap:localhost]
439439

440440
subtest end
441441

‎pkg/sql/delegate/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ go_library(
4343
importpath = "github.com/cockroachdb/cockroach/pkg/sql/delegate",
4444
visibility = ["//visibility:public"],
4545
deps = [
46+
"//pkg/clusterversion",
4647
"//pkg/jobs",
4748
"//pkg/jobs/jobspb",
4849
"//pkg/security/username",
@@ -56,6 +57,7 @@ go_library(
5657
"//pkg/sql/pgrepl/pgreplparser",
5758
"//pkg/sql/pgwire/pgcode",
5859
"//pkg/sql/pgwire/pgerror",
60+
"//pkg/sql/pgwire/pgnotice",
5961
"//pkg/sql/privilege",
6062
"//pkg/sql/roleoption",
6163
"//pkg/sql/sem/catconstants",

‎pkg/sql/delegate/show_roles.go‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
package delegate
77

88
import (
9+
"github.com/cockroachdb/cockroach/pkg/clusterversion"
10+
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
911
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
1012
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
1113
)
@@ -14,15 +16,24 @@ import (
1416
// Privileges: SELECT on system.users.
1517
func (d *delegator) delegateShowRoles() (tree.Statement, error) {
1618
sqltelemetry.IncrementShowCounter(sqltelemetry.Roles)
17-
return d.parse(`
19+
selectClause := `
1820
SELECT
1921
u.username,
20-
IFNULL(string_agg(o.option || COALESCE('=' || o.value, ''), ', ' ORDER BY o.option), '') AS options,
21-
ARRAY (SELECT role FROM system.role_members AS rm WHERE rm.member = u.username ORDER BY 1) AS member_of
22+
COALESCE(array_remove(array_agg(o.option || COALESCE('=' || o.value, '') ORDER BY o.option), NULL), ARRAY[]::STRING[]) AS options,
23+
ARRAY (SELECT role FROM system.role_members AS rm WHERE rm.member = u.username ORDER BY 1) AS member_of`
24+
selectLastLoginTime := `,
25+
u.estimated_last_login_time`
26+
endingClauses := `
2227
FROM
2328
system.users AS u LEFT JOIN system.role_options AS o ON u.username = o.username
2429
GROUP BY
2530
u.username
2631
ORDER BY 1;
27-
`)
32+
`
33+
if d.evalCtx.Settings.Version.IsActive(d.ctx, clusterversion.V25_3) {
34+
d.evalCtx.ClientNoticeSender.BufferClientNotice(d.ctx, pgnotice.Newf(
35+
"estimated_last_login_time is computed on a best effort basis; it is not guaranteed to capture every login event"))
36+
return d.parse(selectClause + selectLastLoginTime + endingClauses)
37+
}
38+
return d.parse(selectClause + endingClauses)
2839
}

‎pkg/sql/logictest/testdata/logic_test/drop_user‎

Lines changed: 58 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,52 @@
11
statement ok
22
CREATE USER user1
33

4-
query TTT colnames,rowsort
4+
skipif config local-mixed-25.2
5+
query TTTT colnames,rowsort
56
SHOW USERS
67
----
7-
username options member_of
8-
admin · {}
9-
root · {admin}
10-
testuser · {}
11-
user1 · {}
8+
username options member_of estimated_last_login_time
9+
admin {} {} NULL
10+
root {} {admin} NULL
11+
testuser {} {} NULL
12+
user1 {} {} NULL
1213

1314
statement ok
1415
DROP USER user1
1516

16-
query TTT colnames,rowsort
17+
skipif config local-mixed-25.2
18+
query TTTT colnames,rowsort
1719
SHOW USERS
1820
----
19-
username options member_of
20-
admin · {}
21-
root · {admin}
22-
testuser · {}
21+
username options member_of estimated_last_login_time
22+
admin {} {} NULL
23+
root {} {admin} NULL
24+
testuser {} {} NULL
2325

2426
statement ok
2527
CREATE USER user1
2628

27-
query TTT colnames,rowsort
29+
skipif config local-mixed-25.2
30+
query TTTT colnames,rowsort
2831
SHOW USERS
2932
----
30-
username options member_of
31-
admin · {}
32-
root · {admin}
33-
testuser · {}
34-
user1 · {}
33+
username options member_of estimated_last_login_time
34+
admin {} {} NULL
35+
root {} {admin} NULL
36+
testuser {} {} NULL
37+
user1 {} {} NULL
3538

3639
statement ok
3740
DROP USER USEr1
3841

39-
query TTT colnames,rowsort
42+
skipif config local-mixed-25.2
43+
query TTTT colnames,rowsort
4044
SHOW USERS
4145
----
42-
username options member_of
43-
admin · {}
44-
root · {admin}
45-
testuser · {}
46+
username options member_of estimated_last_login_time
47+
admin {} {} NULL
48+
root {} {admin} NULL
49+
testuser {} {} NULL
4650

4751
statement error user "user1" does not exist
4852
DROP USER user1
@@ -83,43 +87,46 @@ CREATE USER user3
8387
statement ok
8488
CREATE USER user4
8589

86-
query TTT colnames,rowsort
90+
skipif config local-mixed-25.2
91+
query TTTT colnames,rowsort
8792
SHOW USERS
8893
----
89-
username options member_of
90-
admin · {}
91-
root · {admin}
92-
testuser · {}
93-
user1 · {}
94-
user2 · {}
95-
user3 · {}
96-
user4 · {}
94+
username options member_of estimated_last_login_time
95+
admin {} {} NULL
96+
root {} {admin} NULL
97+
testuser {} {} NULL
98+
user1 {} {} NULL
99+
user2 {} {} NULL
100+
user3 {} {} NULL
101+
user4 {} {} NULL
97102

98103
statement ok
99104
DROP USER user1,user2
100105

101-
query TTT colnames,rowsort
106+
skipif config local-mixed-25.2
107+
query TTTT colnames,rowsort
102108
SHOW USERS
103109
----
104-
username options member_of
105-
admin · {}
106-
root · {admin}
107-
testuser · {}
108-
user3 · {}
109-
user4 · {}
110+
username options member_of estimated_last_login_time
111+
admin {} {} NULL
112+
root {} {admin} NULL
113+
testuser {} {} NULL
114+
user3 {} {} NULL
115+
user4 {} {} NULL
110116

111117
statement error user "user1" does not exist
112118
DROP USER user1,user3
113119

114-
query TTT colnames,rowsort
120+
skipif config local-mixed-25.2
121+
query TTTT colnames,rowsort
115122
SHOW USERS
116123
----
117-
username options member_of
118-
admin · {}
119-
root · {admin}
120-
testuser · {}
121-
user3 · {}
122-
user4 · {}
124+
username options member_of estimated_last_login_time
125+
admin {} {} NULL
126+
root {} {admin} NULL
127+
testuser {} {} NULL
128+
user3 {} {} NULL
129+
user4 {} {} NULL
123130

124131
statement ok
125132
CREATE USER user1
@@ -148,13 +155,14 @@ statement ok
148155
PREPARE du AS DROP USER user4;
149156
EXECUTE du
150157

151-
query TTT colnames,rowsort
158+
skipif config local-mixed-25.2
159+
query TTTT colnames,rowsort
152160
SHOW USERS
153161
----
154-
username options member_of
155-
admin · {}
156-
root · {admin}
157-
testuser · {}
162+
username options member_of estimated_last_login_time
163+
admin {} {} NULL
164+
root {} {admin} NULL
165+
testuser {} {} NULL
158166

159167
user testuser
160168

0 commit comments

Comments
 (0)