Cluster Configuration #1430
Cluster Configuration #1430
Conversation
session is recorded.
fd478b1 to
432a7ad
Compare
lib/auth/permissions.go
Outdated
| @@ -27,8 +27,8 @@ import ( | |||
| ) | |||
|
|
|||
| // NewRoleAuthorizer authorizes everyone as predefined role | |||
| func NewRoleAuthorizer(r teleport.Role) (Authorizer, error) { | |||
| authContext, err := contextForBuiltinRole(r) | |||
| func NewRoleAuthorizer(recordAtProxy bool, r teleport.Role) (Authorizer, error) { | |||
There was a problem hiding this comment.
this is not scalable, I would assume that proxy will switch to different role name based on whether it is recording or not, why does authorizer has to know about this flag?
There was a problem hiding this comment.
Ideally the proxy would have it's own role, but to access the Auth Server to find out if it's recording or not, it first it has to register itself with the Auth Server with the role it will assume, which it can't do because it has not connected to the Auth Server yet.
So we have two choices, either we make the authorizer smarter (what I did in this PR) or we create another role that can just be used to query the Auth Server for services.ClusterConfig. With the second approach, we can't change roles on the fly.
I can change the behavior here so it takes a services.ClusterConfiguration that way it can change it's value on the fly.
There was a problem hiding this comment.
@klizhentas As per our discussion, I've backed out these changes. I'll address this in another PR since this one has gotten too big already: #1431
lib/auth/permissions.go
Outdated
| return nil, trace.Wrap(err) | ||
| } | ||
| var recordAtProxy bool | ||
| if clusterConfig.GetSessionRecording() == services.RecordAtProxy { |
There was a problem hiding this comment.
I would assume two different builtin proxy roles based on whether proxy is recording or not, rather than modifying the role on the fly.
lib/events/doc.go
Outdated
| package events | ||
|
|
||
| /* | ||
| Package events currently implements the audit log using a simple filesystem backend. |
There was a problem hiding this comment.
this line should go right above package entry to be viewable in godoc, to check simply do godoc and the name of the module
| if cfg.Auth.ClusterConfig.GetSessionRecording() == services.RecordOff { | ||
| recordSessions = false | ||
|
|
||
| warningMessage := "Warning: Teleport session recording have been turned off. " + |
There was a problem hiding this comment.
same message duplicated twice, make a constant?
There was a problem hiding this comment.
It's a different message. In the other message, the audit log and session log were disabled.
lib/services/clusterconfig.go
Outdated
| return &cc, nil | ||
| } | ||
|
|
||
| // ClusterConfigV2 implements the ClusterConfig interface. |
There was a problem hiding this comment.
I think we are on V3 as a baseline for resources
lib/services/clusterconfig.go
Outdated
| all := []string{string(RecordAtNode), string(RecordAtProxy), string(RecordOff)} | ||
| ok := utils.SliceContainsStr(all, string(c.Spec.SessionRecording)) | ||
| if !ok { | ||
| return trace.BadParameter(`session_recording must either be "node", "proxy", or "off".`) |
4ca6829 to
4765e32
Compare
Purpose
Create a
services.ClusterConfigwhich contains a field that controls where (and if) a session is recorded.Implementation
services.ClusterConfigresource which holds cluster level configuration. At the moment it only stores where a session should be recorded.events.discardSessionLogger.Related Issues
Fixes #1329
Fixes #1263