From e25158dda6f0a540f21851a7d3db71d5bd3abbdd Mon Sep 17 00:00:00 2001 From: Jan Rotter Date: Sun, 26 Sep 2021 22:13:48 +0200 Subject: [PATCH 1/3] Add a test for htpasswd-user-groups in the session The groups configured in the `htpasswd-user-group` are not stored in the session, resulting in unauthorized errors when group membership is required. Please see: https://gist.github.com/janrotter/b3d806a59292f07fe83bc52c061226e0 for instructions on reproducing the issue. --- oauthproxy_test.go | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 9add52ed..001d7347 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -587,6 +587,53 @@ func (sipTest *SignInPageTest) GetEndpoint(endpoint string) (int, string) { return rw.Code, rw.Body.String() } +type AlwaysSuccessfulValidator struct { +} + +func (AlwaysSuccessfulValidator) Validate(user, password string) bool { + return true +} + +func TestManualSignInStoresUserGroupsInTheSession(t *testing.T) { + userGroups := []string{"somegroup", "someothergroup"} + + opts := baseTestOptions() + opts.HtpasswdUserGroups = userGroups + err := validation.Validate(opts) + if err != nil { + t.Fatal(err) + } + + proxy, err := NewOAuthProxy(opts, func(email string) bool { + return true + }) + if err != nil { + t.Fatal(err) + } + proxy.basicAuthValidator = AlwaysSuccessfulValidator{} + + rw := httptest.NewRecorder() + formData := url.Values{} + formData.Set("username", "someuser") + formData.Set("password", "somepass") + signInReq, _ := http.NewRequest(http.MethodPost, "/oauth2/sign_in", strings.NewReader(formData.Encode())) + signInReq.Header.Add("Content-Type", "application/x-www-form-urlencoded") + proxy.ServeHTTP(rw, signInReq) + + assert.Equal(t, http.StatusFound, rw.Code) + + req, _ := http.NewRequest(http.MethodGet, "/something", strings.NewReader(formData.Encode())) + for _, c := range rw.Result().Cookies() { + req.AddCookie(c) + } + + s, err := proxy.sessionStore.Load(req) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, userGroups, s.Groups) +} + func TestSignInPageIncludesTargetRedirect(t *testing.T) { sipTest, err := NewSignInPageTest(false) if err != nil { From 81cfd24962cf465f9c8a9dc11809f164fe298943 Mon Sep 17 00:00:00 2001 From: Jan Rotter Date: Sun, 26 Sep 2021 22:25:34 +0200 Subject: [PATCH 2/3] Store the group membership in the session This change puts the groups from the htpasswd-user-group in the session during the manual sign in process. This fixes the issue with being unable to properly authenticate using the manual sign in form when certain group membership is required (e.g. when the --gitlab-group option is used). --- oauthproxy.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/oauthproxy.go b/oauthproxy.go index fb6ef0bc..d45bc692 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -78,6 +78,7 @@ type OAuthProxy struct { sessionStore sessionsapi.SessionStore ProxyPrefix string basicAuthValidator basic.Validator + basicAuthGroups []string SkipProviderButton bool skipAuthPreflight bool skipJwtBearerTokens bool @@ -200,6 +201,7 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr trustedIPs: trustedIPs, basicAuthValidator: basicAuthValidator, + basicAuthGroups: opts.HtpasswdUserGroups, sessionChain: sessionChain, headersChain: headersChain, preAuthChain: preAuthChain, @@ -534,7 +536,7 @@ func (p *OAuthProxy) isTrustedIP(req *http.Request) bool { return p.trustedIPs.Has(remoteAddr) } -// SignInPage writes the sing in template to the response +// SignInPage writes the sign in template to the response func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code int) { prepareNoCache(rw) err := p.ClearSessionCookie(rw, req) @@ -589,7 +591,7 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) { user, ok := p.ManualSignIn(req) if ok { - session := &sessionsapi.SessionState{User: user} + session := &sessionsapi.SessionState{User: user, Groups: p.basicAuthGroups} err = p.SaveSession(rw, req, session) if err != nil { logger.Printf("Error saving session: %v", err) From 826ebc230ac66951af387fd75e6f294256837973 Mon Sep 17 00:00:00 2001 From: Jan Rotter Date: Sun, 26 Sep 2021 23:47:28 +0200 Subject: [PATCH 3/3] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5111659..6fc6fcaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ ## Changes since v7.1.3 +- [#1379](https://github.com/oauth2-proxy/oauth2-proxy/pull/1379) Fix the manual sign in with --htpasswd-user-group switch (@janrotter) - [#1337](https://github.com/oauth2-proxy/oauth2-proxy/pull/1337) Changing user field type to text when using htpasswd (@pburgisser) - [#1239](https://github.com/oauth2-proxy/oauth2-proxy/pull/1239) Base GitLab provider implementation on OIDCProvider (@NickMeves) - [#1276](https://github.com/oauth2-proxy/oauth2-proxy/pull/1276) Update crypto and switched to new github.com/golang-jwt/jwt (@JVecsei)