fix: say the true thing when Gmail, AI or mail is not set up

Two bugs with one shape: the interface confidently told you something
that was not so.

Connecting Gmail without a registered OAuth client said "Det er der
allerede." Thirteen different situations answered a plain `conflict`, and
the frontend renders that as "that already exists" — which for four of
them is not vague but wrong. An unconfigured Gmail is not a Gmail that is
already connected; a second factor that is off is not one that is on; the
Inbox is not a project that already exists somewhere else. Those four
have their own codes now, and their own sentences.

The mail-to-task address came out as todo+…@localhost. VERDANDE_SMTP_FROM
defaults to verdande@localhost and the domain was taken from it
unconditionally, so an instance with no mail server handed out an address
that looks real, can never receive anything, and gives no hint which of
the two it is. The default no longer wins over the public hostname, and
the response now carries whether a mail server exists at all — because
even with the right domain, nothing routes to it until one does.

errors.go has always claimed the code list is kept in one place "so the
frontend's mapping can be checked against it". Nothing checked it, and
the mapping was missing bad_request. Now a test reads both files and
fails on a code with no message.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kristian Wind
2026-08-17 14:35:36 +02:00
committed by Kristian Wind
co-authored by Claude Opus 5
parent 7e28d8bb92
commit 78ec9b08a1
11 changed files with 298 additions and 14 deletions
+4
View File
@@ -52,6 +52,10 @@ what a person reads is decided by the client, which knows what language they are
| `forbidden` | 403 | Cross-site request, or admin needed. |
| `not_found` | 404 | Does not exist, or you cannot see it. |
| `conflict` | 409 | Already exists, or the state does not allow it. |
| `gmail_not_configured` | 409 | No Google OAuth client is registered on this server. |
| `ai_not_configured` | 409 | No AI provider has been set up. |
| `totp_not_enabled` | 409 | Two-factor is off, and the request needs it on. |
| `inbox_protected` | 409 | The Inbox cannot be deleted. |
| `validation_failed` | 422 | Check `fields`. |
| `rate_limited` | 429 | Too many attempts. |
| `internal_error` | 500 | Our fault. |
+11
View File
@@ -1946,6 +1946,13 @@ components:
type: object
properties:
address: { type: string, example: todo+abc123@example.dk }
configured:
type: boolean
description: |
Whether a mail server is set up at all. The address is real
either way, but with no mail server nothing on the internet
routes to it — and an interface that does not say so is
promising something that will not happen.
Gmail:
description: OK
content:
@@ -1995,6 +2002,10 @@ components:
- rate_limited
- internal_error
- payload_too_large
- gmail_not_configured
- ai_not_configured
- totp_not_enabled
- inbox_protected
error: { type: string }
fields:
type: object
+149
View File
@@ -0,0 +1,149 @@
package httpapi
import (
"os"
"path/filepath"
"regexp"
"sort"
"strings"
"testing"
)
// The comment above the code list in errors.go says they are kept in one place
// "so the frontend's mapping can be checked against it rather than discovered
// from whatever a handler happened to write". Nothing checked it, and the
// frontend was quietly missing entries — which is not a cosmetic gap: a code
// with no message falls through to the server's English log prose, and a code
// mapped to the wrong sentence tells the person something untrue. Connecting
// Gmail without an OAuth client said "Det er der allerede."
var (
codeConst = regexp.MustCompile(`(?m)^\s*Code\w+\s*=\s*"([a-z_]+)"`)
messageKey = regexp.MustCompile(`(?m)^\s*([a-z_]+):\s*$|^\s*([a-z_]+):\s*'`)
messagesTop = "const MESSAGES = {"
)
func TestEveryErrorCodeHasAMessageInTheFrontend(t *testing.T) {
codes := serverErrorCodes(t)
messages := frontendMessageKeys(t)
var missing []string
for _, code := range codes {
if !messages[code] {
missing = append(missing, code)
}
}
sort.Strings(missing)
for _, code := range missing {
t.Errorf("%q is returned by the API and has no message in web/src/lib/api.js — "+
"a person would be shown the server's English log prose", code)
}
}
func serverErrorCodes(t *testing.T) []string {
t.Helper()
source, err := os.ReadFile("errors.go")
if err != nil {
t.Fatalf("read errors.go: %v", err)
}
var codes []string
for _, m := range codeConst.FindAllStringSubmatch(string(source), -1) {
codes = append(codes, m[1])
}
if len(codes) == 0 {
t.Fatal("no codes were read out of errors.go — the scanner and the file disagree")
}
return codes
}
// frontendMessageKeys reads the keys of the MESSAGES object. A small scanner
// rather than a JS parser, for the same reason the OpenAPI check uses one: it
// needs one thing out of a file this test also owns.
func frontendMessageKeys(t *testing.T) map[string]bool {
t.Helper()
source, err := os.ReadFile(filepath.Join("..", "..", "web", "src", "lib", "api.js"))
if err != nil {
t.Fatalf("read api.js: %v", err)
}
body := string(source)
start := strings.Index(body, messagesTop)
if start < 0 {
t.Fatalf("could not find %q in api.js", messagesTop)
}
body = body[start+len(messagesTop):]
end := strings.Index(body, "\n};")
if end < 0 {
t.Fatal("the MESSAGES object does not end where this scanner expects")
}
keys := map[string]bool{}
for _, line := range strings.Split(body[:end], "\n") {
line = strings.TrimSpace(line)
if line == "" || strings.HasPrefix(line, "//") {
continue
}
name, _, ok := strings.Cut(line, ":")
if ok {
keys[strings.TrimSpace(name)] = true
}
}
if len(keys) == 0 {
t.Fatal("no message keys were read out of api.js")
}
return keys
}
// The four that used to be indistinguishable from "that already exists".
func TestTheMisleadingConflictsHaveTheirOwnCodes(t *testing.T) {
for _, code := range []string{
CodeGmailNotConfigured,
CodeAINotConfigured,
CodeTOTPNotEnabled,
CodeInboxProtected,
} {
if code == CodeConflict {
t.Errorf("%q is still plain conflict", code)
}
}
messages := frontendMessageKeys(t)
for _, code := range []string{"gmail_not_configured", "ai_not_configured",
"totp_not_enabled", "inbox_protected"} {
if !messages[code] {
t.Errorf("%q has no Danish message", code)
}
}
}
// The Inbox is the one project that cannot be deleted, and saying "that already
// exists" about it helps nobody.
func TestDeletingTheInboxSaysWhy(t *testing.T) {
ts := newTestServer(t)
ts.bootstrap(t)
_, projects := ts.do(t, "GET", "/api/v1/projects", nil)
list, _ := projects["projects"].([]any)
var inboxID string
for _, raw := range list {
p := raw.(map[string]any)
if p["is_inbox"] == true {
inboxID = p["id"].(string)
}
}
if inboxID == "" {
t.Fatal("no inbox was found")
}
resp, body := ts.do(t, "DELETE", "/api/v1/projects/"+inboxID, nil)
if resp.StatusCode != 409 {
t.Fatalf("status %d, want 409", resp.StatusCode)
}
if body["code"] != CodeInboxProtected {
t.Errorf("code = %v, want %q", body["code"], CodeInboxProtected)
}
}
+11
View File
@@ -36,6 +36,17 @@ const (
CodeRateLimited = "rate_limited"
CodeInternal = "internal_error"
CodePayloadTooLarge = "payload_too_large"
// Four situations that used to answer plain `conflict`, which the frontend
// renders as "that already exists". For these that is not merely vague, it is
// wrong: an unconfigured Gmail is not a Gmail that is already there, and a
// second factor that is off is not one that is on. A code exists so the
// interface can say the true thing — the server's own prose is English for a
// log, and deciding what a person reads is the frontend's job.
CodeGmailNotConfigured = "gmail_not_configured"
CodeAINotConfigured = "ai_not_configured"
CodeTOTPNotEnabled = "totp_not_enabled"
CodeInboxProtected = "inbox_protected"
)
func writeError(w http.ResponseWriter, status int, code, message string) {
+2 -2
View File
@@ -30,8 +30,8 @@ func (s *Server) handleGmailAuthorize(w http.ResponseWriter, r *http.Request) {
cfg := s.gmailConfig()
if !cfg.Configured() {
writeError(w, http.StatusConflict, CodeConflict,
"Gmail er ikke sat op på denne server — administratoren skal registrere en OAuth-klient hos Google")
writeError(w, http.StatusConflict, CodeGmailNotConfigured,
"no Google OAuth client is registered on this server")
return
}
+27 -7
View File
@@ -18,6 +18,12 @@ import (
type mailAddressResponse struct {
Address string `json:"address"`
// Configured says whether a mail server has been set up at all. Without one
// the address is still minted and still valid — the token is real, and the
// endpoint that receives mail works — but nothing on the internet routes to
// it yet, and an interface that shows the address without saying so is
// promising something that will not happen.
Configured bool `json:"configured"`
}
// handleGetMailAddress returns the personal address that turns an email into a
@@ -34,7 +40,10 @@ func (s *Server) handleGetMailAddress(w http.ResponseWriter, r *http.Request) {
s.internal(w, "mail token", err)
return
}
writeJSON(w, http.StatusOK, mailAddressResponse{Address: s.mailAddress(token)})
writeJSON(w, http.StatusOK, mailAddressResponse{
Address: s.mailAddress(token),
Configured: s.cfg.SMTP.Host != "",
})
}
func (s *Server) handleRotateMailAddress(w http.ResponseWriter, r *http.Request) {
@@ -47,7 +56,10 @@ func (s *Server) handleRotateMailAddress(w http.ResponseWriter, r *http.Request)
s.internal(w, "set mail token", err)
return
}
writeJSON(w, http.StatusOK, mailAddressResponse{Address: s.mailAddress(token)})
writeJSON(w, http.StatusOK, mailAddressResponse{
Address: s.mailAddress(token),
Configured: s.cfg.SMTP.Host != "",
})
}
func (s *Server) mailAddress(token string) string {
@@ -55,7 +67,15 @@ func (s *Server) mailAddress(token string) string {
if from := s.cfg.SMTP.From; strings.Contains(from, "@") {
// The mail server's own domain, which is the one that can actually route
// this — the web address may well be a tunnel hostname that receives no mail.
domain = from[strings.Index(from, "@")+1:]
//
// Except when it is the default. VERDANDE_SMTP_FROM falls back to
// verdande@localhost, so an instance with no mail server at all was
// handing out todo+…@localhost: an address that looks real, cannot
// possibly receive anything, and gives no hint which of the two it is.
// The public hostname is at least a domain that exists.
if d := from[strings.Index(from, "@")+1:]; d != "" && d != "localhost" {
domain = d
}
}
return "todo+" + token + "@" + domain
}
@@ -251,8 +271,8 @@ func (s *Server) handleAISplit(w http.ResponseWriter, r *http.Request) {
}
if !cfg.Configured() {
// Not an error the user did anything to cause: the feature is simply off.
writeError(w, http.StatusConflict, CodeConflict,
"der er ikke sat en AI-udbyder op under indstillinger")
writeError(w, http.StatusConflict, CodeAINotConfigured,
"no AI provider is configured")
return
}
@@ -299,8 +319,8 @@ func (s *Server) handleAISummary(w http.ResponseWriter, r *http.Request) {
return
}
if !cfg.Configured() {
writeError(w, http.StatusConflict, CodeConflict,
"der er ikke sat en AI-udbyder op under indstillinger")
writeError(w, http.StatusConflict, CodeAINotConfigured,
"no AI provider is configured")
return
}
+49
View File
@@ -880,3 +880,52 @@ func TestMCPWithKeyIgnoresTheSessionCookie(t *testing.T) {
resp.StatusCode)
}
}
// --- mail to task ------------------------------------------------------------------
// The address used to come out as todo+…@localhost on any instance without a
// mail server, because VERDANDE_SMTP_FROM defaults to verdande@localhost and the
// domain was taken from it unconditionally. That address looks real, cannot
// receive anything, and gives no hint which of the two it is.
func TestMailAddressDoesNotClaimToBeLocalhost(t *testing.T) {
ts := newTestServerWith(t, func(c *config.Config) {
c.BaseURL = "https://todo.example.dk"
// Left at the default, as an instance with no mail server has it.
c.SMTP.From = "verdande@localhost"
})
ts.bootstrap(t)
_, body := ts.do(t, "GET", "/api/v1/mail-address", nil)
address, _ := body["address"].(string)
if strings.HasSuffix(address, "@localhost") {
t.Errorf("address = %q — that can never receive mail", address)
}
if !strings.HasSuffix(address, "@todo.example.dk") {
t.Errorf("address = %q, want the public hostname", address)
}
if body["configured"] != false {
t.Error("no SMTP host is set, so configured should be false")
}
}
// A real sender address still wins: the mail server's own domain is the one that
// can actually route this, and the web address may be a tunnel hostname.
func TestMailAddressPrefersTheConfiguredSender(t *testing.T) {
ts := newTestServerWith(t, func(c *config.Config) {
c.BaseURL = "https://tunnel-abc123.example.net"
c.SMTP.Host = "mail.firma.dk"
c.SMTP.From = "verdande@firma.dk"
})
ts.bootstrap(t)
_, body := ts.do(t, "GET", "/api/v1/mail-address", nil)
address, _ := body["address"].(string)
if !strings.HasSuffix(address, "@firma.dk") {
t.Errorf("address = %q, want the sender's domain", address)
}
if body["configured"] != true {
t.Error("an SMTP host is set, so configured should be true")
}
}
+1 -1
View File
@@ -141,7 +141,7 @@ func (s *Server) handleDeleteProject(w http.ResponseWriter, r *http.Request) {
projectID := chi.URLParam(r, "projectID")
if err := s.db.DeleteProject(r.Context(), projectID); err != nil {
if strings.Contains(err.Error(), "Inbox cannot be deleted") {
writeError(w, http.StatusConflict, CodeConflict, "the Inbox cannot be deleted")
writeError(w, http.StatusConflict, CodeInboxProtected, "the Inbox cannot be deleted")
return
}
s.storeError(w, "delete project", err)
+1 -1
View File
@@ -116,7 +116,7 @@ func (s *Server) handleRecoveryCodesRegenerate(w http.ResponseWriter, r *http.Re
user := userFrom(r.Context())
if !user.TOTPEnabled {
writeError(w, http.StatusConflict, CodeConflict,
writeError(w, http.StatusConflict, CodeTOTPNotEnabled,
"two-factor authentication is not switched on")
return
}
+21 -1
View File
@@ -34,9 +34,29 @@ const MESSAGES = {
validation_failed: 'Tjek felterne herunder.',
payload_too_large: 'Det er for stort.',
internal_error: 'Noget gik galt. Prøv igen.',
offline: 'Ingen forbindelse til serveren.'
bad_request: 'Anmodningen kunne ikke læses.',
// Client-only: a network failure is not an HTTP status, so the server has no
// code for it.
offline: 'Ingen forbindelse til serveren.',
// Situations that used to arrive as a plain `conflict` and were therefore
// shown as "Det er der allerede." — which was not merely vague but wrong. An
// unconfigured Gmail is not a Gmail that is already connected.
gmail_not_configured:
'Gmail er ikke sat op på denne server. Administratoren skal registrere en OAuth-klient hos Google og sætte VERDANDE_GMAIL_CLIENT_ID og _SECRET.',
ai_not_configured: 'Der er ikke valgt en AI-udbyder. Sæt den op under Indstillinger → AI.',
totp_not_enabled: 'To-faktor er ikke slået til.',
inbox_protected: 'Indbakken kan ikke slettes.'
};
/**
* What to show a person for a failed request.
*
* A code with no message here falls back to the server's prose, which is English
* and written for a log — not good, but better than swallowing the only
* explanation there is. A code that turns up in that fallback is a code this
* table is missing.
*/
export function humanMessage(err) {
if (!(err instanceof ApiError)) return MESSAGES.offline;
return MESSAGES[err.code] ?? err.message ?? MESSAGES.internal_error;
@@ -96,10 +96,18 @@
let feedURL = $state('');
let mailAddress = $state('');
/** Null until loaded, so the warning does not flash before the answer is in. */
let mailConfigured = $state(null);
$effect(() => {
api.feed().then((r) => (feedURL = r.url)).catch(() => {});
api.getMailAddress().then((r) => (mailAddress = r.address)).catch(() => {});
api
.getMailAddress()
.then((r) => {
mailAddress = r.address;
mailConfigured = r.configured;
})
.catch(() => {});
});
async function rotateFeed() {
@@ -114,7 +122,9 @@
async function rotateMail() {
if (!confirm('Ny adresse? Mail til den gamle bliver afvist.')) return;
try {
mailAddress = (await api.rotateMailAddress()).address;
const r = await api.rotateMailAddress();
mailAddress = r.address;
mailConfigured = r.configured;
} catch (e) {
app.toast(humanMessage(e));
}
@@ -218,6 +228,16 @@
</p>
</header>
{#if mailConfigured === false}
<p class="callback" data-tone="bad">
Der er ingen mailserver sat op på denne server, så adressen herunder kan
ikke modtage noget endnu. Sæt <span class="mono">VERDANDE_SMTP_HOST</span> og
de øvrige SMTP-indstillinger, og få mailserveren til at aflevere post til
<span class="mono">todo+*</span> videre til
<span class="mono">/inbound/mail</span>.
</p>
{/if}
<div class="field">
<label for="mail">Din adresse</label>
<input id="mail" class="mono" value={mailAddress} readonly />