Skip to content

Commit

Permalink
Respect windows_desktop_labels for dynamic desktops (#48941)
Browse files Browse the repository at this point in the history 
* Respect windows_desktop_labels for dynamic desktops

* fix imports

* Fix test

* add access check test
  • Loading branch information
@probakowski
probakowski authored Nov 15, 2024
1 parent 7b520cc commit 1ebf8a3
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 7 deletions.
47 changes: 45 additions & 2 deletions lib/auth/dynamicwindows/dynamicwindowsv1/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
dynamicwindowspb "github.com/gravitational/teleport/api/gen/proto/go/teleport/dynamicwindows/v1"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/authz"
"github.com/gravitational/teleport/lib/services"
)

// Service implements the teleport.trust.v1.TrustService RPC service.
Expand Down Expand Up @@ -108,6 +109,9 @@ func (s *Service) GetDynamicWindowsDesktop(ctx context.Context, request *dynamic
if err != nil {
return nil, trace.Wrap(err)
}
if err := checkAccess(auth, d); err != nil {
return nil, trace.Wrap(err)
}

desktop, ok := d.(*types.DynamicWindowsDesktopV1)
if !ok {
Expand Down Expand Up @@ -136,6 +140,9 @@ func (s *Service) ListDynamicWindowsDesktops(ctx context.Context, request *dynam
NextPageToken: next,
}
for _, d := range desktops {
if err := checkAccess(auth, d); err != nil {
continue
}
desktop, ok := d.(*types.DynamicWindowsDesktopV1)
if !ok {
return nil, trace.BadParameter("unexpected type %T", d)
Expand All @@ -158,6 +165,9 @@ func (s *Service) CreateDynamicWindowsDesktop(ctx context.Context, req *dynamicw
if err := auth.CheckAccessToKind(types.KindDynamicWindowsDesktop, types.VerbCreate); err != nil {
return nil, trace.Wrap(err)
}
if err := checkAccess(auth, req.GetDesktop()); err != nil {
return nil, trace.Wrap(err)
}
d, err := s.backend.CreateDynamicWindowsDesktop(ctx, types.DynamicWindowsDesktop(req.Desktop))
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -171,6 +181,10 @@ func (s *Service) CreateDynamicWindowsDesktop(ctx context.Context, req *dynamicw
return createdDesktop, nil
}

func checkAccess(auth *authz.Context, desktop types.DynamicWindowsDesktop) error {
return auth.Checker.CheckAccess(desktop, services.AccessState{MFAVerified: true})
}

// UpdateDynamicWindowsDesktop updates an existing dynamic Windows desktop.
func (s *Service) UpdateDynamicWindowsDesktop(ctx context.Context, req *dynamicwindowspb.UpdateDynamicWindowsDesktopRequest) (*types.DynamicWindowsDesktopV1, error) {
auth, err := s.authorizer.Authorize(ctx)
Expand All @@ -183,7 +197,17 @@ func (s *Service) UpdateDynamicWindowsDesktop(ctx context.Context, req *dynamicw
if err := auth.CheckAccessToKind(types.KindDynamicWindowsDesktop, types.VerbUpdate); err != nil {
return nil, trace.Wrap(err)
}
d, err := s.backend.UpdateDynamicWindowsDesktop(ctx, req.Desktop)
d, err := s.cache.GetDynamicWindowsDesktop(ctx, req.GetDesktop().GetName())
if err != nil {
return nil, trace.Wrap(err)
}
if err := checkAccess(auth, d); err != nil {
return nil, trace.Wrap(err)
}
if err := checkAccess(auth, req.GetDesktop()); err != nil {
return nil, trace.Wrap(err)
}
d, err = s.backend.UpdateDynamicWindowsDesktop(ctx, req.Desktop)
if err != nil {
return nil, trace.Wrap(err)
}
Expand All @@ -208,7 +232,19 @@ func (s *Service) UpsertDynamicWindowsDesktop(ctx context.Context, req *dynamicw
if err := auth.CheckAccessToKind(types.KindDynamicWindowsDesktop, types.VerbCreate, types.VerbUpdate); err != nil {
return nil, trace.Wrap(err)
}
d, err := s.backend.UpsertDynamicWindowsDesktop(ctx, req.Desktop)
d, err := s.cache.GetDynamicWindowsDesktop(ctx, req.GetDesktop().GetName())
if !trace.IsNotFound(err) {
if err != nil {
return nil, trace.Wrap(err)
}
if err := checkAccess(auth, d); err != nil {
return nil, trace.Wrap(err)
}
}
if err := checkAccess(auth, req.GetDesktop()); err != nil {
return nil, trace.Wrap(err)
}
d, err = s.backend.UpsertDynamicWindowsDesktop(ctx, req.Desktop)
if err != nil {
return nil, trace.Wrap(err)
}
Expand All @@ -233,6 +269,13 @@ func (s *Service) DeleteDynamicWindowsDesktop(ctx context.Context, req *dynamicw
if err := auth.CheckAccessToKind(types.KindDynamicWindowsDesktop, types.VerbDelete); err != nil {
return nil, trace.Wrap(err)
}
d, err := s.cache.GetDynamicWindowsDesktop(ctx, req.GetName())
if err != nil {
return nil, trace.Wrap(err)
}
if err := checkAccess(auth, d); err != nil {
return nil, trace.Wrap(err)
}
if err := s.backend.DeleteDynamicWindowsDesktop(ctx, req.GetName()); err != nil {
return nil, trace.Wrap(err)
}
Expand Down
58 changes: 53 additions & 5 deletions lib/auth/dynamicwindows/dynamicwindowsv1/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,43 @@ import (
"github.com/gravitational/teleport/lib/utils"
)

func TestFailedAccessCheck(t *testing.T) {
t.Parallel()
checker := fakeChecker{
allowedVerbs: []string{types.VerbRead, types.VerbList, types.VerbCreate, types.VerbUpdate},
}
s := newService(t, authz.AdminActionAuthMFAVerified, &checker)
desktop, err := types.NewDynamicWindowsDesktopV1("test2", nil, types.DynamicWindowsDesktopSpecV1{Addr: "addr"})
require.NoError(t, err)
req := dynamicwindowsv1.CreateDynamicWindowsDesktopRequest{
Desktop: desktop,
}
_, err = s.CreateDynamicWindowsDesktop(context.Background(), &req)
require.NoError(t, err)
checker.failAccess = true
testCases := []string{
"CreateDynamicWindowsDesktop",
"UpdateDynamicWindowsDesktop",
"UpsertDynamicWindowsDesktop",
"DeleteDynamicWindowsDesktop",
"GetDynamicWindowsDesktop",
}
for _, tt := range testCases {
t.Run(fmt.Sprintf("%s failed access check", tt), func(t *testing.T) {
err := callMethod(s, tt)
require.True(t, trace.IsAccessDenied(err))
})
}
t.Run("ListDynamicWindowsDesktops failed access check", func(t *testing.T) {
req := dynamicwindowsv1.ListDynamicWindowsDesktopsRequest{
PageSize: 10,
}
resp, err := s.ListDynamicWindowsDesktops(context.Background(), &req)
require.NoError(t, err)
require.Empty(t, resp.Desktops)
})
}

func TestServiceAccess(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -87,7 +124,7 @@ func TestServiceAccess(t *testing.T) {
for _, state := range tt.allowedStates {
for _, verbs := range utils.Combinations(tt.allowedVerbs) {
t.Run(fmt.Sprintf("%v,allowed:%v,verbs:%v", tt.name, stateToString(state), verbs), func(t *testing.T) {
service := newService(t, state, fakeChecker{allowedVerbs: verbs})
service := newService(t, state, &fakeChecker{allowedVerbs: verbs})
err := callMethod(service, tt.name)
// expect access denied except with full set of verbs.
if len(verbs) == len(tt.allowedVerbs) {
Expand All @@ -105,7 +142,7 @@ func TestServiceAccess(t *testing.T) {
t.Run(fmt.Sprintf("%v,disallowed:%v", tt.name, stateToString(state)), func(t *testing.T) {
// it is enough to test against tt.allowedVerbs,
// this is the only different data point compared to the test cases above.
service := newService(t, state, fakeChecker{allowedVerbs: tt.allowedVerbs})
service := newService(t, state, &fakeChecker{allowedVerbs: tt.allowedVerbs})
err := callMethod(service, tt.name)
require.True(t, trace.IsAccessDenied(err))
})
Expand Down Expand Up @@ -157,16 +194,19 @@ func callMethod(service *Service, method string) error {
if desc.MethodName == method {
_, err := desc.Handler(service, context.Background(), func(arg any) error {
switch arg := arg.(type) {
case *dynamicwindowsv1.GetDynamicWindowsDesktopRequest:
arg.Name = "test2"

case *dynamicwindowsv1.CreateDynamicWindowsDesktopRequest:
arg.Desktop, _ = types.NewDynamicWindowsDesktopV1("test", nil, types.DynamicWindowsDesktopSpecV1{
Addr: "test",
})
case *dynamicwindowsv1.UpdateDynamicWindowsDesktopRequest:
arg.Desktop, _ = types.NewDynamicWindowsDesktopV1("test", nil, types.DynamicWindowsDesktopSpecV1{
arg.Desktop, _ = types.NewDynamicWindowsDesktopV1("test2", nil, types.DynamicWindowsDesktopSpecV1{
Addr: "test",
})
case *dynamicwindowsv1.UpsertDynamicWindowsDesktopRequest:
arg.Desktop, _ = types.NewDynamicWindowsDesktopV1("test", nil, types.DynamicWindowsDesktopSpecV1{
arg.Desktop, _ = types.NewDynamicWindowsDesktopV1("test2", nil, types.DynamicWindowsDesktopSpecV1{
Addr: "test",
})
}
Expand All @@ -180,10 +220,11 @@ func callMethod(service *Service, method string) error {

type fakeChecker struct {
allowedVerbs []string
failAccess bool
services.AccessChecker
}

func (f fakeChecker) CheckAccessToRule(_ services.RuleContext, _ string, resource string, verb string) error {
func (f *fakeChecker) CheckAccessToRule(_ services.RuleContext, _ string, resource string, verb string) error {
if resource == types.KindDynamicWindowsDesktop {
if slices.Contains(f.allowedVerbs, verb) {
return nil
Expand All @@ -193,6 +234,13 @@ func (f fakeChecker) CheckAccessToRule(_ services.RuleContext, _ string, resourc
return trace.AccessDenied("access denied to rule=%v/verb=%v", resource, verb)
}

func (f *fakeChecker) CheckAccess(r services.AccessCheckable, state services.AccessState, matchers ...services.RoleMatcher) error {
if f.failAccess {
return trace.AccessDenied("denied")
}
return nil
}

func newService(t *testing.T, authState authz.AdminActionAuthState, checker services.AccessChecker) *Service {
t.Helper()

Expand Down
1 change: 1 addition & 0 deletions lib/services/role.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ func validateRoleExpressions(r types.Role) error {
{"db_labels", types.KindDatabase},
{"db_service_labels", types.KindDatabaseService},
{"windows_desktop_labels", types.KindWindowsDesktop},
{"windows_desktop_labels", types.KindDynamicWindowsDesktop},
{"group_labels", types.KindUserGroup},
} {
labelMatchers, err := r.GetLabelMatchers(condition.condition, labels.kind)
Expand Down

0 comments on commit 1ebf8a3

Please sign in to comment.