Skip to content

Commit

Permalink
Limit decision service evaluation to local admins (#51840)
Browse files Browse the repository at this point in the history
  • Loading branch information
rosstimothy authored Feb 5, 2025
1 parent 1d4398a commit 602465e
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 6 deletions.
36 changes: 36 additions & 0 deletions lib/decision/decisionv1/decision_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,30 @@
package decisionv1

import (
"cmp"
"context"
"log/slog"

"github.com/gravitational/trace"

decisionpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/decision/v1alpha1"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/authz"
)

// ServiceConfig holds creation parameters for [Service].
type ServiceConfig struct {
// Authorizer used by the service.
Authorizer authz.Authorizer
Logger *slog.Logger
}

// Service implements the teleport.decision.v1alpha1.DecisionService gRPC API.
type Service struct {
decisionpb.UnimplementedDecisionServiceServer

authorizer authz.Authorizer
logger *slog.Logger
}

// NewService creates a new [Service] instance.
Expand All @@ -44,5 +51,34 @@ func NewService(cfg ServiceConfig) (*Service, error) {

return &Service{
authorizer: cfg.Authorizer,
logger: cmp.Or(cfg.Logger, slog.Default()),
}, nil
}

func (s *Service) EvaluateSSHAccess(ctx context.Context, req *decisionpb.EvaluateSSHAccessRequest) (*decisionpb.EvaluateSSHAccessResponse, error) {
authzContext, err := s.authorizer.Authorize(ctx)
if err != nil {
return nil, trace.Wrap(err)
}

if !authz.HasBuiltinRole(*authzContext, string(types.RoleAdmin)) {
s.logger.WarnContext(ctx, "user does not have permission to evaluate SSH access", "user", authzContext.User.GetName())
return nil, trace.AccessDenied("user %q does not have permission to evaluate SSH access", authzContext.User.GetName())
}

return s.UnimplementedDecisionServiceServer.EvaluateSSHAccess(ctx, req)
}

func (s *Service) EvaluateDatabaseAccess(ctx context.Context, req *decisionpb.EvaluateDatabaseAccessRequest) (*decisionpb.EvaluateDatabaseAccessResponse, error) {
authzContext, err := s.authorizer.Authorize(ctx)
if err != nil {
return nil, trace.Wrap(err)
}

if !authz.HasBuiltinRole(*authzContext, string(types.RoleAdmin)) {
s.logger.WarnContext(ctx, "user does not have permission to evaluate database access", "user", authzContext.User.GetName())
return nil, trace.AccessDenied("user %q does not have permission to evaluate database access", authzContext.User.GetName())
}

return s.UnimplementedDecisionServiceServer.EvaluateDatabaseAccess(ctx, req)
}
51 changes: 45 additions & 6 deletions lib/decision/decisionv1/decision_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,63 @@ import (

"github.com/gravitational/trace"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

decisionpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/decision/v1alpha1"
"github.com/gravitational/teleport/lib/auth"
)

func TestDecisionService_notImplemented(t *testing.T) {
func TestDecisionServiceRequiresLocalAdmin(t *testing.T) {
t.Parallel()

env := NewTestenv(t)
decisionClient := env.DecisionClient
ctx := context.Background()

_, _, err := auth.CreateUserAndRoleWithoutRoles(env.AuthAdminClient, "alice", []string{"alice"})
require.NoError(t, err, "Creating use alice failed")

aliceClient, err := env.TestServer.NewClient(auth.TestUser("alice"))
require.NoError(t, err, "NewClient failed")
t.Cleanup(func() {
assert.NoError(t, aliceClient.Close(), "aliceClient.Close() failed")
})

tests := []struct {
name string
client decisionpb.DecisionServiceClient
expectedError any
}{
{
name: "admin",
client: env.DecisionClient,
expectedError: &trace.NotImplementedError{},
},
{
name: "alice",
client: aliceClient.DecisionClient(),
expectedError: &trace.AccessDeniedError{},
},
}

t.Run("EvaluateSSHAccess", func(t *testing.T) {
_, err := decisionClient.EvaluateSSHAccess(ctx, &decisionpb.EvaluateSSHAccessRequest{})
assert.ErrorAs(t, err, new(*trace.NotImplementedError), "EvaluateSSHAccess error mismatch")
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()

_, err := test.client.EvaluateSSHAccess(ctx, &decisionpb.EvaluateSSHAccessRequest{})
assert.ErrorAs(t, err, &test.expectedError, "EvaluateSSHAccess error mismatch")
})
}
})

t.Run("EvaluateDatabaseAccess", func(t *testing.T) {
_, err := decisionClient.EvaluateDatabaseAccess(ctx, &decisionpb.EvaluateDatabaseAccessRequest{})
assert.ErrorAs(t, err, new(*trace.NotImplementedError), "EvaluateDatabaseAccess error mismatch")
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()

_, err := test.client.EvaluateDatabaseAccess(ctx, &decisionpb.EvaluateDatabaseAccessRequest{})
assert.ErrorAs(t, err, &test.expectedError, "EvaluateDatabaseAccess error mismatch")
})
}
})
}

0 comments on commit 602465e

Please sign in to comment.