From b640a69d638445ad0f6a28c68e2e5fc2569bb446 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Wed, 21 Jun 2017 15:02:34 -0700 Subject: [PATCH 1/9] oauthproxy: fix #284 -skip-provider-button for /sign_in route --- oauthproxy.go | 6 +++++- oauthproxy_test.go | 51 ++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index dd2b58e9..91608acc 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -482,7 +482,11 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) { p.SaveSession(rw, req, session) http.Redirect(rw, req, redirect, 302) } else { - p.SignInPage(rw, req, 200) + if p.SkipProviderButton { + p.OAuthStart(rw, req) + } else { + p.SignInPage(rw, req, http.StatusOK) + } } } diff --git a/oauthproxy_test.go b/oauthproxy_test.go index a0bcc5c1..43e165a2 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -3,9 +3,6 @@ package main import ( "crypto" "encoding/base64" - "github.com/18F/hmacauth" - "github.com/bitly/oauth2_proxy/providers" - "github.com/bmizerany/assert" "io" "io/ioutil" "log" @@ -17,6 +14,10 @@ import ( "strings" "testing" "time" + + "github.com/18F/hmacauth" + "github.com/bitly/oauth2_proxy/providers" + "github.com/bmizerany/assert" ) func init() { @@ -359,26 +360,30 @@ func TestDoNotForwardAccessTokenUpstream(t *testing.T) { } type SignInPageTest struct { - opts *Options - proxy *OAuthProxy - sign_in_regexp *regexp.Regexp + opts *Options + proxy *OAuthProxy + sign_in_regexp *regexp.Regexp + sign_in_provider_regexp *regexp.Regexp } const signInRedirectPattern = `` +const signInSkipProvider = `>Found<` -func NewSignInPageTest() *SignInPageTest { +func NewSignInPageTest(skipProvider bool) *SignInPageTest { var sip_test SignInPageTest sip_test.opts = NewOptions() sip_test.opts.CookieSecret = "foobar" sip_test.opts.ClientID = "bazquux" sip_test.opts.ClientSecret = "xyzzyplugh" + sip_test.opts.SkipProviderButton = skipProvider sip_test.opts.Validate() sip_test.proxy = NewOAuthProxy(sip_test.opts, func(email string) bool { return true }) sip_test.sign_in_regexp = regexp.MustCompile(signInRedirectPattern) + sip_test.sign_in_provider_regexp = regexp.MustCompile(signInSkipProvider) return &sip_test } @@ -391,7 +396,7 @@ func (sip_test *SignInPageTest) GetEndpoint(endpoint string) (int, string) { } func TestSignInPageIncludesTargetRedirect(t *testing.T) { - sip_test := NewSignInPageTest() + sip_test := NewSignInPageTest(false) const endpoint = "/some/random/endpoint" code, body := sip_test.GetEndpoint(endpoint) @@ -409,7 +414,7 @@ func TestSignInPageIncludesTargetRedirect(t *testing.T) { } func TestSignInPageDirectAccessRedirectsToRoot(t *testing.T) { - sip_test := NewSignInPageTest() + sip_test := NewSignInPageTest(false) code, body := sip_test.GetEndpoint("/oauth2/sign_in") assert.Equal(t, 200, code) @@ -423,6 +428,34 @@ func TestSignInPageDirectAccessRedirectsToRoot(t *testing.T) { } } +func TestSignInPageSkipProvider(t *testing.T) { + sip_test := NewSignInPageTest(true) + const endpoint = "/some/random/endpoint" + + code, body := sip_test.GetEndpoint(endpoint) + assert.Equal(t, 302, code) + + match := sip_test.sign_in_provider_regexp.FindStringSubmatch(body) + if match == nil { + t.Fatal("Did not find pattern in body: " + + signInSkipProvider + "\nBody:\n" + body) + } +} + +func TestSignInPageSkipProviderDirect(t *testing.T) { + sip_test := NewSignInPageTest(true) + const endpoint = "/sign_in" + + code, body := sip_test.GetEndpoint(endpoint) + assert.Equal(t, 302, code) + + match := sip_test.sign_in_provider_regexp.FindStringSubmatch(body) + if match == nil { + t.Fatal("Did not find pattern in body: " + + signInSkipProvider + "\nBody:\n" + body) + } +} + type ProcessCookieTest struct { opts *Options proxy *OAuthProxy From 28336540066282e3f77835d7eba1f9e238a3191d Mon Sep 17 00:00:00 2001 From: Pavel Sorokin Date: Wed, 28 Jun 2017 12:26:17 +0800 Subject: [PATCH 2/9] Return back default provider in Options --- options.go | 1 + 1 file changed, 1 insertion(+) diff --git a/options.go b/options.go index 951fa84f..70fbec5a 100644 --- a/options.go +++ b/options.go @@ -111,6 +111,7 @@ func NewOptions() *Options { PassHostHeader: true, ApprovalPrompt: "", RequestLogging: true, + Provider: "google", } } From e6e60c4b60aacc280520ca211cf9b9f9d35047c4 Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Thu, 29 Jun 2017 09:36:31 +0300 Subject: [PATCH 3/9] Updates README.md with svg badge --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 85fd9201..f2dd362e 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ oauth2_proxy A reverse proxy and static file server that provides authentication using Providers (Google, GitHub, and others) to validate accounts by email, domain or group. -[![Build Status](https://secure.travis-ci.org/bitly/oauth2_proxy.png?branch=master)](http://travis-ci.org/bitly/oauth2_proxy) +[![Build Status](https://secure.travis-ci.org/bitly/oauth2_proxy.svg?branch=master)](http://travis-ci.org/bitly/oauth2_proxy) ![Sign In Page](https://cloud.githubusercontent.com/assets/45028/4970624/7feb7dd8-6886-11e4-93e0-c9904af44ea8.png) From ba67e5c847c3e0ab2000a8f12de6b03ca9f0925c Mon Sep 17 00:00:00 2001 From: Colin Arnott Date: Thu, 13 Jul 2017 18:33:48 +0000 Subject: [PATCH 4/9] strip all log statements with the endpoint var --- providers/internal_util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/internal_util.go b/providers/internal_util.go index 6d853ab8..b396993b 100644 --- a/providers/internal_util.go +++ b/providers/internal_util.go @@ -57,7 +57,7 @@ func validateToken(p Provider, access_token string, header http.Header) bool { } resp, err := api.RequestUnparsedResponse(endpoint, header) if err != nil { - log.Printf("GET %s", endpoint) + log.Printf("GET %s", stripToken(endpoint)) log.Printf("token validation request failed: %s", err) return false } From f4321c4b45fb8a9fc67ff521dd2fd1caefbf1463 Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Thu, 20 Jul 2017 13:28:41 +0200 Subject: [PATCH 5/9] Update cookie generation to match base64 encoding Current code is using URLEncoding but example was using the standard RFC 4648 encoding. Switch to using the URL encoding in the example as well. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 85fd9201..eb1ef102 100644 --- a/README.md +++ b/README.md @@ -147,7 +147,7 @@ To authorize by email domain use `--email-domain=yourcompany.com`. To authorize `oauth2_proxy` can be configured via [config file](#config-file), [command line options](#command-line-options) or [environment variables](#environment-variables). -To generate a strong cookie secret use `python -c 'import os,base64; print base64.b64encode(os.urandom(16))'` +To generate a strong cookie secret use `python -c 'import os,base64; print base64.urlsafe_b64encode(os.urandom(16))'` ### Config File From 0b117133b949ba5834a6f9c3ce4c172b6a9366bd Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Thu, 20 Jul 2017 13:35:06 +0200 Subject: [PATCH 6/9] Remove check for >0 upstreams When used solely for auth_request there is no upstream. Instead of forcing users to set a dummy upstream, remove the check. --- options.go | 3 --- options_test.go | 1 - 2 files changed, 4 deletions(-) diff --git a/options.go b/options.go index f1df9169..f30b6aac 100644 --- a/options.go +++ b/options.go @@ -121,9 +121,6 @@ func parseURL(to_parse string, urltype string, msgs []string) (*url.URL, []strin func (o *Options) Validate() error { msgs := make([]string, 0) - if len(o.Upstreams) < 1 { - msgs = append(msgs, "missing setting: upstream") - } if o.CookieSecret == "" { msgs = append(msgs, "missing setting: cookie-secret") } diff --git a/options_test.go b/options_test.go index 27dd8524..07193dde 100644 --- a/options_test.go +++ b/options_test.go @@ -35,7 +35,6 @@ func TestNewOptions(t *testing.T) { assert.NotEqual(t, nil, err) expected := errorMsg([]string{ - "missing setting: upstream", "missing setting: cookie-secret", "missing setting: client-id", "missing setting: client-secret"}) From e9bbecfacecbccc08f359f2f2ee3f13d5e4e0c70 Mon Sep 17 00:00:00 2001 From: Pierce Lopez Date: Sat, 5 Aug 2017 12:48:36 -0400 Subject: [PATCH 7/9] options: gracefully report un-parsed upstream URL upstreamURL is a nil pointer if there is an error parsing --upstream --- options.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/options.go b/options.go index f1df9169..e6c6c4e8 100644 --- a/options.go +++ b/options.go @@ -142,14 +142,13 @@ func (o *Options) Validate() error { for _, u := range o.Upstreams { upstreamURL, err := url.Parse(u) if err != nil { - msgs = append(msgs, fmt.Sprintf( - "error parsing upstream=%q %s", - upstreamURL, err)) + msgs = append(msgs, fmt.Sprintf("error parsing upstream: %s", err)) + } else { + if upstreamURL.Path == "" { + upstreamURL.Path = "/" + } + o.proxyURLs = append(o.proxyURLs, upstreamURL) } - if upstreamURL.Path == "" { - upstreamURL.Path = "/" - } - o.proxyURLs = append(o.proxyURLs, upstreamURL) } for _, u := range o.SkipAuthRegex { From 3d8b59ef716756370df2903266d81e80c7e708a3 Mon Sep 17 00:00:00 2001 From: Pierce Lopez Date: Sat, 5 Aug 2017 12:54:31 -0400 Subject: [PATCH 8/9] options: wrap missing-email-validation error message --- options.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/options.go b/options.go index e6c6c4e8..ac0a8061 100644 --- a/options.go +++ b/options.go @@ -134,7 +134,8 @@ func (o *Options) Validate() error { msgs = append(msgs, "missing setting: client-secret") } if o.AuthenticatedEmailsFile == "" && len(o.EmailDomains) == 0 && o.HtpasswdFile == "" { - msgs = append(msgs, "missing setting for email validation: email-domain or authenticated-emails-file required.\n use email-domain=* to authorize all email addresses") + msgs = append(msgs, "missing setting for email validation: email-domain or authenticated-emails-file required."+ + "\n use email-domain=* to authorize all email addresses") } o.redirectURL, msgs = parseURL(o.RedirectURL, "redirect", msgs) From 4f67c1acd44d7c236d860e86f2a975f72bba4b2c Mon Sep 17 00:00:00 2001 From: Pavel Sorokin Date: Thu, 7 Sep 2017 13:53:53 +0800 Subject: [PATCH 9/9] Add "resource" to LoginURL if defined. That helps to get 2FA --- providers/azure.go | 4 ++++ providers/azure_test.go | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/providers/azure.go b/providers/azure.go index 5ce210e1..baff0bc3 100644 --- a/providers/azure.go +++ b/providers/azure.go @@ -206,7 +206,11 @@ func (p *AzureProvider) GetLoginURL(redirectURI, state string) string { params.Add("state", state) params.Set("prompt", p.ApprovalPrompt) params.Set("nonce", "FIXME") + if p.ProtectedResource != nil && p.ProtectedResource.String() != "" { + params.Add("resource", p.ProtectedResource.String()) + } a.RawQuery = params.Encode() + return a.String() } diff --git a/providers/azure_test.go b/providers/azure_test.go index c8102901..09768cf2 100644 --- a/providers/azure_test.go +++ b/providers/azure_test.go @@ -351,3 +351,18 @@ func TestAzureRightPermittedGroups(t *testing.T) { assert.Equal(t, true, result) } + +func TestAzureLoginURLnoResource(t *testing.T) { + p := testAzureProvider("") + p.ProtectedResource = nil + + result := p.GetLoginURL("http://redirect/url", "state") + assert.Equal(t, "?client_id=&nonce=FIXME&prompt=&redirect_uri=http%3A%2F%2Fredirect%2Furl&response_mode=form_post&response_type=id_token+code&scope=openid&state=state", result) +} + +func TestAzureLoginURL(t *testing.T) { + p := testAzureProvider("") + + result := p.GetLoginURL("http://redirect/url", "state") + assert.Equal(t, "?client_id=&nonce=FIXME&prompt=&redirect_uri=http%3A%2F%2Fredirect%2Furl&resource=https%3A%2F%2Fgraph.microsoft.com&response_mode=form_post&response_type=id_token+code&scope=openid&state=state", result) +}