Skip to content

Commit

Permalink
[v10] Fix resource access requests for apps (#14026)
Browse files Browse the repository at this point in the history
  • Loading branch information
nklaassen authored Jul 1, 2022
1 parent 1ffeb51 commit d84f7fb
Show file tree
Hide file tree
Showing 3 changed files with 160 additions and 139 deletions.
125 changes: 77 additions & 48 deletions lib/services/access_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ package services

import (
"context"
"fmt"
"sort"
"strings"
"time"

"github.com/google/go-cmp/cmp"

"github.com/gravitational/teleport/api/client/proto"
apidefaults "github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
apiutils "github.com/gravitational/teleport/api/utils"
Expand Down Expand Up @@ -1301,11 +1304,7 @@ func MarshalAccessRequest(accessRequest types.AccessRequest, opts ...MarshalOpti
// PruneResourceRequestGetter is the access interface necessary for PruneResourceRequestRoles.
type PruneResourceRequestGetter interface {
GetRole(ctx context.Context, name string) (types.Role, error)
GetNode(ctx context.Context, namespace string, name string) (types.Server, error)
GetKubeServices(ctx context.Context) ([]types.Server, error)
GetDatabase(ctx context.Context, name string) (types.Database, error)
GetApp(ctx context.Context, name string) (types.Application, error)
GetWindowsDesktops(ctx context.Context, filter types.WindowsDesktopFilter) ([]types.WindowsDesktop, error)
ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error)
}

// PruneResourceRequestRoles takes an access request and does one of two things:
Expand Down Expand Up @@ -1450,55 +1449,85 @@ func roleAllowsResource(
}

func getResources(ctx context.Context, getter PruneResourceRequestGetter, resourceIDs []types.ResourceID) ([]types.ResourceWithLabels, error) {
var resources []types.ResourceWithLabels
resourceNamesByKind := make(map[string][]string)
for _, resourceID := range resourceIDs {
switch resourceID.Kind {
case types.KindNode:
node, err := getter.GetNode(ctx, apidefaults.Namespace, resourceID.Name)
if err != nil {
return nil, trace.Wrap(err)
}
resources = append(resources, node)
case types.KindKubernetesCluster:
kubeServices, err := getter.GetKubeServices(ctx)
resourceNamesByKind[resourceID.Kind] = append(resourceNamesByKind[resourceID.Kind], resourceID.Name)
}
var resources []types.ResourceWithLabels
for kind, resourceNames := range resourceNamesByKind {
req := proto.ListResourcesRequest{
ResourceType: MapResourceKindToListResourcesType(kind),
PredicateExpression: anyNameMatcher(resourceNames),
Limit: int32(len(resourceNames)),
}
resp, err := getter.ListResources(ctx, req)
if err != nil {
return nil, trace.Wrap(err)
}
for _, result := range resp.Resources {
leafResources, err := MapListResourcesResultToLeafResource(result, kind)
if err != nil {
return nil, trace.Wrap(err)
}
for _, kubeService := range kubeServices {
for _, kubeCluster := range kubeService.GetKubernetesClusters() {
if kubeCluster.Name != resourceID.Name {
continue
}
kubeV3, err := types.NewKubernetesClusterV3FromLegacyCluster(kubeService.GetNamespace(), kubeCluster)
if err != nil {
return nil, trace.Wrap(err)
}
resources = append(resources, kubeV3)
resources = append(resources, leafResources...)
}
}
return resources, nil
}

// anyNameMatcher returns a PredicateExpression which matches any of a given list
// of names. Given names will be escaped and quoted when building the expression.
func anyNameMatcher(names []string) string {
matchers := make([]string, len(names))
for i := range names {
matchers[i] = fmt.Sprintf(`name == %q`, names[i])
}
return strings.Join(matchers, " || ")
}

// MapResourceKindToListResourcesType returns the value to use for ResourceType in a
// ListResourcesRequest based on the kind of resource you're searching for.
// Necessary because some resource kinds don't support ListResources directly,
// so you have to list the parent kind. Use MapListResourcesResultToLeafResource to map back
// to the given kind.
func MapResourceKindToListResourcesType(kind string) string {
switch kind {
case types.KindApp:
return types.KindAppServer
case types.KindDatabase:
return types.KindDatabaseServer
case types.KindKubernetesCluster:
return types.KindKubeService
default:
return kind
}
}

// MapListResourcesResultToLeafResource is the inverse of
// MapResourceKindToListResourcesType, after the ListResources call it maps the
// result back to the kind we really want. `hint` should be the name of the
// desired resource kind, used to disambiguate normal SSH nodes and kubernetes
// services which are both returned as `types.Server`.
func MapListResourcesResultToLeafResource(resource types.ResourceWithLabels, hint string) (types.ResourcesWithLabels, error) {
switch r := resource.(type) {
case types.AppServer:
return types.ResourcesWithLabels{r.GetApp()}, nil
case types.DatabaseServer:
return types.ResourcesWithLabels{r.GetDatabase()}, nil
case types.Server:
if hint == types.KindKubernetesCluster {
kubeClusters := r.GetKubernetesClusters()
resources := make(types.ResourcesWithLabels, len(kubeClusters))
for i := range kubeClusters {
resource, err := types.NewKubernetesClusterV3FromLegacyCluster(apidefaults.Namespace, kubeClusters[i])
if err != nil {
return nil, trace.Wrap(err)
}
resources[i] = resource
}
case types.KindDatabase:
db, err := getter.GetDatabase(ctx, resourceID.Name)
if err != nil {
return nil, trace.Wrap(err)
}
resources = append(resources, db)
case types.KindApp:
app, err := getter.GetApp(ctx, resourceID.Name)
if err != nil {
return nil, trace.Wrap(err)
}
resources = append(resources, app)
case types.KindWindowsDesktop:
desktops, err := getter.GetWindowsDesktops(ctx, types.WindowsDesktopFilter{
Name: resourceID.Name,
})
if err != nil {
return nil, trace.Wrap(err)
}
for _, desktop := range desktops {
resources = append(resources, desktop)
}
return resources, nil
}
default:
}
return resources, nil
return types.ResourcesWithLabels{resource}, nil
}
126 changes: 80 additions & 46 deletions lib/services/access_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ package services

import (
"context"
"strings"
"testing"

"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/fixtures"

Expand All @@ -31,13 +33,13 @@ import (

// mockGetter mocks the UserAndRoleGetter interface.
type mockGetter struct {
users map[string]types.User
roles map[string]types.Role
nodes map[string]types.Server
kubeServices []types.Server
dbs map[string]types.Database
apps map[string]types.Application
desktops map[string]types.WindowsDesktop
users map[string]types.User
roles map[string]types.Role
nodes map[string]types.Server
kubeServers map[string]types.Server
dbServers map[string]types.DatabaseServer
appServers map[string]types.AppServer
desktops map[string]types.WindowsDesktop
}

// user inserts a new user with the specified roles and returns the username.
Expand Down Expand Up @@ -78,40 +80,37 @@ func (m *mockGetter) GetRoles(ctx context.Context) ([]types.Role, error) {
return roles, nil
}

func (m *mockGetter) GetNode(ctx context.Context, namespace string, name string) (types.Server, error) {
node, ok := m.nodes[name]
if !ok {
return nil, trace.NotFound("no such node: %q", name)
// ListResources is a very dumb implementation for the mockGetter that just
// returns all resources which have names matching the request
// PredicateExpression.
func (m *mockGetter) ListResources(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error) {
resp := &types.ListResourcesResponse{}
for nodeName, node := range m.nodes {
if strings.Contains(req.PredicateExpression, nodeName) {
resp.Resources = append(resp.Resources, types.ResourceWithLabels(node))
}
}
return node, nil
}

func (m *mockGetter) GetKubeServices(ctx context.Context) ([]types.Server, error) {
return append([]types.Server{}, m.kubeServices...), nil
}

func (m *mockGetter) GetDatabase(ctx context.Context, name string) (types.Database, error) {
db, ok := m.dbs[name]
if !ok {
return nil, trace.NotFound("no such db: %q", name)
for kubeName, kubeService := range m.kubeServers {
if strings.Contains(req.PredicateExpression, kubeName) {
resp.Resources = append(resp.Resources, types.ResourceWithLabels(kubeService))
}
}
return db, nil
}

func (m *mockGetter) GetApp(ctx context.Context, name string) (types.Application, error) {
app, ok := m.apps[name]
if !ok {
return nil, trace.NotFound("no such app: %q", name)
for dbName, dbServer := range m.dbServers {
if strings.Contains(req.PredicateExpression, dbName) {
resp.Resources = append(resp.Resources, dbServer)
}
}
return app, nil
}

func (m *mockGetter) GetWindowsDesktops(ctx context.Context, filter types.WindowsDesktopFilter) ([]types.WindowsDesktop, error) {
desktop, ok := m.desktops[filter.Name]
if !ok {
return nil, trace.NotFound("no such desktop: %q", filter.Name)
for appName, appServer := range m.appServers {
if strings.Contains(req.PredicateExpression, appName) {
resp.Resources = append(resp.Resources, appServer)
}
}
return []types.WindowsDesktop{desktop}, nil
for desktopName, desktop := range m.desktops {
if strings.Contains(req.PredicateExpression, desktopName) {
resp.Resources = append(resp.Resources, desktop)
}
}
return resp, nil
}

// TestReviewThresholds tests various review threshold scenarios
Expand Down Expand Up @@ -1044,12 +1043,13 @@ func TestPruneRequestRoles(t *testing.T) {
ctx := context.Background()

g := &mockGetter{
roles: make(map[string]types.Role),
users: make(map[string]types.User),
nodes: make(map[string]types.Server),
dbs: make(map[string]types.Database),
apps: make(map[string]types.Application),
desktops: make(map[string]types.WindowsDesktop),
roles: make(map[string]types.Role),
users: make(map[string]types.User),
nodes: make(map[string]types.Server),
kubeServers: make(map[string]types.Server),
dbServers: make(map[string]types.DatabaseServer),
appServers: make(map[string]types.AppServer),
desktops: make(map[string]types.WindowsDesktop),
}

// set up test roles
Expand Down Expand Up @@ -1142,6 +1142,12 @@ func TestPruneRequestRoles(t *testing.T) {
"owner": "node-admins",
},
},
{
name: "admins-node-2",
labels: map[string]string{
"owner": "node-admins",
},
},
{
name: "denied-node",
},
Expand All @@ -1161,7 +1167,7 @@ func TestPruneRequestRoles(t *testing.T) {
},
}, nil)
require.NoError(t, err)
g.kubeServices = append(g.kubeServices, kube)
g.kubeServers[kube.GetName()] = kube

db, err := types.NewDatabaseV3(types.Metadata{
Name: "db",
Expand All @@ -1170,15 +1176,25 @@ func TestPruneRequestRoles(t *testing.T) {
URI: "example.com:3000",
})
require.NoError(t, err)
g.dbs[db.GetName()] = db
dbServer, err := types.NewDatabaseServerV3(types.Metadata{
Name: db.GetName(),
}, types.DatabaseServerSpecV3{
HostID: "db-server",
Hostname: "db-server",
Database: db,
})
require.NoError(t, err)
g.dbServers[dbServer.GetName()] = dbServer

app, err := types.NewAppV3(types.Metadata{
Name: "app",
}, types.AppSpecV3{
URI: "example.com:3000",
})
require.NoError(t, err)
g.apps[app.GetName()] = app
appServer, err := types.NewAppServerV3FromApp(app, "app-server", "app-server")
require.NoError(t, err)
g.appServers[app.GetName()] = appServer

desktop, err := types.NewWindowsDesktopV3("windows", nil, types.WindowsDesktopSpecV3{
Addr: "example.com:3001",
Expand Down Expand Up @@ -1222,6 +1238,24 @@ func TestPruneRequestRoles(t *testing.T) {
// With "responder" login hint, only request node-access.
expectRoles: []string{"node-access"},
},
{
desc: "multiple nodes",
requestResourceIDs: []types.ResourceID{
{
ClusterName: clusterName,
Kind: types.KindNode,
Name: "admins-node",
},
{
ClusterName: clusterName,
Kind: types.KindNode,
Name: "admins-node-2",
},
},
loginHint: "responder",
// With "responder" login hint, only request node-access.
expectRoles: []string{"node-access"},
},
{
desc: "root login hint",
requestResourceIDs: []types.ResourceID{
Expand Down
Loading

0 comments on commit d84f7fb

Please sign in to comment.