Skip to content

Commit

Permalink
proxy: fix bug that would allow failed refresh session to continue (#762
Browse files Browse the repository at this point in the history
)

Signed-off-by: Bobby DeSimone <bobbydesimone@gmail.com>
  • Loading branch information
desimone committed May 24, 2020
1 parent d31245f commit 82d5f9c
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
8 changes: 7 additions & 1 deletion internal/httputil/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package httputil

import (
"encoding/json"
"errors"
"html/template"
"net/http"

Expand All @@ -11,6 +12,8 @@ import (
"github.com/pomerium/pomerium/internal/version"
)

var ErrRedirectOnly = errors.New("httputil: redirecting to authenticate service")

var errorTemplate = template.Must(frontend.NewTemplates())
var fullVersion = version.FullVersion()

Expand Down Expand Up @@ -60,11 +63,14 @@ type errResponse struct {
// It does not otherwise end the request; the caller should ensure no further
// writes are done to w.
func (e *HTTPError) ErrorResponse(w http.ResponseWriter, r *http.Request) {
log.FromRequest(r).Info().Err(e).Msg("httputil: ErrorResponse")
if errors.Is(e, ErrRedirectOnly) {
return
}
// indicate to clients that the error originates from Pomerium, not the app
w.Header().Set(HeaderPomeriumResponse, "true")
w.WriteHeader(e.Status)

log.FromRequest(r).Info().Err(e).Msg("httputil: ErrorResponse")
var requestID string
if id, ok := log.IDFromRequest(r); ok {
requestID = id
Expand Down
2 changes: 1 addition & 1 deletion proxy/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (p *Proxy) redirectToSignin(w http.ResponseWriter, r *http.Request) error {
log.FromRequest(r).Debug().Str("url", signinURL.String()).Msg("proxy: redirectToSignin")
httputil.Redirect(w, r, urlutil.NewSignedURL(p.SharedKey, &signinURL).String(), http.StatusFound)
p.sessionStore.ClearSession(w, r)
return nil
return httputil.ErrRedirectOnly
}

// AuthorizeSession is middleware to enforce a user is authorized for a request.
Expand Down

0 comments on commit 82d5f9c

Please sign in to comment.