diff --git a/libraries/fixture-composition/go/README.md b/libraries/fixture-composition/go/README.md index 1b55cbd..1470414 100644 --- a/libraries/fixture-composition/go/README.md +++ b/libraries/fixture-composition/go/README.md @@ -15,9 +15,16 @@ composed module ships next to its code, plus the composition's own - `Merge` reads each `Source` with `Decode` (an error names the source) and combines them. Every statement has exactly one source: a role's grant on one kind, one installation, one stand-in module. A second source stating one is - `ErrConflict`, naming both sources, even when they agree. Sources are merged - in the order of their names, so the result does not depend on the order they - were found in. + `ErrConflict`, naming both sources, even when they agree; so is a second + statement of it **within one document**, which `Decode` refuses, since the + grants of a kind are a union and a file stating one twice widens it exactly + as two files would. Sources are merged in the order of their names, so the + result does not depend on the order they were found in. +- A statement is keyed by organisation, role and kind — never by which module + ships the fragment — so the merge does not enforce that a module grants only + at its own kinds, and a composition's fragment deliberately grants at a + module's (its seeded collection). See the local service's README, "What the + merge does not enforce: ownership". - `Encode` writes YAML that `Decode` reads back unchanged. `go test -race ./...` covers the merge of two module fragments and a diff --git a/libraries/fixture-composition/go/fixturecomposition.go b/libraries/fixture-composition/go/fixturecomposition.go index a8f63f8..2ff172a 100644 --- a/libraries/fixture-composition/go/fixturecomposition.go +++ b/libraries/fixture-composition/go/fixturecomposition.go @@ -48,9 +48,10 @@ var ( actionGrammar = regexp.MustCompile(`^[a-z][a-z0-9_-]*$`) ) -// ErrConflict is two sources stating the same thing: the same role's grant on -// one kind, the same installation, or the same stand-in module. -var ErrConflict = errors.New("fixture-composition: two sources state the same grant") +// ErrConflict is the same thing stated twice — the same role's grant on one +// kind, the same installation, the same stand-in module — whether by two +// sources (Merge) or twice within one document (Decode). +var ErrConflict = errors.New("fixture-composition: one statement, stated twice") // Document is one fixture-composition document. type Document struct { @@ -150,7 +151,19 @@ func Decode(data []byte) (Document, error) { return doc, doc.validate() } +// validate holds one document — a single fragment, or a merged one — to the +// rules that do not depend on the fixture, the one-statement-one-source rule +// included: the same (organisation, role, kind), the same installation and the +// same stand-in module are each stated once in a document, as they are stated +// by one source across documents (Merge). +// +// Within one document that rule is what the cross-source rule exists for. The +// grants of a kind are a UNION — a role holding `documents.collection:read` on +// `{id: seeded}` and again on `{all: true}` reads every collection, the exact +// widening the rule refuses between two fragments — and whoever reads the +// first statement has no reason to look for a second further down the file. func (d Document) validate() error { + grants := map[string]bool{} for _, r := range d.Roles { key := r.Key() switch { @@ -163,24 +176,42 @@ func (d Document) validate() error { if err := CheckScope(sc.Kind, sc.Actions); err != nil { return fmt.Errorf("%s: %w", key, err) } - grants := make([]resources.Grant, 0, len(sc.Resources)) + if grants[key+" "+sc.Kind] { + return fmt.Errorf("%w: %s's grant on %s is stated twice in one document; state it once, with every action and resource it covers", + ErrConflict, key, sc.Kind) + } + grants[key+" "+sc.Kind] = true + compiled := make([]resources.Grant, 0, len(sc.Resources)) for _, m := range sc.Resources { - grants = append(grants, m.Grant()) + compiled = append(compiled, m.Grant()) } - if _, err := resources.Compile(grants); err != nil { + if _, err := resources.Compile(compiled); err != nil { return fmt.Errorf("%s scope %s: %w", key, sc.Kind, err) } } } + installations := map[string]bool{} for _, i := range d.Installations { if i.Target == "" || i.Role == "" { return errors.New("an installation names no target or no role") } + key := i.Organization + "/" + i.Target + if installations[key] { + return fmt.Errorf("%w: the installation of %s at %s is stated twice in one document; state it once", + ErrConflict, i.Target, i.Organization) + } + installations[key] = true } + modules := map[string]bool{} for _, m := range d.Modules { if !kindGrammar.MatchString(m.Module) || len(m.Kinds) == 0 { return fmt.Errorf("stand-in module %q names no valid module or no kind", m.Module) } + if modules[m.Module] { + return fmt.Errorf("%w: stand-in module %s is stated twice in one document; state it once, with every kind it serves", + ErrConflict, m.Module) + } + modules[m.Module] = true for _, k := range m.Kinds { if err := CheckScope(k.Kind, k.Actions); err != nil { return fmt.Errorf("stand-in module %s: %w", m.Module, err) @@ -230,7 +261,9 @@ type Source struct { // owns it is the one a reader goes to: a composition narrowing a module's grant // to a seeded resource must not be silently widened by the module's, and a // module's grant copied into a composition is the drift this exists to end. -// Within one source the document reads as it does alone. +// Within ONE source the same rule holds, and for the same reason — the grants +// of a kind are a union, so a fragment stating one twice widens it exactly as +// two fragments would — which Decode refuses (validate). // // The result does not depend on the order the sources are given in: they are // merged in the order of their names, and two sources of one name are refused. diff --git a/libraries/fixture-composition/go/fixturecomposition_test.go b/libraries/fixture-composition/go/fixturecomposition_test.go index 42798e5..79bfad6 100644 --- a/libraries/fixture-composition/go/fixturecomposition_test.go +++ b/libraries/fixture-composition/go/fixturecomposition_test.go @@ -167,3 +167,59 @@ func TestMergeOfNothingIsTheSchemaAlone(t *testing.T) { t.Fatalf("%q %v", encoded, err) } } + +// The one-statement-one-source rule holds inside a fragment too: the grants of +// a kind are a union, so a fragment stating the same (organisation, role, kind) +// twice widens it exactly as two fragments would — `{id: seeded}` and again +// `{all: true}` reads every resource of the kind, the failure the rule exists +// to stop. The same holds for an installation and a stand-in module. +func TestADocumentStatingOneThingTwiceIsRefused(t *testing.T) { + for name, tc := range map[string]struct{ doc, reason string }{ + "widening grant": {`roles: + - organization: acme + name: viewer + scopes: + - {kind: documents.collection, actions: [read], resources: [{id: seeded}]} + - {kind: documents.collection, actions: [read], resources: [{all: true}]}`, + "acme/viewer's grant on documents.collection is stated twice in one document"}, + "two role entries": {`roles: + - organization: acme + name: viewer + scopes: [{kind: documents, actions: [read]}] + - organization: acme + name: viewer + scopes: [{kind: documents, actions: [write]}]`, + "acme/viewer's grant on documents is stated twice"}, + "installation": {`installations: + - {organization: acme, target: atlas, role: viewer, owner: alex} + - {organization: acme, target: atlas, role: editor, owner: alex}`, + "the installation of atlas at acme is stated twice"}, + "stand-in": {`modules: + - {module: oracle, kinds: [{kind: oracle.answers, actions: [read]}]} + - {module: oracle, kinds: [{kind: oracle.answers, actions: [write]}]}`, + "stand-in module oracle is stated twice"}, + } { + t.Run(name, func(t *testing.T) { + _, err := fixturecomposition.Decode([]byte(head + tc.doc + "\n")) + if !errors.Is(err, fixturecomposition.ErrConflict) || !strings.Contains(err.Error(), tc.reason) { + t.Fatalf("got %v, want a conflict naming %q", err, tc.reason) + } + // The merge names the fragment it came from. + _, err = fixturecomposition.Merge([]fixturecomposition.Source{{Name: "module ledger", Data: []byte(head + tc.doc + "\n")}}) + if !errors.Is(err, fixturecomposition.ErrConflict) || !strings.HasPrefix(err.Error(), "module ledger: ") { + t.Fatalf("got %v, want the conflict named against module ledger", err) + } + }) + } + // One role stating two different kinds, and two roles stating one kind, + // are not duplicates. + for _, doc := range []string{ + `roles: [{organization: acme, name: viewer, scopes: [{kind: documents, actions: [read]}, {kind: robin.conversations, actions: [read]}]}]`, + `roles: [{organization: acme, name: viewer, scopes: [{kind: documents, actions: [read]}]}, {organization: acme, name: editor, scopes: [{kind: documents, actions: [read]}]}]`, + `installations: [{organization: acme, target: atlas, role: viewer, owner: alex}, {organization: globex, target: atlas, role: viewer, owner: zoe}]`, + } { + if _, err := fixturecomposition.Decode([]byte(head + doc + "\n")); err != nil { + t.Errorf("%s: %v", doc, err) + } + } +} diff --git a/modules/authority/services/local/README.md b/modules/authority/services/local/README.md index dec7e17..7eae879 100644 --- a/modules/authority/services/local/README.md +++ b/modules/authority/services/local/README.md @@ -130,9 +130,27 @@ the one parser of the document: refused naming both, even when they agree, so a composition narrowing a module's grant to its seeded collection is never silently widened by the module's, and a module's grant copied into a playground is refused rather - than left to drift; + than left to drift. One **fragment** stating the same one twice is refused + too, and for the same reason: the grants of a kind are a union, so a file + granting `{id: seeded}` and again `{all: true}` widens exactly as two files + would; - the result does not depend on the order fragments are found in. +**What the merge does not enforce: ownership.** A statement is keyed by +organisation, role and kind alone, never by which module ships the fragment, so +a module's fragment *can* grant at another module's kinds — and a composition's +fragment does, by design: the robin playground grants its viewer +`documents.collection` on the collection its own seed writes, a kind the +document store owns. "One definition per module" is therefore a convention +about where a grant is written, enforced only in that two fragments cannot +state the same grant; it is not a check that a module grants on its own kinds. +Nothing is lost by that: these fragments reach the development fixture alone, +never a host, and a grant at a kind no composed module serves resolves to +nothing (the authority intersects every capability with the delivered +manifests), so the failure mode is a refusal, not a widening. A composition +that wanted ownership enforced would have to name each kind's owning module in +the document, which v1 does not. + The authority module's own fragment (`modules/authority/configurations/local/fixture-composition.yaml`) is the schema line alone, so a composition of the authority and nothing else boots diff --git a/modules/authority/services/local/code/pkg/business/state/composition.go b/modules/authority/services/local/code/pkg/business/state/composition.go index 40be0d2..a9e5148 100644 --- a/modules/authority/services/local/code/pkg/business/state/composition.go +++ b/modules/authority/services/local/code/pkg/business/state/composition.go @@ -32,7 +32,9 @@ const CompositionSchema = fixturecomposition.Schema // until its own delivery replaces it. The document is read by the library's // one parser, which holds every rule that does not depend on the fixture; // this checks it against the fixture. Nothing is applied unless the whole -// document is valid. It returns one line per effect, for the boot log. +// document is valid, and the one applying step that can fail runs before the +// one that cannot, so a refused installation leaves every role as it was. It +// returns one line per effect, for the boot log. func (s *State) ApplyComposition(ctx context.Context, data []byte) ([]string, error) { doc, err := fixturecomposition.Decode(data) if err != nil { @@ -46,12 +48,13 @@ func (s *State) ApplyComposition(ctx context.Context, data []byte) ([]string, er if err != nil { return nil, fmt.Errorf("%s: %w", CompositionDocument, err) } - s.mu.Lock() - for key, role := range roles { - s.roles[key] = role - } - s.refreshRolesLocked() - s.mu.Unlock() + // The installations and stand-in modules go first, and the role grants + // after: everything above was validation, which changed nothing, and of the + // two applying steps only this one can fail. Granting first meant a + // composition whose installation was then refused had already widened the + // fixture's roles — the document was part-applied, against what this says. + // (LoadFixture's own internal steps are not individually reversible; a + // failure inside it still leaves what it applied before failing.) if len(doc.Installations) > 0 || len(doc.Modules) > 0 { setup, err := yaml.Marshal(fixtureFile{Installations: doc.Installations, Modules: doc.Modules}) if err != nil { @@ -61,6 +64,12 @@ func (s *State) ApplyComposition(ctx context.Context, data []byte) ([]string, er return nil, fmt.Errorf("%s: %w", CompositionDocument, err) } } + s.mu.Lock() + for key, role := range roles { + s.roles[key] = role + } + s.refreshRolesLocked() + s.mu.Unlock() return append(lines, more...), nil } diff --git a/modules/authority/services/local/code/pkg/business/state/composition_test.go b/modules/authority/services/local/code/pkg/business/state/composition_test.go index bb3447f..47c9a32 100644 --- a/modules/authority/services/local/code/pkg/business/state/composition_test.go +++ b/modules/authority/services/local/code/pkg/business/state/composition_test.go @@ -130,7 +130,12 @@ modules: // A composition grants concrete permissions to roles the fixture defines, and // a refused document changes nothing. func TestCompositionRefusesByName(t *testing.T) { - valid := `{organization: acme, name: editor, scopes: [{kind: ledger.entry, actions: [read]}]}` + // A valid entry prepended to every roles case below, so a refused later + // entry cannot leave it applied. Its kind differs from the cases' own: + // one document states each (organisation, role, kind) once, so reusing + // ledger.entry here would make every case a duplicate and hide what it + // is actually testing. + valid := `{organization: acme, name: editor, scopes: [{kind: ledger.audit, actions: [read]}]}` for name, tc := range map[string]struct{ doc, reason string }{ "empty": {"", "the document is empty"}, "null": {"null", "not a mapping"}, @@ -185,3 +190,38 @@ func TestTheShippedDefaultAddsNothing(t *testing.T) { t.Fatalf("%v %v", lines, err) } } + +// A document the fixture validates but that fails while being APPLIED leaves +// every role as it was: the applying step that can fail runs before the one +// that cannot. +// +// The document has to fail INSIDE LoadFixture to exercise that, which is the +// whole point of the ordering. A refused installation does not: an owner the +// fixture does not define is caught by checkComposedSetup, pure validation +// that runs before either applying step in both orderings, so such a document +// never reaches the code the ordering moves. A stand-in module named after a +// fixture person does: checkComposedSetup only checks its kinds and logs a +// line, and the core refuses it when the manifest is applied ("names a +// person; a module's principal is its own"). With the role writes first, that +// left the editor holding the grant from a document that was refused. +func TestADocumentRefusedWhileApplyingLeavesTheRolesAlone(t *testing.T) { + k := fixtureOnly(t) + before := k.State.RoleScopes("acme", "editor") + lines, err := k.State.ApplyComposition(context.Background(), []byte(head+` +roles: + - organization: acme + name: editor + scopes: + - {kind: ledger.entry, actions: [approve]} +modules: + - module: maya + kinds: [{kind: maya.things, actions: [read]}] +`)) + if err == nil || !strings.Contains(err.Error(), "names a person") { + t.Fatalf("got %v (%v), want the stand-in module refused while applying", err, lines) + } + after := k.State.RoleScopes("acme", "editor") + if !state.Attenuates(before, after) || !state.Attenuates(after, before) { + t.Fatalf("a refused document granted the editor anyway: %v → %v", before, after) + } +} diff --git a/modules/authority/services/local/code/pkg/business/state/state.go b/modules/authority/services/local/code/pkg/business/state/state.go index 007d1d4..ffc500f 100644 --- a/modules/authority/services/local/code/pkg/business/state/state.go +++ b/modules/authority/services/local/code/pkg/business/state/state.go @@ -7,7 +7,6 @@ package state import ( - fixturecomposition "github.com/codefly-dev/authority/libraries/fixture-composition/go" "context" "crypto/ed25519" "errors" @@ -20,6 +19,7 @@ import ( "github.com/codefly-dev/authority/libraries/authority-core/go/domain" "github.com/codefly-dev/authority/libraries/authority-core/go/memstore" "github.com/codefly-dev/authority/libraries/authority-core/go/scope" + fixturecomposition "github.com/codefly-dev/authority/libraries/fixture-composition/go" resources "github.com/codefly-dev/authority/libraries/resources/go" basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" "github.com/codefly-dev/core/workloadidentity"