7a784a4 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Merge pull request #1709 from aiciobanu/basic-login-failed-message

Show an alert message when basic auth credentials are invalid

commit7a784a460dbccad2fff4837cdad5a9fb414f971c authorJoel Speed <Joel.speed@hotmail.co.uk> dateWed 13 Jul 2022, 14:50 (4 years ago) parentdb74661 33a3a60

8 files changed+110−19

compare with master

unifiedsplit

CHANGELOG.md+2 oauthproxy.go+8−8 oauthproxy_test.go+53 pkg/app/pagewriter/pagewriter.go+4−4 pkg/app/pagewriter/pagewriter_test.go+4−4 pkg/app/pagewriter/sign_in.html+34 pkg/app/pagewriter/sign_in_page.go+3−1 pkg/app/pagewriter/sign_in_page_test.go+2−2

CHANGELOG.md+2

@@ -8,6 +8,8 @@

99## Changes since v7.3.0

1010

11+- [#1709](https://github.com/oauth2-proxy/oauth2-proxy/pull/1709) Show an alert message when basic auth credentials are invalid (@aiciobanu)

12+

1113# V7.3.0

1214

1315## Release Highlights

oauthproxy.go+8−8

@@ -568,26 +568,26 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code

568568 redirectURL = "/"

569569 }

570570

571− p.pageWriter.WriteSignInPage(rw, req, redirectURL)

571+ p.pageWriter.WriteSignInPage(rw, req, redirectURL, code)

572572}

573573

574574// ManualSignIn handles basic auth logins to the proxy

575−func (p *OAuthProxy) ManualSignIn(req *http.Request) (string, bool) {

575+func (p *OAuthProxy) ManualSignIn(req *http.Request) (string, bool, int) {

576576 if req.Method != "POST" || p.basicAuthValidator == nil {

577− return "", false

577+ return "", false, http.StatusOK

578578 }

579579 user := req.FormValue("username")

580580 passwd := req.FormValue("password")

581581 if user == "" {

582− return "", false

582+ return "", false, http.StatusBadRequest

583583 }

584584 // check auth

585585 if p.basicAuthValidator.Validate(user, passwd) {

586586 logger.PrintAuthf(user, req, logger.AuthSuccess, "Authenticated via HtpasswdFile")

587− return user, true

587+ return user, true, http.StatusOK

588588 }

589589 logger.PrintAuthf(user, req, logger.AuthFailure, "Invalid authentication via HtpasswdFile")

590− return "", false

590+ return "", false, http.StatusUnauthorized

591591}

592592

593593// SignIn serves a page prompting users to sign in

@@ -599,7 +599,7 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) {

599599 return

600600 }

601601

602− user, ok := p.ManualSignIn(req)

602+ user, ok, statusCode := p.ManualSignIn(req)

603603 if ok {

604604 session := &sessionsapi.SessionState{User: user, Groups: p.basicAuthGroups}

605605 err = p.SaveSession(rw, req, session)

@@ -614,7 +614,7 @@ func (p *OAuthProxy) SignIn(rw http.ResponseWriter, req *http.Request) {

614614 p.OAuthStart(rw, req)

615615 } else {

616616 // TODO - should we pass on /oauth2/sign_in query params to /oauth2/start?

617− p.SignInPage(rw, req, http.StatusOK)

617+ p.SignInPage(rw, req, statusCode)

618618 }

619619 }

620620}

oauthproxy_test.go+53

@@ -657,6 +657,59 @@ func TestManualSignInStoresUserGroupsInTheSession(t *testing.T) {

657657 assert.Equal(t, userGroups, s.Groups)

658658}

659659

660+type ManualSignInValidator struct{}

661+

662+func (ManualSignInValidator) Validate(user, password string) bool {

663+ switch {

664+ case user == "admin" && password == "adminPass":

665+ return true

666+ default:

667+ return false

668+ }

669+}

670+

671+func ManualSignInWithCredentials(t *testing.T, user, pass string) int {

672+ opts := baseTestOptions()

673+ err := validation.Validate(opts)

674+ if err != nil {

675+ t.Fatal(err)

676+ }

677+

678+ proxy, err := NewOAuthProxy(opts, func(email string) bool {

679+ return true

680+ })

681+ if err != nil {

682+ t.Fatal(err)

683+ }

684+

685+ proxy.basicAuthValidator = ManualSignInValidator{}

686+

687+ rw := httptest.NewRecorder()

688+ formData := url.Values{}

689+ formData.Set("username", user)

690+ formData.Set("password", pass)

691+ signInReq, _ := http.NewRequest(http.MethodPost, "/oauth2/sign_in", strings.NewReader(formData.Encode()))

692+ signInReq.Header.Add("Content-Type", "application/x-www-form-urlencoded")

693+ proxy.ServeHTTP(rw, signInReq)

694+

695+ return rw.Code

696+}

697+

698+func TestManualSignInEmptyUsernameAlert(t *testing.T) {

699+ statusCode := ManualSignInWithCredentials(t, "", "")

700+ assert.Equal(t, http.StatusBadRequest, statusCode)

701+}

702+

703+func TestManualSignInInvalidCredentialsAlert(t *testing.T) {

704+ statusCode := ManualSignInWithCredentials(t, "admin", "")

705+ assert.Equal(t, http.StatusUnauthorized, statusCode)

706+}

707+

708+func TestManualSignInCorrectCredentials(t *testing.T) {

709+ statusCode := ManualSignInWithCredentials(t, "admin", "adminPass")

710+ assert.Equal(t, http.StatusFound, statusCode)

711+}

712+

660713func TestSignInPageIncludesTargetRedirect(t *testing.T) {

661714 sipTest, err := NewSignInPageTest(false)

662715 if err != nil {

pkg/app/pagewriter/pagewriter.go+4−4

@@ -10,7 +10,7 @@ import (

1010// It can also be used to write errors for the http.ReverseProxy used in the

1111// upstream package.

1212type Writer interface {

13− WriteSignInPage(rw http.ResponseWriter, req *http.Request, redirectURL string)

13+ WriteSignInPage(rw http.ResponseWriter, req *http.Request, redirectURL string, statusCode int)

1414 WriteErrorPage(rw http.ResponseWriter, opts ErrorPageOpts)

1515 ProxyErrorHandler(rw http.ResponseWriter, req *http.Request, proxyErr error)

1616 WriteRobotsTxt(rw http.ResponseWriter, req *http.Request)

@@ -108,7 +108,7 @@ func NewWriter(opts Opts) (Writer, error) {

108108// If any of the funcs are not provided, a default implementation will be used.

109109// This is primarily for us in testing.

110110type WriterFuncs struct {

111− SignInPageFunc func(rw http.ResponseWriter, req *http.Request, redirectURL string)

111+ SignInPageFunc func(rw http.ResponseWriter, req *http.Request, redirectURL string, statusCode int)

112112 ErrorPageFunc func(rw http.ResponseWriter, opts ErrorPageOpts)

113113 ProxyErrorFunc func(rw http.ResponseWriter, req *http.Request, proxyErr error)

114114 RobotsTxtfunc func(rw http.ResponseWriter, req *http.Request)

@@ -117,9 +117,9 @@ type WriterFuncs struct {

117117// WriteSignInPage implements the Writer interface.

118118// If the SignInPageFunc is provided, this will be used, else a default

119119// implementation will be used.

120−func (w *WriterFuncs) WriteSignInPage(rw http.ResponseWriter, req *http.Request, redirectURL string) {

120+func (w *WriterFuncs) WriteSignInPage(rw http.ResponseWriter, req *http.Request, redirectURL string, statusCode int) {

121121 if w.SignInPageFunc != nil {

122− w.SignInPageFunc(rw, req, redirectURL)

122+ w.SignInPageFunc(rw, req, redirectURL, statusCode)

123123 return

124124 }

125125

pkg/app/pagewriter/pagewriter_test.go+4−4

@@ -57,7 +57,7 @@ var _ = Describe("Writer", func() {

5757

5858 It("Writes the default sign in template", func() {

5959 recorder := httptest.NewRecorder()

60− writer.WriteSignInPage(recorder, request, "/redirect")

60+ writer.WriteSignInPage(recorder, request, "/redirect", http.StatusOK)

6161

6262 body, err := ioutil.ReadAll(recorder.Result().Body)

6363 Expect(err).ToNot(HaveOccurred())

@@ -104,7 +104,7 @@ var _ = Describe("Writer", func() {

104104

105105 It("Writes the custom sign in template", func() {

106106 recorder := httptest.NewRecorder()

107− writer.WriteSignInPage(recorder, request, "/redirect")

107+ writer.WriteSignInPage(recorder, request, "/redirect", http.StatusOK)

108108

109109 body, err := ioutil.ReadAll(recorder.Result().Body)

110110 Expect(err).ToNot(HaveOccurred())

@@ -151,7 +151,7 @@ var _ = Describe("Writer", func() {

151151 rw := httptest.NewRecorder()

152152 req := httptest.NewRequest("", "/sign-in", nil)

153153 redirectURL := "<redirectURL>"

154− in.writer.WriteSignInPage(rw, req, redirectURL)

154+ in.writer.WriteSignInPage(rw, req, redirectURL, http.StatusOK)

155155

156156 Expect(rw.Result().StatusCode).To(Equal(in.expectedStatus))

157157

@@ -166,7 +166,7 @@ var _ = Describe("Writer", func() {

166166 }),

167167 Entry("With an override function", writerFuncsTableInput{

168168 writer: &WriterFuncs{

169− SignInPageFunc: func(rw http.ResponseWriter, req *http.Request, redirectURL string) {

169+ SignInPageFunc: func(rw http.ResponseWriter, req *http.Request, redirectURL string, statusCode int) {

170170 rw.WriteHeader(202)

171171 rw.Write([]byte(fmt.Sprintf("%s %s", req.URL.Path, redirectURL)))

172172 },

pkg/app/pagewriter/sign_in.html+34

@@ -18,6 +18,28 @@

1818 .logo-box {

1919 margin: 1.5rem 3rem;

2020 }

21+ .alert {

22+ padding: 5px;

23+ background-color: #f44336; /* Red */

24+ color: white;

25+ margin-bottom: 5px;

26+ border-radius: 5px

27+ }

28+ /* The close button */

29+ .closebtn {

30+ margin-left: 10px;

31+ color: white;

32+ font-weight: bold;

33+ float: right;

34+ font-size: 22px;

35+ line-height: 20px;

36+ cursor: pointer;

37+ transition: 0.3s;

38+ }

39+ /* When moving the mouse over the close button */

40+ .closebtn:hover {

41+ color: black;

42+ }

2143 footer a {

2244 text-decoration: underline;

2345 }

@@ -62,6 +84,18 @@

6284 <button class="button is-primary">Sign in</button>

6385 </form>

6486 {{ end }}

87+

88+ {{ if eq .StatusCode 400 401 }}

89+ <div class="alert">

90+ <span class="closebtn" onclick="this.parentElement.style.display='none';">×</span>

91+ {{ if eq .StatusCode 400 }}

92+ {{.StatusCode}}: Username cannot be empty

93+ {{ else }}

94+ {{.StatusCode}}: Invalid Username or Password

95+ {{ end }}

96+ </div>

97+ {{ end }}

98+

6599 </div>

66100 </section>

67101

pkg/app/pagewriter/sign_in_page.go+3−1

@@ -54,12 +54,13 @@ type signInPageWriter struct {

5454

5555// WriteSignInPage writes the sign-in page to the given response writer.

5656// It uses the redirectURL to be able to set the final destination for the user post login.

57−func (s *signInPageWriter) WriteSignInPage(rw http.ResponseWriter, req *http.Request, redirectURL string) {

57+func (s *signInPageWriter) WriteSignInPage(rw http.ResponseWriter, req *http.Request, redirectURL string, statusCode int) {

5858 // We allow unescaped template.HTML since it is user configured options

5959 /* #nosec G203 */

6060 t := struct {

6161 ProviderName string

6262 SignInMessage template.HTML

63+ StatusCode int

6364 CustomLogin bool

6465 Redirect string

6566 Version string

@@ -69,6 +70,7 @@ func (s *signInPageWriter) WriteSignInPage(rw http.ResponseWriter, req *http.Req

6970 }{

7071 ProviderName: s.providerName,

7172 SignInMessage: template.HTML(s.signInMessage),

73+ StatusCode: statusCode,

7274 CustomLogin: s.displayLoginForm,

7375 Redirect: redirectURL,

7476 Version: s.version,

pkg/app/pagewriter/sign_in_page_test.go+2−2

@@ -54,7 +54,7 @@ var _ = Describe("SignIn Page", func() {

5454 Context("WriteSignInPage", func() {

5555 It("Writes the template to the response writer", func() {

5656 recorder := httptest.NewRecorder()

57− signInPage.WriteSignInPage(recorder, request, "/redirect")

57+ signInPage.WriteSignInPage(recorder, request, "/redirect", http.StatusOK)

5858

5959 body, err := ioutil.ReadAll(recorder.Result().Body)

6060 Expect(err).ToNot(HaveOccurred())

@@ -68,7 +68,7 @@ var _ = Describe("SignIn Page", func() {

6868 signInPage.template = tmpl

6969

7070 recorder := httptest.NewRecorder()

71− signInPage.WriteSignInPage(recorder, request, "/redirect")

71+ signInPage.WriteSignInPage(recorder, request, "/redirect", http.StatusOK)

7272

7373 body, err := ioutil.ReadAll(recorder.Result().Body)

7474 Expect(err).ToNot(HaveOccurred())