Merge pull request 'Read the grant file on keycloak's moved path, so a consumer not moved gets its client (issue 1332)' (#378) from fix/1332-keycloak-reads-the-grant-file into main

modules/keycloak/cmd/keycloak-provider/grant_moved_test.go

@@ -0,0 +1,143 @@
+package main
+
+// Keycloak runs the moved path with the grant file still open for its consumers not moved (issue 937), fixed for
+// keycloak by issue 1332: mesh-mail-web, grafana and car-hunter are not moved, so without this fix keycloak never
+// made their clients and held them for retirement. These tests drive the SDK's harness.Moved exactly as main.go
+// wires it, against a fake Keycloak and an in-memory bus — no real backend.
+
+import (
+	"context"
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"git.novox.be/novox/mesh-sdk/go/provider"
+	"git.novox.be/novox/mesh-sdk/go/provider/harness"
+)
+
+// grantGives writes a grant file (MESH_RECEIVES) naming consumers the plans do not move (issue 937): each one's
+// secret file, and the json the grant path reads.
+func grantGives(t *testing.T, given ...map[string]any) string {
+	t.Helper()
+	dir := t.TempDir()
+	for _, g := range given {
+		as := g["as"].(string)
+		secret := filepath.Join(dir, as+".secret")
+		if err := os.WriteFile(secret, []byte("pw-"+as+"\n"), 0o600); err != nil {
+			t.Fatal(err)
+		}
+		g["secret"] = secret
+	}
+	raw, _ := json.Marshal(map[string]any{"requirement": ProvisionName, "given": given})
+	at := filepath.Join(dir, "mesh.json")
+	if err := os.WriteFile(at, raw, 0o600); err != nil {
+		t.Fatal(err)
+	}
+	return at
+}
+
+// noOpPlan puts one assignment the bus can read that binds nothing of keycloak's, so the moved path has plans it
+// could read (m.asked answers "read", issue 241) even when nothing is moved.
+func noOpPlan(bus *bucketBus, node, module string) {
+	raw, _ := json.Marshal(map[string]any{"node": node, "module": module})
+	_ = bus.Put(ctx, provider.AssignmentsBucket, node+"."+module, raw)
+}
+
+// waitUntil polls cond until it is true or the timeout passes.
+func waitUntil(t *testing.T, timeout time.Duration, cond func() bool) {
+	t.Helper()
+	deadline := time.Now().Add(timeout)
+	for {
+		if cond() {
+			return
+		}
+		if time.Now().After(deadline) {
+			t.Fatalf("condition never held within %s", timeout)
+		}
+		time.Sleep(2 * time.Millisecond)
+	}
+}
+
+// movedWorld builds keycloak's moved path, as main.go wires it, against a fake Keycloak and a bucket bus. Nothing
+// runs until start is called, so a test can seed the backend first; it is stopped when the test ends.
+func movedWorld(t *testing.T, h *harness.Harness) (f *fakeKeycloak, start func()) {
+	t.Helper()
+	var o OidcClients
+	f, o = oidcWorld(t)
+	h.Adapter = provisioner{clients: o, announce: func(string, map[string]any) {}, log: func(string, ...any) {}}
+	if h.Every == 0 {
+		h.Every = 5 * time.Millisecond
+	}
+	consumersFile := filepath.Join(t.TempDir(), "consumers.json")
+	if err := os.WriteFile(consumersFile, []byte("[]"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	run, _, _, err := moved(consumersFile, h, testIssuer)
+	if err != nil {
+		t.Fatal(err)
+	}
+	bus := &bucketBus{buckets: map[string]map[string][]byte{}}
+	noOpPlan(bus, "elsewhere", "unrelated")
+	p := provider.New(provider.Options{Node: h.Node, Module: "keycloak", Bus: bus,
+		Vault:  provider.ConsumersFile{Paths: []string{consumersFile}},
+		Offers: map[string]provider.Offer{ProvisionName: {Credential: provider.PerConsumer, Serves: servedValues(testIssuer)}},
+		Make:   run.Make, Gone: run.Gone, Every: h.Every, Logf: func(string, ...any) {}})
+	return f, func() {
+		runCtx, cancel := context.WithTimeout(ctx, 2*time.Second)
+		t.Cleanup(cancel)
+		go func() { _ = run.Run(runCtx, p, bus) }()
+	}
+}
+
+// 1332/AC1: a consumer only the grant file gives (oidc-client for novox.mesh-mail-web, not moved) is wanted and
+// Adapter.Create is asked for its client, though keycloak runs on the moved path.
+func TestAGrantFileConsumerNotMovedIsMadeOnTheMovedPath(t *testing.T) {
+	mailWeb := "mesh_novox_mail_web"
+	receives := grantGives(t, map[string]any{"as": mailWeb, "node": "novox", "values": map[string]any{
+		"label": "mail-web", "endpoint": "web", "callback": "/login/generic_oauth", "name": "mail.example.org"}})
+	h := &harness.Harness{Resource: ProvisionName, Receives: receives, Node: "novox"}
+	f, start := movedWorld(t, h)
+	start()
+	waitUntil(t, time.Second, func() bool {
+		found := false
+		f.set(func() {
+			for _, c := range f.clients {
+				if c["clientId"] == mailWeb {
+					found = true
+				}
+			}
+		})
+		return found
+	})
+}
+
+// 1332/AC2: a grant-file consumer whose client already exists in the backend (mesh_ace_grafana) counts as asked and
+// is not held for retirement on the moved path.
+func TestAGrantFileConsumerAlreadyInKeycloakIsNotRetiredOnTheMovedPath(t *testing.T) {
+	grafana := "mesh_ace_grafana"
+	receives := grantGives(t, map[string]any{"as": grafana, "node": "ace", "values": map[string]any{
+		"label": "grafana", "endpoint": "web", "callback": "/login/generic_oauth", "name": "grafana.example.org"}})
+	h := &harness.Harness{Resource: ProvisionName, Receives: receives, Node: "ace",
+		StablePasses: 1, StableFor: time.Millisecond}
+	f, start := movedWorld(t, h)
+	// Seed the backend with the client the mesh made earlier, exactly as grafana's already is — before anything
+	// runs, so the harness's one-time seed (issue 1332) sees it.
+	f.set(func() {
+		f.clients["existing-grafana"] = Rep{"id": "existing-grafana", "clientId": grafana, "enabled": true,
+			"attributes": map[string]any{Mark: "true"}}
+	})
+	start()
+	// Many passes, well past StablePasses and StableFor if the login were ever counted unasked.
+	time.Sleep(200 * time.Millisecond)
+	f.set(func() {
+		c := f.clients["existing-grafana"]
+		if c["enabled"] == false {
+			t.Fatalf("retired %v though its grant still gives it (issue 1332)", c)
+		}
+		if _, retired := attributes(c)[MarkRetired]; retired {
+			t.Fatalf("marked retired %v though its grant still gives it (issue 1332)", c)
+		}
+	})
+}

modules/keycloak/cmd/keycloak-provider/main.go

@@ -51,15 +51,9 @@ func main() {
 		say("%v — the provisioner cannot run without it", err)
 	} else if consumers := handedConsumersFile(os.Getenv, os.Stat); consumers != "" {
 		// Handed to the provisioner (novox/hq ADR 0373): the provider helper answers the consumers its
-		// machine's client fills the file with; the grant directory is not read. Retirement is the
-		// harness's, never run, so its guards and its tools stand (issue 805).
-		h = &harness.Harness{
-			Resource: ProvisionName,
-			Adapter:  provisioner{clients: OidcClients{KC: kc, Realm: realm}, guard: guard, announce: announce, log: logStderr},
-			Log:      logStderr,
-			Announce: announce,
-			Node:     os.Getenv("MESH_NODE"),
-		}
+		// machine's client fills the file with, and the grant file still gives the consumers not moved
+		// (issue 937, 1332). Retirement is the harness's, never run, so its guards and its tools stand (issue 805).
+		h = movedHarness(os.Getenv, provisioner{clients: OidcClients{KC: kc, Realm: realm}, guard: guard, announce: announce, log: logStderr})
 		run, p, bus, err := moved(consumers, h, issuer)
 		if err != nil {
 			say("%v", err)
@@ -101,6 +95,20 @@ func main() {
 	}
 }
 
+// movedHarness is the harness the moved path shares with the SDK's helper: what main wires together, pulled out so
+// it can be built and inspected without starting the process. Receives is MESH_RECEIVES, read exactly as the grant
+// path reads it (novox/hq issue 937), so the grant file still gives every consumer not moved (novox/hq issue 1332).
+func movedHarness(getenv func(string) string, adapter harness.Adapter) *harness.Harness {
+	return &harness.Harness{
+		Resource: ProvisionName,
+		Receives: getenv("MESH_RECEIVES"),
+		Adapter:  adapter,
+		Log:      logStderr,
+		Announce: announce,
+		Node:     getenv("MESH_NODE"),
+	}
+}
+
 // handedConsumersFile is the consumers' file when keycloak is handed to the provisioner: the file MESH_CONSUMERS_FILE
 // names, which the controller places only where it hands the module. Empty, where it is not set or the file is not
 // there, and keycloak then reads the grant directory exactly as it always did (issue 805).

modules/keycloak/cmd/keycloak-provider/main_test.go

@@ -0,0 +1,19 @@
+package main
+
+// What main wires for the moved path (novox/hq issue 1332): the harness it builds from the environment, the same
+// way the grant path reads its own settings.
+
+import "testing"
+
+// 1332/AC3: the moved-path harness, built from the environment, reads MESH_RECEIVES the way the grant path does
+// (main.go's other branch, line 67), as postgres's moved harness does (postgres-provider/main.go:43).
+func TestTheMovedPathHarnessReadsMESH_RECEIVESAsTheGrantPathDoes(t *testing.T) {
+	env := map[string]string{"MESH_RECEIVES": "/grants/mesh.json", "MESH_NODE": "novox"}
+	getenv := func(k string) string { return env[k] }
+	h := movedHarness(getenv, provisioner{})
+	if h.Receives != env["MESH_RECEIVES"] {
+		t.Fatalf("Receives = %q, want %q — the grant file still gives every consumer not moved (issue 937), "+
+			"read exactly as MESH_RECEIVES on the grant path, as postgres's moved harness does "+
+			"(postgres-provider/main.go:43)", h.Receives, env["MESH_RECEIVES"])
+	}
+}

modules/keycloak/cmd/keycloak-provider/moved.go

@@ -43,10 +43,16 @@ func valuesOf(e provider.Entry) map[string]any {
 // moved is keycloak handed to the provisioner: the SDK's moved path (harness.Moved) answering its consumers, and the
 // harness (never run) whose retirement and tools it shares. What differs from postgres is only this declaration
 // (issue 849).
+//
+// **A consumer not moved is still the grant file's** (issue 937, ADR 0349 clause 17, fixed here by issue 1332):
+// only a moved consumer's plan names keycloak as its provider, so the controller still gives every other consumer
+// through keycloak's grant file (MESH_RECEIVES) until R1 (issue 682) — mesh-mail-web, grafana and car-hunter among
+// them. Each client it gives is made and checked by the harness's grant pass exactly as on the grant path; a client
+// a plan names is the helper's alone.
 func moved(consumersFile string, h *harness.Harness, issuer string) (*harness.Moved, *provider.Provider, provider.Bus, error) {
 	return harness.NewMoved(consumersFile, h, harness.MovedSpec{Module: "keycloak", Provision: ProvisionName,
 		Offer:  provider.Offer{Credential: provider.PerConsumer, Serves: servedValues(issuer)},
 		Login:  func(e provider.Entry) string { return e.As },
 		Values: valuesOf,
-		Noun:   "client id", Made: "client", Backend: "Keycloak", WithGrants: false})
+		Noun:   "client id", Made: "client", Backend: "Keycloak", WithGrants: true})
 }