From c30c7b9957f953b70015c58603f9a817d0d539c2 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Tue, 8 Mar 2022 10:52:43 -0500 Subject: [PATCH] sql: make session_revival_token.enabled tenant-ro I was hoping to wait for the new cluster setting syntax to be completed, but since it's getting close to the branch cut time I'd rather merge this now so we don't forget at the last minute. Release justification: low risk change to new functionality. Release note: None --- pkg/ccl/testccl/sqlccl/session_revival_test.go | 4 +++- pkg/ccl/testccl/sqlccl/show_transfer_state_test.go | 14 ++++++++------ .../logictest/testdata/logic_test/builtin_function | 8 ++++++-- pkg/sql/session_revival_token.go | 2 +- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/pkg/ccl/testccl/sqlccl/session_revival_test.go b/pkg/ccl/testccl/sqlccl/session_revival_test.go index 83f9a36cab26..97861122eaaf 100644 --- a/pkg/ccl/testccl/sqlccl/session_revival_test.go +++ b/pkg/ccl/testccl/sqlccl/session_revival_test.go @@ -43,7 +43,9 @@ func TestAuthenticateWithSessionRevivalToken(t *testing.T) { _, err := tenantDB.Exec("CREATE USER testuser WITH PASSWORD 'hunter2'") require.NoError(t, err) - _, err = tenantDB.Exec("SET CLUSTER SETTING server.user_login.session_revival_token.enabled = true") + // TODO(rafi): use ALTER TENANT ALL when available. + _, err = mainDB.Exec(`INSERT INTO system.tenant_settings (tenant_id, name, value, value_type) VALUES + (0, 'server.user_login.session_revival_token.enabled', 'true', 'b')`) require.NoError(t, err) var token string diff --git a/pkg/ccl/testccl/sqlccl/show_transfer_state_test.go b/pkg/ccl/testccl/sqlccl/show_transfer_state_test.go index 815951ad99b8..7946c7e96d48 100644 --- a/pkg/ccl/testccl/sqlccl/show_transfer_state_test.go +++ b/pkg/ccl/testccl/sqlccl/show_transfer_state_test.go @@ -28,15 +28,17 @@ func TestShowTransferState(t *testing.T) { ctx := context.Background() params, _ := tests.CreateTestServerParams() - s, _, _ := serverutils.StartServer(t, params) + s, mainDB, _ := serverutils.StartServer(t, params) defer s.Stopper().Stop(ctx) - tenant, mainDB := serverutils.StartTenant(t, s, tests.CreateTestTenantParams(serverutils.TestTenantID())) + tenant, tenantDB := serverutils.StartTenant(t, s, tests.CreateTestTenantParams(serverutils.TestTenantID())) defer tenant.Stopper().Stop(ctx) - defer mainDB.Close() + defer tenantDB.Close() - _, err := mainDB.Exec("CREATE USER testuser WITH PASSWORD 'hunter2'") + _, err := tenantDB.Exec("CREATE USER testuser WITH PASSWORD 'hunter2'") require.NoError(t, err) - _, err = mainDB.Exec("SET CLUSTER SETTING server.user_login.session_revival_token.enabled = true") + // TODO(rafi): use ALTER TENANT ALL when available. + _, err = mainDB.Exec(`INSERT INTO system.tenant_settings (tenant_id, name, value, value_type) VALUES + (0, 'server.user_login.session_revival_token.enabled', 'true', 'b')`) require.NoError(t, err) t.Run("without_transfer_key", func(t *testing.T) { @@ -170,7 +172,7 @@ func TestShowTransferState(t *testing.T) { t.Run("root_user", func(t *testing.T) { var key string var errVal, sessionState, sessionRevivalToken gosql.NullString - err := mainDB.QueryRow(`SHOW TRANSFER STATE WITH 'bar'`).Scan(&errVal, &sessionState, &sessionRevivalToken, &key) + err := tenantDB.QueryRow(`SHOW TRANSFER STATE WITH 'bar'`).Scan(&errVal, &sessionState, &sessionRevivalToken, &key) require.NoError(t, err) require.True(t, errVal.Valid) diff --git a/pkg/sql/logictest/testdata/logic_test/builtin_function b/pkg/sql/logictest/testdata/logic_test/builtin_function index 7e371b699ed2..fcb9a068fa67 100644 --- a/pkg/sql/logictest/testdata/logic_test/builtin_function +++ b/pkg/sql/logictest/testdata/logic_test/builtin_function @@ -3136,8 +3136,10 @@ SELECT hmac('dog', 'key', 'made up alg') subtest session_revival_token +# TODO(rafi): use ALTER TENANT ALL when available. statement ok -SET CLUSTER SETTING server.user_login.session_revival_token.enabled = true; +INSERT INTO system.tenant_settings (tenant_id, name, value, value_type) VALUES + (0, 'server.user_login.session_revival_token.enabled', 'true', 'b'); CREATE USER parentuser; GRANT parentuser TO testuser @@ -3198,5 +3200,7 @@ Ed25519 testuser true true true true user root +# TODO(rafi): use ALTER TENANT ALL when available. statement ok -SET CLUSTER SETTING server.user_login.session_revival_token.enabled = false +INSERT INTO system.tenant_settings (tenant_id, name, value, value_type) VALUES + (0, 'server.user_login.session_revival_token.enabled', 'false', 'b') diff --git a/pkg/sql/session_revival_token.go b/pkg/sql/session_revival_token.go index 4a178368a615..ca640d08aa20 100644 --- a/pkg/sql/session_revival_token.go +++ b/pkg/sql/session_revival_token.go @@ -25,7 +25,7 @@ import ( // setting since this is only intended to be used by CockroachDB-serverless // at the time of this writing. var AllowSessionRevival = settings.RegisterBoolSetting( - settings.TenantWritable, + settings.TenantReadOnly, "server.user_login.session_revival_token.enabled", "if set, the cluster is able to create session revival tokens and use them "+ "to authenticate a new session",