fix(auth): close OAuth open-redirect by wiring AllowedRedirectHosts (#2049)
* fix(auth): close OAuth open-redirect by wiring AllowedRedirectHosts Bump go-pkgz/auth/v2 to master (v2.1.2-0.20260421203319-686683f19cf7) which carries the `from` redirect validator from go-pkgz/auth#275. The library default with a nil AllowedRedirectHosts is permissive (preserves legacy behavior for existing consumers on a dep bump), so just bumping the dep leaves remark42 vulnerable — a crafted /auth/<provider>/login?from=https://evil.example.com/... still issues the 307 to the attacker host after the user completes legitimate OAuth. Verified end-to-end against a local dev-auth instance before and after this commit. Wire Opts.AllowedRedirectHosts in getAuthenticator to the operator's existing --allowed-hosts config, stripping the CSP "self" sentinel which is not a real hostname. RemarkURL's own host is always implicit per the library contract, so a default single-site deployment gains the protection with no config change. Multi-host embeds work as soon as their embedding hosts are added to AllowedHosts (they already need to be there for CSP frame-ancestors). Refreshed vendor tree to match the new module version. * chore(lint): suppress G703 false positives on image Save CI's newer gosec flags os.MkdirAll/os.WriteFile in FileSystem.Save with G703 because id flows in from the caller. id is validated at the HTTP layer (safePictureSegment in rest_public.go) and dst is derived via f.location — not a real traversal. Targeted //nolint with reason. * fix(auth): normalise AllowedRedirectHosts entries + add unit test Address Copilot review on PR #2049. The previous closure passed raw s.AllowedHosts entries straight to the auth library, but --allowed-hosts holds CSP frame-ancestors source expressions: scheme-prefixed values (https://blog.example.com), entries with ports, and wildcards (*.cdn.example.com) are all valid there but the auth library compares against u.Hostname() and would silently drop them — breaking legitimate redirects on multi-host deployments. Extract getAllowedRedirectHosts that: * trims whitespace, drops empty / 'self' / "self" / wildcard entries * prepends https:// if scheme missing then url.Parse to extract Hostname * logs a warning on parse failure rather than poisoning the allowlist Wire the closure in getAuthenticator to call the helper. Test_getAllowedRedirectHosts covers all the edge cases Copilot flagged (scheme stripping, port handling, self spellings, wildcards, empty, mixed real-world). * fix(auth): preserve explicit port in AllowedRedirectHosts + clarify fs_store nolint Address Copilot follow-up on PR #2049: * getAllowedRedirectHosts stripped explicit ports via u.Hostname(), which broadened the allowlist. The auth validator checks both Hostname() and Host, so an entry like admin.example.com:8443 can and should be kept host:port — allowing only that port, not any. Emit u.Host when u.Port() != "", u.Hostname() otherwise. Updated tests. * fs_store Save nolint rationale said "id validated at HTTP layer", but Save is reached via image.Service.Save and SaveWithID (cache), neither of which is HTTP validation. id is actually a server-generated hash in both paths. Updated the comment.
This commit is contained in:
@@ -794,6 +794,42 @@ func (s *ServerCommand) getAllowedDomains() []string {
|
||||
return allowedDomains
|
||||
}
|
||||
|
||||
// getAllowedRedirectHosts normalises s.AllowedHosts into the form that
|
||||
// go-pkgz/auth's redirect validator expects. Strips http(s) schemes and
|
||||
// paths; preserves explicit ports (the validator matches both host-only
|
||||
// and host:port, so an entry without a port accepts any port while an
|
||||
// entry with a port restricts to that port). Skips CSP sentinels
|
||||
// ('self' / "self") and wildcard entries (*, *.example.com) that are
|
||||
// valid CSP source expressions but not valid hostnames.
|
||||
func (s *ServerCommand) getAllowedRedirectHosts() []string {
|
||||
out := make([]string, 0, len(s.AllowedHosts))
|
||||
for _, raw := range s.AllowedHosts {
|
||||
raw = strings.TrimSpace(raw)
|
||||
if raw == "" || raw == "self" || raw == "'self'" || raw == `"self"` {
|
||||
continue
|
||||
}
|
||||
if strings.ContainsRune(raw, '*') { // CSP wildcard, not a host
|
||||
continue
|
||||
}
|
||||
// add scheme so url.Parse populates Hostname()/Host consistently for bare hosts
|
||||
toParse := raw
|
||||
if !strings.HasPrefix(toParse, "http://") && !strings.HasPrefix(toParse, "https://") {
|
||||
toParse = "https://" + toParse
|
||||
}
|
||||
u, err := url.Parse(toParse)
|
||||
if err != nil || u.Hostname() == "" {
|
||||
log.Printf("[WARN] skipping invalid AllowedHosts entry %q for redirect allowlist: %v", raw, err)
|
||||
continue
|
||||
}
|
||||
if u.Port() != "" {
|
||||
out = append(out, u.Host) // preserve explicit host:port so allowlist is port-specific
|
||||
continue
|
||||
}
|
||||
out = append(out, u.Hostname())
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// Run all application objects
|
||||
func (a *serverApp) run(ctx context.Context) error {
|
||||
if a.AdminPasswd != "" {
|
||||
@@ -1351,6 +1387,12 @@ func (s *ServerCommand) getAuthenticator(ds *service.DataStore, avas avatar.Stor
|
||||
SendJWTHeader: s.Auth.SendJWTHeader,
|
||||
SameSiteCookie: s.parseSameSite(s.Auth.SameSite),
|
||||
SecureCookies: strings.HasPrefix(s.RemarkURL, "https://"),
|
||||
// enable the `from` redirect allowlist in go-pkgz/auth v2.1.2+ — limits
|
||||
// post-auth redirects to RemarkURL's own host plus any configured
|
||||
// AllowedHosts. Prevents the OAuth open-redirect / phishing vector.
|
||||
AllowedRedirectHosts: token.AllowedHostsFunc(func() ([]string, error) {
|
||||
return s.getAllowedRedirectHosts(), nil
|
||||
}),
|
||||
SecretReader: token.SecretFunc(func(aud string) (string, error) { // get secret per site
|
||||
return admns.Key(aud)
|
||||
}),
|
||||
|
||||
@@ -902,6 +902,34 @@ func Test_getAllowedDomains(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func Test_getAllowedRedirectHosts(t *testing.T) {
|
||||
tbl := []struct {
|
||||
name string
|
||||
hosts []string
|
||||
want []string
|
||||
}{
|
||||
{name: "empty", hosts: nil, want: []string{}},
|
||||
{name: "bare hostnames pass through", hosts: []string{"example.com", "admin.example.com"}, want: []string{"example.com", "admin.example.com"}},
|
||||
{name: "https scheme stripped", hosts: []string{"https://example.com"}, want: []string{"example.com"}},
|
||||
{name: "http scheme stripped", hosts: []string{"http://example.com"}, want: []string{"example.com"}},
|
||||
{name: "scheme with path strips path", hosts: []string{"https://example.com/embed"}, want: []string{"example.com"}},
|
||||
{name: "explicit port preserved as host:port", hosts: []string{"example.com:8080"}, want: []string{"example.com:8080"}},
|
||||
{name: "scheme with explicit port preserved", hosts: []string{"https://example.com:8443"}, want: []string{"example.com:8443"}},
|
||||
{name: "scheme without port stays bare host", hosts: []string{"https://example.com"}, want: []string{"example.com"}},
|
||||
{name: "self sentinel filtered", hosts: []string{"'self'", "self", `"self"`, "example.com"}, want: []string{"example.com"}},
|
||||
{name: "wildcards filtered", hosts: []string{"*", "*.example.com", "https://*.example.com", "example.com"}, want: []string{"example.com"}},
|
||||
{name: "empty entries filtered", hosts: []string{"", " ", "example.com"}, want: []string{"example.com"}},
|
||||
{name: "mixed real-world", hosts: []string{"'self'", "https://blog.example.com", "admin.example.com:8443", "*.cdn.example.com"},
|
||||
want: []string{"blog.example.com", "admin.example.com:8443"}},
|
||||
}
|
||||
for _, tt := range tbl {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
s := ServerCommand{AllowedHosts: tt.hosts}
|
||||
assert.Equal(t, tt.want, s.getAllowedRedirectHosts())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func chooseRandomUnusedPort() (port int) {
|
||||
for range 10 {
|
||||
port = 40000 + int(rand.Int31n(10000))
|
||||
|
||||
@@ -37,11 +37,11 @@ type FileSystem struct {
|
||||
func (f *FileSystem) Save(id string, img []byte) error {
|
||||
dst := f.location(f.Staging, id)
|
||||
|
||||
if err := os.MkdirAll(path.Dir(dst), 0o700); err != nil {
|
||||
if err := os.MkdirAll(path.Dir(dst), 0o700); err != nil { //nolint:gosec // id is server-generated hash via image.Service (Save / SaveWithID); dst computed via f.location
|
||||
return fmt.Errorf("can't make image directory: %w", err)
|
||||
}
|
||||
|
||||
if err := os.WriteFile(dst, img, 0o600); err != nil {
|
||||
if err := os.WriteFile(dst, img, 0o600); err != nil { //nolint:gosec // same as MkdirAll above
|
||||
return fmt.Errorf("can't write image file with id %s: %w", id, err)
|
||||
}
|
||||
|
||||
|
||||
+2
-2
@@ -9,7 +9,7 @@ require (
|
||||
github.com/didip/tollbooth/v8 v8.0.1
|
||||
github.com/go-chi/chi/v5 v5.2.5
|
||||
github.com/go-chi/cors v1.2.2
|
||||
github.com/go-pkgz/auth/v2 v2.1.2-0.20260211003156-fbba7f2baa6b
|
||||
github.com/go-pkgz/auth/v2 v2.1.2-0.20260421203319-686683f19cf7
|
||||
github.com/go-pkgz/jrpc v0.4.0
|
||||
github.com/go-pkgz/lcw/v2 v2.0.0
|
||||
github.com/go-pkgz/lgr v0.12.3
|
||||
@@ -33,6 +33,7 @@ require (
|
||||
golang.org/x/crypto v0.50.0
|
||||
golang.org/x/image v0.39.0
|
||||
golang.org/x/net v0.53.0
|
||||
golang.org/x/oauth2 v0.36.0
|
||||
)
|
||||
|
||||
require (
|
||||
@@ -66,7 +67,6 @@ require (
|
||||
github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78 // indirect
|
||||
go.mongodb.org/mongo-driver v1.17.9 // indirect
|
||||
go.uber.org/atomic v1.11.0 // indirect
|
||||
golang.org/x/oauth2 v0.36.0 // indirect
|
||||
golang.org/x/sync v0.20.0 // indirect
|
||||
golang.org/x/sys v0.43.0 // indirect
|
||||
golang.org/x/text v0.36.0 // indirect
|
||||
|
||||
+2
-2
@@ -48,8 +48,8 @@ github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE=
|
||||
github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58=
|
||||
github.com/go-oauth2/oauth2/v4 v4.5.4 h1:YjI0tmGW8oxVhn9QSBIxlr641QugWrJY5UWa6XmLcW0=
|
||||
github.com/go-oauth2/oauth2/v4 v4.5.4/go.mod h1:BXiOY+QZtZy2ewbsGk2B5P8TWmtz/Rf7ES5ZttQFxfQ=
|
||||
github.com/go-pkgz/auth/v2 v2.1.2-0.20260211003156-fbba7f2baa6b h1:N8iS/o/LgbSL4NLabOuLgfmROjtMLW2Qc3EsMmdYNGs=
|
||||
github.com/go-pkgz/auth/v2 v2.1.2-0.20260211003156-fbba7f2baa6b/go.mod h1:9LwzESczjMavmXNZo1XhYpfYdKWtoCbXt/ZIi0GTvF0=
|
||||
github.com/go-pkgz/auth/v2 v2.1.2-0.20260421203319-686683f19cf7 h1:NKcMUFdfoJY8e9zYcNBXQotrxTUaQwBc3nNPjE7nv0g=
|
||||
github.com/go-pkgz/auth/v2 v2.1.2-0.20260421203319-686683f19cf7/go.mod h1:IvxxhJIrwd1hKqFwQgBF9i+sMTmGfzAw66wmhw1zfJc=
|
||||
github.com/go-pkgz/email v0.6.0 h1:snZnXldjeF4PgKSjnx9Fa25mtOgFpAOEeWvnQvrxjLE=
|
||||
github.com/go-pkgz/email v0.6.0/go.mod h1:+wgi4x7S33IuCzfcCM5euN0GwQG6XvO/PBLxrNffYLI=
|
||||
github.com/go-pkgz/expirable-cache/v3 v3.1.0 h1:s05P851/O6QJ6Mc+7o2bh9aGtD3romB1SxDTXifdoqc=
|
||||
|
||||
+69
-52
@@ -63,6 +63,15 @@ type Opts struct {
|
||||
URL string // root url for the rest service, i.e. http://blah.example.com, required
|
||||
Validator token.Validator // validator allows to reject some valid tokens with user-defined logic
|
||||
|
||||
// AllowedRedirectHosts lists hostnames accepted in the "from" query
|
||||
// parameter of OAuth/verify login flows. Setting this field enables
|
||||
// host validation: the host of URL is always implicit, and any other
|
||||
// host must appear here. Nil (the default) disables validation and
|
||||
// preserves legacy permissive behavior — any non-empty "from" value
|
||||
// is honored. Hardening is opt-in; to restrict to the service host
|
||||
// only, pass a getter returning an empty slice.
|
||||
AllowedRedirectHosts token.AllowedHosts
|
||||
|
||||
AvatarStore avatar.Store // store to save/load avatars, required (use avatar.NoOp to disable avatars support)
|
||||
AvatarResizeLimit int // resize avatar's limit in pixels
|
||||
AvatarRoutePath string // avatar routing prefix, i.e. "/api/v1/avatar", default `/avatar`
|
||||
@@ -227,14 +236,15 @@ func (s *Service) Middleware() middleware.Authenticator {
|
||||
// AddProviderWithUserAttributes adds provider with user attributes mapping
|
||||
func (s *Service) AddProviderWithUserAttributes(name, cid, csecret string, userAttributes provider.UserAttributes) {
|
||||
p := provider.Params{
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
Cid: cid,
|
||||
Csecret: csecret,
|
||||
L: s.logger,
|
||||
UserAttributes: userAttributes,
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
Cid: cid,
|
||||
Csecret: csecret,
|
||||
L: s.logger,
|
||||
UserAttributes: userAttributes,
|
||||
AllowedRedirectHosts: s.opts.AllowedRedirectHosts,
|
||||
}
|
||||
s.addProviderByName(name, p)
|
||||
}
|
||||
@@ -309,14 +319,15 @@ func (s *Service) isValidProviderName(name string) bool {
|
||||
// AddProvider adds provider for given name
|
||||
func (s *Service) AddProvider(name, cid, csecret string) {
|
||||
p := provider.Params{
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
Cid: cid,
|
||||
Csecret: csecret,
|
||||
L: s.logger,
|
||||
UserAttributes: map[string]string{},
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
Cid: cid,
|
||||
Csecret: csecret,
|
||||
L: s.logger,
|
||||
UserAttributes: map[string]string{},
|
||||
AllowedRedirectHosts: s.opts.AllowedRedirectHosts,
|
||||
}
|
||||
s.addProviderByName(name, p)
|
||||
}
|
||||
@@ -327,15 +338,16 @@ func (s *Service) AddProvider(name, cid, csecret string) {
|
||||
// For advanced configuration (e.g., UserAttributes), construct provider.Params directly.
|
||||
func (s *Service) AddMicrosoftProvider(cid, csecret, tenant string) {
|
||||
p := provider.Params{
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
Cid: cid,
|
||||
Csecret: csecret,
|
||||
L: s.logger,
|
||||
UserAttributes: map[string]string{},
|
||||
MicrosoftTenant: tenant,
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
Cid: cid,
|
||||
Csecret: csecret,
|
||||
L: s.logger,
|
||||
UserAttributes: map[string]string{},
|
||||
MicrosoftTenant: tenant,
|
||||
AllowedRedirectHosts: s.opts.AllowedRedirectHosts,
|
||||
}
|
||||
s.addProvider(provider.NewMicrosoft(p))
|
||||
}
|
||||
@@ -343,13 +355,14 @@ func (s *Service) AddMicrosoftProvider(cid, csecret, tenant string) {
|
||||
// AddDevProvider with a custom host and port
|
||||
func (s *Service) AddDevProvider(host string, port int) {
|
||||
p := provider.Params{
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
L: s.logger,
|
||||
Port: port,
|
||||
Host: host,
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
L: s.logger,
|
||||
Port: port,
|
||||
Host: host,
|
||||
AllowedRedirectHosts: s.opts.AllowedRedirectHosts,
|
||||
}
|
||||
s.addProvider(provider.NewDev(p))
|
||||
}
|
||||
@@ -357,11 +370,12 @@ func (s *Service) AddDevProvider(host string, port int) {
|
||||
// AddAppleProvider allow SignIn with Apple ID
|
||||
func (s *Service) AddAppleProvider(appleConfig provider.AppleConfig, privKeyLoader provider.PrivateKeyLoaderInterface) error {
|
||||
p := provider.Params{
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
L: s.logger,
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
L: s.logger,
|
||||
AllowedRedirectHosts: s.opts.AllowedRedirectHosts,
|
||||
}
|
||||
|
||||
// error checking at create need for catch one when apple private key init
|
||||
@@ -377,13 +391,14 @@ func (s *Service) AddAppleProvider(appleConfig provider.AppleConfig, privKeyLoad
|
||||
// AddCustomProvider adds custom provider (e.g. https://gopkg.in/oauth2.v3)
|
||||
func (s *Service) AddCustomProvider(name string, client Client, copts provider.CustomHandlerOpt) {
|
||||
p := provider.Params{
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
Cid: client.Cid,
|
||||
Csecret: client.Csecret,
|
||||
L: s.logger,
|
||||
URL: s.opts.URL,
|
||||
JwtService: s.jwtService,
|
||||
Issuer: s.issuer,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
Cid: client.Cid,
|
||||
Csecret: client.Csecret,
|
||||
L: s.logger,
|
||||
AllowedRedirectHosts: s.opts.AllowedRedirectHosts,
|
||||
}
|
||||
s.addProvider(provider.NewCustom(name, p, copts))
|
||||
}
|
||||
@@ -421,14 +436,16 @@ func (s *Service) AddDirectProviderWithUserIDFunc(name string, credChecker provi
|
||||
// AddVerifProvider adds provider user's verification sent by sender
|
||||
func (s *Service) AddVerifProvider(name, msgTmpl string, sender provider.Sender) {
|
||||
dh := provider.VerifyHandler{
|
||||
L: s.logger,
|
||||
ProviderName: name,
|
||||
Issuer: s.issuer,
|
||||
TokenService: s.jwtService,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
Sender: sender,
|
||||
Template: msgTmpl,
|
||||
UseGravatar: s.useGravatar,
|
||||
L: s.logger,
|
||||
ProviderName: name,
|
||||
Issuer: s.issuer,
|
||||
TokenService: s.jwtService,
|
||||
AvatarSaver: s.avatarProxy,
|
||||
Sender: sender,
|
||||
Template: msgTmpl,
|
||||
UseGravatar: s.useGravatar,
|
||||
URL: s.opts.URL,
|
||||
AllowedRedirectHosts: s.opts.AllowedRedirectHosts,
|
||||
}
|
||||
s.addProvider(dh)
|
||||
}
|
||||
|
||||
+5
@@ -393,6 +393,11 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
// redirect to back url if presented in login query params
|
||||
if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" {
|
||||
if !isAllowedRedirect(oauthClaims.Handshake.From, ah.URL, ah.AllowedRedirectHosts) {
|
||||
ah.Logf("[WARN] rejected redirect to disallowed host: %s", redirectHostForLog(oauthClaims.Handshake.From))
|
||||
rest.RenderJSON(w, &u)
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, oauthClaims.Handshake.From, http.StatusTemporaryRedirect)
|
||||
return
|
||||
}
|
||||
|
||||
+5
@@ -163,6 +163,11 @@ func (h Oauth1Handler) AuthHandler(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
// redirect to back url if presented in login query params
|
||||
if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" {
|
||||
if !isAllowedRedirect(oauthClaims.Handshake.From, h.URL, h.AllowedRedirectHosts) {
|
||||
h.Logf("[WARN] rejected redirect to disallowed host: %s", redirectHostForLog(oauthClaims.Handshake.From))
|
||||
rest.RenderJSON(w, &u)
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, oauthClaims.Handshake.From, http.StatusTemporaryRedirect)
|
||||
return
|
||||
}
|
||||
|
||||
+13
@@ -42,6 +42,14 @@ type Params struct {
|
||||
AvatarSaver AvatarSaver
|
||||
UserAttributes UserAttributes
|
||||
|
||||
// AllowedRedirectHosts lists hostnames accepted in the "from" query
|
||||
// parameter. Setting this field enables host validation: the host of
|
||||
// URL is always implicit, and any other host must appear here. Nil
|
||||
// disables validation and preserves legacy permissive behavior — any
|
||||
// non-empty "from" value is honored. See isAllowedRedirect for the
|
||||
// full policy.
|
||||
AllowedRedirectHosts token.AllowedHosts
|
||||
|
||||
Port int // relevant for providers supporting port customization, for example dev oauth2
|
||||
Host string // relevant for providers supporting host customization, for example dev oauth2
|
||||
|
||||
@@ -239,6 +247,11 @@ func (p Oauth2Handler) AuthHandler(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
// redirect to back url if presented in login query params
|
||||
if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" {
|
||||
if !isAllowedRedirect(oauthClaims.Handshake.From, p.URL, p.AllowedRedirectHosts) {
|
||||
p.Logf("[WARN] rejected redirect to disallowed host: %s", redirectHostForLog(oauthClaims.Handshake.From))
|
||||
rest.RenderJSON(w, &u)
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, oauthClaims.Handshake.From, http.StatusTemporaryRedirect)
|
||||
return
|
||||
}
|
||||
|
||||
+69
@@ -0,0 +1,69 @@
|
||||
package provider
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
"strings"
|
||||
|
||||
"github.com/go-pkgz/auth/v2/token"
|
||||
)
|
||||
|
||||
// isAllowedRedirect reports whether the "from" URL is safe to redirect to
|
||||
// after a successful auth handshake.
|
||||
//
|
||||
// The check is opt-in: when allowed is nil the function returns true for any
|
||||
// non-empty input, preserving the behavior of versions before the redirect
|
||||
// validator existed. This keeps a dependency bump from breaking existing
|
||||
// consumers; hardening is enabled by setting Opts.AllowedRedirectHosts.
|
||||
//
|
||||
// When allowed is non-nil:
|
||||
// - only http/https schemes are accepted
|
||||
// - relative paths and unparseable URLs are rejected
|
||||
// - the service's own host (derived from serviceURL) is always allowed
|
||||
// - any other host must appear in the allowed list
|
||||
//
|
||||
// Hostname comparison is case-insensitive and ignores the port:
|
||||
// https://app.example.com:443 and https://App.Example.Com are treated as the
|
||||
// same host. Operators wanting strict port-aware checks should list each
|
||||
// host:port form explicitly via AllowedHosts.
|
||||
func isAllowedRedirect(from, serviceURL string, allowed token.AllowedHosts) bool {
|
||||
// permissive default: no allowlist configured = legacy behavior.
|
||||
// guard against typed-nil AllowedHostsFunc values (non-nil interface
|
||||
// wrapping a nil func) to avoid panicking in Get().
|
||||
if allowed == nil {
|
||||
return from != ""
|
||||
}
|
||||
if fn, ok := allowed.(token.AllowedHostsFunc); ok && fn == nil {
|
||||
return from != ""
|
||||
}
|
||||
u, err := url.Parse(from)
|
||||
if err != nil || u.Hostname() == "" {
|
||||
return false
|
||||
}
|
||||
if u.Scheme != "http" && u.Scheme != "https" {
|
||||
return false
|
||||
}
|
||||
fromHost := u.Hostname()
|
||||
if svc, sErr := url.Parse(serviceURL); sErr == nil && svc.Hostname() != "" && strings.EqualFold(svc.Hostname(), fromHost) {
|
||||
return true
|
||||
}
|
||||
hosts, hErr := allowed.Get()
|
||||
if hErr != nil {
|
||||
return false
|
||||
}
|
||||
for _, h := range hosts {
|
||||
if strings.EqualFold(h, fromHost) || strings.EqualFold(h, u.Host) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// redirectHostForLog extracts just the hostname from a from-URL for logging
|
||||
// on rejection, so attacker-supplied paths/queries do not leak into operator
|
||||
// logs. Returns a sentinel if the URL cannot be parsed.
|
||||
func redirectHostForLog(from string) string {
|
||||
if u, err := url.Parse(from); err == nil && u.Hostname() != "" {
|
||||
return u.Hostname()
|
||||
}
|
||||
return "<unparseable>"
|
||||
}
|
||||
+15
@@ -28,6 +28,16 @@ type VerifyHandler struct {
|
||||
Sender Sender
|
||||
Template string
|
||||
UseGravatar bool
|
||||
|
||||
// URL is the service's own root URL; its host is always permitted as
|
||||
// a "from" redirect target. Optional but recommended.
|
||||
URL string
|
||||
// AllowedRedirectHosts lists additional hostnames permitted as "from"
|
||||
// redirect targets. Setting this field enables host validation: the
|
||||
// host of URL is always implicit, and any other host must appear
|
||||
// here. Nil disables validation and preserves legacy permissive
|
||||
// behavior — any non-empty "from" value is honored.
|
||||
AllowedRedirectHosts token.AllowedHosts
|
||||
}
|
||||
|
||||
// Sender defines interface to send emails
|
||||
@@ -127,6 +137,11 @@ func (e VerifyHandler) LoginHandler(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
if confClaims.Handshake != nil && confClaims.Handshake.From != "" {
|
||||
if !isAllowedRedirect(confClaims.Handshake.From, e.URL, e.AllowedRedirectHosts) {
|
||||
e.Logf("[WARN] rejected redirect to disallowed host: %s", redirectHostForLog(confClaims.Handshake.From))
|
||||
rest.RenderJSON(w, claims.User)
|
||||
return
|
||||
}
|
||||
http.Redirect(w, r, confClaims.Handshake.From, http.StatusTemporaryRedirect)
|
||||
return
|
||||
}
|
||||
|
||||
+21
-1
@@ -178,7 +178,7 @@ func (j *Service) Parse(tokenString string) (Claims, error) {
|
||||
return Claims{}, fmt.Errorf("can't get secret: %w", err)
|
||||
}
|
||||
|
||||
token, err := parser.ParseWithClaims(tokenString, &Claims{}, func(token *jwt.Token) (interface{}, error) {
|
||||
token, err := parser.ParseWithClaims(tokenString, &Claims{}, func(token *jwt.Token) (any, error) {
|
||||
if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok {
|
||||
return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"])
|
||||
}
|
||||
@@ -451,3 +451,23 @@ type AudienceFunc func() ([]string, error)
|
||||
func (f AudienceFunc) Get() ([]string, error) {
|
||||
return f()
|
||||
}
|
||||
|
||||
// AllowedHosts defines interface returning list of hostnames allowed in the
|
||||
// "from" redirect parameter of OAuth and verify flows. The service's own host
|
||||
// (derived from Opts.URL) is always allowed implicitly; this list is for
|
||||
// additional hosts.
|
||||
type AllowedHosts interface {
|
||||
Get() ([]string, error)
|
||||
}
|
||||
|
||||
// AllowedHostsFunc adapter to allow ordinary functions to be used as AllowedHosts.
|
||||
// Assigning a nil AllowedHostsFunc to an interface field (e.g.
|
||||
// Opts.AllowedRedirectHosts) produces a typed-nil interface that would panic
|
||||
// when Get is called; the provider-side validator recognizes this form and
|
||||
// treats it as "no allowlist configured".
|
||||
type AllowedHostsFunc func() ([]string, error)
|
||||
|
||||
// Get calls f()
|
||||
func (f AllowedHostsFunc) Get() ([]string, error) {
|
||||
return f()
|
||||
}
|
||||
|
||||
Vendored
+1
-1
@@ -56,7 +56,7 @@ github.com/go-chi/cors
|
||||
github.com/go-oauth2/oauth2/v4
|
||||
github.com/go-oauth2/oauth2/v4/errors
|
||||
github.com/go-oauth2/oauth2/v4/server
|
||||
# github.com/go-pkgz/auth/v2 v2.1.2-0.20260211003156-fbba7f2baa6b
|
||||
# github.com/go-pkgz/auth/v2 v2.1.2-0.20260421203319-686683f19cf7
|
||||
## explicit; go 1.24.0
|
||||
github.com/go-pkgz/auth/v2
|
||||
github.com/go-pkgz/auth/v2/avatar
|
||||
|
||||
Reference in New Issue
Block a user