From 688103c7ffc59b7012c606f2c7c375f12337c35f Mon Sep 17 00:00:00 2001 From: Wyatt Anderson Date: Wed, 14 Jun 2017 10:54:37 -0400 Subject: [PATCH] oauth2: use issuer-prefixed auth URL in challenge redirect (#509) In order to support running Hydra with a different path prefix behind a proxy, issue a challenge token with an issuer-prefixed auth redirect URL instead of the URL received with the auth request. Signed-off-by: Wyatt Anderson --- cmd/host.go | 2 +- cmd/root.go | 2 +- cmd/server/handler.go | 12 ++++++++ oauth2/handler.go | 29 +++++++++++++---- oauth2/handler_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++ oauth2/oauth2_test.go | 4 +++ 6 files changed, 111 insertions(+), 8 deletions(-) diff --git a/cmd/host.go b/cmd/host.go index b4829126e9e..fbb39e62f96 100644 --- a/cmd/host.go +++ b/cmd/host.go @@ -77,7 +77,7 @@ OAUTH2 CONTROLS Example: CONSENT_URL=https://id.myapp.com/consent - ISSUER: The issuer is used for identification in all OAuth2 tokens. Should be the public url of the server. - Defaults to ISSUER=hydra.localhost + Defaults to ISSUER=http://localhost:4444 - AUTH_CODE_LIFESPAN: Lifespan of OAuth2 authorize codes. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". Defaults to AUTH_CODE_LIFESPAN=10m diff --git a/cmd/root.go b/cmd/root.go index ac624b11e7a..846c966dad9 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -110,7 +110,7 @@ func initConfig() { viper.SetDefault("PORT", 4444) viper.BindEnv("ISSUER") - viper.SetDefault("ISSUER", "hydra.localhost") + viper.SetDefault("ISSUER", "http://localhost:4444") viper.BindEnv("BCRYPT_COST") viper.SetDefault("BCRYPT_COST", 10) diff --git a/cmd/server/handler.go b/cmd/server/handler.go index 6e1d2cd1fa2..33ea6a07154 100644 --- a/cmd/server/handler.go +++ b/cmd/server/handler.go @@ -4,6 +4,7 @@ import ( "crypto/tls" "fmt" "net/http" + "net/url" "os" @@ -37,6 +38,17 @@ func RunHost(c *config.Config) func(cmd *cobra.Command, args []string) { serverHandler.registerRoutes(router) c.ForceHTTP, _ = cmd.Flags().GetBool("dangerous-force-http") + if !c.ForceHTTP { + if c.Issuer == "" { + logger.Fatalln("Issuer must be explicitly specified unless --dangerous-force-http is passed.") + } + issuer, err := url.Parse(c.Issuer) + pkg.Must(err, "Could not parse issuer URL: %s", err) + if issuer.Scheme != "https" { + logger.Fatalln("Issuer must use HTTPS unless --dangerous-force-http is passed.") + } + } + if c.ClusterURL == "" { proto := "https" if c.ForceHTTP { diff --git a/oauth2/handler.go b/oauth2/handler.go index 43f3c65ba3b..70e199dd7c1 100644 --- a/oauth2/handler.go +++ b/oauth2/handler.go @@ -2,6 +2,7 @@ package oauth2 import ( "encoding/json" + "net" "net/http" "net/url" @@ -365,15 +366,31 @@ func (h *Handler) AuthHandler(w http.ResponseWriter, r *http.Request, _ httprout } func (h *Handler) redirectToConsent(w http.ResponseWriter, r *http.Request, authorizeRequest fosite.AuthorizeRequester) error { - schema := "https" - if h.ForcedHTTP { - schema = "http" - } - // Error can be ignored because a session will always be returned cookie, _ := h.CookieStore.Get(r, consentCookieName) - challenge, err := h.Consent.IssueChallenge(authorizeRequest, schema+"://"+r.Host+r.URL.String(), cookie) + host, _, err := net.SplitHostPort(r.Host) + if err != nil { + host = r.Host + } + + authUrl, err := url.Parse(h.Issuer + AuthPath) + if err != nil { + return err + } + authHost, _, err := net.SplitHostPort(authUrl.Host) + if err != nil { + authHost = authUrl.Host + } + if authHost != host { + h.L.WithFields(logrus.Fields{ + "request_host": host, + "issuer_host": authHost, + }).Warnln("Host from auth request does not match issuer host. The consent return redirect may fail.") + } + authUrl.RawQuery = r.URL.RawQuery + + challenge, err := h.Consent.IssueChallenge(authorizeRequest, authUrl.String(), cookie) if err != nil { return err } diff --git a/oauth2/handler_test.go b/oauth2/handler_test.go index 26240b47aa2..24e36ddc1db 100644 --- a/oauth2/handler_test.go +++ b/oauth2/handler_test.go @@ -1,13 +1,21 @@ package oauth2 import ( + "crypto/rand" + "crypto/rsa" "net/http" "net/http/httptest" + "net/url" "testing" "encoding/json" + "github.com/Sirupsen/logrus" + "github.com/gorilla/sessions" "github.com/julienschmidt/httprouter" + "github.com/ory/fosite" + "github.com/ory/fosite/compose" + "github.com/ory/fosite/storage" "github.com/ory/herodot" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -45,3 +53,65 @@ func TestHandlerWellKnown(t *testing.T) { require.NoError(t, err, "problem decoding wellknown json response: %+v", err) assert.Equal(t, trueConfig, wellKnownResp) } + +type FakeConsentStrategy struct { + RedirectURL string +} + +func (s *FakeConsentStrategy) ValidateResponse(authorizeRequest fosite.AuthorizeRequester, token string, session *sessions.Session) (claims *Session, err error) { + return nil, nil +} + +func (s *FakeConsentStrategy) IssueChallenge(authorizeRequest fosite.AuthorizeRequester, redirectURL string, session *sessions.Session) (token string, err error) { + s.RedirectURL = redirectURL + return "token", nil +} + +func TestIssuerRedirect(t *testing.T) { + storage := storage.NewExampleStore() + secret := []byte("my super secret password") + config := compose.Config{} + privateKey, _ := rsa.GenerateKey(rand.Reader, 2048) + + consentUrl, _ := url.Parse("http://consent.localhost") + + cs := &FakeConsentStrategy{} + + h := &Handler{ + H: herodot.NewJSONWriter(nil), + Issuer: "http://127.0.0.1/some/proxied/path", + OAuth2: compose.ComposeAllEnabled(&config, storage, secret, privateKey), + ConsentURL: *consentUrl, + CookieStore: sessions.NewCookieStore([]byte("my super secret password")), + Consent: cs, + L: logrus.New(), + } + + r := httprouter.New() + h.SetRoutes(r) + ts := httptest.NewServer(r) + + authUrl, _ := url.Parse(ts.URL) + v := url.Values{} + v.Set("response_type", "code") + v.Set("client_id", "my-client") + v.Set("redirect_uri", "http://localhost:3846/callback") + v.Set("scope", "openid") + v.Set("state", "my super secret state") + authUrl.Path = "/oauth2/auth" + authUrl.RawQuery = v.Encode() + + client := &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } + + res, _ := client.Get(authUrl.String()) + + authRedirect, _ := url.Parse(cs.RedirectURL) + assert.Equal(t, "/some/proxied/path/oauth2/auth", authRedirect.Path, "The redirect URL sent in the challenge includes the full issuer path") + assert.Equal(t, authUrl.Query(), authRedirect.Query(), "The auth redirect should have the same parameters with the addition of challenge") + + defer res.Body.Close() +} diff --git a/oauth2/oauth2_test.go b/oauth2/oauth2_test.go index a1e10e01f69..7120eec5a77 100644 --- a/oauth2/oauth2_test.go +++ b/oauth2/oauth2_test.go @@ -6,6 +6,7 @@ import ( "net/url" "time" + "github.com/Sirupsen/logrus" "github.com/dgrijalva/jwt-go" "github.com/gorilla/sessions" "github.com/julienschmidt/httprouter" @@ -67,6 +68,7 @@ var handler = &Handler{ }, CookieStore: sessions.NewCookieStore([]byte("foo-secret")), ForcedHTTP: true, + L: logrus.New(), } var router = httprouter.New() @@ -84,6 +86,8 @@ func init() { keyManager.AddKeySet(ConsentEndpointKey, keys) ts = httptest.NewServer(router) + handler.Issuer = ts.URL + handler.SetRoutes(router) h, _ := hasher.Hash([]byte("secret")) store.Manager.(*hc.MemoryManager).Clients["app"] = hc.Client{