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 {