Skip to content

Commit f2aa36f

Browse files
committed
fix(auth): polish OIDC integration
- Let the SPA shell (HTML/JS/CSS, /search, /view/<id>, /auth/callback) load without an auth challenge when OIDC is enabled, so the SPA can run the redirect itself. API and direct .html/.txt previews stay gated. - Suppress WWW-Authenticate: Basic on 401s when OIDC is on, so the browser's native Basic dialog doesn't pop up on a SPA-side 401. Basic Auth still works for clients that send the header proactively. - Add the IdP origin to connect-src in the CSP so the SPA can reach the discovery / JWKS / token endpoints. - Drop the duplicate Basic Auth check in websockets.ServeWs — auth is the middleware's job, and the duplicate broke browser WS upgrades (browsers can't send Authorization on WS handshakes). - Move the SPA-side OIDC config to a cached Promise so callers race- free await the same init. AuthCallbackView does a hard navigation after exchange so the router guard sees the freshly stored user. - Store tokens in localStorage so a new tab inherits the session. - Move the logout button next to "About Mailpit" and show the signed- in user's name beside it.
1 parent 98eb383 commit f2aa36f

10 files changed

Lines changed: 221 additions & 78 deletions

File tree

config/config.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"net"
9+
"net/url"
910
"os"
1011
"path"
1112
"path/filepath"
@@ -301,9 +302,18 @@ func VerifyConfig() error {
301302
// The default Content Security Policy is updates on every application page load to replace script-src 'self'
302303
// with a random nonce ID to prevent XSS. This applies to the Mailpit app & API.
303304
// See server.middleWareFunc()
305+
connectSrc := "'self' ws: wss:"
306+
// When OIDC is enabled the SPA must be allowed to fetch the IdP's
307+
// discovery doc / JWKS / token endpoint. Add the issuer's origin
308+
// to connect-src.
309+
if UIOIDCIssuer != "" {
310+
if u, err := url.Parse(UIOIDCIssuer); err == nil && u.Scheme != "" && u.Host != "" {
311+
connectSrc += " " + u.Scheme + "://" + u.Host
312+
}
313+
}
304314
ContentSecurityPolicy = fmt.Sprintf(
305-
"default-src 'self'; script-src 'self'; style-src %s 'unsafe-inline'; frame-src 'self'; img-src * data: blob:; font-src %s data:; media-src 'self'; connect-src 'self' ws: wss:; object-src 'none'; base-uri 'self';",
306-
cssFontRestriction, cssFontRestriction,
315+
"default-src 'self'; script-src 'self'; style-src %s 'unsafe-inline'; frame-src 'self'; img-src * data: blob:; font-src %s data:; media-src 'self'; connect-src %s; object-src 'none'; base-uri 'self';",
316+
cssFontRestriction, cssFontRestriction, connectSrc,
307317
)
308318

309319
if Database != "" && isDir(Database) {

server/server.go

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ func Listen() {
9292
r.HandleFunc("GET "+config.Webroot+"view/", middleWareFunc(viewHandler))
9393

9494
r.Handle("GET "+config.Webroot+"search", middleWareFunc(index))
95+
// OIDC callback — served by the SPA (Vue Router handles the code+state).
96+
r.Handle("GET "+config.Webroot+"auth/callback", middleWareFunc(index))
9597
// Exact-match the webroot; stdlib "/" is always a subtree so we guard inside.
9698
r.HandleFunc("GET "+config.Webroot, func(w http.ResponseWriter, r *http.Request) {
9799
if r.URL.Path != config.Webroot {
@@ -228,6 +230,15 @@ func checkUIAuth(r *http.Request) bool {
228230
if auth.UICredentials == nil && auth.OIDCVerifier == nil {
229231
return true
230232
}
233+
// When OIDC is configured, the SPA shell (HTML + JS/CSS + SPA-served
234+
// routes) must boot without an auth challenge so it can run the OIDC
235+
// redirect itself. Otherwise the browser's native Basic Auth dialog
236+
// would fire on first navigation and the SPA would never get a chance
237+
// to redirect to the IdP. The API and direct message-preview routes
238+
// remain gated.
239+
if auth.OIDCVerifier != nil && isSPAShellRequest(r) {
240+
return true
241+
}
231242
if auth.OIDCVerifier != nil {
232243
authz := r.Header.Get("Authorization")
233244
raw := strings.TrimPrefix(authz, "Bearer ")
@@ -247,15 +258,56 @@ func checkUIAuth(r *http.Request) bool {
247258
return false
248259
}
249260

261+
// isSPAShellRequest reports whether the request targets the SPA shell —
262+
// the HTML page, its bundled JS/CSS, favicons, and the client-side
263+
// routes that all serve the same index template. It deliberately
264+
// excludes /api/*, direct HTML/TXT message previews, and /view/latest
265+
// (which leaks the latest message ID via redirect).
266+
func isSPAShellRequest(r *http.Request) bool {
267+
if r.Method != http.MethodGet && r.Method != http.MethodHead {
268+
return false
269+
}
270+
p := r.URL.Path
271+
wr := config.Webroot
272+
wrTrim := strings.TrimRight(wr, "/")
273+
if p == wr || p == wrTrim {
274+
return true
275+
}
276+
if strings.HasPrefix(p, wr+"dist/") {
277+
return true
278+
}
279+
switch p {
280+
case wr + "favicon.ico",
281+
wr + "favicon.svg",
282+
wr + "mailpit.svg",
283+
wr + "notification.png",
284+
wr + "search",
285+
wr + "auth/callback":
286+
return true
287+
}
288+
if strings.HasPrefix(p, wr+"view/") && p != wr+"view/latest" {
289+
if strings.HasSuffix(p, ".html") || strings.HasSuffix(p, ".txt") {
290+
return false
291+
}
292+
return true
293+
}
294+
return false
295+
}
296+
250297
// unauthorizedResponse writes a 401. When OIDC is enabled it adds the
251298
// X-Mp-Auth-Required header so the SPA's axios interceptor can trigger
252299
// signinRedirect. The Basic challenge is preserved when htpasswd is
253300
// configured so curl-based integrations keep working.
254301
func unauthorizedResponse(w http.ResponseWriter) {
302+
// When OIDC is enabled, do NOT advertise a Basic challenge. The
303+
// browser would otherwise pop its native Basic Auth dialog on every
304+
// SPA-side 401 (e.g. a new tab with empty sessionStorage racing the
305+
// OIDC redirect) — even when Basic Auth is also configured for API
306+
// integrations. Basic Auth still works for clients that proactively
307+
// send `Authorization: Basic …` (curl -u, automation scripts).
255308
if auth.OIDCVerifier != nil {
256309
w.Header().Set("X-Mp-Auth-Required", "oidc")
257-
}
258-
if auth.UICredentials != nil {
310+
} else if auth.UICredentials != nil {
259311
w.Header().Set("WWW-Authenticate", `Basic realm="Login"`)
260312
}
261313
w.WriteHeader(http.StatusUnauthorized)

server/server_test.go

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -920,18 +920,89 @@ func TestUIAuthOIDC(t *testing.T) {
920920

921921
t.Run("TamperedBearer_Returns401", func(t *testing.T) {
922922
tok := idp.issue("mailpit", "alice", time.Now().Add(time.Hour))
923-
// Flip the last char of the signature.
924-
tampered := tok[:len(tok)-1] + "A"
925-
if tampered == tok {
926-
tampered = tok[:len(tok)-1] + "B"
923+
// Flip a char in the middle of the signature segment so we are
924+
// always changing real signature bytes (the last base64url char
925+
// may encode unused padding bits and produce identical bytes).
926+
dot := strings.LastIndex(tok, ".")
927+
sigStart := dot + 1
928+
mid := sigStart + (len(tok)-sigStart)/2
929+
swap := byte('A')
930+
if tok[mid] == 'A' {
931+
swap = 'B'
927932
}
933+
tampered := tok[:mid] + string(swap) + tok[mid+1:]
928934
resp, err := clientGetWithBearer(ts.URL+"/api/v1/messages", tampered)
929935
if err != nil {
930936
t.Fatalf("get: %v", err)
931937
}
932938
defer func() { _ = resp.Body.Close() }()
933939
assertEqual(t, http.StatusUnauthorized, resp.StatusCode, "expected 401 for tampered token")
934940
})
941+
942+
t.Run("SPAShell_ServesWithoutAuth_WhenOIDCEnabled", func(t *testing.T) {
943+
// The SPA shell must load without an auth challenge so the SPA
944+
// can run the OIDC redirect itself. Otherwise the browser pops
945+
// up its native Basic Auth dialog and the SPA never boots.
946+
for _, path := range []string{"/", "/search", "/view/abc", "/dist/app.js", "/favicon.svg"} {
947+
resp, err := clientGetRaw(ts.URL + path)
948+
if err != nil {
949+
t.Fatalf("get %s: %v", path, err)
950+
}
951+
_ = resp.Body.Close()
952+
if resp.StatusCode == http.StatusUnauthorized {
953+
t.Errorf("%s: expected SPA shell to load without auth, got 401 (WWW-Authenticate=%q)",
954+
path, resp.Header.Get("Www-Authenticate"))
955+
}
956+
if got := resp.Header.Get("Www-Authenticate"); got != "" {
957+
t.Errorf("%s: SPA shell must not return WWW-Authenticate, got %q", path, got)
958+
}
959+
}
960+
})
961+
962+
}
963+
964+
func TestIsSPAShellRequest(t *testing.T) {
965+
// Webroot is "/" in tests by default.
966+
cases := []struct {
967+
method string
968+
path string
969+
want bool
970+
}{
971+
{"GET", "/", true},
972+
{"GET", "/search", true},
973+
{"GET", "/auth/callback", true},
974+
{"GET", "/view/abc", true},
975+
{"GET", "/view/abc123XYZ", true},
976+
{"GET", "/dist/app.js", true},
977+
{"GET", "/dist/app.css", true},
978+
{"GET", "/favicon.ico", true},
979+
{"GET", "/favicon.svg", true},
980+
{"GET", "/mailpit.svg", true},
981+
{"GET", "/notification.png", true},
982+
// Must remain gated:
983+
{"GET", "/view/abc.html", false},
984+
{"GET", "/view/abc.txt", false},
985+
{"GET", "/view/latest", false},
986+
{"GET", "/api/v1/messages", false},
987+
{"GET", "/api/v1/webui", false},
988+
{"GET", "/api/events", false},
989+
{"GET", "/proxy", false},
990+
// HEAD also qualifies (browsers + curl -I).
991+
{"HEAD", "/", true},
992+
{"HEAD", "/dist/app.js", true},
993+
// Non-GET/HEAD methods never qualify.
994+
{"POST", "/", false},
995+
{"PUT", "/dist/app.js", false},
996+
}
997+
for _, tc := range cases {
998+
r, err := http.NewRequest(tc.method, tc.path, nil)
999+
if err != nil {
1000+
t.Fatalf("NewRequest: %v", err)
1001+
}
1002+
if got := isSPAShellRequest(r); got != tc.want {
1003+
t.Errorf("isSPAShellRequest(%s %s) = %v, want %v", tc.method, tc.path, got, tc.want)
1004+
}
1005+
}
9351006
}
9361007

9371008
func TestUIAuthOIDCAndBasicCoexist(t *testing.T) {
@@ -976,17 +1047,19 @@ func TestUIAuthOIDCAndBasicCoexist(t *testing.T) {
9761047
}
9771048
})
9781049

979-
t.Run("NoAuth_401_WithBothChallenges", func(t *testing.T) {
1050+
t.Run("NoAuth_401_OIDCHintOnly_NoBasicChallenge", func(t *testing.T) {
1051+
// When OIDC is enabled the server must NOT advertise a Basic
1052+
// challenge, even if htpasswd is also configured — otherwise
1053+
// the browser pops its native dialog on any SPA-side 401.
1054+
// Basic Auth still works for clients that proactively send it.
9801055
resp, err := clientGetRaw(ts.URL + "/api/v1/messages")
9811056
if err != nil {
9821057
t.Fatalf("get: %v", err)
9831058
}
9841059
defer func() { _ = resp.Body.Close() }()
9851060
assertEqual(t, http.StatusUnauthorized, resp.StatusCode, "expected 401")
9861061
assertEqual(t, "oidc", resp.Header.Get("X-Mp-Auth-Required"), "expected OIDC hint")
987-
if resp.Header.Get("WWW-Authenticate") == "" {
988-
t.Fatalf("expected WWW-Authenticate Basic challenge")
989-
}
1062+
assertEqual(t, "", resp.Header.Get("WWW-Authenticate"), "Basic challenge must be suppressed when OIDC is enabled")
9901063
})
9911064
}
9921065

server/ui-src/App.vue

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
import CommonMixins from "./mixins/CommonMixins";
33
import Favicon from "./components/AppFavicon.vue";
44
import AppBadge from "./components/AppBadge.vue";
5-
import AppLogout from "./components/AppLogout.vue";
65
import Notifications from "./components/AppNotifications.vue";
76
import EditTags from "./components/EditTags.vue";
87
import { mailbox } from "./stores/mailbox";
@@ -11,7 +10,6 @@ export default {
1110
components: {
1211
Favicon,
1312
AppBadge,
14-
AppLogout,
1513
Notifications,
1614
EditTags,
1715
},
@@ -44,7 +42,6 @@ export default {
4442
<RouterView />
4543
<Favicon />
4644
<AppBadge />
47-
<AppLogout />
4845
<Notifications />
4946
<EditTags />
5047
</template>

server/ui-src/components/AppAbout.vue

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
<script>
22
import AjaxLoader from "./AjaxLoader.vue";
3+
import AppLogout from "./AppLogout.vue";
34
import Settings from "./AppSettings.vue";
45
import CommonMixins from "../mixins/CommonMixins";
56
import { mailbox } from "../stores/mailbox";
67
78
export default {
89
components: {
910
AjaxLoader,
11+
AppLogout,
1012
Settings,
1113
},
1214
@@ -88,6 +90,7 @@ export default {
8890
<i class="bi bi-bell"></i>
8991
</button>
9092
</div>
93+
<AppLogout />
9194
</template>
9295

9396
<template v-else>
Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,23 @@
11
<script>
2-
import { logout, oidcEnabled } from "../services/oidcAuth";
2+
import { getUser, logout, oidcEnabled } from "../services/oidcAuth";
33
44
export default {
5+
data() {
6+
return {
7+
displayName: "",
8+
};
9+
},
510
computed: {
611
show() {
712
return oidcEnabled();
813
},
914
},
15+
async mounted() {
16+
if (!this.show) return;
17+
const u = await getUser();
18+
if (!u || !u.profile) return;
19+
this.displayName = u.profile.name || u.profile.preferred_username || u.profile.email || "Signed in";
20+
},
1021
methods: {
1122
signOut() {
1223
logout();
@@ -16,15 +27,13 @@ export default {
1627
</script>
1728

1829
<template>
19-
<button
20-
v-if="show"
21-
type="button"
22-
class="btn btn-sm btn-outline-light position-fixed top-0 end-0 mt-2 me-2 d-print-none"
23-
style="z-index: 1050"
24-
title="Sign out"
25-
@click="signOut"
26-
>
27-
<i class="bi bi-box-arrow-right"></i>
28-
<span class="d-none d-md-inline ms-1">Sign out</span>
29-
</button>
30+
<div v-if="show" class="bg-body ms-sm-n1 me-sm-n1 pb-2 text-muted small">
31+
<button type="button" class="text-muted btn btn-sm" :title="displayName">
32+
<i class="bi bi-person-fill me-1"></i>
33+
{{ displayName || "Signed in" }}
34+
</button>
35+
<button type="button" class="btn btn-sm btn-outline-secondary float-end" title="Sign out" @click="signOut">
36+
<i class="bi bi-box-arrow-right"></i>
37+
</button>
38+
</div>
3039
</template>

server/ui-src/components/AppNotifications.vue

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import CommonMixins from "../mixins/CommonMixins";
33
import { Toast } from "bootstrap";
44
import { mailbox } from "../stores/mailbox";
55
import { pagination } from "../stores/pagination";
6-
import { getToken, oidcEnabled } from "../services/oidcAuth";
6+
import { configureOIDC, getToken, oidcEnabled } from "../services/oidcAuth";
77
88
export default {
99
mixins: [CommonMixins],
@@ -51,6 +51,11 @@ export default {
5151
const proto = location.protocol === "https:" ? "wss" : "ws";
5252
let uri = proto + "://" + document.location.host + this.resolve("/api/events");
5353
if (oidcEnabled()) {
54+
// configureOIDC is idempotent: the first call sets up the
55+
// UserManager, subsequent calls await the same promise.
56+
// Awaiting here closes the race between component mount
57+
// and the async OIDC init in router/index.js.
58+
await configureOIDC();
5459
const t = await getToken();
5560
if (t) {
5661
uri += "?access_token=" + encodeURIComponent(t);

0 commit comments

Comments
 (0)