Skip to content

Commit

Permalink
feat:Allow wildcard domains for redirect_to checks
Browse files Browse the repository at this point in the history
  • Loading branch information
abador committed Jul 12, 2021
1 parent 953c6d6 commit e558525
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 2 deletions.
2 changes: 2 additions & 0 deletions docs/docs/reference/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,7 @@ selfservice:
# - - https://app.my-app.com/dashboard
# - /dashboard
# - https://www.my-app.com/
# - https://*.my-app.com/
#
# Set this value using environment variables on
# - Linux/macOS:
Expand All @@ -1029,6 +1030,7 @@ selfservice:
- https://app.my-app.com/dashboard
- /dashboard
- https://www.my-app.com/
- https://*.my-app.com/

## serve ##
#
Expand Down
13 changes: 13 additions & 0 deletions driver/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"testing"
"time"

"golang.org/x/net/publicsuffix"

"github.com/google/uuid"

"github.com/ory/x/dbal"
Expand Down Expand Up @@ -672,6 +674,17 @@ func (p *Config) SelfServiceBrowserWhitelistedReturnToDomains() (us []url.URL) {
p.l.WithError(err).Warnf("Ignoring URL \"%s\" from configuration key \"%s.%d\".", u, ViperKeyURLsWhitelistedReturnToDomains, k)
continue
}
if parsed.Host == "*" {
p.l.Warnf("Ignoring wildcard \"%s\" from configuration key \"%s.%d\".", u, ViperKeyURLsWhitelistedReturnToDomains, k)
continue
}
eTLD, icann := publicsuffix.PublicSuffix(parsed.Host)
if parsed.Host[:1] == "*" &&
icann &&
parsed.Host == fmt.Sprintf("*.%s", eTLD) {
p.l.Warnf("Ignoring wildcard \"%s\" from configuration key \"%s.%d\".", u, ViperKeyURLsWhitelistedReturnToDomains, k)
continue
}

us = append(us, *parsed)
}
Expand Down
1 change: 1 addition & 0 deletions driver/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func TestViperProvider(t *testing.T) {
assert.Equal(t, []string{
"http://return-to-1-test.ory.sh/",
"http://return-to-2-test.ory.sh/",
"http://*.wildcards.ory.sh",
}, ds)

pWithFragments := config.MustNew(t, logrusx.New("", ""),
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ require (
github.com/urfave/negroni v1.0.0
github.com/ziutek/mymysql v1.5.4 // indirect
golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110
golang.org/x/oauth2 v0.0.0-20210323180902-22b0adad7558
golang.org/x/sync v0.0.0-20201207232520-09787c993a3a
golang.org/x/tools v0.1.0
Expand Down
1 change: 0 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,6 @@ github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk=
github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
github.com/globalsign/mgo v0.0.0-20180905125535-1ca0a4f7cbcb/go.mod h1:xkRDCp4j0OGD1HRkm4kmhM+pmpv3AKq5SU7GMg4oO/Q=
github.com/globalsign/mgo v0.0.0-20181015135952-eeefdecb41b8/go.mod h1:xkRDCp4j0OGD1HRkm4kmhM+pmpv3AKq5SU7GMg4oO/Q=
github.com/go-bindata/go-bindata v3.1.1+incompatible h1:tR4f0e4VTO7LK6B2YWyAoVEzG9ByG1wrXB4TL9+jiYg=
github.com/go-bindata/go-bindata v3.1.1+incompatible/go.mod h1:xK8Dsgwmeed+BBsSy2XTopBn/8uK2HWuGSnA11C3Joo=
github.com/go-errors/errors v1.0.1 h1:LUHzmkK3GUKUrL/1gfBUxAHzcev3apQlezX/+O7ma6w=
github.com/go-errors/errors v1.0.1/go.mod h1:f4zRHt4oKfwPJE5k8C9vpYG+aDHdBFUsgrm6/TyX73Q=
Expand Down
5 changes: 5 additions & 0 deletions internal/.kratos.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ selfservice:
whitelisted_return_urls:
- http://return-to-1-test.ory.sh/
- http://return-to-2-test.ory.sh/
- http://*.wildcards.ory.sh
- http://*.sh
- http://*.com
- http://*.com.pl
- http://*
methods:
password:
enabled: true
Expand Down
10 changes: 9 additions & 1 deletion x/http_secure_redirect.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ func SecureRedirectOverrideDefaultReturnTo(defaultReturnTo *url.URL) SecureRedir
}
}

// SecureRedirectToIsWhitelisted validates if the redirect_to param is allowed for a given wildcard
func SecureRedirectToIsWhiteListedHost(returnTo *url.URL, allowed url.URL) bool {
if allowed.Host != "" && allowed.Host[:1] == "*" {
strings.HasSuffix(returnTo.Host, allowed.Host[1:])
}
return strings.EqualFold(allowed.Host, returnTo.Host)
}

// SecureRedirectTo implements a HTTP redirector who mitigates open redirect vulnerabilities by
// working with whitelisting.
func SecureRedirectTo(r *http.Request, defaultReturnTo *url.URL, opts ...SecureRedirectOption) (returnTo *url.URL, err error) {
Expand Down Expand Up @@ -87,7 +95,7 @@ func SecureRedirectTo(r *http.Request, defaultReturnTo *url.URL, opts ...SecureR
var found bool
for _, allowed := range o.whitelist {
if strings.EqualFold(allowed.Scheme, returnTo.Scheme) &&
strings.EqualFold(allowed.Host, returnTo.Host) &&
SecureRedirectToIsWhiteListedHost(returnTo, allowed) &&
strings.HasPrefix(
stringsx.Coalesce(returnTo.Path, "/"),
stringsx.Coalesce(allowed.Path, "/")) {
Expand Down

0 comments on commit e558525

Please sign in to comment.