Skip to content

Commit

Permalink
sql: don't no-op SET SESSION AUTHORIZATION DEFAULT
Browse files Browse the repository at this point in the history
Release note (bug fix): Previously, SET SESSION AUTHORIZATION DEFAULT
would have no effect. Now, it causes the current role to be reset to the
original user who logged into the session.

Release justification: low risk bug fix to existing functionality
  • Loading branch information
rafiss committed Aug 20, 2022
1 parent 04c6a1a commit 42b67ff
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 4 deletions.
8 changes: 7 additions & 1 deletion pkg/sql/logictest/testdata/logic_test/set_role
Original file line number Diff line number Diff line change
Expand Up @@ -374,5 +374,11 @@ WHERE active_queries LIKE 'SELECT user_name%'
----
root

# Verify that SET SESSION AUTHORIZATION *does* reset the role.
statement ok
RESET ROLE
SET SESSION AUTHORIZATION DEFAULT

query TTTT
SELECT current_user(), current_user, session_user(), session_user
----
root root root root
2 changes: 2 additions & 0 deletions pkg/sql/sessiondatapb/local_only_session_data.proto
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ message LocalOnlySessionData {
// established the connection before SET ROLE was first performed.
// This is only populated when SET ROLE is used, otherwise the session_user
// is the same as the UserProto in SessionData.
// Postgres allows the SessionUser to be changed with SET SESSION AUTHORIZATION
// but CockroachDB doesn't allow that at the time of this writing.
string session_user_proto = 46 [(gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/security/username.SQLUsernameProto"];
// TxnRowsWrittenLog is the threshold for the number of rows written by a SQL
// transaction which - once exceeded - will trigger a logging event to SQL_PERF
Expand Down
24 changes: 21 additions & 3 deletions pkg/sql/set_session_authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,26 @@

package sql

import (
"context"

"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
)

func (p *planner) SetSessionAuthorizationDefault() (planNode, error) {
// This is currently a no-op - we don't support changing the session
// authorization, and the parser only accepts DEFAULT.
return newZeroNode(nil /* columns */), nil
return &setSessionAuthorizationDefaultNode{}, nil
}

type setSessionAuthorizationDefaultNode struct{}

func (n *setSessionAuthorizationDefaultNode) Next(_ runParams) (bool, error) { return false, nil }
func (n *setSessionAuthorizationDefaultNode) Values() tree.Datums { return nil }
func (n *setSessionAuthorizationDefaultNode) Close(_ context.Context) {}
func (n *setSessionAuthorizationDefaultNode) startExec(params runParams) error {
// This is currently the same as `SET ROLE = DEFAULT`, which means that it
// only changes the "current user." In Postgres, `SET SESSION AUTHORIZATION`
// also changes the "session user," but since the session user cannot be
// modified in CockroachDB (at the time of writing), we just need to change
// the current user here.
return params.p.setRole(params.ctx, false /* local */, params.p.SessionData().SessionUser())
}
1 change: 1 addition & 0 deletions pkg/sql/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ var planNodeNames = map[reflect.Type]string{
reflect.TypeOf(&sequenceSelectNode{}): "sequence select",
reflect.TypeOf(&serializeNode{}): "run",
reflect.TypeOf(&setClusterSettingNode{}): "set cluster setting",
reflect.TypeOf(&setSessionAuthorizationDefaultNode{}): "set session authorization",
reflect.TypeOf(&setVarNode{}): "set",
reflect.TypeOf(&setZoneConfigNode{}): "configure zone",
reflect.TypeOf(&showFingerprintsNode{}): "show fingerprints",
Expand Down

0 comments on commit 42b67ff

Please sign in to comment.