From b6bd878f276f762ab816e10eb40a55e9147e7eca Mon Sep 17 00:00:00 2001 From: Reed Loden Date: Tue, 18 Apr 2017 20:33:50 -0700 Subject: [PATCH 01/14] Don't set the cookie domain to the host by default, as it breaks Cookie Prefixes The Cookie Prefixes spec disallows the use of the `domain` attribute in cookies if the `__Host-` prefix is used (https://tools.ietf.org/html/draft-ietf-httpbis-cookie-prefixes-00#section-3.2). There's no need to set it to the host by default, so make it optional. If it is set to a non-empty value, still output a warning if it is not a suffix of the host, as that's likely not wanted. Fixes #352. --- README.md | 2 +- oauthproxy.go | 17 ++++++----------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index be73f363..093a981f 100644 --- a/README.md +++ b/README.md @@ -164,7 +164,7 @@ Usage of oauth2_proxy: -client-id string: the OAuth Client ID: ie: "123456.apps.googleusercontent.com" -client-secret string: the OAuth Client Secret -config string: path to config file - -cookie-domain string: an optional cookie domain to force cookies to (ie: .yourcompany.com)* + -cookie-domain string: an optional cookie domain to force cookies to (ie: .yourcompany.com) -cookie-expire duration: expire timeframe for cookie (default 168h0m0s) -cookie-httponly: set HttpOnly cookie flag (default true) -cookie-name string: the name of the cookie that the oauth_proxy creates (default "_oauth2_proxy") diff --git a/oauthproxy.go b/oauthproxy.go index dd2b58e9..1c62aa88 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -155,16 +155,12 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy { redirectURL.Path = fmt.Sprintf("%s/callback", opts.ProxyPrefix) log.Printf("OAuthProxy configured for %s Client ID: %s", opts.provider.Data().ProviderName, opts.ClientID) - domain := opts.CookieDomain - if domain == "" { - domain = "" - } refresh := "disabled" if opts.CookieRefresh != time.Duration(0) { refresh = fmt.Sprintf("after %s", opts.CookieRefresh) } - log.Printf("Cookie settings: name:%s secure(https):%v httponly:%v expiry:%s domain:%s refresh:%s", opts.CookieName, opts.CookieSecure, opts.CookieHttpOnly, opts.CookieExpire, domain, refresh) + log.Printf("Cookie settings: name:%s secure(https):%v httponly:%v expiry:%s domain:%s refresh:%s", opts.CookieName, opts.CookieSecure, opts.CookieHttpOnly, opts.CookieExpire, opts.CookieDomain, refresh) var cipher *cookie.Cipher if opts.PassAccessToken || (opts.CookieRefresh != time.Duration(0)) { @@ -267,22 +263,21 @@ func (p *OAuthProxy) MakeCSRFCookie(req *http.Request, value string, expiration } func (p *OAuthProxy) makeCookie(req *http.Request, name string, value string, expiration time.Duration, now time.Time) *http.Cookie { - domain := req.Host - if h, _, err := net.SplitHostPort(domain); err == nil { - domain = h - } if p.CookieDomain != "" { + domain := req.Host + if h, _, err := net.SplitHostPort(domain); err == nil { + domain = h + } if !strings.HasSuffix(domain, p.CookieDomain) { log.Printf("Warning: request host is %q but using configured cookie domain of %q", domain, p.CookieDomain) } - domain = p.CookieDomain } return &http.Cookie{ Name: name, Value: value, Path: "/", - Domain: domain, + Domain: p.CookieDomain, HttpOnly: p.CookieHttpOnly, Secure: p.CookieSecure, Expires: now.Add(expiration), From 8d6e16bf224cc429c95b6860bb438de7ed001053 Mon Sep 17 00:00:00 2001 From: Colin Arnott Date: Thu, 13 Jul 2017 18:29:58 +0000 Subject: [PATCH 02/14] use base64.RawURLEncoding.DecodeString() in place of a bespoke function --- providers/google.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/providers/google.go b/providers/google.go index a9cb487a..4bf108e7 100644 --- a/providers/google.go +++ b/providers/google.go @@ -67,7 +67,7 @@ func emailFromIdToken(idToken string) (string, error) { // id_token is a base64 encode ID token payload // https://developers.google.com/accounts/docs/OAuth2Login#obtainuserinfo jwt := strings.Split(idToken, ".") - b, err := jwtDecodeSegment(jwt[1]) + b, err := base64.RawURLEncoding.DecodeString(jwt[1]) if err != nil { return "", err } @@ -89,14 +89,6 @@ func emailFromIdToken(idToken string) (string, error) { return email.Email, nil } -func jwtDecodeSegment(seg string) ([]byte, error) { - if l := len(seg) % 4; l > 0 { - seg += strings.Repeat("=", 4-l) - } - - return base64.URLEncoding.DecodeString(seg) -} - func (p *GoogleProvider) Redeem(redirectURL, code string) (s *SessionState, err error) { if code == "" { err = errors.New("missing code") From 94574df274834609b4f132cc88fdc75c94313c30 Mon Sep 17 00:00:00 2001 From: Hans Kristian Flaatten Date: Tue, 5 Sep 2017 22:58:53 +0200 Subject: [PATCH 03/14] Clarify that GitHub team slug name should be used for the `-github-team` option --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ce371f42..ae0dac29 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ The Azure AD auth provider uses `openid` as it default scope. It uses `https://g The GitHub auth provider supports two additional parameters to restrict authentication to Organization or Team level access. Restricting by org and team is normally accompanied with `--email-domain=*` -github-org="": restrict logins to members of this organisation - -github-team="": restrict logins to members of any of these teams, separated by a comma + -github-team="": restrict logins to members of any of these teams (slug), separated by a comma If you are using GitHub enterprise, make sure you set the following to the appropriate url: @@ -176,7 +176,7 @@ Usage of oauth2_proxy: -email-domain value: authenticate emails with the specified domain (may be given multiple times). Use * to authenticate any email -footer string: custom footer string. Use "-" to disable default footer. -github-org string: restrict logins to members of this organisation - -github-team string: restrict logins to members of this team + -github-team string: restrict logins to members of any of these teams (slug), separated by a comma -google-admin-email string: the google admin to impersonate for api calls -google-group value: restrict logins to members of this google group (may be given multiple times). -google-service-account-json string: the path to the service account json credentials From cb48577ede89837567a4d030570cc7cbdbf245dc Mon Sep 17 00:00:00 2001 From: Eric Chiang Date: Tue, 9 May 2017 11:20:35 -0700 Subject: [PATCH 04/14] *: add an OpenID Connect provider See the README for usage with Dex or any other OIDC provider. To test run a backend: python3 -m http.server Run dex and modify the example config with the proxy callback: go get github.com/coreos/dex/cmd/dex cd $GOPATH/src/github.com/coreos/dex sed -i.bak \ 's|http://127.0.0.1:5555/callback|http://127.0.0.1:5555/oauth2/callback|g' \ examples/config-dev.yaml make ./bin/dex serve examples/config-dev.yaml Then run the oauth2_proxy oauth2_proxy \ --oidc-issuer-url http://127.0.0.1:5556/dex \ --upstream http://localhost:8000 \ --client-id example-app \ --client-secret ZXhhbXBsZS1hcHAtc2VjcmV0 \ --cookie-secret foo \ --email-domain '*' \ --http-address http://127.0.0.1:5555 \ --redirect-url http://127.0.0.1:5555/oauth2/callback \ --cookie-secure=false Login with the username/password "admin@example.com:password" --- Godeps | 3 ++ README.md | 16 ++++++++ main.go | 1 + options.go | 41 +++++++++++++++++---- providers/oidc.go | 84 ++++++++++++++++++++++++++++++++++++++++++ providers/providers.go | 2 + 6 files changed, 140 insertions(+), 7 deletions(-) create mode 100644 providers/oidc.go diff --git a/Godeps b/Godeps index 21884cd2..efbaf7ab 100644 --- a/Godeps +++ b/Godeps @@ -8,3 +8,6 @@ golang.org/x/oauth2 7fdf09982454086d5570c7db3e11f360194830c golang.org/x/net/context 242b6b35177ec3909636b6cf6a47e8c2c6324b5d google.golang.org/api/admin/directory/v1 650535c7d6201e8304c92f38c922a9a3a36c6877 cloud.google.com/go/compute/metadata v0.7.0 +github.com/coreos/go-oidc c797a55f1c1001ec3169f1d0fbb4c5523563bec6 +gopkg.in/square/go-jose.v2 v2.1.1 +github.com/pquerna/cachecontrol 9299cc36e57c32f83e47ffb3c25d8a3dec10ea0b diff --git a/README.md b/README.md index be73f363..a332ec56 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,22 @@ For adding an application to the Microsoft Azure AD follow [these steps to add a Take note of your `TenantId` if applicable for your situation. The `TenantId` can be used to override the default `common` authorization server with a tenant specific server. +### OpenID Connect Provider + +OpenID Connect is a spec for OAUTH 2.0 + identity that is implemented by many major providers and several open source projects. This provider was originally built against CoreOS Dex and we will use it as an example. + +1. Launch a Dex instance using the [getting started guide](https://github.com/coreos/dex/blob/master/Documentation/getting-started.md). +2. Setup oauth2_proxy with the correct provider and using the default ports and callbacks. +3. Login with the fixture use in the dex guide and run the oauth2_proxy with the following args: + + -provider oidc + -client-id oauth2_proxy + -client-secret proxy + -redirect-url http://127.0.0.1:4180/oauth2/callback + -oidc-issuer-url http://127.0.0.1:5556 + -cookie-secure=false + -email-domain example.com + ## Email Authentication To authorize by email domain use `--email-domain=yourcompany.com`. To authorize individual email addresses use `--authenticated-emails-file=/path/to/file` with one email per line. To authorize all email addresses use `--email-domain=*`. diff --git a/main.go b/main.go index ab0e4d35..b9d9c96c 100644 --- a/main.go +++ b/main.go @@ -69,6 +69,7 @@ func main() { flagSet.Bool("request-logging", true, "Log requests to stdout") flagSet.String("provider", "google", "OAuth provider") + flagSet.String("oidc-issuer-url", "", "OpenID Connect issuer URL (ie: https://accounts.google.com)") flagSet.String("login-url", "", "Authentication endpoint") flagSet.String("redeem-url", "", "Token redemption endpoint") flagSet.String("profile-url", "", "Profile access endpoint") diff --git a/options.go b/options.go index f1df9169..9ed02ad3 100644 --- a/options.go +++ b/options.go @@ -1,6 +1,7 @@ package main import ( + "context" "crypto" "crypto/tls" "encoding/base64" @@ -14,6 +15,7 @@ import ( "github.com/18F/hmacauth" "github.com/bitly/oauth2_proxy/providers" + oidc "github.com/coreos/go-oidc" ) // Configuration Options that can be set by Command Line Flag, or Config File @@ -63,6 +65,7 @@ type Options struct { // These options allow for other providers besides Google, with // potential overrides. Provider string `flag:"provider" cfg:"provider"` + OIDCIssuerURL string `flag:"oidc-issuer-url" cfg:"oidc_issuer_url"` LoginURL string `flag:"login-url" cfg:"login_url"` RedeemURL string `flag:"redeem-url" cfg:"redeem_url"` ProfileURL string `flag:"profile-url" cfg:"profile_url"` @@ -81,6 +84,7 @@ type Options struct { CompiledRegex []*regexp.Regexp provider providers.Provider signatureData *SignatureData + oidcVerifier *oidc.IDTokenVerifier } type SignatureData struct { @@ -120,6 +124,14 @@ func parseURL(to_parse string, urltype string, msgs []string) (*url.URL, []strin } func (o *Options) Validate() error { + if o.SSLInsecureSkipVerify { + // TODO: Accept a certificate bundle. + insecureTransport := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + http.DefaultClient = &http.Client{Transport: insecureTransport} + } + msgs := make([]string, 0) if len(o.Upstreams) < 1 { msgs = append(msgs, "missing setting: upstream") @@ -137,6 +149,22 @@ func (o *Options) Validate() error { msgs = append(msgs, "missing setting for email validation: email-domain or authenticated-emails-file required.\n use email-domain=* to authorize all email addresses") } + if o.OIDCIssuerURL != "" { + // Configure discoverable provider data. + provider, err := oidc.NewProvider(context.Background(), o.OIDCIssuerURL) + if err != nil { + return err + } + o.oidcVerifier = provider.Verifier(&oidc.Config{ + ClientID: o.ClientID, + }) + o.LoginURL = provider.Endpoint().AuthURL + o.RedeemURL = provider.Endpoint().TokenURL + if o.Scope == "" { + o.Scope = "openid email profile" + } + } + o.redirectURL, msgs = parseURL(o.RedirectURL, "redirect", msgs) for _, u := range o.Upstreams { @@ -210,13 +238,6 @@ func (o *Options) Validate() error { msgs = parseSignatureKey(o, msgs) msgs = validateCookieName(o, msgs) - if o.SSLInsecureSkipVerify { - insecureTransport := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - } - http.DefaultClient = &http.Client{Transport: insecureTransport} - } - if len(msgs) != 0 { return fmt.Errorf("Invalid configuration:\n %s", strings.Join(msgs, "\n ")) @@ -252,6 +273,12 @@ func parseProviderInfo(o *Options, msgs []string) []string { p.SetGroupRestriction(o.GoogleGroups, o.GoogleAdminEmail, file) } } + case *providers.OIDCProvider: + if o.oidcVerifier == nil { + msgs = append(msgs, "oidc provider requires an oidc issuer URL") + } else { + p.Verifier = o.oidcVerifier + } } return msgs } diff --git a/providers/oidc.go b/providers/oidc.go new file mode 100644 index 00000000..ec0152ab --- /dev/null +++ b/providers/oidc.go @@ -0,0 +1,84 @@ +package providers + +import ( + "context" + "fmt" + "time" + + "golang.org/x/oauth2" + + oidc "github.com/coreos/go-oidc" +) + +type OIDCProvider struct { + *ProviderData + + Verifier *oidc.IDTokenVerifier +} + +func NewOIDCProvider(p *ProviderData) *OIDCProvider { + return &OIDCProvider{ProviderData: p} +} + +func (p *OIDCProvider) Redeem(redirectURL, code string) (s *SessionState, err error) { + ctx := context.Background() + c := oauth2.Config{ + ClientID: p.ClientID, + ClientSecret: p.ClientSecret, + Endpoint: oauth2.Endpoint{ + TokenURL: p.RedeemURL.String(), + }, + RedirectURL: redirectURL, + } + token, err := c.Exchange(ctx, code) + if err != nil { + return nil, fmt.Errorf("token exchange: %v", err) + } + + rawIDToken, ok := token.Extra("id_token").(string) + if !ok { + return nil, fmt.Errorf("token response did not contain an id_token") + } + + // Parse and verify ID Token payload. + idToken, err := p.Verifier.Verify(ctx, rawIDToken) + if err != nil { + return nil, fmt.Errorf("could not verify id_token: %v", err) + } + + // Extract custom claims. + var claims struct { + Email string `json:"email"` + Verified *bool `json:"email_verified"` + } + if err := idToken.Claims(&claims); err != nil { + return nil, fmt.Errorf("failed to parse id_token claims: %v", err) + } + + if claims.Email == "" { + return nil, fmt.Errorf("id_token did not contain an email") + } + if claims.Verified != nil && !*claims.Verified { + return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email) + } + + s = &SessionState{ + AccessToken: token.AccessToken, + RefreshToken: token.RefreshToken, + ExpiresOn: token.Expiry, + Email: claims.Email, + } + + return +} + +func (p *OIDCProvider) RefreshSessionIfNeeded(s *SessionState) (bool, error) { + if s == nil || s.ExpiresOn.After(time.Now()) || s.RefreshToken == "" { + return false, nil + } + + origExpiration := s.ExpiresOn + s.ExpiresOn = time.Now().Add(time.Second).Truncate(time.Second) + fmt.Printf("refreshed access token %s (expired on %s)\n", s, origExpiration) + return false, nil +} diff --git a/providers/providers.go b/providers/providers.go index fb2e5fc5..3aa4f398 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -30,6 +30,8 @@ func New(provider string, p *ProviderData) Provider { return NewAzureProvider(p) case "gitlab": return NewGitLabProvider(p) + case "oidc": + return NewOIDCProvider(p) default: return NewGoogleProvider(p) } From 982439a8d8a7fa4a19b29781a08042ccd670a644 Mon Sep 17 00:00:00 2001 From: Miouge1 Date: Tue, 12 Sep 2017 23:42:07 +0200 Subject: [PATCH 05/14] Reduce the default GitLab scope --- providers/gitlab.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/gitlab.go b/providers/gitlab.go index 708283ad..d036ac74 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -36,7 +36,7 @@ func NewGitLabProvider(p *ProviderData) *GitLabProvider { } } if p.Scope == "" { - p.Scope = "api" + p.Scope = "read_user" } return &GitLabProvider{ProviderData: p} } From a32ff08d6865707a437281c54e0f53581138f8eb Mon Sep 17 00:00:00 2001 From: Miouge1 Date: Tue, 12 Sep 2017 23:43:49 +0200 Subject: [PATCH 06/14] Update test for default GitLab scope --- providers/gitlab_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/gitlab_test.go b/providers/gitlab_test.go index 3df001c2..54b10529 100644 --- a/providers/gitlab_test.go +++ b/providers/gitlab_test.go @@ -53,7 +53,7 @@ func TestGitLabProviderDefaults(t *testing.T) { p.Data().RedeemURL.String()) assert.Equal(t, "https://gitlab.com/api/v3/user", p.Data().ValidateURL.String()) - assert.Equal(t, "api", p.Data().Scope) + assert.Equal(t, "read_user", p.Data().Scope) } func TestGitLabProviderOverrides(t *testing.T) { From 34d96f8d84581db7899da80396181ef04e832459 Mon Sep 17 00:00:00 2001 From: Joshua Carp Date: Sun, 8 Oct 2017 00:40:36 -0400 Subject: [PATCH 07/14] Add OpenID Connect provider name. --- providers/oidc.go | 1 + templates.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/providers/oidc.go b/providers/oidc.go index ec0152ab..0c0fa52a 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -17,6 +17,7 @@ type OIDCProvider struct { } func NewOIDCProvider(p *ProviderData) *OIDCProvider { + p.ProviderName = "OpenID Connect" return &OIDCProvider{ProviderData: p} } diff --git a/templates.go b/templates.go index c5619b25..80408bd0 100644 --- a/templates.go +++ b/templates.go @@ -115,7 +115,7 @@ func getTemplates() *template.Template { {{ if .SignInMessage }}

{{.SignInMessage}}

{{ end}} -
+
From d118cb7bbb0a71718ec33fb7060bee21e23b393c Mon Sep 17 00:00:00 2001 From: Joshua Carp Date: Sat, 7 Oct 2017 23:36:48 -0400 Subject: [PATCH 08/14] Drop deprecated MyUSA provider. [Resolves #390] --- README.md | 5 -- providers/internal_util.go | 5 ++ providers/myusa.go | 58 --------------- providers/myusa_test.go | 141 ------------------------------------- providers/providers.go | 2 - 5 files changed, 5 insertions(+), 206 deletions(-) delete mode 100644 providers/myusa.go delete mode 100644 providers/myusa_test.go diff --git a/README.md b/README.md index 79de61e7..555e0676 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,6 @@ Valid providers are : * [GitHub](#github-auth-provider) * [GitLab](#gitlab-auth-provider) * [LinkedIn](#linkedin-auth-provider) -* [MyUSA](#myusa-auth-provider) The provider can be selected using the `provider` configuration value. @@ -129,10 +128,6 @@ For LinkedIn, the registration steps are: 3. Fill in the remaining required fields and Save. 4. Take note of the **Consumer Key / API Key** and **Consumer Secret / Secret Key** -### MyUSA Auth Provider - -The [MyUSA](https://alpha.my.usa.gov) authentication service ([GitHub](https://github.com/18F/myusa)) - ### Microsoft Azure AD Provider For adding an application to the Microsoft Azure AD follow [these steps to add an application](https://azure.microsoft.com/en-us/documentation/articles/active-directory-integrating-applications/). diff --git a/providers/internal_util.go b/providers/internal_util.go index b396993b..924d4119 100644 --- a/providers/internal_util.go +++ b/providers/internal_util.go @@ -72,3 +72,8 @@ func validateToken(p Provider, access_token string, header http.Header) bool { log.Printf("token validation request failed: status %d - %s", resp.StatusCode, body) return false } + +func updateURL(url *url.URL, hostname string) { + url.Scheme = "http" + url.Host = hostname +} diff --git a/providers/myusa.go b/providers/myusa.go deleted file mode 100644 index ae76d343..00000000 --- a/providers/myusa.go +++ /dev/null @@ -1,58 +0,0 @@ -package providers - -import ( - "log" - "net/http" - "net/url" - - "github.com/bitly/oauth2_proxy/api" -) - -type MyUsaProvider struct { - *ProviderData -} - -func NewMyUsaProvider(p *ProviderData) *MyUsaProvider { - const myUsaHost string = "alpha.my.usa.gov" - - p.ProviderName = "MyUSA" - if p.LoginURL.String() == "" { - p.LoginURL = &url.URL{Scheme: "https", - Host: myUsaHost, - Path: "/oauth/authorize"} - } - if p.RedeemURL.String() == "" { - p.RedeemURL = &url.URL{Scheme: "https", - Host: myUsaHost, - Path: "/oauth/token"} - } - if p.ProfileURL.String() == "" { - p.ProfileURL = &url.URL{Scheme: "https", - Host: myUsaHost, - Path: "/api/v1/profile"} - } - if p.ValidateURL.String() == "" { - p.ValidateURL = &url.URL{Scheme: "https", - Host: myUsaHost, - Path: "/api/v1/tokeninfo"} - } - if p.Scope == "" { - p.Scope = "profile.email" - } - return &MyUsaProvider{ProviderData: p} -} - -func (p *MyUsaProvider) GetEmailAddress(s *SessionState) (string, error) { - req, err := http.NewRequest("GET", - p.ProfileURL.String()+"?access_token="+s.AccessToken, nil) - if err != nil { - log.Printf("failed building request %s", err) - return "", err - } - json, err := api.Request(req) - if err != nil { - log.Printf("failed making request %s", err) - return "", err - } - return json.Get("email").String() -} diff --git a/providers/myusa_test.go b/providers/myusa_test.go deleted file mode 100644 index d058845c..00000000 --- a/providers/myusa_test.go +++ /dev/null @@ -1,141 +0,0 @@ -package providers - -import ( - "net/http" - "net/http/httptest" - "net/url" - "testing" - - "github.com/bmizerany/assert" -) - -func updateURL(url *url.URL, hostname string) { - url.Scheme = "http" - url.Host = hostname -} - -func testMyUsaProvider(hostname string) *MyUsaProvider { - p := NewMyUsaProvider( - &ProviderData{ - ProviderName: "", - LoginURL: &url.URL{}, - RedeemURL: &url.URL{}, - ProfileURL: &url.URL{}, - ValidateURL: &url.URL{}, - Scope: ""}) - if hostname != "" { - updateURL(p.Data().LoginURL, hostname) - updateURL(p.Data().RedeemURL, hostname) - updateURL(p.Data().ProfileURL, hostname) - updateURL(p.Data().ValidateURL, hostname) - } - return p -} - -func testMyUsaBackend(payload string) *httptest.Server { - path := "/api/v1/profile" - query := "access_token=imaginary_access_token" - - return httptest.NewServer(http.HandlerFunc( - func(w http.ResponseWriter, r *http.Request) { - url := r.URL - if url.Path != path || url.RawQuery != query { - w.WriteHeader(404) - } else { - w.WriteHeader(200) - w.Write([]byte(payload)) - } - })) -} - -func TestMyUsaProviderDefaults(t *testing.T) { - p := testMyUsaProvider("") - assert.NotEqual(t, nil, p) - assert.Equal(t, "MyUSA", p.Data().ProviderName) - assert.Equal(t, "https://alpha.my.usa.gov/oauth/authorize", - p.Data().LoginURL.String()) - assert.Equal(t, "https://alpha.my.usa.gov/oauth/token", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://alpha.my.usa.gov/api/v1/profile", - p.Data().ProfileURL.String()) - assert.Equal(t, "https://alpha.my.usa.gov/api/v1/tokeninfo", - p.Data().ValidateURL.String()) - assert.Equal(t, "profile.email", p.Data().Scope) -} - -func TestMyUsaProviderOverrides(t *testing.T) { - p := NewMyUsaProvider( - &ProviderData{ - LoginURL: &url.URL{ - Scheme: "https", - Host: "example.com", - Path: "/oauth/auth"}, - RedeemURL: &url.URL{ - Scheme: "https", - Host: "example.com", - Path: "/oauth/token"}, - ProfileURL: &url.URL{ - Scheme: "https", - Host: "example.com", - Path: "/oauth/profile"}, - ValidateURL: &url.URL{ - Scheme: "https", - Host: "example.com", - Path: "/oauth/tokeninfo"}, - Scope: "profile"}) - assert.NotEqual(t, nil, p) - assert.Equal(t, "MyUSA", p.Data().ProviderName) - assert.Equal(t, "https://example.com/oauth/auth", - p.Data().LoginURL.String()) - assert.Equal(t, "https://example.com/oauth/token", - p.Data().RedeemURL.String()) - assert.Equal(t, "https://example.com/oauth/profile", - p.Data().ProfileURL.String()) - assert.Equal(t, "https://example.com/oauth/tokeninfo", - p.Data().ValidateURL.String()) - assert.Equal(t, "profile", p.Data().Scope) -} - -func TestMyUsaProviderGetEmailAddress(t *testing.T) { - b := testMyUsaBackend("{\"email\": \"michael.bland@gsa.gov\"}") - defer b.Close() - - b_url, _ := url.Parse(b.URL) - p := testMyUsaProvider(b_url.Host) - - session := &SessionState{AccessToken: "imaginary_access_token"} - email, err := p.GetEmailAddress(session) - assert.Equal(t, nil, err) - assert.Equal(t, "michael.bland@gsa.gov", email) -} - -// Note that trying to trigger the "failed building request" case is not -// practical, since the only way it can fail is if the URL fails to parse. -func TestMyUsaProviderGetEmailAddressFailedRequest(t *testing.T) { - b := testMyUsaBackend("unused payload") - defer b.Close() - - b_url, _ := url.Parse(b.URL) - p := testMyUsaProvider(b_url.Host) - - // We'll trigger a request failure by using an unexpected access - // token. Alternatively, we could allow the parsing of the payload as - // JSON to fail. - session := &SessionState{AccessToken: "unexpected_access_token"} - email, err := p.GetEmailAddress(session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) -} - -func TestMyUsaProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) { - b := testMyUsaBackend("{\"foo\": \"bar\"}") - defer b.Close() - - b_url, _ := url.Parse(b.URL) - p := testMyUsaProvider(b_url.Host) - - session := &SessionState{AccessToken: "imaginary_access_token"} - email, err := p.GetEmailAddress(session) - assert.NotEqual(t, nil, err) - assert.Equal(t, "", email) -} diff --git a/providers/providers.go b/providers/providers.go index 3aa4f398..8a4e7caf 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -18,8 +18,6 @@ type Provider interface { func New(provider string, p *ProviderData) Provider { switch provider { - case "myusa": - return NewMyUsaProvider(p) case "linkedin": return NewLinkedInProvider(p) case "facebook": From 8a77cfcac3570afb977fe71073eabb0a6a2c1f7c Mon Sep 17 00:00:00 2001 From: Tanvir Alam Date: Mon, 23 Oct 2017 12:23:46 -0400 Subject: [PATCH 09/14] Swap out bmizerany/assert package that is deprecated in favor of stretchr/testify/assert --- Godeps | 2 +- api/api_test.go | 3 ++- cookie/cookies_test.go | 2 +- env_options_test.go | 2 +- htpasswd_test.go | 2 +- oauthproxy_test.go | 2 +- options_test.go | 2 +- providers/azure_test.go | 3 ++- providers/gitlab_test.go | 2 +- providers/google_test.go | 2 +- providers/internal_util_test.go | 2 +- providers/linkedin_test.go | 3 ++- providers/provider_default_test.go | 2 +- providers/session_state_test.go | 2 +- templates_test.go | 3 ++- 15 files changed, 19 insertions(+), 15 deletions(-) diff --git a/Godeps b/Godeps index efbaf7ab..68f4ce6b 100644 --- a/Godeps +++ b/Godeps @@ -2,7 +2,7 @@ github.com/18F/hmacauth 1.0.1 github.com/BurntSushi/toml d94612f9fc140360834f9742158c70b5c5b5535b github.com/bitly/go-simplejson da1a8928f709389522c8023062a3739f3b4af419 github.com/mreiferson/go-options 77551d20752b54535462404ad9d877ebdb26e53d -github.com/bmizerany/assert e17e99893cb6509f428e1728281c2ad60a6b31e3 +github.com/stretchr/testify v1.1.4 gopkg.in/fsnotify.v1 v1.2.0 golang.org/x/oauth2 7fdf09982454086d5570c7db3e11f360194830ca golang.org/x/net/context 242b6b35177ec3909636b6cf6a47e8c2c6324b5d diff --git a/api/api_test.go b/api/api_test.go index 515d4da9..4f9ae2a5 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -2,12 +2,13 @@ package api import ( "github.com/bitly/go-simplejson" - "github.com/bmizerany/assert" "io/ioutil" "net/http" "net/http/httptest" "strings" "testing" + + "github.com/stretchr/testify/assert" ) func testBackend(response_code int, payload string) *httptest.Server { diff --git a/cookie/cookies_test.go b/cookie/cookies_test.go index 5c4a9434..74e78fb9 100644 --- a/cookie/cookies_test.go +++ b/cookie/cookies_test.go @@ -4,7 +4,7 @@ import ( "encoding/base64" "testing" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" ) func TestEncodeAndDecodeAccessToken(t *testing.T) { diff --git a/env_options_test.go b/env_options_test.go index 354dc424..e9277f7d 100644 --- a/env_options_test.go +++ b/env_options_test.go @@ -4,7 +4,7 @@ import ( "os" "testing" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" ) type envTest struct { diff --git a/htpasswd_test.go b/htpasswd_test.go index 5cfc9e61..17ce37b4 100644 --- a/htpasswd_test.go +++ b/htpasswd_test.go @@ -2,7 +2,7 @@ package main import ( "bytes" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" "testing" ) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 43e165a2..5e0beeba 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -17,7 +17,7 @@ import ( "github.com/18F/hmacauth" "github.com/bitly/oauth2_proxy/providers" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" ) func init() { diff --git a/options_test.go b/options_test.go index 07193dde..e1fed8a1 100644 --- a/options_test.go +++ b/options_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" ) func testOptions() *Options { diff --git a/providers/azure_test.go b/providers/azure_test.go index 1c25e12a..f2cf3533 100644 --- a/providers/azure_test.go +++ b/providers/azure_test.go @@ -1,11 +1,12 @@ package providers import ( - "github.com/bmizerany/assert" "net/http" "net/http/httptest" "net/url" "testing" + + "github.com/stretchr/testify/assert" ) func testAzureProvider(hostname string) *AzureProvider { diff --git a/providers/gitlab_test.go b/providers/gitlab_test.go index 54b10529..050b5c49 100644 --- a/providers/gitlab_test.go +++ b/providers/gitlab_test.go @@ -6,7 +6,7 @@ import ( "net/url" "testing" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" ) func testGitLabProvider(hostname string) *GitLabProvider { diff --git a/providers/google_test.go b/providers/google_test.go index 8f9b0542..fedd8da0 100644 --- a/providers/google_test.go +++ b/providers/google_test.go @@ -8,7 +8,7 @@ import ( "net/url" "testing" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" ) func newRedeemServer(body []byte) (*url.URL, *httptest.Server) { diff --git a/providers/internal_util_test.go b/providers/internal_util_test.go index ccb5ed41..5fe0e8ea 100644 --- a/providers/internal_util_test.go +++ b/providers/internal_util_test.go @@ -7,7 +7,7 @@ import ( "net/url" "testing" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" ) type ValidateSessionStateTestProvider struct { diff --git a/providers/linkedin_test.go b/providers/linkedin_test.go index f43c96bf..a0d255b3 100644 --- a/providers/linkedin_test.go +++ b/providers/linkedin_test.go @@ -1,11 +1,12 @@ package providers import ( - "github.com/bmizerany/assert" "net/http" "net/http/httptest" "net/url" "testing" + + "github.com/stretchr/testify/assert" ) func testLinkedInProvider(hostname string) *LinkedInProvider { diff --git a/providers/provider_default_test.go b/providers/provider_default_test.go index e60aa544..abff0a9b 100644 --- a/providers/provider_default_test.go +++ b/providers/provider_default_test.go @@ -4,7 +4,7 @@ import ( "testing" "time" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" ) func TestRefresh(t *testing.T) { diff --git a/providers/session_state_test.go b/providers/session_state_test.go index 6044bae1..0cf6d3ed 100644 --- a/providers/session_state_test.go +++ b/providers/session_state_test.go @@ -6,7 +6,7 @@ import ( "time" "github.com/bitly/oauth2_proxy/cookie" - "github.com/bmizerany/assert" + "github.com/stretchr/testify/assert" ) const secret = "0123456789abcdefghijklmnopqrstuv" diff --git a/templates_test.go b/templates_test.go index ed4ed706..49e1a9dd 100644 --- a/templates_test.go +++ b/templates_test.go @@ -1,8 +1,9 @@ package main import ( - "github.com/bmizerany/assert" "testing" + + "github.com/stretchr/testify/assert" ) func TestTemplatesCompile(t *testing.T) { From f2a995b8d98054e9ee57a3da359783cf090c2f71 Mon Sep 17 00:00:00 2001 From: Tanvir Alam Date: Mon, 6 Nov 2017 12:01:48 -0500 Subject: [PATCH 10/14] providers: update gitlab api endpoint to use latest version, v4 --- README.md | 2 +- providers/gitlab.go | 2 +- providers/gitlab_test.go | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index bc6d3aef..0e79061b 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ If you are using self-hosted GitLab, make sure you set the following to the appr -login-url="/oauth/authorize" -redeem-url="/oauth/token" - -validate-url="/api/v3/user" + -validate-url="/api/v4/user" ### LinkedIn Auth Provider diff --git a/providers/gitlab.go b/providers/gitlab.go index d036ac74..124d7198 100644 --- a/providers/gitlab.go +++ b/providers/gitlab.go @@ -32,7 +32,7 @@ func NewGitLabProvider(p *ProviderData) *GitLabProvider { p.ValidateURL = &url.URL{ Scheme: "https", Host: "gitlab.com", - Path: "/api/v3/user", + Path: "/api/v4/user", } } if p.Scope == "" { diff --git a/providers/gitlab_test.go b/providers/gitlab_test.go index 050b5c49..0eec5aa6 100644 --- a/providers/gitlab_test.go +++ b/providers/gitlab_test.go @@ -28,7 +28,7 @@ func testGitLabProvider(hostname string) *GitLabProvider { } func testGitLabBackend(payload string) *httptest.Server { - path := "/api/v3/user" + path := "/api/v4/user" query := "access_token=imaginary_access_token" return httptest.NewServer(http.HandlerFunc( @@ -51,7 +51,7 @@ func TestGitLabProviderDefaults(t *testing.T) { p.Data().LoginURL.String()) assert.Equal(t, "https://gitlab.com/oauth/token", p.Data().RedeemURL.String()) - assert.Equal(t, "https://gitlab.com/api/v3/user", + assert.Equal(t, "https://gitlab.com/api/v4/user", p.Data().ValidateURL.String()) assert.Equal(t, "read_user", p.Data().Scope) } @@ -70,7 +70,7 @@ func TestGitLabProviderOverrides(t *testing.T) { ValidateURL: &url.URL{ Scheme: "https", Host: "example.com", - Path: "/api/v3/user"}, + Path: "/api/v4/user"}, Scope: "profile"}) assert.NotEqual(t, nil, p) assert.Equal(t, "GitLab", p.Data().ProviderName) @@ -78,7 +78,7 @@ func TestGitLabProviderOverrides(t *testing.T) { p.Data().LoginURL.String()) assert.Equal(t, "https://example.com/oauth/token", p.Data().RedeemURL.String()) - assert.Equal(t, "https://example.com/api/v3/user", + assert.Equal(t, "https://example.com/api/v4/user", p.Data().ValidateURL.String()) assert.Equal(t, "profile", p.Data().Scope) } From e241fe86d33f973935869e7526460988dbb43c40 Mon Sep 17 00:00:00 2001 From: Mike Bland Date: Tue, 12 Sep 2017 18:59:00 -0400 Subject: [PATCH 11/14] Switch from 18F/hmacauth to mbland/hmacauth Since I'm no longer with 18F, I've re-released hmacauth under the ISC license as opposed to the previous CC0 license. There have been no changes to the hmacauth code itself, and all tests still pass. --- Godeps | 2 +- oauthproxy.go | 2 +- oauthproxy_test.go | 2 +- options.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Godeps b/Godeps index 68f4ce6b..f783a27c 100644 --- a/Godeps +++ b/Godeps @@ -1,4 +1,4 @@ -github.com/18F/hmacauth 1.0.1 +github.com/mbland/hmacauth 1.0.2 github.com/BurntSushi/toml d94612f9fc140360834f9742158c70b5c5b5535b github.com/bitly/go-simplejson da1a8928f709389522c8023062a3739f3b4af419 github.com/mreiferson/go-options 77551d20752b54535462404ad9d877ebdb26e53d diff --git a/oauthproxy.go b/oauthproxy.go index c39bbf67..bc693693 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -14,9 +14,9 @@ import ( "strings" "time" - "github.com/18F/hmacauth" "github.com/bitly/oauth2_proxy/cookie" "github.com/bitly/oauth2_proxy/providers" + "github.com/mbland/hmacauth" ) const SignatureHeader = "GAP-Signature" diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 5e0beeba..1e6b3140 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -15,8 +15,8 @@ import ( "testing" "time" - "github.com/18F/hmacauth" "github.com/bitly/oauth2_proxy/providers" + "github.com/mbland/hmacauth" "github.com/stretchr/testify/assert" ) diff --git a/options.go b/options.go index d884f28b..06337258 100644 --- a/options.go +++ b/options.go @@ -13,9 +13,9 @@ import ( "strings" "time" - "github.com/18F/hmacauth" "github.com/bitly/oauth2_proxy/providers" oidc "github.com/coreos/go-oidc" + "github.com/mbland/hmacauth" ) // Configuration Options that can be set by Command Line Flag, or Config File From c4905f234744c0ad0a2695180a127bea80396c30 Mon Sep 17 00:00:00 2001 From: Jehiah Czebotar Date: Tue, 31 Oct 2017 09:12:15 -0400 Subject: [PATCH 12/14] Switch from gpm -> dep for dependency management --- .gitignore | 11 ++-- .travis.yml | 10 ++-- Godeps | 13 ----- Gopkg.lock | 117 ++++++++++++++++++++++++++++++++++++++ Gopkg.toml | 40 +++++++++++++ test.sh | 21 ++++--- validator_test.go | 2 +- validator_watcher_test.go | 2 +- 8 files changed, 182 insertions(+), 34 deletions(-) delete mode 100644 Godeps create mode 100644 Gopkg.lock create mode 100644 Gopkg.toml diff --git a/.gitignore b/.gitignore index c51af8d0..74de2e73 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,10 @@ -google_auth_proxy oauth2_proxy +vendor +dist +.godeps +*.exe + + # Go.gitignore # Compiled Object files, Static and Dynamic libs (Shared Objects) *.o @@ -22,9 +27,5 @@ _cgo_export.* _testmain.go -*.exe -dist -.godeps - # Editor swap/temp files .*.swp diff --git a/.travis.yml b/.travis.yml index 8c830da6..da7885ac 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,11 @@ language: go go: - - 1.7.5 - - 1.8.1 + - 1.8.x + - 1.9.x script: - - curl -s https://raw.githubusercontent.com/pote/gpm/v1.4.0/bin/gpm > gpm - - chmod +x gpm - - ./gpm install + - wget -O dep https://github.com/golang/dep/releases/download/v0.3.2/dep-linux-amd64 + - chmod +x dep + - ./dep ensure - ./test.sh sudo: false notifications: diff --git a/Godeps b/Godeps deleted file mode 100644 index f783a27c..00000000 --- a/Godeps +++ /dev/null @@ -1,13 +0,0 @@ -github.com/mbland/hmacauth 1.0.2 -github.com/BurntSushi/toml d94612f9fc140360834f9742158c70b5c5b5535b -github.com/bitly/go-simplejson da1a8928f709389522c8023062a3739f3b4af419 -github.com/mreiferson/go-options 77551d20752b54535462404ad9d877ebdb26e53d -github.com/stretchr/testify v1.1.4 -gopkg.in/fsnotify.v1 v1.2.0 -golang.org/x/oauth2 7fdf09982454086d5570c7db3e11f360194830ca -golang.org/x/net/context 242b6b35177ec3909636b6cf6a47e8c2c6324b5d -google.golang.org/api/admin/directory/v1 650535c7d6201e8304c92f38c922a9a3a36c6877 -cloud.google.com/go/compute/metadata v0.7.0 -github.com/coreos/go-oidc c797a55f1c1001ec3169f1d0fbb4c5523563bec6 -gopkg.in/square/go-jose.v2 v2.1.1 -github.com/pquerna/cachecontrol 9299cc36e57c32f83e47ffb3c25d8a3dec10ea0b diff --git a/Gopkg.lock b/Gopkg.lock new file mode 100644 index 00000000..f4474968 --- /dev/null +++ b/Gopkg.lock @@ -0,0 +1,117 @@ +# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. + + +[[projects]] + name = "cloud.google.com/go" + packages = ["compute/metadata"] + revision = "2d3a6656c17a60b0815b7e06ab0be04eacb6e613" + version = "v0.16.0" + +[[projects]] + name = "github.com/BurntSushi/toml" + packages = ["."] + revision = "b26d9c308763d68093482582cea63d69be07a0f0" + version = "v0.3.0" + +[[projects]] + name = "github.com/bitly/go-simplejson" + packages = ["."] + revision = "aabad6e819789e569bd6aabf444c935aa9ba1e44" + version = "v0.5.0" + +[[projects]] + branch = "v2" + name = "github.com/coreos/go-oidc" + packages = ["."] + revision = "77e7f2010a464ade7338597afe650dfcffbe2ca8" + +[[projects]] + name = "github.com/davecgh/go-spew" + packages = ["spew"] + revision = "346938d642f2ec3594ed81d874461961cd0faa76" + version = "v1.1.0" + +[[projects]] + branch = "master" + name = "github.com/golang/protobuf" + packages = ["proto"] + revision = "1e59b77b52bf8e4b449a57e6f79f21226d571845" + +[[projects]] + name = "github.com/mbland/hmacauth" + packages = ["."] + revision = "107c17adcc5eccc9935cd67d9bc2feaf5255d2cb" + version = "1.0.2" + +[[projects]] + branch = "master" + name = "github.com/mreiferson/go-options" + packages = ["."] + revision = "77551d20752b54535462404ad9d877ebdb26e53d" + +[[projects]] + name = "github.com/pmezard/go-difflib" + packages = ["difflib"] + revision = "792786c7400a136282c1664665ae0a8db921c6c2" + version = "v1.0.0" + +[[projects]] + branch = "master" + name = "github.com/pquerna/cachecontrol" + packages = [".","cacheobject"] + revision = "0dec1b30a0215bb68605dfc568e8855066c9202d" + +[[projects]] + name = "github.com/stretchr/testify" + packages = ["assert"] + revision = "69483b4bd14f5845b5a1e55bca19e954e827f1d0" + version = "v1.1.4" + +[[projects]] + branch = "master" + name = "golang.org/x/crypto" + packages = ["ed25519","ed25519/internal/edwards25519"] + revision = "9f005a07e0d31d45e6656d241bb5c0f2efd4bc94" + +[[projects]] + branch = "master" + name = "golang.org/x/net" + packages = ["context","context/ctxhttp"] + revision = "9dfe39835686865bff950a07b394c12a98ddc811" + +[[projects]] + branch = "master" + name = "golang.org/x/oauth2" + packages = [".","google","internal","jws","jwt"] + revision = "9ff8ebcc8e241d46f52ecc5bff0e5a2f2dbef402" + +[[projects]] + branch = "master" + name = "google.golang.org/api" + packages = ["admin/directory/v1","gensupport","googleapi","googleapi/internal/uritemplates"] + revision = "8791354e7ab150705ede13637a18c1fcc16b62e8" + +[[projects]] + name = "google.golang.org/appengine" + packages = [".","internal","internal/app_identity","internal/base","internal/datastore","internal/log","internal/modules","internal/remote_api","internal/urlfetch","urlfetch"] + revision = "150dc57a1b433e64154302bdc40b6bb8aefa313a" + version = "v1.0.0" + +[[projects]] + name = "gopkg.in/fsnotify.v1" + packages = ["."] + revision = "836bfd95fecc0f1511dd66bdbf2b5b61ab8b00b6" + version = "v1.2.11" + +[[projects]] + name = "gopkg.in/square/go-jose.v2" + packages = [".","cipher","json"] + revision = "f8f38de21b4dcd69d0413faf231983f5fd6634b1" + version = "v2.1.3" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + inputs-digest = "efab48a0e196c2a849bfbe9aa02d2ae28d281ce1bfe9f23720d648858eefc8e6" + solver-name = "gps-cdcl" + solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml new file mode 100644 index 00000000..97f83d01 --- /dev/null +++ b/Gopkg.toml @@ -0,0 +1,40 @@ + +# Refer to https://github.com/golang/dep/blob/master/docs/Gopkg.toml.md +# for detailed Gopkg.toml documentation. +# + +[[constraint]] + name = "github.com/18F/hmacauth" + version = "~1.0.1" + +[[constraint]] + name = "github.com/BurntSushi/toml" + version = "~0.3.0" + +[[constraint]] + name = "github.com/bitly/go-simplejson" + version = "~0.5.0" + +[[constraint]] + branch = "v2" + name = "github.com/coreos/go-oidc" + +[[constraint]] + branch = "master" + name = "github.com/mreiferson/go-options" + +[[constraint]] + name = "github.com/stretchr/testify" + version = "~1.1.4" + +[[constraint]] + branch = "master" + name = "golang.org/x/oauth2" + +[[constraint]] + branch = "master" + name = "google.golang.org/api" + +[[constraint]] + name = "gopkg.in/fsnotify.v1" + version = "~1.2.0" diff --git a/test.sh b/test.sh index ec343663..acc17a23 100755 --- a/test.sh +++ b/test.sh @@ -1,11 +1,14 @@ #!/bin/bash -set -e - +EXIT_CODE=0 echo "gofmt" -diff -u <(echo -n) <(gofmt -d $(find . -type f -name '*.go' -not -path "./.godeps/*")) -echo "go vet" -go vet ./... -echo "go test" -go test -timeout 60s ./... -echo "go test -race" -GOMAXPROCS=4 go test -timeout 60s -race ./... +diff -u <(echo -n) <(gofmt -d $(find . -type f -name '*.go' -not -path "./vendor/*")) || EXIT_CODE=1 +for pkg in $(go list ./... | grep -v '/vendor/' ); do + echo "testing $pkg" + echo "go vet $pkg" + go vet "$pkg" || EXIT_CODE=1 + echo "go test -v $pkg" + go test -v -timeout 90s "$pkg" || EXIT_CODE=1 + echo "go test -v -race $pkg" + GOMAXPROCS=4 go test -v -timeout 90s0s -race "$pkg" || EXIT_CODE=1 +done +exit $EXIT_CODE \ No newline at end of file diff --git a/validator_test.go b/validator_test.go index b87d419b..f91f41ce 100644 --- a/validator_test.go +++ b/validator_test.go @@ -20,7 +20,7 @@ func NewValidatorTest(t *testing.T) *ValidatorTest { if err != nil { t.Fatal("failed to create temp file: " + err.Error()) } - vt.done = make(chan bool) + vt.done = make(chan bool, 1) return vt } diff --git a/validator_watcher_test.go b/validator_watcher_test.go index 70eaa104..dc16a7da 100644 --- a/validator_watcher_test.go +++ b/validator_watcher_test.go @@ -86,7 +86,7 @@ func TestValidatorOverwriteEmailListViaRenameAndReplace(t *testing.T) { vt.WriteEmails(t, []string{"xyzzy@example.com"}) domains := []string(nil) - updated := make(chan bool) + updated := make(chan bool, 1) validator := vt.NewValidator(domains, updated) if !validator("xyzzy@example.com") { From e955d2be0e14fbd9bc1bfcee8388712ec60d7ab6 Mon Sep 17 00:00:00 2001 From: Dave Nicponski Date: Fri, 5 May 2017 15:47:40 -0400 Subject: [PATCH 13/14] options: update options parsing for better handling of incorrect values * don't add in failed compiled regexes for skip auth regex option * improve test coverage for skip auth regex option to handle partial success case * add tests for incorrect upstream options parsing errors --- options.go | 4 ++-- options_test.go | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/options.go b/options.go index 06337258..214c8156 100644 --- a/options.go +++ b/options.go @@ -180,8 +180,8 @@ func (o *Options) Validate() error { for _, u := range o.SkipAuthRegex { CompiledRegex, err := regexp.Compile(u) if err != nil { - msgs = append(msgs, fmt.Sprintf( - "error compiling regex=%q %s", u, err)) + msgs = append(msgs, fmt.Sprintf("error compiling regex=%q %s", u, err)) + continue } o.CompiledRegex = append(o.CompiledRegex, CompiledRegex) } diff --git a/options_test.go b/options_test.go index e1fed8a1..23ae9fb8 100644 --- a/options_test.go +++ b/options_test.go @@ -95,6 +95,18 @@ func TestProxyURLs(t *testing.T) { assert.Equal(t, expected, o.proxyURLs) } +func TestProxyURLsError(t *testing.T) { + o := testOptions() + o.Upstreams = append(o.Upstreams, "127.0.0.1:8081") + err := o.Validate() + assert.NotEqual(t, nil, err) + + expected := errorMsg([]string{ + "error parsing upstream: parse 127.0.0.1:8081: " + + "first path segment in URL cannot contain colon"}) + assert.Equal(t, expected, err.Error()) +} + func TestCompiledRegex(t *testing.T) { o := testOptions() regexps := []string{"/foo/.*", "/ba[rz]/quux"} @@ -119,6 +131,15 @@ func TestCompiledRegexError(t *testing.T) { "error compiling regex=\"barquux)\" error parsing regexp: " + "unexpected ): `barquux)`"}) assert.Equal(t, expected, err.Error()) + + o.SkipAuthRegex = []string{"foobaz", "barquux)"} + err = o.Validate() + assert.NotEqual(t, nil, err) + + expected = errorMsg([]string{ + "error compiling regex=\"barquux)\" error parsing regexp: " + + "unexpected ): `barquux)`"}) + assert.Equal(t, expected, err.Error()) } func TestDefaultProviderApiSettings(t *testing.T) { From 731fa9f8e00b294ff1bf4a687e63d2ecdd9a4a50 Mon Sep 17 00:00:00 2001 From: Carlo Lobrano Date: Tue, 26 Sep 2017 23:31:27 +0200 Subject: [PATCH 14/14] Github provider: use login as user - Save both user and email in session state: Encoding/decoding methods save both email and user field in session state, for use cases when User is not derived from email's local-parth, like for GitHub provider. For retrocompatibility, if no user is obtained by the provider, (e.g. User is an empty string) the encoding/decoding methods fall back to the previous behavior and use the email's local-part Updated also related tests and added two more tests to show behavior when session contains a non-empty user value. - Added first basic GitHub provider tests - Added GetUserName method to Provider interface The new GetUserName method is intended to return the User value when this is not the email's local-part. Added also the default implementation to provider_default.go - Added call to GetUserName in redeemCode the new GetUserName method is used in redeemCode to get SessionState User value. For backward compatibility, if GetUserName error is "not implemented", the error is ignored. - Added GetUserName method and tests to github provider. --- oauthproxy.go | 7 ++ providers/github.go | 47 +++++++++- providers/github_test.go | 146 ++++++++++++++++++++++++++++++++ providers/provider_default.go | 5 ++ providers/providers.go | 1 + providers/session_state.go | 80 ++++++++--------- providers/session_state_test.go | 74 ++++++++++++++-- 7 files changed, 315 insertions(+), 45 deletions(-) create mode 100644 providers/github_test.go diff --git a/oauthproxy.go b/oauthproxy.go index bc693693..f94aa6e4 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -244,6 +244,13 @@ func (p *OAuthProxy) redeemCode(host, code string) (s *providers.SessionState, e if s.Email == "" { s.Email, err = p.provider.GetEmailAddress(s) } + + if s.User == "" { + s.User, err = p.provider.GetUserName(s) + if err != nil && err.Error() == "not implemented" { + err = nil + } + } return } diff --git a/providers/github.go b/providers/github.go index 512eed86..f3af86fe 100644 --- a/providers/github.go +++ b/providers/github.go @@ -218,10 +218,10 @@ func (p *GitHubProvider) GetEmailAddress(s *SessionState) (string, error) { if resp.StatusCode != 200 { return "", fmt.Errorf("got %d from %q %s", resp.StatusCode, endpoint.String(), body) - } else { - log.Printf("got %d from %q %s", resp.StatusCode, endpoint.String(), body) } + log.Printf("got %d from %q %s", resp.StatusCode, endpoint.String(), body) + if err := json.Unmarshal(body, &emails); err != nil { return "", fmt.Errorf("%s unmarshaling %s", err, body) } @@ -234,3 +234,46 @@ func (p *GitHubProvider) GetEmailAddress(s *SessionState) (string, error) { return "", nil } + +func (p *GitHubProvider) GetUserName(s *SessionState) (string, error) { + var user struct { + Login string `json:"login"` + Email string `json:"email"` + } + + endpoint := &url.URL{ + Scheme: p.ValidateURL.Scheme, + Host: p.ValidateURL.Host, + Path: path.Join(p.ValidateURL.Path, "/user"), + } + + req, err := http.NewRequest("GET", endpoint.String(), nil) + if err != nil { + return "", fmt.Errorf("could not create new GET request: %v", err) + } + + req.Header.Set("Authorization", fmt.Sprintf("token %s", s.AccessToken)) + resp, err := http.DefaultClient.Do(req) + if err != nil { + return "", err + } + + body, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() + if err != nil { + return "", err + } + + if resp.StatusCode != 200 { + return "", fmt.Errorf("got %d from %q %s", + resp.StatusCode, endpoint.String(), body) + } + + log.Printf("got %d from %q %s", resp.StatusCode, endpoint.String(), body) + + if err := json.Unmarshal(body, &user); err != nil { + return "", fmt.Errorf("%s unmarshaling %s", err, body) + } + + return user.Login, nil +} diff --git a/providers/github_test.go b/providers/github_test.go new file mode 100644 index 00000000..8080525b --- /dev/null +++ b/providers/github_test.go @@ -0,0 +1,146 @@ +package providers + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" +) + +func testGitHubProvider(hostname string) *GitHubProvider { + p := NewGitHubProvider( + &ProviderData{ + ProviderName: "", + LoginURL: &url.URL{}, + RedeemURL: &url.URL{}, + ProfileURL: &url.URL{}, + ValidateURL: &url.URL{}, + Scope: ""}) + if hostname != "" { + updateURL(p.Data().LoginURL, hostname) + updateURL(p.Data().RedeemURL, hostname) + updateURL(p.Data().ProfileURL, hostname) + updateURL(p.Data().ValidateURL, hostname) + } + return p +} + +func testGitHubBackend(payload string) *httptest.Server { + pathToQueryMap := map[string]string{ + "/user": "", + "/user/emails": "", + } + + return httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + url := r.URL + query, ok := pathToQueryMap[url.Path] + if !ok { + w.WriteHeader(404) + } else if url.RawQuery != query { + w.WriteHeader(404) + } else { + w.WriteHeader(200) + w.Write([]byte(payload)) + } + })) +} + +func TestGitHubProviderDefaults(t *testing.T) { + p := testGitHubProvider("") + assert.NotEqual(t, nil, p) + assert.Equal(t, "GitHub", p.Data().ProviderName) + assert.Equal(t, "https://github.com/login/oauth/authorize", + p.Data().LoginURL.String()) + assert.Equal(t, "https://github.com/login/oauth/access_token", + p.Data().RedeemURL.String()) + assert.Equal(t, "https://api.github.com/", + p.Data().ValidateURL.String()) + assert.Equal(t, "user:email", p.Data().Scope) +} + +func TestGitHubProviderOverrides(t *testing.T) { + p := NewGitHubProvider( + &ProviderData{ + LoginURL: &url.URL{ + Scheme: "https", + Host: "example.com", + Path: "/login/oauth/authorize"}, + RedeemURL: &url.URL{ + Scheme: "https", + Host: "example.com", + Path: "/login/oauth/access_token"}, + ValidateURL: &url.URL{ + Scheme: "https", + Host: "api.example.com", + Path: "/"}, + Scope: "profile"}) + assert.NotEqual(t, nil, p) + assert.Equal(t, "GitHub", p.Data().ProviderName) + assert.Equal(t, "https://example.com/login/oauth/authorize", + p.Data().LoginURL.String()) + assert.Equal(t, "https://example.com/login/oauth/access_token", + p.Data().RedeemURL.String()) + assert.Equal(t, "https://api.example.com/", + p.Data().ValidateURL.String()) + assert.Equal(t, "profile", p.Data().Scope) +} + +func TestGitHubProviderGetEmailAddress(t *testing.T) { + b := testGitHubBackend(`[ {"email": "michael.bland@gsa.gov", "primary": true} ]`) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + + session := &SessionState{AccessToken: "imaginary_access_token"} + email, err := p.GetEmailAddress(session) + assert.Equal(t, nil, err) + assert.Equal(t, "michael.bland@gsa.gov", email) +} + +// Note that trying to trigger the "failed building request" case is not +// practical, since the only way it can fail is if the URL fails to parse. +func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) { + b := testGitHubBackend("unused payload") + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + + // We'll trigger a request failure by using an unexpected access + // token. Alternatively, we could allow the parsing of the payload as + // JSON to fail. + session := &SessionState{AccessToken: "unexpected_access_token"} + email, err := p.GetEmailAddress(session) + assert.NotEqual(t, nil, err) + assert.Equal(t, "", email) +} + +func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) { + b := testGitHubBackend("{\"foo\": \"bar\"}") + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + + session := &SessionState{AccessToken: "imaginary_access_token"} + email, err := p.GetEmailAddress(session) + assert.NotEqual(t, nil, err) + assert.Equal(t, "", email) +} + +func TestGitHubProviderGetUserName(t *testing.T) { + b := testGitHubBackend(`{"email": "michael.bland@gsa.gov", "login": "mbland"}`) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testGitHubProvider(bURL.Host) + + session := &SessionState{AccessToken: "imaginary_access_token"} + email, err := p.GetUserName(session) + assert.Equal(t, nil, err) + assert.Equal(t, "mbland", email) +} diff --git a/providers/provider_default.go b/providers/provider_default.go index 1d1daea4..355e6c37 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -106,6 +106,11 @@ func (p *ProviderData) GetEmailAddress(s *SessionState) (string, error) { return "", errors.New("not implemented") } +// GetUserName returns the Account username +func (p *ProviderData) GetUserName(s *SessionState) (string, error) { + return "", errors.New("not implemented") +} + // ValidateGroup validates that the provided email exists in the configured provider // email group(s). func (p *ProviderData) ValidateGroup(email string) bool { diff --git a/providers/providers.go b/providers/providers.go index 8a4e7caf..70e707b4 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -7,6 +7,7 @@ import ( type Provider interface { Data() *ProviderData GetEmailAddress(*SessionState) (string, error) + GetUserName(*SessionState) (string, error) Redeem(string, string) (*SessionState, error) ValidateGroup(string) bool ValidateSessionState(*SessionState) bool diff --git a/providers/session_state.go b/providers/session_state.go index 214b5a4a..805c702f 100644 --- a/providers/session_state.go +++ b/providers/session_state.go @@ -25,7 +25,7 @@ func (s *SessionState) IsExpired() bool { } func (s *SessionState) String() string { - o := fmt.Sprintf("Session{%s", s.userOrEmail()) + o := fmt.Sprintf("Session{%s", s.accountInfo()) if s.AccessToken != "" { o += " token:true" } @@ -40,17 +40,13 @@ func (s *SessionState) String() string { func (s *SessionState) EncodeSessionState(c *cookie.Cipher) (string, error) { if c == nil || s.AccessToken == "" { - return s.userOrEmail(), nil + return s.accountInfo(), nil } return s.EncryptedString(c) } -func (s *SessionState) userOrEmail() string { - u := s.User - if s.Email != "" { - u = s.Email - } - return u +func (s *SessionState) accountInfo() string { + return fmt.Sprintf("email:%s user:%s", s.Email, s.User) } func (s *SessionState) EncryptedString(c *cookie.Cipher) (string, error) { @@ -60,56 +56,64 @@ func (s *SessionState) EncryptedString(c *cookie.Cipher) (string, error) { } a := s.AccessToken if a != "" { - a, err = c.Encrypt(a) - if err != nil { + if a, err = c.Encrypt(a); err != nil { return "", err } } r := s.RefreshToken if r != "" { - r, err = c.Encrypt(r) - if err != nil { + if r, err = c.Encrypt(r); err != nil { return "", err } } - return fmt.Sprintf("%s|%s|%d|%s", s.userOrEmail(), a, s.ExpiresOn.Unix(), r), nil + return fmt.Sprintf("%s|%s|%d|%s", s.accountInfo(), a, s.ExpiresOn.Unix(), r), nil +} + +func decodeSessionStatePlain(v string) (s *SessionState, err error) { + chunks := strings.Split(v, " ") + if len(chunks) != 2 { + return nil, fmt.Errorf("could not decode session state: expected 2 chunks got %d", len(chunks)) + } + + email := strings.TrimPrefix(chunks[0], "email:") + user := strings.TrimPrefix(chunks[1], "user:") + if user == "" { + user = strings.Split(email, "@")[0] + } + + return &SessionState{User: user, Email: email}, nil } func DecodeSessionState(v string, c *cookie.Cipher) (s *SessionState, err error) { - chunks := strings.Split(v, "|") - if len(chunks) == 1 { - if strings.Contains(chunks[0], "@") { - u := strings.Split(v, "@")[0] - return &SessionState{Email: v, User: u}, nil - } - return &SessionState{User: v}, nil + if c == nil { + return decodeSessionStatePlain(v) } + chunks := strings.Split(v, "|") if len(chunks) != 4 { err = fmt.Errorf("invalid number of fields (got %d expected 4)", len(chunks)) return } - s = &SessionState{} - if c != nil && chunks[1] != "" { - s.AccessToken, err = c.Decrypt(chunks[1]) - if err != nil { + sessionState, err := decodeSessionStatePlain(chunks[0]) + if err != nil { + return nil, err + } + + if chunks[1] != "" { + if sessionState.AccessToken, err = c.Decrypt(chunks[1]); err != nil { return nil, err } } - if c != nil && chunks[3] != "" { - s.RefreshToken, err = c.Decrypt(chunks[3]) - if err != nil { - return nil, err - } - } - if u := chunks[0]; strings.Contains(u, "@") { - s.Email = u - s.User = strings.Split(u, "@")[0] - } else { - s.User = u - } + ts, _ := strconv.Atoi(chunks[2]) - s.ExpiresOn = time.Unix(int64(ts), 0) - return + sessionState.ExpiresOn = time.Unix(int64(ts), 0) + + if chunks[3] != "" { + if sessionState.RefreshToken, err = c.Decrypt(chunks[3]); err != nil { + return nil, err + } + } + + return sessionState, nil } diff --git a/providers/session_state_test.go b/providers/session_state_test.go index 0cf6d3ed..d3cc8f88 100644 --- a/providers/session_state_test.go +++ b/providers/session_state_test.go @@ -1,6 +1,7 @@ package providers import ( + "fmt" "strings" "testing" "time" @@ -30,6 +31,7 @@ func TestSessionStateSerialization(t *testing.T) { ss, err := DecodeSessionState(encoded, c) t.Logf("%#v", ss) assert.Equal(t, nil, err) + assert.Equal(t, "user", ss.User) assert.Equal(t, s.Email, ss.Email) assert.Equal(t, s.AccessToken, ss.AccessToken) assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix()) @@ -39,6 +41,43 @@ func TestSessionStateSerialization(t *testing.T) { ss, err = DecodeSessionState(encoded, c2) t.Logf("%#v", ss) assert.Equal(t, nil, err) + assert.Equal(t, "user", ss.User) + assert.Equal(t, s.Email, ss.Email) + assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix()) + assert.NotEqual(t, s.AccessToken, ss.AccessToken) + assert.NotEqual(t, s.RefreshToken, ss.RefreshToken) +} + +func TestSessionStateSerializationWithUser(t *testing.T) { + c, err := cookie.NewCipher([]byte(secret)) + assert.Equal(t, nil, err) + c2, err := cookie.NewCipher([]byte(altSecret)) + assert.Equal(t, nil, err) + s := &SessionState{ + User: "just-user", + Email: "user@domain.com", + AccessToken: "token1234", + ExpiresOn: time.Now().Add(time.Duration(1) * time.Hour), + RefreshToken: "refresh4321", + } + encoded, err := s.EncodeSessionState(c) + assert.Equal(t, nil, err) + assert.Equal(t, 3, strings.Count(encoded, "|")) + + ss, err := DecodeSessionState(encoded, c) + t.Logf("%#v", ss) + assert.Equal(t, nil, err) + assert.Equal(t, s.User, ss.User) + assert.Equal(t, s.Email, ss.Email) + assert.Equal(t, s.AccessToken, ss.AccessToken) + assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix()) + assert.Equal(t, s.RefreshToken, ss.RefreshToken) + + // ensure a different cipher can't decode properly (ie: it gets gibberish) + ss, err = DecodeSessionState(encoded, c2) + t.Logf("%#v", ss) + assert.Equal(t, nil, err) + assert.Equal(t, s.User, ss.User) assert.Equal(t, s.Email, ss.Email) assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix()) assert.NotEqual(t, s.AccessToken, ss.AccessToken) @@ -46,7 +85,6 @@ func TestSessionStateSerialization(t *testing.T) { } func TestSessionStateSerializationNoCipher(t *testing.T) { - s := &SessionState{ Email: "user@domain.com", AccessToken: "token1234", @@ -55,25 +93,51 @@ func TestSessionStateSerializationNoCipher(t *testing.T) { } encoded, err := s.EncodeSessionState(nil) assert.Equal(t, nil, err) - assert.Equal(t, s.Email, encoded) + expected := fmt.Sprintf("email:%s user:", s.Email) + assert.Equal(t, expected, encoded) // only email should have been serialized ss, err := DecodeSessionState(encoded, nil) assert.Equal(t, nil, err) + assert.Equal(t, "user", ss.User) assert.Equal(t, s.Email, ss.Email) assert.Equal(t, "", ss.AccessToken) assert.Equal(t, "", ss.RefreshToken) } -func TestSessionStateUserOrEmail(t *testing.T) { +func TestSessionStateSerializationNoCipherWithUser(t *testing.T) { + s := &SessionState{ + User: "just-user", + Email: "user@domain.com", + AccessToken: "token1234", + ExpiresOn: time.Now().Add(time.Duration(1) * time.Hour), + RefreshToken: "refresh4321", + } + encoded, err := s.EncodeSessionState(nil) + assert.Equal(t, nil, err) + expected := fmt.Sprintf("email:%s user:%s", s.Email, s.User) + assert.Equal(t, expected, encoded) + // only email should have been serialized + ss, err := DecodeSessionState(encoded, nil) + assert.Equal(t, nil, err) + assert.Equal(t, s.User, ss.User) + assert.Equal(t, s.Email, ss.Email) + assert.Equal(t, "", ss.AccessToken) + assert.Equal(t, "", ss.RefreshToken) +} + +func TestSessionStateAccountInfo(t *testing.T) { s := &SessionState{ Email: "user@domain.com", User: "just-user", } - assert.Equal(t, "user@domain.com", s.userOrEmail()) + expected := fmt.Sprintf("email:%v user:%v", s.Email, s.User) + assert.Equal(t, expected, s.accountInfo()) + s.Email = "" - assert.Equal(t, "just-user", s.userOrEmail()) + expected = fmt.Sprintf("email:%v user:%v", s.Email, s.User) + assert.Equal(t, expected, s.accountInfo()) } func TestExpired(t *testing.T) {