From 568452846ae3f92a4780d55c9f7586f91be1e8c6 Mon Sep 17 00:00:00 2001 From: Vassiliy Yegorov Date: Sat, 4 Jul 2026 20:12:41 +0700 Subject: [PATCH] feat(api): structured provider credentials + trial-auth validation on account create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /accounts now accepts secret as a provider-specific JSON object instead of an opaque string, and validates credentials via provider.Provider.Validate before persisting — invalid credentials get a generic 400 without ever reaching Store.CreateAccount or echoing the secret back. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BwxdSt4reTm7Dj1oxRvpP3 --- internal/api/tenant_dto.go | 11 +++++-- internal/api/tenant_handlers.go | 15 ++++++++-- internal/api/tenant_test.go | 53 ++++++++++++++++++++++++++++----- 3 files changed, 66 insertions(+), 13 deletions(-) diff --git a/internal/api/tenant_dto.go b/internal/api/tenant_dto.go index 613b1c8..1ec0b6c 100644 --- a/internal/api/tenant_dto.go +++ b/internal/api/tenant_dto.go @@ -1,16 +1,21 @@ package api import ( + "encoding/json" + "github.com/google/uuid" "github.com/vasyakrg/dns-autoresolver/internal/store" "github.com/vasyakrg/dns-autoresolver/internal/store/dto" ) +// accountRequest.Secret is a provider-specific JSON object (e.g. Selectel's +// service-user credentials) rather than an opaque string — it is passed +// through as raw bytes to Validate/Encrypt, never parsed here. type accountRequest struct { - Provider string `json:"provider"` - Secret string `json:"secret"` - Comment string `json:"comment"` + Provider string `json:"provider"` + Secret json.RawMessage `json:"secret"` + Comment string `json:"comment"` } // accountResponse deliberately excludes the secret (plaintext or encrypted). diff --git a/internal/api/tenant_handlers.go b/internal/api/tenant_handlers.go index 13711a4..4a93a68 100644 --- a/internal/api/tenant_handlers.go +++ b/internal/api/tenant_handlers.go @@ -31,11 +31,22 @@ func (a *API) handleCreateAccount(w http.ResponseWriter, r *http.Request) { if !decodeBody(w, r, &req) { return } - if req.Provider == "" || req.Secret == "" { + if req.Provider == "" || len(req.Secret) == 0 { writeErr(w, http.StatusBadRequest, "provider and secret are required") return } - secretEnc, err := a.Cipher.Encrypt([]byte(req.Secret)) + p, err := a.Reg.ByName(req.Provider) + if err != nil { + writeErr(w, http.StatusBadRequest, "unknown provider") + return + } + // Trial auth up-front so bad credentials fail at creation, not at import. + // The error text is deliberately generic — never echo the secret back. + if err := p.Validate(r.Context(), provider.Credentials{Secret: string(req.Secret)}); err != nil { + writeErr(w, http.StatusBadRequest, "invalid provider credentials") + return + } + secretEnc, err := a.Cipher.Encrypt(req.Secret) if err != nil { log.Printf("api: encrypt secret failed: %v", err) writeErr(w, http.StatusInternalServerError, "internal error") diff --git a/internal/api/tenant_test.go b/internal/api/tenant_test.go index a3a6db7..2eaea37 100644 --- a/internal/api/tenant_test.go +++ b/internal/api/tenant_test.go @@ -140,15 +140,19 @@ func (mockCipher) Decrypt(enc string) ([]byte, error) { } type mockRegistry struct { - zones []provider.Zone + zones []provider.Zone + validateErr error } func (r *mockRegistry) ByName(name string) (provider.Provider, error) { - return &mockProvider{zones: r.zones}, nil + return &mockProvider{zones: r.zones, validateErr: r.validateErr}, nil } type mockProvider struct { zones []provider.Zone + // validateErr, when set, makes Validate reject the credentials — lets + // tests exercise the 400-before-save path of handleCreateAccount. + validateErr error } func (mockProvider) Name() string { return "mock" } @@ -161,7 +165,9 @@ func (mockProvider) GetRecords(context.Context, provider.Credentials, string) ([ func (mockProvider) ApplyChanges(context.Context, provider.Credentials, string, diff.Changeset) error { return nil } -func (mockProvider) Validate(context.Context, provider.Credentials) error { return nil } +func (p mockProvider) Validate(context.Context, provider.Credentials) error { + return p.validateErr +} // newTenantTestAPI wires a fixed authenticated user who owns whatever // project id is requested (alwaysOwnedAuthStore/alwaysValidSessions, see @@ -179,11 +185,16 @@ func newTenantTestAPI() (*API, *mockTenantStore) { // --- accounts --- -func TestCreateAccount_SecretEncryptedAndNotInResponse(t *testing.T) { +// TestCreateAccount_ValidCredentials_EncryptsRawSecretAndCreates covers the +// happy path of the structured-secret contract: secret is a provider-specific +// JSON object, Validate accepts it, and the *raw* JSON (not a re-serialized +// or unwrapped form) is what gets encrypted and handed to the store. +func TestCreateAccount_ValidCredentials_EncryptsRawSecretAndCreates(t *testing.T) { a, ts := newTenantTestAPI() + a.Reg = &mockRegistry{} router := NewRouter(a) - body := `{"provider":"selectel","secret":"super-secret-token","comment":"prod"}` + body := `{"provider":"selectel","secret":{"username":"u","password":"super-secret-token","account_id":"123","project_name":"proj"},"comment":"prod"}` req := requestWithSessionCookie(http.MethodPost, "/api/v1/projects/"+testPID+"/accounts", strings.NewReader(body)) w := httptest.NewRecorder() router.ServeHTTP(w, req) @@ -198,11 +209,12 @@ func TestCreateAccount_SecretEncryptedAndNotInResponse(t *testing.T) { t.Fatalf("expected 1 CreateAccount call, got %d", len(ts.createAccounts)) } got := ts.createAccounts[0].secretEnc - if got == "super-secret-token" { + if got == `{"username":"u","password":"super-secret-token","account_id":"123","project_name":"proj"}` { t.Fatalf("store received plaintext secret instead of encrypted value") } - if got != "ENC(super-secret-token)" { - t.Fatalf("unexpected encrypted secret stored: %q", got) + wantEnc := `ENC({"username":"u","password":"super-secret-token","account_id":"123","project_name":"proj"})` + if got != wantEnc { + t.Fatalf("unexpected encrypted secret stored: %q, want %q", got, wantEnc) } var resp accountResponse @@ -214,6 +226,31 @@ func TestCreateAccount_SecretEncryptedAndNotInResponse(t *testing.T) { } } +// TestCreateAccount_InvalidCredentials_Returns400BeforeSave covers the +// trial-auth gate: when the provider rejects the credentials, the handler +// must reject with 400 and a generic message, and must never reach +// Store.CreateAccount (so no bad account is persisted). +func TestCreateAccount_InvalidCredentials_Returns400BeforeSave(t *testing.T) { + a, ts := newTenantTestAPI() + a.Reg = &mockRegistry{validateErr: errors.New("identity: invalid password")} + router := NewRouter(a) + + body := `{"provider":"selectel","secret":{"username":"u","password":"wrong","account_id":"123","project_name":"proj"},"comment":"prod"}` + req := requestWithSessionCookie(http.MethodPost, "/api/v1/projects/"+testPID+"/accounts", strings.NewReader(body)) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d body %s", w.Code, w.Body.String()) + } + if len(ts.createAccounts) != 0 { + t.Fatalf("expected CreateAccount not to be called, got %d calls", len(ts.createAccounts)) + } + if strings.Contains(w.Body.String(), "wrong") || strings.Contains(w.Body.String(), "invalid password") { + t.Fatalf("response leaks credential/provider error detail: %s", w.Body.String()) + } +} + func TestListAccounts_NoSecretsInResponse(t *testing.T) { a, ts := newTenantTestAPI() ts.accounts = []store.Account{