git.iko.soy/typeable/oauth2-proxy
on master help sign in
Restructure so that serving data from upstream is only done when explicity allowed, rather

than as implicit dangling else

commit9e59b4f62e4abd2812699ba9ac2d00ae83d43dfd authorAdam Eijdenberg <adam.eijdenberg@digital.gov.au> dateFri 7 Jun 2019, 13:50 (7 years ago) parent572646e
1 file changed+48−30
oauthproxy.go+48−30
oauthproxy.go+48−30
@@ -47,6 +47,11 @@ var SignatureHeaders = []string{
4747 "Gap-Auth",
4848}
4949
50+var (
51+ // ErrNeedsLogin means the user should be redirected to the login page
52+ ErrNeedsLogin = errors.New("redirect to login page")
53+)
54+
5055// OAuthProxy is the main authentication proxy
5156type OAuthProxy struct {
5257 CookieSeed string
@@ -477,20 +482,19 @@ func (p *OAuthProxy) IsValidRedirect(redirect string) bool {
477482}
478483
479484// IsWhitelistedRequest is used to check if auth should be skipped for this request
480−func (p *OAuthProxy) IsWhitelistedRequest(req *http.Request) (ok bool) {
485+func (p *OAuthProxy) IsWhitelistedRequest(req *http.Request) bool {
481486 isPreflightRequestAllowed := p.skipAuthPreflight && req.Method == "OPTIONS"
482487 return isPreflightRequestAllowed || p.IsWhitelistedPath(req.URL.Path)
483488}
484489
485490// IsWhitelistedPath is used to check if the request path is allowed without auth
486−func (p *OAuthProxy) IsWhitelistedPath(path string) (ok bool) {
491+func (p *OAuthProxy) IsWhitelistedPath(path string) bool {
487492 for _, u := range p.compiledRegex {
488− ok = u.MatchString(path)
489− if ok {
490− return
493+ if u.MatchString(path) {
494+ return true
491495 }
492496 }
493− return
497+ return false
494498}
495499
496500func getRemoteAddr(req *http.Request) (s string) {
@@ -641,10 +645,11 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
641645
642646// AuthenticateOnly checks whether the user is currently logged in
643647func (p *OAuthProxy) AuthenticateOnly(rw http.ResponseWriter, req *http.Request) {
644− status := p.Authenticate(rw, req)
645− if status == http.StatusAccepted {
648+ _, err := p.getAuthenticatedSession(rw, req)
649+ switch err {
650+ case nil:
646651 rw.WriteHeader(http.StatusAccepted)
647− } else {
652+ default:
648653 http.Error(rw, "unauthorized request", http.StatusUnauthorized)
649654 }
650655}
@@ -652,25 +657,40 @@ func (p *OAuthProxy) AuthenticateOnly(rw http.ResponseWriter, req *http.Request)
652657// Proxy proxies the user request if the user is authenticated else it prompts
653658// them to authenticate
654659func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) {
655− status := p.Authenticate(rw, req)
656− if status == http.StatusInternalServerError {
657− p.ErrorPage(rw, http.StatusInternalServerError,
658− "Internal Error", "Internal Error")
659− } else if status == http.StatusForbidden {
660+ session, err := p.getAuthenticatedSession(rw, req)
661+ switch err {
662+ case nil:
663+ // we are authenticated
664+ p.addHeadersForProxying(rw, req, session)
665+ p.serveMux.ServeHTTP(rw, req)
666+
667+ case ErrNeedsLogin:
668+ // we need to send the user to a login screen
669+ if isAjax(req) {
670+ // no point redirecting an AJAX request
671+ p.ErrorJSON(rw, http.StatusUnauthorized)
672+ return
673+ }
674+
660675 if p.SkipProviderButton {
661676 p.OAuthStart(rw, req)
662677 } else {
663678 p.SignInPage(rw, req, http.StatusForbidden)
664679 }
665− } else if status == http.StatusUnauthorized {
666− p.ErrorJSON(rw, status)
667− } else {
668− p.serveMux.ServeHTTP(rw, req)
680+
681+ default:
682+ // unknown error
683+ logger.Printf("Unexpected internal error: %s", err)
684+ p.ErrorPage(rw, http.StatusInternalServerError,
685+ "Internal Error", "Internal Error")
669686 }
687+
670688}
671689
672−// Authenticate checks whether a user is authenticated
673−func (p *OAuthProxy) Authenticate(rw http.ResponseWriter, req *http.Request) int {
690+// getAuthenticatedSession checks whether a user is authenticated and returns a session object and nil error if so
691+// Returns nil, ErrNeedsLogin if user needs to login.
692+// Set-Cookie headers may be set on the response as a side-effect of calling this method.
693+func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.Request) (*sessionsapi.SessionState, error) {
674694 var saveSession, clearSession, revalidated bool
675695 remoteAddr := getRemoteAddr(req)
676696
@@ -720,7 +740,7 @@ func (p *OAuthProxy) Authenticate(rw http.ResponseWriter, req *http.Request) int
720740 err = p.SaveSession(rw, req, session)
721741 if err != nil {
722742 logger.PrintAuthf(session.Email, req, logger.AuthError, "Save session error %s", err)
723− return http.StatusInternalServerError
743+ return nil, err
724744 }
725745 }
726746
@@ -736,15 +756,14 @@ func (p *OAuthProxy) Authenticate(rw http.ResponseWriter, req *http.Request) int
736756 }
737757
738758 if session == nil {
739− // Check if is an ajax request and return unauthorized to avoid a redirect
740− // to the login page
741− if p.isAjax(req) {
742− return http.StatusUnauthorized
743− }
744− return http.StatusForbidden
759+ return nil, ErrNeedsLogin
745760 }
746761
747− // At this point, the user is authenticated. proxy normally
762+ return session, nil
763+}
764+
765+// addHeadersForProxying adds the appropriate headers the request / response for proxying
766+func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) {
748767 if p.PassBasicAuth {
749768 req.SetBasicAuth(session.User, p.BasicAuthPassword)
750769 req.Header["X-Forwarded-User"] = []string{session.User}
@@ -781,7 +800,6 @@ func (p *OAuthProxy) Authenticate(rw http.ResponseWriter, req *http.Request) int
781800 } else {
782801 rw.Header().Set("GAP-Auth", session.Email)
783802 }
784− return http.StatusAccepted
785803}
786804
787805// CheckBasicAuth checks the requests Authorization header for basic auth
@@ -815,7 +833,7 @@ func (p *OAuthProxy) CheckBasicAuth(req *http.Request) (*sessionsapi.SessionStat
815833}
816834
817835// isAjax checks if a request is an ajax request
818−func (p *OAuthProxy) isAjax(req *http.Request) bool {
836+func isAjax(req *http.Request) bool {
819837 acceptValues, ok := req.Header["accept"]
820838 if !ok {
821839 acceptValues = req.Header["Accept"]