fix(provider): backport "from" redirect validator to v1 (sibling of #275)

auth.go

@@ -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`
@@ -226,14 +235,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)
 }
@@ -308,14 +318,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)
 }
@@ -326,41 +337,44 @@ 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))
 }
 
 // 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))
 }
 
 // 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
@@ -376,13 +390,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))
 }
@@ -420,14 +435,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)
 }

middleware/auth_test.go

@@ -340,7 +340,7 @@ func TestBasicAdminUserDoesNotLogPassword(t *testing.T) {
 	const secret = "super-secret-attempted-passwd"
 	var buf strings.Builder
 	a := makeTestAuth(t)
-	a.L = logger.Func(func(format string, args ...interface{}) {
+	a.L = logger.Func(func(format string, args ...any) {
 		fmt.Fprintf(&buf, format, args...)
 	})
 

provider/apple.go

@@ -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
 	}

provider/apple_test.go

@@ -327,6 +327,37 @@ func TestAppleHandler_LoginHandler(t *testing.T) {
 
 }
 
+// TestAppleHandler_LoginHandlerFromRejectsExternalHost is the regression
+// test for the redirect validator on the apple login path: with an allowlist
+// policy enabled, /login?from=https://evil.example.com must NOT 302 to evil.
+func TestAppleHandler_LoginHandlerFromRejectsExternalHost(t *testing.T) {
+	enablePolicy := func(p *Params) {
+		p.AllowedRedirectHosts = token.AllowedHostsFunc(func() ([]string, error) { return nil, nil })
+	}
+	teardown := prepareAppleOauthTest(t, 8987, 8988, nil, enablePolicy)
+	defer teardown()
+
+	jar, err := cookiejar.New(nil)
+	require.NoError(t, err)
+
+	client := &http.Client{Jar: jar, Timeout: 5 * time.Second,
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			if !strings.HasPrefix(req.URL.Host, "localhost") {
+				return fmt.Errorf("blocked external redirect to %s", req.URL)
+			}
+			return nil
+		},
+	}
+
+	resp, err := client.Get("http://localhost:8987/login?site=remark&from=https://evil.example.com/phish")
+	require.NoError(t, err, "no external redirect expected -- fix should reject evil host")
+	defer resp.Body.Close()
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+	body, err := io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	assert.NotContains(t, string(body), "evil.example.com")
+}
+
 func TestAppleHandler_LogoutHandler(t *testing.T) {
 
 	teardown := prepareAppleOauthTest(t, 8691, 8692, nil)
@@ -457,7 +488,7 @@ func prepareAppleHandlerTest(responseMode string, scopes []string) (*AppleHandle
 	return NewApple(p, aCfg, cl)
 }
 
-func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *string) func() {
+func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *string, paramOpts ...func(*Params)) func() {
 	signKey, testJWK := createTestSignKeyPairs(t)
 	provider, err := prepareAppleHandlerTest("", []string{})
 	assert.NoError(t, err)
@@ -504,6 +535,9 @@ func prepareAppleOauthTest(t *testing.T, loginPort, authPort int, testToken *str
 
 	params := Params{URL: "url", Cid: "cid", Csecret: "csecret", JwtService: jwtService,
 		Issuer: "go-pkgz/auth", L: logger.Std}
+	for _, opt := range paramOpts {
+		opt(&params)
+	}
 	provider.Params = params
 
 	svc := Service{Provider: provider}

provider/oauth1.go

@@ -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
 	}

provider/oauth1_test.go

@@ -184,7 +184,40 @@ func TestOauth1MakeRedirURL(t *testing.T) {
 	}
 }
 
-func prepOauth1Test(t *testing.T, loginPort, authPort int) func() { //nolint
+// TestOauth1LoginFromRejectsExternalHost is the regression test for the
+// redirect validator on the oauth1 path: with an allowlist policy enabled,
+// /login?from=https://evil.example.com must NOT 302 to evil and must fall
+// back to the user-info JSON response.
+func TestOauth1LoginFromRejectsExternalHost(t *testing.T) {
+	enablePolicy := func(p *Params) {
+		p.AllowedRedirectHosts = token.AllowedHostsFunc(func() ([]string, error) { return nil, nil })
+	}
+	teardown := prepOauth1Test(t, loginPort, authPort, enablePolicy)
+	defer teardown()
+
+	jar, err := cookiejar.New(nil)
+	require.NoError(t, err)
+
+	client := &http.Client{Jar: jar, Timeout: timeout * time.Second,
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			if !strings.HasPrefix(req.URL.Host, "localhost") {
+				return fmt.Errorf("blocked external redirect to %s", req.URL)
+			}
+			return nil
+		},
+	}
+
+	resp, err := client.Get(fmt.Sprintf("http://localhost:%d/login?site=remark&from=https://evil.example.com/phish", loginPort))
+	require.NoError(t, err, "no external redirect expected -- fix should reject evil host")
+	defer resp.Body.Close()
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+	body, err := io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	assert.Contains(t, string(body), `"name":"blah"`)
+	assert.NotContains(t, string(body), "evil.example.com")
+}
+
+func prepOauth1Test(t *testing.T, loginPort, authPort int, paramOpts ...func(*Params)) func() { //nolint
 
 	provider := Oauth1Handler{
 		name: "mock",
@@ -223,6 +256,9 @@ func prepOauth1Test(t *testing.T, loginPort, authPort int) func() { //nolint
 
 	params := Params{URL: "url", Cid: "aFdj12348sdja", Csecret: "Dwehsq2387akss", JwtService: jwtService,
 		Issuer: "remark42", AvatarSaver: &mockAvatarSaver{}, L: logger.Std}
+	for _, opt := range paramOpts {
+		opt(&params)
+	}
 
 	provider = initOauth1Handler(params, provider)
 	svc := Service{Provider: provider}

provider/oauth2.go

@@ -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
 	}