Skip to content

session, variable: add privilege check to set session_states #59601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 18, 2025
Merged

session, variable: add privilege check to set session_states #59601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions pkg/session/session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4395,6 +4395,7 @@ func (s *session) EncodeSessionStates(ctx context.Context,
if err := s.sessionVars.EncodeSessionStates(ctx, sessionStates); err != nil {
return err
}
sessionStates.ResourceGroupName = s.sessionVars.ResourceGroupName

hasRestrictVarPriv := false
checker := privilege.GetPrivilegeManager(s)
Expand Down Expand Up @@ -4474,6 +4475,25 @@ func (s *session) DecodeSessionStates(ctx context.Context,
}
}

// Put resource group privilege check from sessionVars to session to avoid circular dependency.
if sessionStates.ResourceGroupName != s.sessionVars.ResourceGroupName {
hasPriv := true
if vardef.EnableResourceControlStrictMode.Load() {
checker := privilege.GetPrivilegeManager(s)
if checker != nil {
hasRgAdminPriv := checker.RequestDynamicVerification(s.sessionVars.ActiveRoles, "RESOURCE_GROUP_ADMIN", false)
hasRgUserPriv := checker.RequestDynamicVerification(s.sessionVars.ActiveRoles, "RESOURCE_GROUP_USER", false)
hasPriv = hasRgAdminPriv || hasRgUserPriv
}
}
if hasPriv {
s.sessionVars.SetResourceGroupName(sessionStates.ResourceGroupName)
} else {
logutil.Logger(ctx).Warn("set session states error, no privilege to set resource group, skip changing resource group",
zap.String("source_resource_group", s.sessionVars.ResourceGroupName), zap.String("target_resource_group", sessionStates.ResourceGroupName))
}
}

// Decoding session vars / prepared statements may override stmt ctx, such as warnings,
// so we decode stmt ctx at last.
return s.sessionVars.DecodeSessionStates(ctx, sessionStates)
Expand Down
2 changes: 0 additions & 2 deletions pkg/sessionctx/variable/session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2826,7 +2826,6 @@ func (s *SessionVars) EncodeSessionStates(_ context.Context, sessionStates *sess
sessionStates.SequenceLatestValues = s.SequenceState.GetAllStates()
sessionStates.FoundInPlanCache = s.PrevFoundInPlanCache
sessionStates.FoundInBinding = s.PrevFoundInBinding
sessionStates.ResourceGroupName = s.ResourceGroupName
sessionStates.HypoIndexes = s.HypoIndexes
sessionStates.HypoTiFlashReplicas = s.HypoTiFlashReplicas

Expand Down Expand Up @@ -2862,7 +2861,6 @@ func (s *SessionVars) DecodeSessionStates(_ context.Context, sessionStates *sess
s.SequenceState.SetAllStates(sessionStates.SequenceLatestValues)
s.FoundInPlanCache = sessionStates.FoundInPlanCache
s.FoundInBinding = sessionStates.FoundInBinding
s.SetResourceGroupName(sessionStates.ResourceGroupName)
s.HypoIndexes = sessionStates.HypoIndexes
s.HypoTiFlashReplicas = sessionStates.HypoTiFlashReplicas

Expand Down
24 changes: 24 additions & 0 deletions tests/integrationtest/r/privilege/privileges.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,30 @@ id
REVOKE SELECT on test_rc.* FROM resource_group_admin;
REVOKE SELECT on test_rc.* FROM resource_group_user;
DROP DATABASE test_rc;
DROP USER resource_group_admin;
DROP USER resource_group_user;
DROP RESOURCE GROUP test;
CREATE USER resource_group_user;
CREATE USER no_resource_group;
CREATE RESOURCE GROUP test RU_PER_SEC = 666;
GRANT RESOURCE_GROUP_USER ON *.* TO resource_group_user;
SET SESSION_STATES '{"rs-group":"test"}';
SELECT CURRENT_RESOURCE_GROUP();
CURRENT_RESOURCE_GROUP()
default
SET SESSION_STATES '{"rs-group":"test"}';
SELECT CURRENT_RESOURCE_GROUP();
CURRENT_RESOURCE_GROUP()
test
set @@global.tidb_resource_control_strict_mode = 0;
SET SESSION_STATES '{"rs-group":"test"}';
SELECT CURRENT_RESOURCE_GROUP();
CURRENT_RESOURCE_GROUP()
test
set @@global.tidb_resource_control_strict_mode = default;
DROP RESOURCE GROUP test;
DROP USER resource_group_user;
DROP USER no_resource_group;
CREATE SCHEMA IF NOT EXISTS privilege__privileges;
USE privilege__privileges;
CREATE TABLE reftest (a int);
Expand Down
34 changes: 34 additions & 0 deletions tests/integrationtest/t/privilege/privileges.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,40 @@ connection default;
REVOKE SELECT on test_rc.* FROM resource_group_admin;
REVOKE SELECT on test_rc.* FROM resource_group_user;
DROP DATABASE test_rc;
DROP USER resource_group_admin;
DROP USER resource_group_user;
DROP RESOURCE GROUP test;

# TestSetSessionStatesPriv
CREATE USER resource_group_user;
CREATE USER no_resource_group;

connection default;
CREATE RESOURCE GROUP test RU_PER_SEC = 666;
GRANT RESOURCE_GROUP_USER ON *.* TO resource_group_user;

connect (no_resource_group,localhost,no_resource_group,,);
SET SESSION_STATES '{"rs-group":"test"}';
SELECT CURRENT_RESOURCE_GROUP();

connect (resource_group_user,localhost,resource_group_user,,);
SET SESSION_STATES '{"rs-group":"test"}';
SELECT CURRENT_RESOURCE_GROUP();

connection default;
set @@global.tidb_resource_control_strict_mode = 0;

connection no_resource_group;
SET SESSION_STATES '{"rs-group":"test"}';
SELECT CURRENT_RESOURCE_GROUP();

disconnect resource_group_user;
disconnect no_resource_group;
connection default;
set @@global.tidb_resource_control_strict_mode = default;
DROP RESOURCE GROUP test;
DROP USER resource_group_user;
DROP USER no_resource_group;

# TestGrantReferences
CREATE SCHEMA IF NOT EXISTS privilege__privileges;
Expand Down