From ac032e99f05625820e354119c548bf5a6ab04d4f Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Mon, 20 Mar 2017 08:38:52 -0700 Subject: [PATCH 1/2] connector/oidc: expose oauth2.RegisterBrokenAuthHeaderProvider --- connector/oidc/oidc.go | 55 +++++++++++++++++++++++++++++++++++++ connector/oidc/oidc_test.go | 23 ++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 connector/oidc/oidc_test.go diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 728bdf6a..0310266c 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -6,6 +6,9 @@ import ( "errors" "fmt" "net/http" + "net/url" + "strings" + "sync" "github.com/Sirupsen/logrus" "github.com/coreos/go-oidc" @@ -21,7 +24,50 @@ type Config struct { ClientSecret string `json:"clientSecret"` RedirectURI string `json:"redirectURI"` + // Causes client_secret to be passed as POST parameters instead of basic + // auth. This is specifically "NOT RECOMMENDED" by the OAuth2 RFC, but some + // providers require it. + // + // https://tools.ietf.org/html/rfc6749#section-2.3.1 + BasicAuthUnsupported *bool `json:"basicAuthUnsupported"` + Scopes []string `json:"scopes"` // defaults to "profile" and "email" + +} + +// Domains that don't support basic auth. golang.org/x/oauth2 has an internal +// list, but it only matches specific URLs, not top level domains. +var brokenAuthHeaderDomains = []string{ + // See: https://github.com/coreos/dex/issues/859 + "okta.com", + "oktapreview.com", +} + +// Detect auth header provider issues for known providers. This lets users +// avoid having to explicitly set "basicAuthUnsupported" in their config. +// +// Setting the config field always overrides values returned by this function. +func knownBrokenAuthHeaderProvider(issuerURL string) bool { + if u, err := url.Parse(issuerURL); err == nil { + for _, host := range brokenAuthHeaderDomains { + if u.Host == host || strings.HasSuffix(u.Host, "."+host) { + return true + } + } + } + return false +} + +// golang.org/x/oauth2 doesn't do internal locking. Need to do it in this +// package ourselves and hope that other packages aren't calling it at the +// same time. +var registerMu = new(sync.Mutex) + +func registerBrokenAuthHeaderProvider(url string) { + registerMu.Lock() + defer registerMu.Unlock() + + oauth2.RegisterBrokenAuthHeaderProvider(url) } // Open returns a connector which can be used to login users through an upstream @@ -35,6 +81,15 @@ func (c *Config) Open(logger logrus.FieldLogger) (conn connector.Connector, err return nil, fmt.Errorf("failed to get provider: %v", err) } + if c.BasicAuthUnsupported != nil { + // Setting "basicAuthUnsupported" always overrides our detection. + if *c.BasicAuthUnsupported { + registerBrokenAuthHeaderProvider(provider.Endpoint().TokenURL) + } + } else if knownBrokenAuthHeaderProvider(c.Issuer) { + registerBrokenAuthHeaderProvider(provider.Endpoint().TokenURL) + } + scopes := []string{oidc.ScopeOpenID} if len(c.Scopes) > 0 { scopes = append(scopes, c.Scopes...) diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go new file mode 100644 index 00000000..b3f609d1 --- /dev/null +++ b/connector/oidc/oidc_test.go @@ -0,0 +1,23 @@ +package oidc + +import "testing" + +func TestKnownBrokenAuthHeaderProvider(t *testing.T) { + tests := []struct { + issuerURL string + expect bool + }{ + {"https://dev.oktapreview.com", true}, + {"https://dev.okta.com", true}, + {"https://okta.com", true}, + {"https://dev.oktaaccounts.com", false}, + {"https://accounts.google.com", false}, + } + + for _, tc := range tests { + got := knownBrokenAuthHeaderProvider(tc.issuerURL) + if got != tc.expect { + t.Errorf("knownBrokenAuthHeaderProvider(%q), want=%t, got=%t", tc.issuerURL, tc.expect, got) + } + } +} From f503ff7950456a097a713a255b482ab0827e1328 Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Mon, 20 Mar 2017 08:39:08 -0700 Subject: [PATCH 2/2] *: add documentation for the OpenID Connect provider --- Documentation/oidc-connector.md | 49 +++++++++++++++++++++++++++++++++ README.md | 1 + 2 files changed, 50 insertions(+) create mode 100644 Documentation/oidc-connector.md diff --git a/Documentation/oidc-connector.md b/Documentation/oidc-connector.md new file mode 100644 index 00000000..57a2b66e --- /dev/null +++ b/Documentation/oidc-connector.md @@ -0,0 +1,49 @@ +# Authentication through an OpenID Connect provider + +## Overview + +Dex is able to use another OpenID Connect provider as an authentication source. When logging in, dex will redirect to the upstream provider and perform the necessary OAuth2 flows to determine the end users email, username, etc. More details on the OpenID Connect protocol can be found in [_An overview of OpenID Connect_][oidc-doc]. + +Prominent examples of OpenID Connect providers include Google Accounts, Salesforce, and Azure AD v2 ([not v1][azure-ad-v1]). + +## Caveats + +Many OpenID Connect providers implement different restrictions on refresh tokens. For example, Google will only issue the first login attempt a refresh token, then not return one after. Because of this, this connector does not refresh the id_token claims when a client of dex redeems a refresh token, which can result in stale user info. + +It's generally recommended to avoid using refresh tokens with the `oidc` connector. + +Progress on this caveat can be tracked in [issue #863][google-refreshing]. + +## Configuration + +```yaml +connectors: +- type: oidc + id: google + name: Google + config: + # Canonical URL of the provider, also used for configuration discovery. + # This value MUST match the value returned in the provider config discovery. + # + # See: https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig + issuer: https://accounts.google.com + + # Connector config values starting with a "$" will read from the environment. + clientID: $GOOGLE_CLIENT_ID + clientSecret: $GOOGLE_CLIENT_SECRET + + # Dex's issuer URL + "/callback" + redirectURI: http://127.0.0.1:5556/callback + + + # Some providers require passing client_secret via POST parameters instead + # of basic auth, despite the OAuth2 RFC discouraging it. Many of these + # cases are caught internally, but some may need to uncommented the + # following field. + # + # basicAuthUnsupported: true +``` + +[oidc-doc]: openid-connect.md +[google-refreshing]: https://github.com/coreos/dex/issues/863 +[azure-ad-v1]: https://github.com/coreos/go-oidc/issues/133 diff --git a/README.md b/README.md index ca3e08d7..10e5ec32 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,7 @@ More docs for running dex as a Kubernetes authenticator can be found [here](Docu * [LDAP](Documentation/ldap-connector.md) * [GitHub](Documentation/github-connector.md) * [SAML 2.0 (experimental)](Documentation/saml-connector.md) + * [OpenID Connect](Documentation/oidc-connector.md) (includes Google, Salesforce, Azure, etc.) * Client libraries * [Go][go-oidc]