Skip to content

Commit b7e1c20

Browse files
authored
session, variable: add privilege check to set session_states (#59601)
close #59600
1 parent 9f5f53a commit b7e1c20

File tree

4 files changed

+78
-2
lines changed

4 files changed

+78
-2
lines changed

pkg/session/session.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4395,6 +4395,7 @@ func (s *session) EncodeSessionStates(ctx context.Context,
43954395
if err := s.sessionVars.EncodeSessionStates(ctx, sessionStates); err != nil {
43964396
return err
43974397
}
4398+
sessionStates.ResourceGroupName = s.sessionVars.ResourceGroupName
43984399

43994400
hasRestrictVarPriv := false
44004401
checker := privilege.GetPrivilegeManager(s)
@@ -4474,6 +4475,25 @@ func (s *session) DecodeSessionStates(ctx context.Context,
44744475
}
44754476
}
44764477

4478+
// Put resource group privilege check from sessionVars to session to avoid circular dependency.
4479+
if sessionStates.ResourceGroupName != s.sessionVars.ResourceGroupName {
4480+
hasPriv := true
4481+
if vardef.EnableResourceControlStrictMode.Load() {
4482+
checker := privilege.GetPrivilegeManager(s)
4483+
if checker != nil {
4484+
hasRgAdminPriv := checker.RequestDynamicVerification(s.sessionVars.ActiveRoles, "RESOURCE_GROUP_ADMIN", false)
4485+
hasRgUserPriv := checker.RequestDynamicVerification(s.sessionVars.ActiveRoles, "RESOURCE_GROUP_USER", false)
4486+
hasPriv = hasRgAdminPriv || hasRgUserPriv
4487+
}
4488+
}
4489+
if hasPriv {
4490+
s.sessionVars.SetResourceGroupName(sessionStates.ResourceGroupName)
4491+
} else {
4492+
logutil.Logger(ctx).Warn("set session states error, no privilege to set resource group, skip changing resource group",
4493+
zap.String("source_resource_group", s.sessionVars.ResourceGroupName), zap.String("target_resource_group", sessionStates.ResourceGroupName))
4494+
}
4495+
}
4496+
44774497
// Decoding session vars / prepared statements may override stmt ctx, such as warnings,
44784498
// so we decode stmt ctx at last.
44794499
return s.sessionVars.DecodeSessionStates(ctx, sessionStates)

pkg/sessionctx/variable/session.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2826,7 +2826,6 @@ func (s *SessionVars) EncodeSessionStates(_ context.Context, sessionStates *sess
28262826
sessionStates.SequenceLatestValues = s.SequenceState.GetAllStates()
28272827
sessionStates.FoundInPlanCache = s.PrevFoundInPlanCache
28282828
sessionStates.FoundInBinding = s.PrevFoundInBinding
2829-
sessionStates.ResourceGroupName = s.ResourceGroupName
28302829
sessionStates.HypoIndexes = s.HypoIndexes
28312830
sessionStates.HypoTiFlashReplicas = s.HypoTiFlashReplicas
28322831

@@ -2862,7 +2861,6 @@ func (s *SessionVars) DecodeSessionStates(_ context.Context, sessionStates *sess
28622861
s.SequenceState.SetAllStates(sessionStates.SequenceLatestValues)
28632862
s.FoundInPlanCache = sessionStates.FoundInPlanCache
28642863
s.FoundInBinding = sessionStates.FoundInBinding
2865-
s.SetResourceGroupName(sessionStates.ResourceGroupName)
28662864
s.HypoIndexes = sessionStates.HypoIndexes
28672865
s.HypoTiFlashReplicas = sessionStates.HypoTiFlashReplicas
28682866

tests/integrationtest/r/privilege/privileges.result

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,30 @@ id
101101
REVOKE SELECT on test_rc.* FROM resource_group_admin;
102102
REVOKE SELECT on test_rc.* FROM resource_group_user;
103103
DROP DATABASE test_rc;
104+
DROP USER resource_group_admin;
105+
DROP USER resource_group_user;
106+
DROP RESOURCE GROUP test;
107+
CREATE USER resource_group_user;
108+
CREATE USER no_resource_group;
109+
CREATE RESOURCE GROUP test RU_PER_SEC = 666;
110+
GRANT RESOURCE_GROUP_USER ON *.* TO resource_group_user;
111+
SET SESSION_STATES '{"rs-group":"test"}';
112+
SELECT CURRENT_RESOURCE_GROUP();
113+
CURRENT_RESOURCE_GROUP()
114+
default
115+
SET SESSION_STATES '{"rs-group":"test"}';
116+
SELECT CURRENT_RESOURCE_GROUP();
117+
CURRENT_RESOURCE_GROUP()
118+
test
119+
set @@global.tidb_resource_control_strict_mode = 0;
120+
SET SESSION_STATES '{"rs-group":"test"}';
121+
SELECT CURRENT_RESOURCE_GROUP();
122+
CURRENT_RESOURCE_GROUP()
123+
test
124+
set @@global.tidb_resource_control_strict_mode = default;
125+
DROP RESOURCE GROUP test;
126+
DROP USER resource_group_user;
127+
DROP USER no_resource_group;
104128
CREATE SCHEMA IF NOT EXISTS privilege__privileges;
105129
USE privilege__privileges;
106130
CREATE TABLE reftest (a int);

tests/integrationtest/t/privilege/privileges.test

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,40 @@ connection default;
139139
REVOKE SELECT on test_rc.* FROM resource_group_admin;
140140
REVOKE SELECT on test_rc.* FROM resource_group_user;
141141
DROP DATABASE test_rc;
142+
DROP USER resource_group_admin;
143+
DROP USER resource_group_user;
144+
DROP RESOURCE GROUP test;
145+
146+
# TestSetSessionStatesPriv
147+
CREATE USER resource_group_user;
148+
CREATE USER no_resource_group;
149+
150+
connection default;
151+
CREATE RESOURCE GROUP test RU_PER_SEC = 666;
152+
GRANT RESOURCE_GROUP_USER ON *.* TO resource_group_user;
153+
154+
connect (no_resource_group,localhost,no_resource_group,,);
155+
SET SESSION_STATES '{"rs-group":"test"}';
156+
SELECT CURRENT_RESOURCE_GROUP();
157+
158+
connect (resource_group_user,localhost,resource_group_user,,);
159+
SET SESSION_STATES '{"rs-group":"test"}';
160+
SELECT CURRENT_RESOURCE_GROUP();
161+
162+
connection default;
163+
set @@global.tidb_resource_control_strict_mode = 0;
164+
165+
connection no_resource_group;
166+
SET SESSION_STATES '{"rs-group":"test"}';
167+
SELECT CURRENT_RESOURCE_GROUP();
168+
169+
disconnect resource_group_user;
170+
disconnect no_resource_group;
171+
connection default;
172+
set @@global.tidb_resource_control_strict_mode = default;
173+
DROP RESOURCE GROUP test;
174+
DROP USER resource_group_user;
175+
DROP USER no_resource_group;
142176

143177
# TestGrantReferences
144178
CREATE SCHEMA IF NOT EXISTS privilege__privileges;

0 commit comments

Comments
 (0)