Skip to content

Commit

Permalink
Block non admin role to access system services (#4227)
Browse files Browse the repository at this point in the history 
* Block non admin role to access system services
  • Loading branch information
@yiminc
yiminc authored Apr 26, 2023
1 parent 5797648 commit ec3ff8b
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 105 deletions.
50 changes: 42 additions & 8 deletions common/authorization/default_authorizer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ type (
}
)

const (
operatorServicePrefix = "/temporal.api.operatorservice.v1.OperatorService/"
adminServicePrefix = "/temporal.server.api.adminservice.v1.AdminService/"
)

var _ Authorizer = (*defaultAuthorizer)(nil)

// NewDefaultAuthorizer creates a default authorizer
Expand All @@ -44,6 +49,16 @@ func NewDefaultAuthorizer() Authorizer {
var resultAllow = Result{Decision: DecisionAllow}
var resultDeny = Result{Decision: DecisionDeny}

// Authorize determines if an API call by given claims should be allowed or denied.
// Rules:
//
// Health check APIs are allowed to everyone.
// System Admin is allowed to access all APIs on all namespaces.
// System Writer is allowed to access non admin APIs on all namespaces.
// System Reader is allowed to access readonly APIs on all namespaces.
// Namespace Admin is allowed to access all APIs on their namespaces.
// Namespace Writer is allowed to access non admin APIs on their namespaces.
// Namespace Reader is allowed to access non admin readonly APIs on their namespaces.
func (a *defaultAuthorizer) Authorize(_ context.Context, claims *Claims, target *CallTarget) (Result, error) {
// APIs that are essentially read-only health checks with no sensitive information are
// always allowed
Expand All @@ -54,27 +69,46 @@ func (a *defaultAuthorizer) Authorize(_ context.Context, claims *Claims, target
if claims == nil {
return resultDeny, nil
}
// Check system level permissions
if claims.System >= RoleWriter {
// System Admin is allowed for everything
if claims.System >= RoleAdmin {
return resultAllow, nil
}

// admin service means admin / operator service
isAdminService := strings.HasPrefix(target.APIName, adminServicePrefix) || strings.HasPrefix(target.APIName, operatorServicePrefix)

// System Writer is allowed for non admin service APIs
if claims.System >= RoleWriter && !isAdminService {
return resultAllow, nil
}

api := ApiName(target.APIName)
readOnlyNamespaceAPI := IsReadOnlyNamespaceAPI(api)
readOnlyGlobalAPI := IsReadOnlyGlobalAPI(api)
// System Reader is allowed for all read only APIs
if claims.System >= RoleReader && (readOnlyNamespaceAPI || readOnlyGlobalAPI) {
return resultAllow, nil
}

role, found := claims.Namespaces[strings.ToLower(target.Namespace)]
// Below are for non system roles.
role, found := claims.Namespaces[target.Namespace]
if !found || role == RoleUndefined {
return resultDeny, nil
}
if role >= RoleWriter {
return resultAllow, nil
}
if role >= RoleReader && readOnlyNamespaceAPI {
return resultAllow, nil

if isAdminService {
// for admin service APIs, only RoleAdmin of given namespace can access
if role >= RoleAdmin {
return resultAllow, nil
}
} else {
// for non admin service APIs
if role >= RoleWriter {
return resultAllow, nil
}
if role >= RoleReader && readOnlyNamespaceAPI {
return resultAllow, nil
}
}

return resultDeny, nil
Expand Down
184 changes: 87 additions & 97 deletions common/authorization/default_authorizer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,31 @@ import (
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

"go.temporal.io/server/common/config"
)

var (
claimsNone = Claims{}
claimsNamespaceOnly = Claims{
claimsNone = Claims{}
claimsNamespaceAdmin = Claims{
Namespaces: map[string]Role{
testNamespace: RoleAdmin,
},
}
claimsNamespaceWriter = Claims{
Namespaces: map[string]Role{
testNamespace: RoleWriter,
},
}
claimsNamespaceReader = Claims{
Namespaces: map[string]Role{
testNamespace: RoleReader,
},
}
claimsBarAdmin = Claims{
Namespaces: map[string]Role{
"bar": RoleAdmin,
},
}
claimsSystemAdmin = Claims{
System: RoleAdmin,
}
Expand All @@ -52,19 +66,6 @@ var (
claimsSystemReader = Claims{
System: RoleReader,
}
claimsSystemReaderNamespaceUndefined = Claims{
System: RoleReader,
Namespaces: map[string]Role{
"bar": RoleUndefined,
},
}
claimsSystemUndefinedNamespaceReader = Claims{
System: RoleUndefined,
Namespaces: map[string]Role{
"bar": RoleReader,
},
}

targetFooBar = CallTarget{
APIName: "Foo",
Namespace: "bar",
Expand All @@ -89,6 +90,18 @@ var (
APIName: "/temporal.api.workflowservice.v1.WorkflowService/GetSystemInfo",
Namespace: "",
}
targetStartWorkflow = CallTarget{
Namespace: testNamespace,
APIName: "/temporal.api.workflowservice.v1.WorkflowService/StartWorkflowExecution",
}
targetAdminAPI = CallTarget{
Namespace: testNamespace,
APIName: "/temporal.server.api.adminservice.v1.AdminService/AddSearchAttributes",
}
targetAdminReadonlyAPI = CallTarget{
Namespace: testNamespace,
APIName: "/temporal.server.api.adminservice.v1.AdminService/GetSearchAttributes",
}
)

type (
Expand Down Expand Up @@ -116,87 +129,64 @@ func (s *defaultAuthorizerSuite) TearDownTest() {
s.controller.Finish()
}

func (s *defaultAuthorizerSuite) TestSystemAdminAuthZ() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemAdmin, &targetFooBar)
s.NoError(err)
s.Equal(DecisionAllow, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemWriterAuthZ() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemWriter, &targetFooBar)
s.NoError(err)
s.Equal(DecisionAllow, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemReaderAuthZ() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemReader, &targetFooBar)
s.NoError(err)
s.Equal(DecisionDeny, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemReaderBarUndefinedAuthZ() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemReaderNamespaceUndefined, &targetFooBar)
s.NoError(err)
s.Equal(DecisionDeny, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemUndefinedNamespaceReaderAuthZ() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemUndefinedNamespaceReader, &targetFooBar)
s.NoError(err)
s.Equal(DecisionDeny, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemUndefinedNamespaceCaseMismatch() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemUndefinedNamespaceReader, &targetFooBAR)
s.NoError(err)
s.Equal(DecisionDeny, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemUndefinedNamespaceReaderListNamespaces() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemUndefinedNamespaceReader, &targetListNamespaces)
s.NoError(err)
s.Equal(DecisionDeny, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemUndefinedNamespaceReaderDescribeNamespace() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemUndefinedNamespaceReader, &targetDescribeNamespace)
s.NoError(err)
s.Equal(DecisionAllow, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemWriterDescribeNamespace() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemWriter, &targetDescribeNamespace)
s.NoError(err)
s.Equal(DecisionAllow, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemWriterListNamespaces() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemWriter, &targetListNamespaces)
s.NoError(err)
s.Equal(DecisionAllow, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemAdminDescribeNamespace() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemAdmin, &targetDescribeNamespace)
s.NoError(err)
s.Equal(DecisionAllow, result.Decision)
}
func (s *defaultAuthorizerSuite) TestSystemAdminListNamespaces() {
result, err := s.authorizer.Authorize(context.TODO(), &claimsSystemAdmin, &targetListNamespaces)
s.NoError(err)
s.Equal(DecisionAllow, result.Decision)
}
func (s *defaultAuthorizerSuite) TestNamespaceOnly() {
// don't need any system-level claims to do namespace-level apis
result, err := s.authorizer.Authorize(context.TODO(), &claimsNamespaceOnly, startWorkflowExecutionTarget)
s.NoError(err)
s.Equal(DecisionAllow, result.Decision)
}
func (s *defaultAuthorizerSuite) TestHealthChecks() {
// all health checks should work all the time
for _, claims := range []*Claims{
nil,
&claimsNone,
&claimsNamespaceOnly,
} {
for _, target := range []*CallTarget{
&targetGrpcHealthCheck,
&targetGetSystemInfo,
} {
result, err := s.authorizer.Authorize(context.TODO(), claims, target)
s.NoError(err)
s.Equal(DecisionAllow, result.Decision)
}
func (s *defaultAuthorizerSuite) TestAuthorize() {
testCases := []struct {
Name string
Claims Claims
Target CallTarget
Decision Decision
}{
// SystemAdmin is allowed on everything
{"SystemAdminOnFooBar", claimsSystemAdmin, targetFooBar, DecisionAllow},
{"SystemAdminOnAdminAPI", claimsSystemAdmin, targetAdminAPI, DecisionAllow},
{"SystemAdminOnReadonlyAPI", claimsSystemAdmin, targetAdminReadonlyAPI, DecisionAllow},
{"SystemAdminOnStartWorkflow", claimsSystemAdmin, targetStartWorkflow, DecisionAllow},

// SystemWriter is allowed on all read only APIs and non-admin APIs on every namespaces
{"SystemWriterOnFooBar", claimsSystemWriter, targetFooBar, DecisionAllow},
{"SystemWriterOnAdminAPI", claimsSystemWriter, targetAdminAPI, DecisionDeny},
{"SystemWriterOnReadonlyAPI", claimsSystemWriter, targetAdminReadonlyAPI, DecisionAllow},
{"SystemWriterOnStartWorkflow", claimsSystemWriter, targetStartWorkflow, DecisionAllow},

// SystemReader is allowed on all read only APIs and blocked
{"SystemReaderOnFooBar", claimsSystemReader, targetFooBar, DecisionDeny},
{"SystemReaderOnAdminAPI", claimsSystemReader, targetAdminAPI, DecisionDeny},
{"SystemReaderOnReadonlyAPI", claimsSystemReader, targetAdminReadonlyAPI, DecisionAllow},
{"SystemReaderOnStartWorkflow", claimsSystemReader, targetStartWorkflow, DecisionDeny},

// NamespaceAdmin is allowed on admin service to their own namespaces (test-namespace)
{"NamespaceAdminOnAdminAPI", claimsNamespaceAdmin, targetAdminAPI, DecisionAllow},
{"NamespaceAdminOnReadonlyAPI", claimsNamespaceAdmin, targetAdminReadonlyAPI, DecisionAllow},
{"NamespaceAdminOnStartWorkflow", claimsNamespaceAdmin, targetStartWorkflow, DecisionAllow},
{"NamespaceAdminOnFooBar", claimsNamespaceAdmin, targetFooBar, DecisionDeny}, // namespace mismatch

{"BarAdminOnFooBar", claimsBarAdmin, targetFooBar, DecisionAllow},
{"BarAdminOnFooBAR", claimsBarAdmin, targetFooBAR, DecisionDeny}, // namespace case mismatch

// NamespaceWriter is not allowed on admin APIs
{"NamespaceWriterOnAdminAPI", claimsNamespaceWriter, targetAdminAPI, DecisionDeny},
{"NamespaceWriterOnReadonlyAPI", claimsNamespaceWriter, targetAdminReadonlyAPI, DecisionDeny},
{"NamespaceWriterOnStartWorkflow", claimsNamespaceWriter, targetStartWorkflow, DecisionAllow},
{"NamespaceWriterOnFooBar", claimsNamespaceWriter, targetFooBar, DecisionDeny}, // namespace mismatch

// NamespaceReader is allowed on read-only APIs on non admin service
{"NamespaceReaderOnAdminAPI", claimsNamespaceReader, targetAdminAPI, DecisionDeny},
{"NamespaceReaderOnReadonlyAPI", claimsNamespaceReader, targetAdminReadonlyAPI, DecisionDeny},
{"NamespaceReaderOnStartWorkflow", claimsNamespaceReader, targetStartWorkflow, DecisionDeny},
{"NamespaceReaderOnFooBar", claimsNamespaceReader, targetFooBar, DecisionDeny}, // namespace mismatch
{"NamespaceReaderOnListWorkflow", claimsNamespaceReader, targetGetSystemInfo, DecisionAllow},

// healthcheck allowed to everyone
{"RoleNoneOnGetSystemInfo", claimsNone, targetGetSystemInfo, DecisionAllow},
{"NamespaceReaderOnGetSystemInfo", claimsNamespaceReader, targetGetSystemInfo, DecisionAllow},
{"RoleNoneOnHealthCheck", claimsNone, targetGrpcHealthCheck, DecisionAllow},
{"NamespaceReaderOnHealthCheck", claimsNamespaceReader, targetGrpcHealthCheck, DecisionAllow},
}

for _, tt := range testCases {
result, err := s.authorizer.Authorize(context.TODO(), &tt.Claims, &tt.Target)
s.NoError(err)
s.Equal(tt.Decision, result.Decision, "Failed case: %v", tt.Name)
}
}

Expand Down

0 comments on commit ec3ff8b

Please sign in to comment.