From 3bb9621f5de4d32e95877ed0337b1c1947aa99c5 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Wed, 16 Feb 2022 13:25:59 +0000 Subject: [PATCH] Add DiscoveryProvider to perform OIDC discovery --- go.mod | 4 +- go.sum | 12 ++- pkg/providers/oidc/provider.go | 81 ++++++++++++++++++++ pkg/providers/oidc/provider_test.go | 114 ++++++++++++++++++++++++++++ 4 files changed, 208 insertions(+), 3 deletions(-) create mode 100644 pkg/providers/oidc/provider.go create mode 100644 pkg/providers/oidc/provider_test.go diff --git a/go.mod b/go.mod index 1df17f69..2621368c 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/bitly/go-simplejson v0.5.0 github.com/bsm/redislock v0.7.0 github.com/coreos/go-oidc/v3 v3.0.0 + github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/fsnotify/fsnotify v1.4.9 github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 github.com/go-redis/redis/v8 v8.2.3 @@ -18,6 +19,7 @@ require ( github.com/justinas/alice v1.2.0 github.com/mbland/hmacauth v0.0.0-20170912233209-44256dfd4bfa github.com/mitchellh/mapstructure v1.1.2 + github.com/oauth2-proxy/mockoidc v0.0.0-20210703044157-382d3faf2671 github.com/oauth2-proxy/tools/reference-gen v0.0.0-20210118095127-56ffd7384404 github.com/onsi/ginkgo v1.14.1 github.com/onsi/gomega v1.10.2 @@ -26,7 +28,7 @@ require ( github.com/spf13/cast v1.3.0 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.6.3 - github.com/stretchr/testify v1.6.1 + github.com/stretchr/testify v1.7.0 github.com/vmihailenco/msgpack/v4 v4.3.11 golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97 golang.org/x/net v0.0.0-20210226172049-e18ecbb05110 diff --git a/go.sum b/go.sum index a0940db2..54be4caf 100644 --- a/go.sum +++ b/go.sum @@ -81,6 +81,7 @@ github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7Do github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dgrijalva/jwt-go v3.2.0+incompatible h1:7qlOGliEKZXTDg6OTjfoBKDXWrumCAMpl/TFQ4/5kLM= github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc= @@ -169,8 +170,9 @@ github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5a github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k= github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= +github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= @@ -293,6 +295,8 @@ github.com/nats-io/nkeys v0.1.3/go.mod h1:xpnFELMwJABBLVhffcfd1MZx6VsNRFpEugbxzi github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c= github.com/nxadm/tail v1.4.4 h1:DQuhQpB1tVlglWS2hLQ5OV6B5r8aGxSrPc5Qo6uTN78= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= +github.com/oauth2-proxy/mockoidc v0.0.0-20210703044157-382d3faf2671 h1:rCw45DEDqAy46HsVu2WZQgEqKth79c8k6HZP2uPDTls= +github.com/oauth2-proxy/mockoidc v0.0.0-20210703044157-382d3faf2671/go.mod h1:ejAP84CdQJiV7GcnIF0wNhbhcWsxew0qXV04CB+r+Mw= github.com/oauth2-proxy/tools/reference-gen v0.0.0-20210118095127-56ffd7384404 h1:ZpzR4Ou1nhldBG/vEzauoqyaUlofaUcLkv1C/gBK8ls= github.com/oauth2-proxy/tools/reference-gen v0.0.0-20210118095127-56ffd7384404/go.mod h1:YpORG8zs14vNlpXvuHYnnDvWazIRaDk02MaY8lafqdI= github.com/oklog/oklog v0.3.2/go.mod h1:FCV+B7mhrz4o+ueLpx+KqkyXRGMWOYEvfiXtdGtbWGs= @@ -412,8 +416,9 @@ github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXf github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= -github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= @@ -456,6 +461,7 @@ golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8U golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97 h1:/UOmuWzQfxxo9UtlXMwuQU8CMgg1eZXqTRwkSQJWKOI= golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -535,6 +541,7 @@ golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20191220142924-d4481acd189f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200106162015-b016eb3dc98e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -547,6 +554,7 @@ golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20201214210602-f9fddec55a1e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1 h1:SrN+KX8Art/Sf4HNj6Zcz06G7VEz+7w9tdXTPOZ7+l4= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= diff --git a/pkg/providers/oidc/provider.go b/pkg/providers/oidc/provider.go new file mode 100644 index 00000000..ddc8a2e1 --- /dev/null +++ b/pkg/providers/oidc/provider.go @@ -0,0 +1,81 @@ +package oidc + +import ( + "context" + "fmt" + "strings" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" +) + +// providerJSON resresents the information we need from an OIDC discovery +type providerJSON struct { + Issuer string `json:"issuer"` + AuthURL string `json:"authorization_endpoint"` + TokenURL string `json:"token_endpoint"` + JWKsURL string `json:"jwks_uri"` + UserInfoURL string `json:"userinfo_endpoint"` +} + +// Endpoints represents the endpoints discovered as part of the OIDC discovery process +// that will be used by the authentication providers. +type Endpoints struct { + AuthURL string + TokenURL string + JWKsURL string + UserInfoURL string +} + +// DiscoveryProvider holds information about an identity provider having +// used OIDC discovery to retrieve the information. +type DiscoveryProvider interface { + Endpoints() Endpoints +} + +// NewProvider allows a user to perform an OIDC discovery and returns the DiscoveryProvider. +// We implement this here as opposed to using oidc.Provider so that we can override the Issuer verification check. +// As we have our own verifier and fetch the userinfo separately, the rest of the oidc.Provider implementation is not +// useful to us. +func NewProvider(ctx context.Context, issuerURL string, skipIssuerVerification bool) (DiscoveryProvider, error) { + // go-oidc doesn't let us pass bypass the issuer check this in the oidc.NewProvider call + // (which uses discovery to get the URLs), so we'll do a quick check ourselves and if + // we get the URLs, we'll just use the non-discovery path. + + logger.Printf("Performing OIDC Discovery...") + + var p providerJSON + requestURL := strings.TrimSuffix(issuerURL, "/") + "/.well-known/openid-configuration" + if err := requests.New(requestURL).WithContext(ctx).Do().UnmarshalInto(&p); err != nil { + return nil, fmt.Errorf("failed to discover OIDC configuration: %v", err) + } + + if !skipIssuerVerification && p.Issuer != issuerURL { + return nil, fmt.Errorf("oidc: issuer did not match the issuer returned by provider, expected %q got %q", issuerURL, p.Issuer) + } + + return &discoveryProvider{ + authURL: p.AuthURL, + tokenURL: p.TokenURL, + jwksURL: p.JWKsURL, + userInfoURL: p.UserInfoURL, + }, nil +} + +// discoveryProvider holds the discovered endpoints +type discoveryProvider struct { + authURL string + tokenURL string + jwksURL string + userInfoURL string +} + +// Endpoints returns the discovered endpoints needed for an authentication provider. +func (p *discoveryProvider) Endpoints() Endpoints { + return Endpoints{ + AuthURL: p.authURL, + TokenURL: p.tokenURL, + JWKsURL: p.jwksURL, + UserInfoURL: p.userInfoURL, + } +} diff --git a/pkg/providers/oidc/provider_test.go b/pkg/providers/oidc/provider_test.go new file mode 100644 index 00000000..731bfb93 --- /dev/null +++ b/pkg/providers/oidc/provider_test.go @@ -0,0 +1,114 @@ +package oidc + +import ( + "context" + "encoding/json" + "net" + "net/http" + + "github.com/oauth2-proxy/mockoidc" + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" +) + +var _ = Describe("Provider", func() { + type newProviderTableInput struct { + skipIssuerVerification bool + expectedError string + middlewares func(*mockoidc.MockOIDC) []func(http.Handler) http.Handler + } + + DescribeTable("NewProvider", func(in *newProviderTableInput) { + m, err := mockoidc.NewServer(nil) + Expect(err).ToNot(HaveOccurred()) + + if in.middlewares != nil { + middlewares := in.middlewares(m) + for _, middlware := range middlewares { + m.AddMiddleware(middlware) + } + } + + ln, err := net.Listen("tcp", "127.0.0.1:0") + Expect(err).ToNot(HaveOccurred()) + + Expect(m.Start(ln, nil)).To(Succeed()) + defer func() { + Expect(m.Shutdown()).To(Succeed()) + }() + + provider, err := NewProvider(context.Background(), m.Issuer(), in.skipIssuerVerification) + if in.expectedError != "" { + Expect(err).To(MatchError(HavePrefix(in.expectedError))) + return + } + Expect(err).ToNot(HaveOccurred()) + + endpoints := provider.Endpoints() + Expect(endpoints.AuthURL).To(Equal(m.AuthorizationEndpoint())) + Expect(endpoints.TokenURL).To(Equal(m.TokenEndpoint())) + Expect(endpoints.JWKsURL).To(Equal(m.JWKSEndpoint())) + Expect(endpoints.UserInfoURL).To(Equal(m.UserinfoEndpoint())) + }, + Entry("with issuer verification and the issuer matches", &newProviderTableInput{ + skipIssuerVerification: false, + }), + Entry("with skip issuer verification and the issuer matches", &newProviderTableInput{ + skipIssuerVerification: true, + }), + Entry("with issuer verification and an invalid issuer", &newProviderTableInput{ + skipIssuerVerification: false, + middlewares: func(m *mockoidc.MockOIDC) []func(http.Handler) http.Handler { + return []func(http.Handler) http.Handler{ + newInvalidIssuerMiddleware(m), + } + }, + expectedError: "oidc: issuer did not match the issuer returned by provider", + }), + Entry("with skip issuer verification and an invalid issuer", &newProviderTableInput{ + skipIssuerVerification: true, + middlewares: func(m *mockoidc.MockOIDC) []func(http.Handler) http.Handler { + return []func(http.Handler) http.Handler{ + newInvalidIssuerMiddleware(m), + } + }, + }), + Entry("when the issuer returns a bad response", &newProviderTableInput{ + skipIssuerVerification: false, + middlewares: func(m *mockoidc.MockOIDC) []func(http.Handler) http.Handler { + return []func(http.Handler) http.Handler{ + newBadRequestMiddleware(), + } + }, + expectedError: "failed to discover OIDC configuration: unexpected status \"400\"", + }), + ) +}) + +func newInvalidIssuerMiddleware(m *mockoidc.MockOIDC) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + p := providerJSON{ + Issuer: "invalid", + AuthURL: m.AuthorizationEndpoint(), + TokenURL: m.TokenEndpoint(), + JWKsURL: m.JWKSEndpoint(), + UserInfoURL: m.UserinfoEndpoint(), + } + data, err := json.Marshal(p) + if err != nil { + rw.WriteHeader(500) + } + rw.Write(data) + }) + } +} + +func newBadRequestMiddleware() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.WriteHeader(400) + }) + } +}