Skip to content

Commit 430e828

Browse files
authored
fix: api-gateway Redis URL, cleanup safety, and auth error handling (#2038)
* fix: api-gateway Redis URL parsing, partial cleanup, and auth error handling - Parse Redis URL with redis.ParseURL() (with fallback for bare addr) so REDIS_URL values like redis://host:port are handled correctly (task 13.1) - Defer buildServerOptions cleanups before the error check so partial wiring (e.g. ssoCleanup after wireBFFSSO) always runs on failure (task 13.2) - Return 403 Forbidden for ErrEmailNotVerified instead of falling through to generic 500 in handleLoginError (task 14) - Log internal FindByEmail failures in issueResendVerificationToken while preserving timing-safe 200 response for email enumeration prevention (task 14) * fix: redact Redis URL credentials in fallback warn log * fix: align test stub with connector contract and harden URL redaction fallback --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent 8b595f3 commit 430e828

5 files changed

Lines changed: 83 additions & 6 deletions

File tree

services/api-gateway/auth_handler.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"time"
1212

1313
"github.com/meridianhub/meridian/services/identity/connector"
14+
identitydomain "github.com/meridianhub/meridian/services/identity/domain"
1415
platformauth "github.com/meridianhub/meridian/shared/platform/auth"
1516
"github.com/meridianhub/meridian/shared/platform/tenant"
1617
)
@@ -173,6 +174,12 @@ func (h *AuthHandler) handleLoginError(ctx context.Context, w http.ResponseWrite
173174
})
174175
return
175176
}
177+
if errors.Is(err, identitydomain.ErrEmailNotVerified) {
178+
writeJSON(w, http.StatusForbidden, map[string]string{
179+
"error": "email address has not been verified",
180+
})
181+
return
182+
}
176183
if strings.HasPrefix(err.Error(), "sign:") {
177184
h.logger.ErrorContext(ctx, "auth: failed to sign token",
178185
"tenant_id", tenantID, "error", err)

services/api-gateway/auth_handler_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
gateway "github.com/meridianhub/meridian/services/api-gateway"
1515
"github.com/meridianhub/meridian/services/identity/connector"
16+
identitydomain "github.com/meridianhub/meridian/services/identity/domain"
1617
platformauth "github.com/meridianhub/meridian/shared/platform/auth"
1718
"github.com/meridianhub/meridian/shared/platform/tenant"
1819
"github.com/stretchr/testify/assert"
@@ -227,3 +228,40 @@ func TestAuthHandler_MethodNotAllowed(t *testing.T) {
227228
handler.HandleLogin(rec, req)
228229
assert.Equal(t, http.StatusMethodNotAllowed, rec.Code)
229230
}
231+
232+
func TestAuthHandler_EmailNotVerified_Returns403(t *testing.T) {
233+
signer := newTestSigner(t)
234+
235+
conn := &stubConnector{
236+
loginFn: func(_ context.Context, _ []string, _, _ string) (connector.Identity, bool, error) {
237+
return connector.Identity{}, false, identitydomain.ErrEmailNotVerified
238+
},
239+
}
240+
241+
handler, err := gateway.NewAuthHandler(gateway.AuthHandlerConfig{
242+
Connector: conn,
243+
Signer: signer,
244+
Logger: slog.Default(),
245+
})
246+
require.NoError(t, err)
247+
248+
body, _ := json.Marshal(map[string]string{
249+
"email": "unverified@example.com",
250+
"password": "correct",
251+
})
252+
253+
req := httptest.NewRequest(http.MethodPost, "/api/auth/login", bytes.NewReader(body))
254+
req.Header.Set("Content-Type", "application/json")
255+
tid, _ := tenant.NewTenantID("volterra")
256+
req = req.WithContext(tenant.WithTenant(req.Context(), tid))
257+
258+
rec := httptest.NewRecorder()
259+
handler.HandleLogin(rec, req)
260+
261+
assert.Equal(t, http.StatusForbidden, rec.Code)
262+
263+
var resp map[string]string
264+
err = json.NewDecoder(rec.Body).Decode(&resp)
265+
require.NoError(t, err)
266+
assert.Equal(t, "email address has not been verified", resp["error"])
267+
}

services/api-gateway/cmd/main.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"log/slog"
99
"os"
10+
"net/url"
1011
"strings"
1112
"time"
1213

@@ -89,14 +90,16 @@ func run(logger *slog.Logger) error {
8990

9091
// Build server options
9192
serverOpts, eventRouter, optCleanups, err := buildServerOptions(config, logger, healthChecker, redisClient)
92-
if err != nil {
93-
return err
94-
}
93+
// Defer cleanups before checking err: buildServerOptions may return partial
94+
// cleanups (e.g., ssoCleanup) even when it fails midway through wiring.
9595
defer func() {
9696
for _, cleanup := range optCleanups {
9797
cleanup()
9898
}
9999
}()
100+
if err != nil {
101+
return err
102+
}
100103

101104
// Create server
102105
server := gateway.NewServer(config, logger, nil, serverOpts...)
@@ -141,9 +144,16 @@ func initRedisAndHealth(config *gateway.Config, dbPool *db.PostgresPool, logger
141144
var redisClient *redis.Client
142145
var cleanup func()
143146
if config.RedisURL != "" {
144-
redisClient = redis.NewClient(&redis.Options{
145-
Addr: config.RedisURL,
146-
})
147+
opt, err := redis.ParseURL(config.RedisURL)
148+
if err != nil {
149+
redacted := "<unparseable>"
150+
if u, parseErr := url.Parse(config.RedisURL); parseErr == nil {
151+
redacted = u.Redacted()
152+
}
153+
logger.Warn("redis URL parse failed, falling back to direct addr", "url", redacted, "error", err)
154+
opt = &redis.Options{Addr: config.RedisURL}
155+
}
156+
redisClient = redis.NewClient(opt)
147157
cleanup = func() { _ = redisClient.Close() }
148158

149159
if err := redisClient.Ping(context.Background()).Err(); err != nil {

services/api-gateway/verification_handler.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ func (h *VerificationHandler) issueResendVerificationToken(ctx context.Context,
199199

200200
identity, err := h.identityRepo.FindByEmail(tenantCtx, req.Email)
201201
if err != nil {
202+
if !errors.Is(err, identitydomain.ErrIdentityNotFound) {
203+
h.logger.ErrorContext(ctx, "verification: failed to find identity by email", "error", err)
204+
}
202205
return
203206
}
204207

services/api-gateway/verification_handler_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"encoding/json"
7+
"errors"
78
"log/slog"
89
"net/http"
910
"net/http/httptest"
@@ -357,6 +358,24 @@ func TestResendVerification_RateLimited(t *testing.T) {
357358
assert.Empty(t, or.entries)
358359
}
359360

361+
func TestResendVerification_InternalFindByEmailError_StillReturns200(t *testing.T) {
362+
ir := &stubIdentityRepo{
363+
findByEmailFn: func(_ context.Context, _ string) (*identitydomain.Identity, error) {
364+
return nil, errors.New("db connection lost")
365+
},
366+
}
367+
or := &stubOutboxRepo{}
368+
h := newVerificationHandler(t, ir, or)
369+
370+
w := postResendVerification(h, map[string]string{
371+
"email": "user@test.com",
372+
"tenant_id": "test_tenant",
373+
})
374+
// Timing-safe: always returns 200 to prevent email enumeration.
375+
assert.Equal(t, http.StatusOK, w.Code)
376+
assert.Empty(t, or.entries)
377+
}
378+
360379
func TestResendVerification_MissingFields(t *testing.T) {
361380
ir := &stubIdentityRepo{}
362381
or := &stubOutboxRepo{}

0 commit comments

Comments
 (0)