Skip to content

Commit

Permalink
Merge #55612
Browse files Browse the repository at this point in the history
55612: sql: write to the event log table when changing privileges r=knz,arulajmani a=neeral

Helps towards #13492.

Release note (admin ui change): Changing privileges (i.e. grants or
revokes) now causes an avent to be logged and displayed in the admin ui

cc: @jordanlewis could you please review this or suggest someone who would be suitable?

Co-authored-by: neeral <[email protected]>
  • Loading branch information
craig[bot] and neeral committed Oct 19, 2020
2 parents ef2481c + 93be162 commit 9481ff7
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 3 deletions.
7 changes: 7 additions & 0 deletions pkg/sql/event_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@ const (
// EventLogCreateStatistics is recorded when statistics are collected for a
// table.
EventLogCreateStatistics EventLogType = "create_statistics"

// EventLogGrantPrivilege is recorded when privileges are added to a user
// for a database object.
EventLogGrantPrivilege EventLogType = "grant_privilege"
// EventLogRevokePrivilege is recorded when privileges are removed from a
// user for a database object.
EventLogRevokePrivilege EventLogType = "revoke_privilege"
)

// EventLogSetClusterSettingDetail is the json details for a settings change.
Expand Down
32 changes: 29 additions & 3 deletions pkg/sql/grant_revoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package sql
import (
"context"
"fmt"
"strings"

"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
Expand Down Expand Up @@ -65,7 +66,8 @@ func (p *planner) Grant(ctx context.Context, n *tree.Grant) (planNode, error) {
changePrivilege: func(privDesc *descpb.PrivilegeDescriptor, grantee string) {
privDesc.Grant(grantee, n.Privileges)
},
grantOn: grantOn,
grantOn: grantOn,
eventLogType: EventLogGrantPrivilege,
}, nil
}

Expand Down Expand Up @@ -106,7 +108,8 @@ func (p *planner) Revoke(ctx context.Context, n *tree.Revoke) (planNode, error)
changePrivilege: func(privDesc *descpb.PrivilegeDescriptor, grantee string) {
privDesc.Revoke(grantee, n.Privileges, grantOn)
},
grantOn: grantOn,
grantOn: grantOn,
eventLogType: EventLogRevokePrivilege,
}, nil
}

Expand All @@ -116,6 +119,7 @@ type changePrivilegesNode struct {
desiredprivs privilege.List
changePrivilege func(*descpb.PrivilegeDescriptor, string)
grantOn privilege.ObjectType
eventLogType EventLogType
}

// ReadingOwnWrites implements the planNodeReadingOwnWrites interface.
Expand Down Expand Up @@ -239,7 +243,29 @@ func (n *changePrivilegesNode) startExec(params runParams) error {
}

// Now update the descriptors transactionally.
return p.txn.Run(ctx, b)
if err := p.txn.Run(ctx, b); err != nil {
return err
}

// Record this index alteration in the event log. This is an auditable log
// event and is recorded in the same transaction as the table descriptor
// update.
fmtCtx := tree.NewFmtCtx(tree.FmtSimple)
n.targets.Format(fmtCtx)
targets := fmtCtx.CloseAndGetString()
return MakeEventLogger(params.extendedEvalCtx.ExecCfg).InsertEventRecord(
params.ctx,
params.p.txn,
n.eventLogType,
0, /* no target */
int32(params.extendedEvalCtx.NodeID.SQLInstanceID()),
struct {
Target string
User string
Grantees string
Privileges string
}{targets, p.SessionData().User, strings.Join(n.grantees.ToStrings(), ","), n.desiredprivs.String()},
)
}

func (*changePrivilegesNode) Next(runParams) (bool, error) { return false, nil }
Expand Down
44 changes: 44 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/event_log
Original file line number Diff line number Diff line change
Expand Up @@ -490,3 +490,47 @@ SELECT "eventType", "reportingID", info::JSONB->>'ViewName'
----
create_view 1 test.public.v
drop_view 1 test.public.v


# Change privileges
##################

statement ok
CREATE TABLE a (id INT PRIMARY KEY)

statement ok
CREATE TABLE b (id INT PRIMARY KEY)

statement ok
CREATE USER u

statement ok
CREATE USER v

statement ok
GRANT INSERT ON TABLE a,b TO u

statement ok
REVOKE UPDATE ON TABLE a FROM u,v

query ITT
SELECT "reportingID", "info", "eventType"
FROM system.eventlog
WHERE "eventType" = 'grant_privilege'
OR "eventType" = 'revoke_privilege'
ORDER BY "eventType"
----
1 {"Target":"TABLE a, b","User":"root","Grantees":"u","Privileges":"INSERT"} grant_privilege
1 {"Target":"TABLE a","User":"root","Grantees":"u,v","Privileges":"UPDATE"} revoke_privilege

statement ok
DROP TABLE a

statement ok
DROP TABLE b

statement ok
DROP USER u

statement ok
DROP USER v
4 changes: 4 additions & 0 deletions pkg/ui/src/util/eventTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ export const SET_ZONE_CONFIG = "set_zone_config";
export const REMOVE_ZONE_CONFIG = "remove_zone_config";
// Recorded when statistics are collected for a table.
export const CREATE_STATISTICS = "create_statistics";
// Recorded when privileges are added to a user(s).
export const GRANT_PRIVILEGE = "grant_privilege";
// Recorded when privileges are removed from a user(s).
export const REVOKE_PRIVILEGE = "revoke_privilege";

// Node Event Types
export const nodeEvents = [NODE_JOIN, NODE_RESTART, NODE_DECOMMISSIONING, NODE_DECOMMISSIONED, NODE_RECOMMISSIONED];
Expand Down
6 changes: 6 additions & 0 deletions pkg/ui/src/util/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ export function getEventDescription(e: Event$Properties): string {
return `Zone Config Removed: User ${info.User} removed the zone config for ${info.Target}`;
case eventTypes.CREATE_STATISTICS:
return `Table statistics refreshed for ${info.TableName}`;
case eventTypes.GRANT_PRIVILEGE:
return `Privileges granted: User ${info.User} granted ${info.Privileges} to ${info.Grantees} on ${info.Target}`;
case eventTypes.REVOKE_PRIVILEGE:
return `Privileges revoked: User ${info.User} revoked ${info.Privileges} from ${info.Grantees} on ${info.Target}`;
default:
return `Unknown Event Type: ${e.event_type}, content: ${JSON.stringify(info, null, 2)}`;
}
Expand All @@ -105,6 +109,8 @@ export interface EventInfo {
Target?: string;
Config?: string;
Statement?: string;
Grantees?: string;
Privileges?: string;
// The following are three names for the same key (it was renamed twice).
// All ar included for backwards compatibility.
DroppedTables?: string[];
Expand Down

0 comments on commit 9481ff7

Please sign in to comment.