From 2697c5c7832d4905e8270c228cc19585a8a7c1d5 Mon Sep 17 00:00:00 2001 From: Mark Mennell Date: Sat, 10 Oct 2026 14:20:20 +0800 Subject: [PATCH] Disable deleted sub-account addresses, reinstate them on re-create Deleting a derived sub-account now stops its address accepting new messages in fmsgid (POST /fmsgid with accepting_new false), so fmsg hosts refuse new mail to it, before the grant is deleted. If fmsgid can't be updated the delete fails with 503 and keeps the grant, so it can be retried rather than leaving an address accepting mail with nothing to retry from. Delegated identities are left alone. Creating a derived sub-account registers its address with accepting_new true and quota_pool set to the owner, so it shares the owner's quota. Only the owner can derive the address, so one that already exists was theirs before: fmsgid reinstates it and the create response reports "reinstated": true (false for a new address). If the grant then can't be created the reinstated address is disabled again. The api-key create CLI does the same and prints reinstated=. An fmsgid without accepting_new answers POST /fmsgid with no body: creation then works as before with reinstated false, and deletion fails rather than claiming the address was disabled. Co-Authored-By: Claude Opus 5.5 --- README.md | 44 ++- cmd/fmsg-webapi/apikey_cli.go | 18 +- .../apikey_cli_integration_test.go | 73 ++++ internal/handlers/subaccounts.go | 74 +++- .../handlers/subaccounts_lifecycle_test.go | 332 ++++++++++++++++++ internal/middleware/fmsgid_test.go | 104 +++++- internal/middleware/jwt.go | 82 ++++- 7 files changed, 693 insertions(+), 34 deletions(-) create mode 100644 internal/handlers/subaccounts_lifecycle_test.go diff --git a/README.md b/README.md index 7b0f977..1d2c2be 100644 --- a/README.md +++ b/README.md @@ -176,9 +176,11 @@ the plaintext API key once. Creation also requires the fmsgid service configured by `FMSG_ID_URL`: - `create` checks that the owner exists and accepts new messages, registers the - derived address with fmsgid's default quotas, and verifies it accepts new - messages before storing the key. Existing address settings are preserved; - quotas are independent of the owner's. + derived address with fmsgid in the owner's quota pool (it shares the owner's + quota), reinstates it if it was used before and stopped accepting new + messages, and verifies it accepts new messages before storing the key. It + prints `reinstated=true` for such an address. Its other settings are + preserved. - `create-delegation` requires both the owner and delegated address to already exist and accept new messages. It does not register either address. @@ -401,8 +403,33 @@ EdDSA owner authentication. The derived address is `@user_bot@domain`. `agent` may contain letters, digits, dots, and hyphens, but not underscores. The derived address is automatically -registered with fmsgid (with default quotas) if it doesn't already exist there -— no manual fmsgid step is needed for derived sub-accounts. +registered with fmsgid, in the owner's quota pool so it shares the owner's +quota — no manual fmsgid step is needed for derived sub-accounts. + +Only the owner can derive `@user_bot@domain`, so if the address already exists +in fmsgid it is the owner's from before — a sub-account they deleted — and is +reinstated: it accepts new messages again and its earlier messages are still +there. The response is the new grant, as from `GET /fmsg/sub-accounts/:agent`, +plus `api_key` (shown once) and `reinstated`, `true` for a reinstated address +and `false` for a new one: + +```json +{ + "agent": "bot", + "addr": "@alice_bot@example.com", + "grant_type": "derived_sub_account", + "key_id": "abc", + "allowed_cidrs": ["203.0.113.0/24"], + "key_expires_at": "2026-12-31T00:00:00Z", + "api_key": "...", + "reinstated": true +} +``` + +If the grant can't be created (e.g. `409` or the sub-account limit), a +reinstated address is stopped accepting new messages again. Reinstating and +pooling need an fmsgid that supports `accepting_new` and `quota_pool` on +`POST /fmsgid`; with an older one `reinstated` is always `false`. Delegated identities such as `@sales@example.com` are not created by this self-service route. They are operator-created with `api-key create-delegation` @@ -437,6 +464,13 @@ new plaintext key once. Requires `key_expires_at`. Does not touch Deletes a grant row and revokes future token exchange. Existing JWTs for that key are rejected on their next protected-route request. +For a derived sub-account the address is first stopped accepting new messages +in fmsgid (`accepting_new` false), so fmsg hosts refuse new mail to it; it and +its messages are kept, and creating the same sub-account again reinstates it. +If fmsgid can't be updated — unavailable, or a version without `accepting_new` +— the delete fails with `503` and the grant is kept, so it can be retried. +Delegated identities are addresses in their own right and are left as they are. + ### GET `/fmsg/ws` Upgrades the connection to a WebSocket over which the server pushes events that diff --git a/cmd/fmsg-webapi/apikey_cli.go b/cmd/fmsg-webapi/apikey_cli.go index 8562919..5164c40 100644 --- a/cmd/fmsg-webapi/apikey_cli.go +++ b/cmd/fmsg-webapi/apikey_cli.go @@ -55,10 +55,22 @@ func runAPIKeyCreate(ctx context.Context, args []string) error { return err } // Match the self-service sub-account flow: derived addresses are created in - // fmsgid before their API key is persisted. - if err := middleware.RegisterFmsgID(idURL, subAddr); err != nil { + // fmsgid, in the owner's quota pool, before their API key is persisted, and + // one used before is reinstated. + reinstated, err := middleware.RegisterLinkedFmsgID(idURL, subAddr, *owner) + if err != nil { return fmt.Errorf("registering derived address with fmsgid: %w", err) } + granted := false + if reinstated { + defer func() { + if !granted { + if err := middleware.DisableFmsgID(idURL, subAddr); err != nil { + fmt.Fprintf(os.Stderr, "re-disabling reinstated address %s: %v\n", subAddr, err) + } + } + }() + } if err := requireAcceptingCLIAddress(idURL, subAddr, "derived address"); err != nil { return err } @@ -72,7 +84,9 @@ func runAPIKeyCreate(ctx context.Context, args []string) error { if err := store.Create(ctx, *owner, *agent, subAddr, key.ID, hash, allowed, expires); err != nil { return err } + granted = true printCLIKey(*owner, *agent, subAddr, key) + fmt.Printf("reinstated=%t\n", reinstated) return nil } diff --git a/cmd/fmsg-webapi/apikey_cli_integration_test.go b/cmd/fmsg-webapi/apikey_cli_integration_test.go index 6bf0b68..d05e65b 100644 --- a/cmd/fmsg-webapi/apikey_cli_integration_test.go +++ b/cmd/fmsg-webapi/apikey_cli_integration_test.go @@ -287,3 +287,76 @@ func TestAPIKeyExchangeRejectsUnregisteredDerivedAddress(t *testing.T) { t.Fatalf("unregistered address exchange status = %d", w.Code) } } + +// TestAPIKeyCLICreateReinstates creates a derived sub-account whose address was +// used before: fmsgid reinstates it, the CLI says so, and when the grant can't +// be created the address is stopped accepting new messages again. +func TestAPIKeyCLICreateReinstates(t *testing.T) { + for _, conflict := range []bool{false, true} { + t.Run(fmt.Sprintf("conflict=%v", conflict), func(t *testing.T) { + database := newAPIKeyCLIDB(t) + const owner = "@alice@example.com" + const target = "@alice_bot@example.com" + if conflict { + key, err := apiauth.GenerateAPIKey() + if err != nil { + t.Fatal(err) + } + if err := apiauth.NewStore(database).Create(context.Background(), owner, "bot", target, key.ID, + apiauth.HashAPIKey(key.Value), []string{"127.0.0.0/8"}, time.Now().Add(time.Hour)); err != nil { + t.Fatal(err) + } + } + var mu sync.Mutex + accepting := false // the address exists and was disabled + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + defer mu.Unlock() + switch { + case r.Method == http.MethodGet && r.URL.Path == "/fmsgid/"+owner: + _, _ = w.Write([]byte(`{"acceptingNew":true}`)) + case r.Method == http.MethodGet && r.URL.Path == "/fmsgid/"+target: + _ = json.NewEncoder(w).Encode(map[string]bool{"acceptingNew": accepting}) + case r.Method == http.MethodPost && r.URL.Path == "/fmsgid": + var body struct { + Address string `json:"address"` + AcceptingNew *bool `json:"accepting_new"` + QuotaPool string `json:"quota_pool"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil || body.Address != target || body.AcceptingNew == nil { + t.Errorf("unexpected fmsgid update: %+v, err=%v", body, err) + w.WriteHeader(http.StatusBadRequest) + return + } + if *body.AcceptingNew && body.QuotaPool != owner { + t.Errorf("registration quota_pool = %q, want %s", body.QuotaPool, owner) + } + reinstated := !accepting && *body.AcceptingNew + accepting = *body.AcceptingNew + _ = json.NewEncoder(w).Encode(map[string]bool{"created": false, "reinstated": reinstated}) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL) + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + t.Setenv("FMSG_ID_URL", server.URL) + output, err := runCapturedAPIKeyCLI(t, []string{"create", "-owner", owner, "-agent", "bot", "-cidr", "127.0.0.0/8", + "-expires", time.Now().Add(time.Hour).UTC().Format(time.RFC3339)}) + mu.Lock() + defer mu.Unlock() + if conflict { + if err == nil { + t.Fatal("create succeeded despite an existing grant") + } + if accepting { + t.Fatal("address left reinstated although its grant wasn't created") + } + return + } + if err != nil || !strings.Contains(output, "\nreinstated=true\n") || !accepting { + t.Fatalf("create: err=%v accepting=%v output=%q", err, accepting, output) + } + }) + } +} diff --git a/internal/handlers/subaccounts.go b/internal/handlers/subaccounts.go index a0bfa70..ccf713c 100644 --- a/internal/handlers/subaccounts.go +++ b/internal/handlers/subaccounts.go @@ -46,6 +46,14 @@ type subAccountResponse struct { APIKey string `json:"api_key,omitempty"` } +type createSubAccountResponse struct { + subAccountResponse + // Reinstated is true when the derived address had been used before (its + // grant was deleted) and is now accepting new messages again, with its + // earlier messages still there. + Reinstated bool `json:"reinstated"` +} + func (h *SubAccountHandler) List(c *gin.Context) { owner, ok := requireIdPOwner(c) if !ok { @@ -126,13 +134,25 @@ func (h *SubAccountHandler) Create(c *gin.Context) { return } // Derived sub-accounts belong to an already-verified owner, so register - // them in fmsgid automatically rather than requiring a manual step. - // RegisterFmsgID never touches an address that already exists there. - if err := middleware.RegisterFmsgID(h.idURL, subAddr); err != nil { + // them in fmsgid automatically, in the owner's quota pool, rather than + // requiring a manual step. Only this owner can derive subAddr, so if it + // already exists there it was theirs before and is reinstated. + reinstated, err := middleware.RegisterLinkedFmsgID(h.idURL, subAddr, owner) + if err != nil { log.Printf("sub-account create: fmsgid registration: addr=%s: %v", subAddr, err) c.JSON(http.StatusServiceUnavailable, gin.H{"error": "identity service unavailable"}) return } + // undo a reinstatement if no grant comes of it, so a deleted address + // doesn't start accepting new messages again without one + granted := false + if reinstated { + defer func() { + if !granted { + disableAfterFailedCreate(h.idURL, subAddr) + } + }() + } if !checkAcceptingFmsgID(c, h.idURL, subAddr) { return } @@ -147,17 +167,30 @@ func (h *SubAccountHandler) Create(c *gin.Context) { respondSubAccountStoreError(c, err) return } - c.JSON(http.StatusCreated, subAccountResponse{ - Agent: in.Agent, - Addr: subAddr, - GrantType: apiauth.GrantTypeDerivedSubAccount, - KeyID: key.ID, - AllowedCIDRs: in.AllowedCIDRs, - KeyExpiresAt: expires.UTC().Format(time.RFC3339), - APIKey: key.Value, + granted = true + c.JSON(http.StatusCreated, createSubAccountResponse{ + subAccountResponse: subAccountResponse{ + Agent: in.Agent, + Addr: subAddr, + GrantType: apiauth.GrantTypeDerivedSubAccount, + KeyID: key.ID, + AllowedCIDRs: in.AllowedCIDRs, + KeyExpiresAt: expires.UTC().Format(time.RFC3339), + APIKey: key.Value, + }, + Reinstated: reinstated, }) } +// disableAfterFailedCreate stops a reinstated derived address accepting new +// messages again after its grant couldn't be created. It is best effort: the +// create has already failed, so a failure here is only logged. +func disableAfterFailedCreate(idURL, addr string) { + if err := middleware.DisableFmsgID(idURL, addr); err != nil { + log.Printf("sub-account create: re-disabling reinstated address after failure: addr=%s: %v", addr, err) + } +} + func (h *SubAccountHandler) RotateKey(c *gin.Context) { owner, ok := requireIdPOwner(c) if !ok { @@ -256,6 +289,25 @@ func (h *SubAccountHandler) Delete(c *gin.Context) { c.JSON(http.StatusBadRequest, gin.H{"error": "invalid agent"}) return } + account, err := h.store.Get(c.Request.Context(), owner, agent) + if err != nil { + respondSubAccountStoreError(c, err) + return + } + // A derived address exists only for its grant, so stop it accepting new + // messages: fmsg hosts then refuse mail to it. Do it before deleting the + // grant so that if fmsgid fails the grant is still there and the owner can + // retry; deleting first could leave the address accepting mail with + // nothing left to retry from. Its messages are kept and creating the + // sub-account again reinstates it. A delegated identity is an address in + // its own right and is left as it is. + if account.GrantType == apiauth.GrantTypeDerivedSubAccount { + if err := middleware.DisableFmsgID(h.idURL, account.Addr); err != nil { + log.Printf("sub-account delete: fmsgid disable: addr=%s: %v", account.Addr, err) + c.JSON(http.StatusServiceUnavailable, gin.H{"error": "identity service unavailable"}) + return + } + } if err := h.store.Delete(c.Request.Context(), owner, agent); err != nil { respondSubAccountStoreError(c, err) return diff --git a/internal/handlers/subaccounts_lifecycle_test.go b/internal/handlers/subaccounts_lifecycle_test.go new file mode 100644 index 0000000..e014dc6 --- /dev/null +++ b/internal/handlers/subaccounts_lifecycle_test.go @@ -0,0 +1,332 @@ +package handlers + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "strings" + "sync" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/markmnl/fmsg-webapi/internal/apiauth" + "github.com/markmnl/fmsg-webapi/internal/db" + "github.com/markmnl/fmsg-webapi/internal/middleware" +) + +// fakeFmsgID is an in-memory fmsgid serving GET /fmsgid/:address and +// POST /fmsgid with accepting_new and quota_pool. +type fakeFmsgID struct { + mu sync.Mutex + accepting map[string]bool + pool map[string]string + posts []map[string]any + // postStatus, when set, is returned by POST /fmsgid without changing anything. + postStatus int + // legacy answers POST /fmsgid like an fmsgid without accepting_new: it + // ignores accepting_new and quota_pool and returns no body. + legacy bool +} + +func newFakeFmsgID(t *testing.T, addrs ...string) (*fakeFmsgID, *httptest.Server) { + t.Helper() + f := &fakeFmsgID{accepting: map[string]bool{}, pool: map[string]string{}} + for _, a := range addrs { + f.accepting[strings.ToLower(a)] = true + } + server := httptest.NewServer(f) + t.Cleanup(server.Close) + return f, server +} + +func (f *fakeFmsgID) ServeHTTP(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + switch { + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/fmsgid/"): + accepting, ok := f.accepting[strings.ToLower(strings.TrimPrefix(r.URL.Path, "/fmsgid/"))] + if !ok { + w.WriteHeader(http.StatusNotFound) + return + } + _ = json.NewEncoder(w).Encode(map[string]bool{"acceptingNew": accepting}) + case r.Method == http.MethodPost && r.URL.Path == "/fmsgid": + var body map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + f.posts = append(f.posts, body) + if f.postStatus != 0 { + w.WriteHeader(f.postStatus) + return + } + addr := strings.ToLower(body["address"].(string)) + was, existed := f.accepting[addr] + if !existed { + f.accepting[addr] = true + } + if f.legacy { + if existed { + w.WriteHeader(http.StatusOK) + } else { + w.WriteHeader(http.StatusCreated) + } + return + } + if a, ok := body["accepting_new"].(bool); ok { + f.accepting[addr] = a + } + if p, ok := body["quota_pool"].(string); ok { + f.pool[addr] = strings.ToLower(p) + } + status := http.StatusCreated + if existed { + status = http.StatusOK + } + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(map[string]bool{ + "created": !existed, + "reinstated": existed && !was && f.accepting[addr], + }) + default: + w.WriteHeader(http.StatusNotFound) + } +} + +func (f *fakeFmsgID) isAccepting(addr string) (accepting, exists bool) { + f.mu.Lock() + defer f.mu.Unlock() + accepting, exists = f.accepting[strings.ToLower(addr)] + return accepting, exists +} + +func (f *fakeFmsgID) postCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.posts) +} + +func newSubAccountTestStore(t *testing.T) *apiauth.Store { + t.Helper() + dsn := os.Getenv("FMSG_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("set FMSG_TEST_DATABASE_URL to run PostgreSQL integration tests") + } + ctx := context.Background() + admin, err := pgxpool.New(ctx, dsn) + if err != nil { + t.Fatal(err) + } + schema := fmt.Sprintf("sub_account_%d", time.Now().UnixNano()) + if _, err := admin.Exec(ctx, "CREATE SCHEMA "+schema); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _, _ = admin.Exec(ctx, "DROP SCHEMA "+schema+" CASCADE"); admin.Close() }) + config, err := pgxpool.ParseConfig(dsn) + if err != nil { + t.Fatal(err) + } + config.ConnConfig.RuntimeParams["search_path"] = schema + pool, err := pgxpool.NewWithConfig(ctx, config) + if err != nil { + t.Fatal(err) + } + t.Cleanup(pool.Close) + dd, err := os.ReadFile("../../dd.sql") + if err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, string(dd)); err != nil { + t.Fatal(err) + } + return apiauth.NewStore(&db.DB{Pool: pool}) +} + +const lifecycleOwner = "@alice@example.com" + +// subAccountRouter serves the sub-account routes as lifecycleOwner, signed in +// with the identity provider. +func subAccountRouter(h *SubAccountHandler) *gin.Engine { + gin.SetMode(gin.TestMode) + r := gin.New() + r.Use(func(c *gin.Context) { + c.Set(middleware.AuthTypeKey, middleware.AuthTypeIdP) + c.Set(middleware.IdentityKey, lifecycleOwner) + c.Set(middleware.OwnerIdentityKey, lifecycleOwner) + }) + r.POST("/fmsg/sub-accounts", h.Create) + r.DELETE("/fmsg/sub-accounts/:agent", h.Delete) + return r +} + +func serveSubAccount(r *gin.Engine, method, path, body string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + return w +} + +func createSubAccount(t *testing.T, r *gin.Engine, agent string, wantCode int) map[string]any { + t.Helper() + body := fmt.Sprintf(`{"agent":%q,"allowed_cidrs":["127.0.0.0/8"],"key_expires_at":%q}`, + agent, time.Now().Add(time.Hour).UTC().Format(time.RFC3339)) + w := serveSubAccount(r, http.MethodPost, "/fmsg/sub-accounts", body) + if w.Code != wantCode { + t.Fatalf("create %s: status %d, want %d: %s", agent, w.Code, wantCode, w.Body.String()) + } + var out map[string]any + _ = json.Unmarshal(w.Body.Bytes(), &out) + return out +} + +// TestSubAccountLifecycle deletes a derived sub-account, which stops its +// address accepting new messages, and creates it again, which reinstates it. +func TestSubAccountLifecycle(t *testing.T) { + store := newSubAccountTestStore(t) + id, server := newFakeFmsgID(t, lifecycleOwner) + r := subAccountRouter(NewSubAccountHandler(store, server.URL)) + const addr = "@alice_bot@example.com" + + out := createSubAccount(t, r, "bot", http.StatusCreated) + if out["reinstated"] != false || out["addr"] != addr || out["api_key"] == "" { + t.Fatalf("new sub-account response: %v", out) + } + if accepting, _ := id.isAccepting(addr); !accepting { + t.Fatal("new sub-account address not accepting new messages") + } + if got := id.pool[addr]; got != lifecycleOwner { + t.Fatalf("new sub-account quota_pool = %q, want the owner %s", got, lifecycleOwner) + } + + if w := serveSubAccount(r, http.MethodDelete, "/fmsg/sub-accounts/bot", ""); w.Code != http.StatusNoContent { + t.Fatalf("delete: status %d: %s", w.Code, w.Body.String()) + } + if accepting, exists := id.isAccepting(addr); accepting || !exists { + t.Fatalf("deleted sub-account address: accepting=%v exists=%v, want kept and not accepting", accepting, exists) + } + if _, err := store.Get(context.Background(), lifecycleOwner, "bot"); err != apiauth.ErrNotFound { + t.Fatalf("grant after delete: err=%v, want not found", err) + } + + out = createSubAccount(t, r, "bot", http.StatusCreated) + if out["reinstated"] != true { + t.Fatalf("re-created sub-account response: %v, want reinstated", out) + } + if accepting, _ := id.isAccepting(addr); !accepting { + t.Fatal("re-created sub-account address not accepting new messages") + } + + // creating it while its grant exists conflicts and leaves it accepting + createSubAccount(t, r, "bot", http.StatusConflict) + if accepting, _ := id.isAccepting(addr); !accepting { + t.Fatal("conflicting create disabled the address") + } +} + +// TestSubAccountDeleteFmsgIDFailure keeps the grant when fmsgid can't stop the +// address accepting new messages, so the delete can be retried. +func TestSubAccountDeleteFmsgIDFailure(t *testing.T) { + for _, tc := range []struct { + name string + postStatus int + legacy bool + }{ + {name: "unavailable", postStatus: http.StatusServiceUnavailable}, + {name: "fmsgid without accepting_new", legacy: true}, + } { + t.Run(tc.name, func(t *testing.T) { + store := newSubAccountTestStore(t) + id, server := newFakeFmsgID(t, lifecycleOwner) + r := subAccountRouter(NewSubAccountHandler(store, server.URL)) + createSubAccount(t, r, "bot", http.StatusCreated) + + id.mu.Lock() + id.postStatus, id.legacy = tc.postStatus, tc.legacy + id.mu.Unlock() + if w := serveSubAccount(r, http.MethodDelete, "/fmsg/sub-accounts/bot", ""); w.Code != http.StatusServiceUnavailable { + t.Fatalf("delete: status %d, want 503", w.Code) + } + if _, err := store.Get(context.Background(), lifecycleOwner, "bot"); err != nil { + t.Fatalf("grant deleted although fmsgid failed: %v", err) + } + + id.mu.Lock() + id.postStatus, id.legacy = 0, false + id.mu.Unlock() + if w := serveSubAccount(r, http.MethodDelete, "/fmsg/sub-accounts/bot", ""); w.Code != http.StatusNoContent { + t.Fatalf("retried delete: status %d", w.Code) + } + if accepting, _ := id.isAccepting("@alice_bot@example.com"); accepting { + t.Fatal("address accepting new messages after retried delete") + } + }) + } +} + +// TestSubAccountDeleteDelegatedIdentity leaves a delegated identity's address +// alone: it is an address in its own right, not one derived for the grant. +func TestSubAccountDeleteDelegatedIdentity(t *testing.T) { + store := newSubAccountTestStore(t) + id, server := newFakeFmsgID(t, lifecycleOwner, "@sales@example.com") + r := subAccountRouter(NewSubAccountHandler(store, server.URL)) + key, err := apiauth.GenerateAPIKey() + if err != nil { + t.Fatal(err) + } + if err := store.CreateDelegated(context.Background(), lifecycleOwner, "sales", "@sales@example.com", "", + key.ID, apiauth.HashAPIKey(key.Value), []string{"127.0.0.0/8"}, time.Now().Add(time.Hour)); err != nil { + t.Fatal(err) + } + if w := serveSubAccount(r, http.MethodDelete, "/fmsg/sub-accounts/sales", ""); w.Code != http.StatusNoContent { + t.Fatalf("delete: status %d", w.Code) + } + if n := id.postCount(); n != 0 { + t.Fatalf("delegated delete sent %d fmsgid updates, want none", n) + } + if accepting, _ := id.isAccepting("@sales@example.com"); !accepting { + t.Fatal("delegated identity stopped accepting new messages") + } + if w := serveSubAccount(r, http.MethodDelete, "/fmsg/sub-accounts/nope", ""); w.Code != http.StatusNotFound { + t.Fatalf("delete missing grant: status %d, want 404", w.Code) + } +} + +// TestSubAccountCreateFailureUndoesReinstatement re-disables a reinstated +// address when its grant can't be created. +func TestSubAccountCreateFailureUndoesReinstatement(t *testing.T) { + store := newSubAccountTestStore(t) + id, server := newFakeFmsgID(t, lifecycleOwner) + r := subAccountRouter(NewSubAccountHandler(store, server.URL)) + + createSubAccount(t, r, "old", http.StatusCreated) + if w := serveSubAccount(r, http.MethodDelete, "/fmsg/sub-accounts/old", ""); w.Code != http.StatusNoContent { + t.Fatalf("delete: status %d", w.Code) + } + for i := range apiauth.DefaultMaxSubAccounts { + createSubAccount(t, r, fmt.Sprintf("bot%d", i), http.StatusCreated) + } + createSubAccount(t, r, "old", http.StatusForbidden) + if accepting, _ := id.isAccepting("@alice_old@example.com"); accepting { + t.Fatal("address reinstated although its grant wasn't created") + } +} + +// TestSubAccountCreateLegacyFmsgID creates a sub-account with an fmsgid that +// answers registration without a body: it is simply not reinstated. +func TestSubAccountCreateLegacyFmsgID(t *testing.T) { + store := newSubAccountTestStore(t) + id, server := newFakeFmsgID(t, lifecycleOwner) + id.legacy = true + r := subAccountRouter(NewSubAccountHandler(store, server.URL)) + if out := createSubAccount(t, r, "bot", http.StatusCreated); out["reinstated"] != false { + t.Fatalf("response %v, want reinstated false", out) + } +} diff --git a/internal/middleware/fmsgid_test.go b/internal/middleware/fmsgid_test.go index 54ac904..5abf046 100644 --- a/internal/middleware/fmsgid_test.go +++ b/internal/middleware/fmsgid_test.go @@ -37,7 +37,7 @@ func TestCheckFmsgIDSeparatesServices(t *testing.T) { } } -func TestRegisterFmsgIDRefreshesExistingAddress(t *testing.T) { +func TestRegisterLinkedFmsgIDRefreshesExistingAddress(t *testing.T) { for _, status := range []int{http.StatusOK, http.StatusCreated} { t.Run(http.StatusText(status), func(t *testing.T) { const addr = "@alice_bot@example.com" @@ -45,14 +45,16 @@ func TestRegisterFmsgIDRefreshesExistingAddress(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodPost && r.URL.Path == "/fmsgid": - var body struct{ Address string } - if err := json.NewDecoder(r.Body).Decode(&body); err != nil || body.Address != addr { - t.Errorf("registration payload: address=%q, err=%v", body.Address, err) + var body map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err != nil || body["address"] != addr || + body["accepting_new"] != true || body["quota_pool"] != "@alice@example.com" { + t.Errorf("registration payload: %v, err=%v", body, err) w.WriteHeader(http.StatusBadRequest) return } accepting.Store(true) w.WriteHeader(status) + _, _ = w.Write([]byte(`{"created":true,"reinstated":false}`)) case r.Method == http.MethodGet && r.URL.Path == "/fmsgid/"+addr: _ = json.NewEncoder(w).Encode(map[string]bool{"acceptingNew": accepting.Load()}) default: @@ -64,8 +66,8 @@ func TestRegisterFmsgIDRefreshesExistingAddress(t *testing.T) { if _, ok, err := CheckFmsgID(server.URL, addr); err != nil || ok { t.Fatalf("initial lookup: accepting=%v, err=%v", ok, err) } - if err := RegisterFmsgID(server.URL+"/", addr); err != nil { - t.Fatal(err) + if reinstated, err := RegisterLinkedFmsgID(server.URL+"/", addr, "@alice@example.com"); err != nil || reinstated { + t.Fatalf("registration: reinstated=%v, err=%v", reinstated, err) } if _, ok, err := CheckFmsgID(server.URL, addr); err != nil || !ok { t.Fatalf("lookup after registration: accepting=%v, err=%v", ok, err) @@ -74,6 +76,96 @@ func TestRegisterFmsgIDRefreshesExistingAddress(t *testing.T) { } } +func TestRegisterLinkedFmsgIDResponses(t *testing.T) { + for _, tc := range []struct { + name string + status int + body string + wantReinstated bool + wantErr bool + }{ + {name: "created", status: http.StatusCreated, body: `{"created":true,"reinstated":false}`}, + {name: "existing", status: http.StatusOK, body: `{"created":false,"reinstated":false}`}, + {name: "reinstated", status: http.StatusOK, body: `{"created":false,"reinstated":true}`, wantReinstated: true}, + {name: "fmsgid without accepting_new", status: http.StatusOK, body: ""}, + {name: "malformed", status: http.StatusOK, body: "", wantErr: true}, + {name: "quota pool refused", status: http.StatusUnprocessableEntity, body: `{"error":"quota_pool address not found"}`, wantErr: true}, + {name: "unavailable", status: http.StatusServiceUnavailable, wantErr: true}, + } { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tc.status) + _, _ = w.Write([]byte(tc.body)) + })) + defer server.Close() + reinstated, err := RegisterLinkedFmsgID(server.URL, "@alice_bot@example.com", "@alice@example.com") + if (err != nil) != tc.wantErr || reinstated != tc.wantReinstated { + t.Fatalf("reinstated=%v, err=%v; want reinstated=%v, error=%v", reinstated, err, tc.wantReinstated, tc.wantErr) + } + }) + } +} + +func TestDisableFmsgID(t *testing.T) { + const addr = "@alice_old@example.com" + var accepting atomic.Bool + accepting.Store(true) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/fmsgid": + var body map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err != nil || body["address"] != addr || + body["accepting_new"] != false || body["quota_pool"] != nil { + t.Errorf("disable payload: %v, err=%v", body, err) + w.WriteHeader(http.StatusBadRequest) + return + } + accepting.Store(false) + _, _ = w.Write([]byte(`{"created":false,"reinstated":false}`)) + case r.Method == http.MethodGet && r.URL.Path == "/fmsgid/"+addr: + _ = json.NewEncoder(w).Encode(map[string]bool{"acceptingNew": accepting.Load()}) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL) + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + if _, ok, err := CheckFmsgID(server.URL, addr); err != nil || !ok { + t.Fatalf("initial lookup: accepting=%v, err=%v", ok, err) + } + if err := DisableFmsgID(server.URL, addr); err != nil { + t.Fatal(err) + } + if _, ok, err := CheckFmsgID(server.URL, addr); err != nil || ok { + t.Fatalf("lookup after disabling (cache must be dropped): accepting=%v, err=%v", ok, err) + } +} + +// An fmsgid that ignores accepting_new answers without a body; disabling must +// not be reported as done. +func TestDisableFmsgIDFailures(t *testing.T) { + for _, tc := range []struct { + name string + status int + body string + }{ + {"fmsgid without accepting_new", http.StatusOK, ""}, + {"unavailable", http.StatusServiceUnavailable, ""}, + {"forbidden", http.StatusForbidden, `{"error":"domain not active"}`}, + } { + t.Run(tc.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tc.status) + _, _ = w.Write([]byte(tc.body)) + })) + defer server.Close() + if err := DisableFmsgID(server.URL, "@alice_bot@example.com"); err == nil { + t.Fatal("disable reported success") + } + }) + } +} + func TestCheckFmsgIDEscapesAddress(t *testing.T) { const addr = "@alice?bot#1%tag@example.com" server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/middleware/jwt.go b/internal/middleware/jwt.go index 7b4f629..c7d08f1 100644 --- a/internal/middleware/jwt.go +++ b/internal/middleware/jwt.go @@ -8,6 +8,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "log" "net/http" "net/url" @@ -516,28 +517,89 @@ func CheckFmsgID(idURL, addr string) (int, bool, error) { return res.code, res.acceptingNew, nil } -// RegisterFmsgID registers addr with fmsgid using default quotas, idempotently. -// It never modifies an address that already exists there. -func RegisterFmsgID(idURL, addr string) error { - payload, err := json.Marshal(struct { - Address string `json:"address"` - }{Address: addr}) +// fmsgIDAddressReq is the body of fmsgid's POST /fmsgid, which registers an +// address idempotently and applies accepting_new and quota_pool, when given, +// whether or not the address already existed. +type fmsgIDAddressReq struct { + Address string `json:"address"` + AcceptingNew *bool `json:"accepting_new,omitempty"` + QuotaPool string `json:"quota_pool,omitempty"` +} + +// fmsgIDAddressResp is fmsgid's answer to POST /fmsgid. fmsgid versions that +// don't support accepting_new and quota_pool answer with no body. +type fmsgIDAddressResp struct { + Created bool `json:"created"` + Reinstated bool `json:"reinstated"` +} + +// errFmsgIDNoAcceptingNew is returned when fmsgid answers POST /fmsgid without +// a body, i.e. it is a version that ignores accepting_new. +var errFmsgIDNoAcceptingNew = errors.New("fmsgid does not support accepting_new on POST /fmsgid; upgrade fmsgid") + +// RegisterLinkedFmsgID registers a linked address — a derived sub-account +// address such as @alice_bot@example.com — with fmsgid, idempotently, in its +// owner's quota pool and accepting new messages. Only its owner can derive a +// linked address, so one that already exists there was this owner's before: +// it is reinstated, with its earlier messages, and reinstated reports whether +// it had stopped accepting new messages (e.g. its grant was deleted). +func RegisterLinkedFmsgID(idURL, addr, owner string) (reinstated bool, err error) { + accepting := true + resp, err := postFmsgIDAddress(idURL, fmsgIDAddressReq{Address: addr, AcceptingNew: &accepting, QuotaPool: owner}) + if err != nil { + return false, err + } + return resp != nil && resp.Reinstated, nil +} + +// DisableFmsgID stops addr accepting new messages in fmsgid, so fmsg hosts +// refuse new messages to it. The address, its messages and its usage are kept, +// so its owner can reinstate it with RegisterLinkedFmsgID. +func DisableFmsgID(idURL, addr string) error { + accepting := false + resp, err := postFmsgIDAddress(idURL, fmsgIDAddressReq{Address: addr, AcceptingNew: &accepting}) if err != nil { return err } + if resp == nil { + return errFmsgIDNoAcceptingNew + } + return nil +} + +// postFmsgIDAddress POSTs req to fmsgid's /fmsgid and drops the cached lookup +// of the address. The response is nil when fmsgid answers without a body. +func postFmsgIDAddress(idURL string, req fmsgIDAddressReq) (*fmsgIDAddressResp, error) { + payload, err := json.Marshal(req) + if err != nil { + return nil, err + } url := strings.TrimRight(idURL, "/") + "/fmsgid" resp, err := fmsgIDClient.Post(url, "application/json", bytes.NewReader(payload)) //nolint:gosec // URL constructed from trusted config if err != nil { - return err + return nil, err } defer resp.Body.Close() + // whatever fmsgid did, the cached lookup may now be stale + fmsgIDCache.Delete(fmsgIDAddressURL(idURL, req.Address)) + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { - return fmt.Errorf("fmsgid registration failed: status %d", resp.StatusCode) + return nil, fmt.Errorf("fmsgid registration failed: status %d", resp.StatusCode) } - fmsgIDCache.Delete(fmsgIDAddressURL(idURL, addr)) - return nil + body, err := io.ReadAll(io.LimitReader(resp.Body, 64<<10)) + if err != nil { + return nil, fmt.Errorf("reading fmsgid registration response: %w", err) + } + if len(bytes.TrimSpace(body)) == 0 { + return nil, nil + } + var out fmsgIDAddressResp + if err := json.Unmarshal(body, &out); err != nil { + return nil, fmt.Errorf("decoding fmsgid registration response: %w", err) + } + return &out, nil } func fmsgIDAddressURL(idURL, addr string) string {