Make tsh db ls lists available db users.
#10458
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -862,6 +862,110 @@ func NewRoleSet(roles ...types.Role) RoleSet { | |||||||
| // RoleSet is a set of roles that implements access control functionality | ||||||||
| type RoleSet []types.Role | ||||||||
|
|
||||||||
| // EnumerationResult is a result of enumerating a role set against some property, e.g. allowed names or logins. | ||||||||
| type EnumerationResult struct { | ||||||||
| allowedDeniedMap map[string]bool | ||||||||
|
There was a problem hiding this comment. nit: splitting in two separate list instead of aggregating a user into single map should simplify the implemeantion:
Suggested change
For example the There was a problem hiding this comment. We need a map to deal with duplicates, so either we do it in one go or we need to deduplicate afterward, which makes the code more complex, not less. There was a problem hiding this comment. The login duplication are already removed in the users = apiutils.Deduplicate(append(users,extraUsers) call There was a problem hiding this comment.
This is done outside of I can switch to two slices, but that makes it possible to have an internal representation which is inconsistent. The easiest way to efficiently remove duplicates is with map. There was a problem hiding this comment. OK. This was just a suggestion. Let's leave the current form. |
||||||||
| wildcardAllowed bool | ||||||||
| wildcardDenied bool | ||||||||
| } | ||||||||
|
|
||||||||
| func (result *EnumerationResult) filtered(value bool) []string { | ||||||||
| var filtered []string | ||||||||
|
|
||||||||
| for entity, allow := range result.allowedDeniedMap { | ||||||||
| if allow == value { | ||||||||
| filtered = append(filtered, entity) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| sort.Strings(filtered) | ||||||||
|
|
||||||||
| return filtered | ||||||||
| } | ||||||||
|
|
||||||||
| // Denied returns all explicitly denied users. | ||||||||
| func (result *EnumerationResult) Denied() []string { | ||||||||
| return result.filtered(false) | ||||||||
| } | ||||||||
|
|
||||||||
| // Allowed returns all known allowed users. | ||||||||
| func (result *EnumerationResult) Allowed() []string { | ||||||||
| if result.WildcardDenied() { | ||||||||
| return nil | ||||||||
| } | ||||||||
| return result.filtered(true) | ||||||||
| } | ||||||||
|
|
||||||||
| // WildcardAllowed is true if there * username allowed for given rule set. | ||||||||
| func (result *EnumerationResult) WildcardAllowed() bool { | ||||||||
| return result.wildcardAllowed && !result.wildcardDenied | ||||||||
| } | ||||||||
|
|
||||||||
| // WildcardDenied is true if there * username deny for given rule set. | ||||||||
| func (result *EnumerationResult) WildcardDenied() bool { | ||||||||
| return result.wildcardDenied | ||||||||
| } | ||||||||
|
|
||||||||
| // NewEnumerationResult returns new EnumerationResult. | ||||||||
| func NewEnumerationResult() EnumerationResult { | ||||||||
| return EnumerationResult{ | ||||||||
| allowedDeniedMap: map[string]bool{}, | ||||||||
| wildcardAllowed: false, | ||||||||
| wildcardDenied: false, | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| // EnumerateDatabaseUsers works on a given role set to return a minimal description of allowed set of usernames. | ||||||||
| // It is biased towards *allowed* usernames; It is meant to describe what the user can do, rather than cannot do. | ||||||||
| // For that reason if the user isn't allowed to pick *any* entities, the output will be empty. | ||||||||
| // | ||||||||
| // In cases where * is listed in set of allowed users, it may be hard for users to figure out the expected username. | ||||||||
| // For this reason the parameter extraUsers provides an extra set of users to be checked against RoleSet. | ||||||||
| // This extra set of users may be sourced e.g. from user connection history. | ||||||||
|
Comment on lines
+922
to
+924
There was a problem hiding this comment. Was this added with Teleterm in mind? I hope we actually get to use this. 🙊 But I'll think about how to utilize this once I actually get around to incorporating that in Teleterm. There was a problem hiding this comment. Yep, Teleterm was the planned user for this parameter. I guessed that it has a mechanism for storing recently used resources etc. Alternatively, this may be a feature of the database itself: on successful auth, store the username which can be queried as a property of the database. This has the benefit of sharing the available usernames across all people who can access that particular database in Teleport. The second mechanism is perhaps more robust and can be shared with |
||||||||
| func (set RoleSet) EnumerateDatabaseUsers(database types.Database, extraUsers ...string) EnumerationResult { | ||||||||
| result := NewEnumerationResult() | ||||||||
|
|
||||||||
| // gather users for checking from the roles, check wildcards. | ||||||||
| var users []string | ||||||||
| for _, role := range set { | ||||||||
| wildcardAllowed := false | ||||||||
| wildcardDenied := false | ||||||||
|
|
||||||||
|
Comment on lines
+931
to
+933
There was a problem hiding this comment. It can be moved outside the for loop. There was a problem hiding this comment. On the contrary: it must be inside the loop, since these two bools refer to particular role. |
||||||||
| for _, user := range role.GetDatabaseUsers(types.Allow) { | ||||||||
| if user == types.Wildcard { | ||||||||
| wildcardAllowed = true | ||||||||
| } else { | ||||||||
| users = append(users, user) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| for _, user := range role.GetDatabaseUsers(types.Deny) { | ||||||||
| if user == types.Wildcard { | ||||||||
| wildcardDenied = true | ||||||||
| } else { | ||||||||
| users = append(users, user) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| result.wildcardDenied = result.wildcardDenied || wildcardDenied | ||||||||
|
|
||||||||
| if err := NewRoleSet(role).CheckAccess(database, AccessMFAParams{Verified: true}); err == nil { | ||||||||
| result.wildcardAllowed = result.wildcardAllowed || wildcardAllowed | ||||||||
| } | ||||||||
|
|
||||||||
| } | ||||||||
|
|
||||||||
| users = apiutils.Deduplicate(append(users, extraUsers...)) | ||||||||
|
|
||||||||
| // check each individual user against the database. | ||||||||
| for _, user := range users { | ||||||||
| err := set.CheckAccess(database, AccessMFAParams{Verified: true}, &DatabaseUserMatcher{User: user}) | ||||||||
| result.allowedDeniedMap[user] = err == nil | ||||||||
| } | ||||||||
|
|
||||||||
| return result | ||||||||
| } | ||||||||
|
|
||||||||
| // MatchNamespace returns true if given list of namespace matches | ||||||||
| // target namespace, wildcard matches everything. | ||||||||
| func MatchNamespace(selectors []string, namespace string) (bool, string) { | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ import ( | |
| dbprofile "github.com/gravitational/teleport/lib/client/db" | ||
| "github.com/gravitational/teleport/lib/client/db/dbcmd" | ||
| "github.com/gravitational/teleport/lib/defaults" | ||
| "github.com/gravitational/teleport/lib/services" | ||
| "github.com/gravitational/teleport/lib/tlsca" | ||
| "github.com/gravitational/teleport/lib/utils" | ||
|
|
||
|
|
@@ -49,11 +50,29 @@ func onListDatabases(cf *CLIConf) error { | |
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
|
|
||
| proxy, err := tc.ConnectToProxy(cf.Context) | ||
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
|
Comment on lines
+54
to
+57
There was a problem hiding this comment. I'm fixing a bug in Connect where we prematurely close the proxy client when fetching db users. I was looking at how tsh does things (since it was working fine in Shouldn't we have There was a problem hiding this comment. The fix in Connect I was working on: #14230. There was a problem hiding this comment. We probably should. There are, in fact, a few other places in codebase that are missing the deferred |
||
|
|
||
| cluster, err := proxy.ConnectToCurrentCluster(cf.Context, false) | ||
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
| defer cluster.Close() | ||
|
|
||
| // Retrieve profile to be able to show which databases user is logged into. | ||
| profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy) | ||
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
|
|
||
| roleSet, err := services.FetchRoles(profile.Roles, cluster, profile.Traits) | ||
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
|
|
||
| sort.Slice(databases, func(i, j int) bool { | ||
| return databases[i].GetName() < databases[j].GetName() | ||
| }) | ||
|
|
@@ -62,7 +81,8 @@ func onListDatabases(cf *CLIConf) error { | |
| if err != nil { | ||
| return trace.Wrap(err) | ||
| } | ||
|
|
||
| showDatabases(cf.SiteName, databases, activeDatabases, roleSet, cf.Verbose) | ||
| return nil | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
EnumerationResultis quite generic name for therolepackage. For exampleroles.EnumerationResultsuggest that roles are enumerated.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any suggestions?
RoleSetEnumerationResultis bit too long for my tastes.