diff --git a/pkg/api/policy/api.go b/pkg/api/policy/api.go index 35edb8d..f53316d 100644 --- a/pkg/api/policy/api.go +++ b/pkg/api/policy/api.go @@ -182,9 +182,9 @@ func (a *API) listAllPolicies() ([]policyTY.Policy, error) { // Save persists a user authored policy. // // System policies are code: their statements are rewritten on every start, so -// accepting edits here would silently discard them. The System flag itself is never -// taken from the request - otherwise a client could mark its own policy -// undeletable. +// accepting edits here would silently discard them. The System flag itself is +// never taken from the request - a client cannot mark a custom policy undeletable +// or create a policy with a built-in id. func (a *API) Save(policy *policyTY.Policy) error { if policy.ID == "" { policy.ID = utils.RandID() @@ -192,8 +192,11 @@ func (a *API) Save(policy *policyTY.Policy) error { existing, err := a.loadPolicyFromStorage(policy.ID) isExisting := err == nil && existing.ID != "" - if isExisting && existing.System { - return fmt.Errorf("cannot modify system policy: %s", policy.ID) + if IsBuiltInPolicyID(policy.ID) || (isExisting && existing.System) { + return fmt.Errorf("%w: %s", ErrSystemPolicyImmutable, policy.ID) + } + if policy.System { + return fmt.Errorf("%w: %s", ErrSystemFlagNotAllowed, policy.ID) } policy.System = false @@ -259,15 +262,11 @@ func (a *API) Import(data interface{}) error { if !ok { return fmt.Errorf("invalid type:%T", data) } - if input.ID == "" { - input.ID = utils.RandID() - } - filters := []storageTY.Filter{{Key: types.KeyID, Value: input.ID}} - if err := a.storage.Upsert(types.EntityPolicy, &input, filters); err != nil { - return err + if IsBuiltInPolicyID(input.ID) { + return nil } - a.cache.PutPolicy(&input) - return nil + input.System = false + return a.Save(&input) } func (a *API) GetEntityInterface() interface{} { @@ -312,4 +311,8 @@ func (a *API) ValidatePoliciesExist(ids []string) error { return nil } -var ErrNotFound = errors.New("not found") +var ( + ErrNotFound = errors.New("not found") + ErrSystemPolicyImmutable = errors.New("cannot modify system policy") + ErrSystemFlagNotAllowed = errors.New("system flag is not allowed on custom policies") +) diff --git a/pkg/api/policy/api_save_test.go b/pkg/api/policy/api_save_test.go new file mode 100644 index 0000000..b926bac --- /dev/null +++ b/pkg/api/policy/api_save_test.go @@ -0,0 +1,139 @@ +package policy + +import ( + "errors" + "testing" + + types "github.com/mycontroller-org/server/v2/pkg/types" + policyTY "github.com/mycontroller-org/server/v2/pkg/types/policy" + svcTokenTY "github.com/mycontroller-org/server/v2/pkg/types/service_token" + userTY "github.com/mycontroller-org/server/v2/pkg/types/user" + storageTY "github.com/mycontroller-org/server/v2/plugin/database/storage/types" +) + +type policyMemStore struct { + policies map[string]policyTY.Policy +} + +func newPolicyMemStore() *policyMemStore { + return &policyMemStore{policies: map[string]policyTY.Policy{}} +} + +func (s *policyMemStore) Name() string { return "test" } +func (s *policyMemStore) Ping() error { return nil } +func (s *policyMemStore) Close() error { return nil } +func (s *policyMemStore) Insert(string, interface{}) error { + return errors.New("not implemented") +} +func (s *policyMemStore) Update(string, interface{}, []storageTY.Filter) error { + return errors.New("not implemented") +} +func (s *policyMemStore) Find(string, interface{}, []storageTY.Filter, *storageTY.Pagination) (*storageTY.Result, error) { + return nil, errors.New("not implemented") +} +func (s *policyMemStore) Delete(string, []storageTY.Filter) (int64, error) { + return 0, errors.New("not implemented") +} +func (s *policyMemStore) Pause() error { return nil } +func (s *policyMemStore) Resume() error { return nil } +func (s *policyMemStore) ClearDatabase() error { return nil } +func (s *policyMemStore) DoStartupImport() (bool, string, string) { return false, "", "" } + +func (s *policyMemStore) FindOne(entityName string, out interface{}, filters []storageTY.Filter) error { + if entityName != types.EntityPolicy || len(filters) == 0 { + return storageTY.ErrNoDocuments + } + id, _ := filters[0].Value.(string) + p, ok := s.policies[id] + if !ok { + return storageTY.ErrNoDocuments + } + *(out.(*policyTY.Policy)) = p + return nil +} + +func (s *policyMemStore) Upsert(entityName string, data interface{}, _ []storageTY.Filter) error { + p, ok := data.(*policyTY.Policy) + if !ok || entityName != types.EntityPolicy { + return errors.New("invalid upsert") + } + s.policies[p.ID] = *p + return nil +} + +func testPolicyAPI(store *policyMemStore) *API { + c := newCache() + c.setLoaders( + func(id string) (*userTY.User, error) { return nil, ErrUserNotFound }, + func(id string) (*policyTY.Policy, error) { return nil, ErrUserNotFound }, + func(id string) (*svcTokenTY.ServiceToken, error) { return nil, ErrTokenNotFound }, + func() ([]policyTY.Policy, error) { return nil, nil }, + ) + return &API{storage: store, cache: c} +} + +func TestSaveRejectsSystemPolicyEdit(t *testing.T) { + store := newPolicyMemStore() + store.policies["admin"] = policyTY.Policy{ID: "admin", System: true} + a := testPolicyAPI(store) + + err := a.Save(&policyTY.Policy{ID: "admin", Description: "hacked", System: false}) + if !errors.Is(err, ErrSystemPolicyImmutable) { + t.Fatalf("Save system policy: got %v, want %v", err, ErrSystemPolicyImmutable) + } +} + +func TestSaveRejectsBuiltInID(t *testing.T) { + a := testPolicyAPI(newPolicyMemStore()) + err := a.Save(&policyTY.Policy{ID: policyTY.PolicyReadOnly, Description: "custom readonly"}) + if !errors.Is(err, ErrSystemPolicyImmutable) { + t.Fatalf("Save built-in id: got %v, want %v", err, ErrSystemPolicyImmutable) + } +} + +func TestSaveRejectsSystemFlagOnCustomPolicy(t *testing.T) { + a := testPolicyAPI(newPolicyMemStore()) + err := a.Save(&policyTY.Policy{ID: "living-room", System: true}) + if !errors.Is(err, ErrSystemFlagNotAllowed) { + t.Fatalf("Save custom with system: got %v, want %v", err, ErrSystemFlagNotAllowed) + } +} + +func TestSavePersistsCustomPolicyWithoutSystem(t *testing.T) { + store := newPolicyMemStore() + a := testPolicyAPI(store) + if err := a.Save(&policyTY.Policy{ID: "living-room", Description: "ok"}); err != nil { + t.Fatalf("Save custom: %v", err) + } + got := store.policies["living-room"] + if got.System { + t.Fatal("expected system=false on stored custom policy") + } + if got.Description != "ok" { + t.Fatalf("description=%q", got.Description) + } +} + +func TestImportSkipsBuiltInAndStripsSystemFlag(t *testing.T) { + store := newPolicyMemStore() + store.policies["admin"] = policyTY.Policy{ID: "admin", System: true, Description: "original"} + a := testPolicyAPI(store) + + if err := a.Import(policyTY.Policy{ID: "admin", Description: "from backup", System: true}); err != nil { + t.Fatalf("Import built-in: %v", err) + } + if store.policies["admin"].Description != "original" { + t.Fatalf("built-in was overwritten: %+v", store.policies["admin"]) + } + + if err := a.Import(policyTY.Policy{ID: "custom-1", System: true, Description: "restored"}); err != nil { + t.Fatalf("Import custom: %v", err) + } + got := store.policies["custom-1"] + if got.System { + t.Fatal("import left system=true on custom policy") + } + if got.Description != "restored" { + t.Fatalf("description=%q", got.Description) + } +} diff --git a/pkg/api/policy/defaults.go b/pkg/api/policy/defaults.go index c468c74..3fe67d9 100644 --- a/pkg/api/policy/defaults.go +++ b/pkg/api/policy/defaults.go @@ -5,6 +5,19 @@ import ( settingsTY "github.com/mycontroller-org/server/v2/pkg/types/settings" ) +// IsBuiltInPolicyID reports whether id is a code-owned system policy. +func IsBuiltInPolicyID(id string) bool { + if id == "" { + return false + } + for _, p := range BuiltInPolicies() { + if p.ID == id { + return true + } + } + return false +} + // BuiltInPolicies returns system policies that should always exist. func BuiltInPolicies() []policyTY.Policy { allActions := []string{policyTY.ActionAll} diff --git a/pkg/http_router/routes/policy.go b/pkg/http_router/routes/policy.go index d217b20..be813e3 100644 --- a/pkg/http_router/routes/policy.go +++ b/pkg/http_router/routes/policy.go @@ -4,6 +4,7 @@ import ( "errors" "net/http" + policyAPI "github.com/mycontroller-org/server/v2/pkg/api/policy" types "github.com/mycontroller-org/server/v2/pkg/types" policyTY "github.com/mycontroller-org/server/v2/pkg/types/policy" handlerUtils "github.com/mycontroller-org/server/v2/pkg/utils/http_handler" @@ -41,7 +42,11 @@ func (h *Routes) updatePolicy(w http.ResponseWriter, r *http.Request) { } err = h.api.Policy().Save(entity) if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + status := http.StatusInternalServerError + if errors.Is(err, policyAPI.ErrSystemPolicyImmutable) || errors.Is(err, policyAPI.ErrSystemFlagNotAllowed) { + status = http.StatusBadRequest + } + http.Error(w, err.Error(), status) return } }