internal/sessions: make user state domain scoped

internal/sessions: session state is domain scoped
internal/sessions: infer csrf cookie, route scoped
proxy & authenticate: use shared cookie name
proxy & authenticate: prevent resaving unchanged session
proxy & authenticate: redirect instead of error for no session on login
internal/config: merge cookies
proxy: remove favicon specific route
proxy: use mock server for tests
proxy: add tests for failures

authenticate/authenticate.go

@@ -68,7 +68,7 @@ func New(opts *config.Options) (*Authenticate, error) {
 	}
 	cookieStore, err := sessions.NewCookieStore(
 		&sessions.CookieStoreOptions{
-			Name:           opts.AuthenticateCookieName,
+			Name:           opts.CookieName,
 			CookieSecure:   opts.CookieSecure,
 			CookieHTTPOnly: opts.CookieHTTPOnly,
 			CookieExpire:   opts.CookieExpire,

authenticate/authenticate_test.go

@@ -18,7 +18,7 @@ func testOptions() *config.Options {
 		CookieSecret:    "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw=",
 		CookieRefresh:   time.Duration(1) * time.Hour,
 		CookieExpire:    time.Duration(168) * time.Hour,
-		AuthenticateCookieName: "pomerium",
+		CookieName:      "pomerium",
 	}
 }
 

authenticate/handlers.go

@@ -45,56 +45,47 @@ func (a *Authenticate) RobotsTxt(w http.ResponseWriter, r *http.Request) {
 	fmt.Fprintf(w, "User-agent: *\nDisallow: /")
 }
 
-func (a *Authenticate) authenticate(w http.ResponseWriter, r *http.Request) (*sessions.SessionState, error) {
+func (a *Authenticate) authenticate(w http.ResponseWriter, r *http.Request, session *sessions.SessionState) error {
-	session, err := a.sessionStore.LoadSession(r)
-	if err != nil {
-		log.FromRequest(r).Error().Err(err).Msg("authenticate: failed to load session")
-		a.sessionStore.ClearSession(w, r)
-		return nil, err
-	}
-
 	if session.RefreshPeriodExpired() {
-		newSession, err := a.provider.Refresh(r.Context(), session)
+		session, err := a.provider.Refresh(r.Context(), session)
 		if err != nil {
-			log.FromRequest(r).Error().Err(err).Msg("authenticate: failed to refresh session")
+			return fmt.Errorf("authenticate: session refresh failed : %v", err)
-			a.sessionStore.ClearSession(w, r)
-			return nil, err
-		}
-		err = a.sessionStore.SaveSession(w, r, newSession)
-		if err != nil {
-			log.FromRequest(r).Error().Err(err).Msg("authenticate: could not save refreshed session")
-			a.sessionStore.ClearSession(w, r)
-			return nil, err
-		}
-	} else {
-		// The session has not exceeded it's lifetime or requires refresh
-		ok, err := a.provider.Validate(r.Context(), session.IDToken)
-		if !ok || err != nil {
-			log.FromRequest(r).Error().Err(err).Msg("authenticate: invalid session state")
-			a.sessionStore.ClearSession(w, r)
-			return nil, httputil.ErrUserNotAuthorized
 		}
 		err = a.sessionStore.SaveSession(w, r, session)
 		if err != nil {
-			log.FromRequest(r).Error().Err(err).Msg("authenticate: failed to save valid session")
+			return fmt.Errorf("authenticate: refresh failed : %v", err)
-			a.sessionStore.ClearSession(w, r)
+		}
-			return nil, err
+	} else {
+		valid, err := a.provider.Validate(r.Context(), session.IDToken)
+		if err != nil || !valid {
+			return fmt.Errorf("authenticate: session valid: %v : %v", valid, err)
 		}
 	}
-
+	return nil
-	return session, nil
 }
 
 // SignIn handles the sign_in endpoint. It attempts to authenticate the user,
 // and if the user is not authenticated, it renders a sign in page.
 func (a *Authenticate) SignIn(w http.ResponseWriter, r *http.Request) {
-	session, err := a.authenticate(w, r)
+	session, err := a.sessionStore.LoadSession(r)
 	if err != nil {
-		log.FromRequest(r).Warn().Err(err).Msg("authenticate: authenticate error")
+		switch err {
+		case http.ErrNoCookie, sessions.ErrLifetimeExpired, sessions.ErrInvalidSession:
+			log.FromRequest(r).Debug().Err(err).Msg("proxy: invalid session")
 			a.sessionStore.ClearSession(w, r)
 			a.OAuthStart(w, r)
+			return
+		default:
+			log.FromRequest(r).Error().Err(err).Msg("proxy: unexpected error")
+			httputil.ErrorResponse(w, r, "An unexpected error occurred", http.StatusInternalServerError)
+			return
+		}
+	}
+	err = a.authenticate(w, r, session)
+	if err != nil {
+		httputil.ErrorResponse(w, r, err.Error(), http.StatusInternalServerError)
+		return
 	}
-	log.FromRequest(r).Debug().Msg("authenticate: user authenticated")
 	a.ProxyCallback(w, r, session)
 }
 
@@ -232,7 +223,6 @@ func (a *Authenticate) OAuthStart(w http.ResponseWriter, r *http.Request) {
 	state := base64.URLEncoding.EncodeToString([]byte(fmt.Sprintf("%v:%v", nonce, authRedirectURL.String())))
 	// build the provider sign in url
 	signInURL := a.provider.GetSignInURL(state)
-
 	http.Redirect(w, r, signInURL, http.StatusFound)
 }
 
@@ -292,7 +282,7 @@ func (a *Authenticate) getOAuthCallback(w http.ResponseWriter, r *http.Request)
 	redirect := s[1]
 	c, err := a.csrfStore.GetCSRF(r)
 	if err != nil {
-		log.FromRequest(r).Error().Err(err).Msg("authenticate: bad csrf")
+		log.FromRequest(r).Error().Err(err).Interface("s", s).Msg("authenticate: bad csrf")
 		return "", httputil.HTTPError{Code: http.StatusForbidden, Message: "Missing CSRF token"}
 	}
 	a.csrfStore.ClearCSRF(w, r)

authenticate/handlers_test.go

@@ -62,56 +62,6 @@ func TestAuthenticate_Handler(t *testing.T) {
 	}
 }
 
-func TestAuthenticate_authenticate(t *testing.T) {
-	// sessions.MockSessionStore{Session: expiredLifetime}
-	goodSession := &sessions.MockSessionStore{
-		Session: &sessions.SessionState{
-			AccessToken:     "AccessToken",
-			RefreshToken:    "RefreshToken",
-			RefreshDeadline: time.Now().Add(10 * time.Second),
-		}}
-
-	expiredRefresPeriod := &sessions.MockSessionStore{
-		Session: &sessions.SessionState{
-			AccessToken:  "AccessToken",
-			RefreshToken: "RefreshToken",
-
-			RefreshDeadline: time.Now().Add(10 * -time.Second),
-		}}
-
-	tests := []struct {
-		name     string
-		session  sessions.SessionStore
-		provider identity.MockProvider
-		want     *sessions.SessionState
-		wantErr  bool
-	}{
-		{"good", goodSession, identity.MockProvider{ValidateResponse: true}, nil, false},
-		{"can't load session", &sessions.MockSessionStore{LoadError: errors.New("error")}, identity.MockProvider{ValidateResponse: true}, nil, true},
-		{"validation fails", goodSession, identity.MockProvider{ValidateResponse: false}, nil, true},
-		{"session fails after good validation", &sessions.MockSessionStore{SaveError: errors.New("error"), Session: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken", RefreshDeadline: time.Now().Add(10 * time.Second)}}, identity.MockProvider{ValidateResponse: true}, nil, true},
-		{"refresh expired", expiredRefresPeriod, identity.MockProvider{ValidateResponse: true, RefreshResponse: &sessions.SessionState{AccessToken: "new token", LifetimeDeadline: time.Now()}}, nil, false},
-		{"refresh expired refresh error", expiredRefresPeriod, identity.MockProvider{ValidateResponse: true, RefreshError: errors.New("error")}, nil, true},
-		{"refresh expired failed save", &sessions.MockSessionStore{SaveError: errors.New("error"), Session: &sessions.SessionState{AccessToken: "AccessToken", RefreshToken: "RefreshToken", RefreshDeadline: time.Now().Add(10 * -time.Second)}}, identity.MockProvider{ValidateResponse: true, RefreshResponse: &sessions.SessionState{AccessToken: "new token", LifetimeDeadline: time.Now()}}, nil, true},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			p := &Authenticate{
-				sessionStore: tt.session,
-				provider:     tt.provider,
-			}
-			r := httptest.NewRequest("GET", "/auth", nil)
-			w := httptest.NewRecorder()
-
-			_, err := p.authenticate(w, r)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("Authenticate.authenticate() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-		})
-	}
-}
-
 func TestAuthenticate_SignIn(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -127,16 +77,67 @@ func TestAuthenticate_SignIn(t *testing.T) {
 					RefreshDeadline: time.Now().Add(10 * time.Second),
 				}},
 			identity.MockProvider{ValidateResponse: true},
-
 			http.StatusForbidden},
-		{"session fails after good validation", &sessions.MockSessionStore{
+		{"session not valid",
+			&sessions.MockSessionStore{
+				Session: &sessions.SessionState{
+					AccessToken:     "AccessToken",
+					RefreshToken:    "RefreshToken",
+					RefreshDeadline: time.Now().Add(10 * time.Second),
+				}},
+			identity.MockProvider{ValidateResponse: false},
+			http.StatusInternalServerError},
+		{"session fails fails to save", &sessions.MockSessionStore{
 			SaveError: errors.New("error"),
 			Session: &sessions.SessionState{
 				AccessToken:     "AccessToken",
 				RefreshToken:    "RefreshToken",
 				RefreshDeadline: time.Now().Add(10 * time.Second),
 			}}, identity.MockProvider{ValidateResponse: true},
+			http.StatusForbidden},
+		{"session refresh error",
+			&sessions.MockSessionStore{
+				Session: &sessions.SessionState{
+					AccessToken:     "AccessToken",
+					RefreshToken:    "RefreshToken",
+					RefreshDeadline: time.Now().Add(-10 * time.Second),
+				}},
+			identity.MockProvider{
+				ValidateResponse: true,
+				RefreshError:     errors.New("error")},
+			http.StatusInternalServerError},
+		{"session save after refresh error",
+			&sessions.MockSessionStore{
+				SaveError: errors.New("error"),
+				Session: &sessions.SessionState{
+					AccessToken:     "AccessToken",
+					RefreshToken:    "RefreshToken",
+					RefreshDeadline: time.Now().Add(-10 * time.Second),
+				}},
+			identity.MockProvider{
+				ValidateResponse: true,
+			},
+			http.StatusInternalServerError},
+		{"no cookie found trying to load",
+			&sessions.MockSessionStore{
+				LoadError: http.ErrNoCookie,
+				Session: &sessions.SessionState{
+					AccessToken:     "AccessToken",
+					RefreshToken:    "RefreshToken",
+					RefreshDeadline: time.Now().Add(10 * time.Second),
+				}},
+			identity.MockProvider{ValidateResponse: true},
 			http.StatusBadRequest},
+		{"unexpected error trying to load session",
+			&sessions.MockSessionStore{
+				LoadError: errors.New("unexpeted"),
+				Session: &sessions.SessionState{
+					AccessToken:     "AccessToken",
+					RefreshToken:    "RefreshToken",
+					RefreshDeadline: time.Now().Add(10 * time.Second),
+				}},
+			identity.MockProvider{ValidateResponse: true},
+			http.StatusInternalServerError},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -154,6 +155,7 @@ func TestAuthenticate_SignIn(t *testing.T) {
 			a.SignIn(w, r)
 			if status := w.Code; status != tt.wantCode {
 				t.Errorf("handler returned wrong status code: got %v want %v", status, tt.wantCode)
+				t.Errorf("\n%+v", w.Body)
 			}
 		})
 	}

internal/config/options.go

@@ -70,8 +70,7 @@ type Options struct {
 
 	// Session/Cookie management
 	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie
-	AuthenticateCookieName string
+	CookieName     string        `envconfig:"COOKIE_NAME"`
-	ProxyCookieName        string
 	CookieSecret   string        `envconfig:"COOKIE_SECRET"`
 	CookieDomain   string        `envconfig:"COOKIE_DOMAIN"`
 	CookieSecure   bool          `envconfig:"COOKIE_SECURE"`
@@ -122,12 +121,11 @@ func NewOptions() *Options {
 		Debug:                  false,
 		LogLevel:               "debug",
 		Services:               "all",
-		AuthenticateCookieName: "_pomerium_authenticate",
 		CookieHTTPOnly:         true,
 		CookieSecure:           true,
 		CookieExpire:           time.Duration(14) * time.Hour,
 		CookieRefresh:          time.Duration(30) * time.Minute,
-		ProxyCookieName:        "_pomerium_proxy",
+		CookieName:             "_pomerium",
 		DefaultUpstreamTimeout: time.Duration(30) * time.Second,
 		Headers: map[string]string{
 			"X-Content-Type-Options":    "nosniff",

internal/httputil/errors.go

@@ -2,7 +2,6 @@ package httputil // import "github.com/pomerium/pomerium/internal/httputil"
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -10,11 +9,6 @@ import (
 	"github.com/pomerium/pomerium/internal/templates"
 )
 
-var (
-	// ErrUserNotAuthorized is an error for unauthorized users.
-	ErrUserNotAuthorized = errors.New("user not authorized")
-)
-
 // HTTPError stores the status code and a message for a given HTTP error.
 type HTTPError struct {
 	Code    int

internal/sessions/cookie_store.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"strings"
 	"time"
 
 	"github.com/pomerium/pomerium/internal/cryptutil"
@@ -30,7 +31,6 @@ type SessionStore interface {
 // CookieStore represents all the cookie related configurations
 type CookieStore struct {
 	Name           string
-	CSRFCookieName string
 	CookieCipher   cryptutil.Cipher
 	CookieExpire   time.Duration
 	CookieRefresh  time.Duration
@@ -59,7 +59,6 @@ func NewCookieStore(opts *CookieStoreOptions) (*CookieStore, error) {
 	}
 	return &CookieStore{
 		Name:           opts.Name,
-		CSRFCookieName: fmt.Sprintf("%v_%v", opts.Name, "csrf"),
 		CookieSecure:   opts.CookieSecure,
 		CookieHTTPOnly: opts.CookieHTTPOnly,
 		CookieDomain:   opts.CookieDomain,
@@ -69,6 +68,7 @@ func NewCookieStore(opts *CookieStoreOptions) (*CookieStore, error) {
 }
 
 func (s *CookieStore) makeCookie(req *http.Request, name string, value string, expiration time.Duration, now time.Time) *http.Cookie {
+	// if csrf, scope cookie to the route or service specific domain
 	domain := req.Host
 	if h, _, err := net.SplitHostPort(domain); err == nil {
 		domain = h
@@ -76,6 +76,12 @@ func (s *CookieStore) makeCookie(req *http.Request, name string, value string, e
 	if s.CookieDomain != "" {
 		domain = s.CookieDomain
 	}
+
+	// Non-CSRF sessions can shared, and set domain-wide
+	if !strings.Contains(name, "csrf") {
+		domain = splitDomain(domain)
+	}
+
 	c := &http.Cookie{
 		Name:     name,
 		Value:    value,
@@ -91,14 +97,19 @@ func (s *CookieStore) makeCookie(req *http.Request, name string, value string, e
 	return c
 }
 
+func (s *CookieStore) csrfName() string {
+	return fmt.Sprintf("%s_csrf", s.Name)
+}
+
 // makeSessionCookie constructs a session cookie given the request, an expiration time and the current time.
 func (s *CookieStore) makeSessionCookie(req *http.Request, value string, expiration time.Duration, now time.Time) *http.Cookie {
 	return s.makeCookie(req, s.Name, value, expiration, now)
 }
 
 // makeCSRFCookie creates a CSRF cookie given the request, an expiration time, and the current time.
+// CSRF cookies should be scoped to the actual domain
 func (s *CookieStore) makeCSRFCookie(req *http.Request, value string, expiration time.Duration, now time.Time) *http.Cookie {
-	return s.makeCookie(req, s.CSRFCookieName, value, expiration, now)
+	return s.makeCookie(req, s.csrfName(), value, expiration, now)
 }
 
 // ClearCSRF clears the CSRF cookie from the request
@@ -113,7 +124,7 @@ func (s *CookieStore) SetCSRF(w http.ResponseWriter, req *http.Request, val stri
 
 // GetCSRF gets the CSRFCookie creates a CSRF cookie in a given request
 func (s *CookieStore) GetCSRF(req *http.Request) (*http.Cookie, error) {
-	return req.Cookie(s.CSRFCookieName)
+	return req.Cookie(s.csrfName())
 }
 
 // ClearSession clears the session cookie from a request
@@ -147,3 +158,11 @@ func (s *CookieStore) SaveSession(w http.ResponseWriter, req *http.Request, sess
 	s.setSessionCookie(w, req, value)
 	return nil
 }
+
+func splitDomain(s string) string {
+	if strings.Count(s, ".") < 2 {
+		return ""
+	}
+	split := strings.SplitN(s, ".", 2)
+	return split[1]
+}

internal/sessions/cookie_store_test.go

@@ -55,7 +55,6 @@ func TestNewCookieStore(t *testing.T) {
 			},
 			&CookieStore{
 				Name:           "_cookie",
-				CSRFCookieName: "_cookie_csrf",
 				CookieSecure:   true,
 				CookieHTTPOnly: true,
 				CookieDomain:   "pomerium.io",
@@ -115,25 +114,26 @@ func TestCookieStore_makeCookie(t *testing.T) {
 		value      string
 		expiration time.Duration
 		want       *http.Cookie
+		wantCSRF   *http.Cookie
 	}{
-		{"good", "http://pomerium.io", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "pomerium.io", Secure: true, HttpOnly: true}},
+		{"good", "http://httpbin.corp.pomerium.io", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}},
-		{"domains with https", "https://pomerium.io", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "pomerium.io", Secure: true, HttpOnly: true}},
+		{"domains with https", "https://httpbin.corp.pomerium.io", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}},
-		{"domain with port", "http://pomerium.io:443", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "pomerium.io", Secure: true, HttpOnly: true}},
+		{"domain with port", "http://httpbin.corp.pomerium.io:443", "_pomerium", "value", 0, &http.Cookie{Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}},
-		{"expiration set", "http://pomerium.io:443", "_pomerium", "value", 10 * time.Second, &http.Cookie{Expires: now.Add(10 * time.Second), Name: "_pomerium", Value: "value", Path: "/", Domain: "pomerium.io", Secure: true, HttpOnly: true}},
+		{"expiration set", "http://httpbin.corp.pomerium.io:443", "_pomerium", "value", 10 * time.Second, &http.Cookie{Expires: now.Add(10 * time.Second), Name: "_pomerium", Value: "value", Path: "/", Domain: "corp.pomerium.io", Secure: true, HttpOnly: true}, &http.Cookie{Expires: now.Add(10 * time.Second), Name: "_pomerium_csrf", Value: "value", Path: "/", Domain: "httpbin.corp.pomerium.io", Secure: true, HttpOnly: true}},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			r := httptest.NewRequest("GET", tt.domain, nil)
 
-			s := &CookieStore{
+			o := &CookieStoreOptions{
 				Name:           "_pomerium",
-				CSRFCookieName: "_pomerium_csrf",
 				CookieSecure:   true,
 				CookieHTTPOnly: true,
-				CookieDomain:   "pomerium.io",
+				CookieDomain:   "httpbin.corp.pomerium.io",
 				CookieExpire:   10 * time.Second,
 				CookieCipher:   cipher}
 
+			s, _ := NewCookieStore(o)
 			if got := s.makeCookie(r, tt.cookieName, tt.value, tt.expiration, now); !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("CookieStore.makeCookie() = \n%#v, \nwant\n%#v", got, tt.want)
 			}
@@ -141,16 +141,16 @@ func TestCookieStore_makeCookie(t *testing.T) {
 				t.Errorf("CookieStore.makeCookie() = \n%#v, \nwant\n%#v", got, tt.want)
 			}
 			got := s.makeCSRFCookie(r, tt.value, tt.expiration, now)
-			tt.want.Name = "_pomerium_csrf"
+			tt.wantCSRF.Name = "_pomerium_csrf"
-			if !reflect.DeepEqual(got, tt.want) {
+			if !reflect.DeepEqual(got, tt.wantCSRF) {
-				t.Errorf("CookieStore.makeCookie() = \n%#v, \nwant\n%#v", got, tt.want)
+				t.Errorf("CookieStore.makeCookie() = \n%#v, \nwant\n%#v", got, tt.wantCSRF)
 			}
 			w := httptest.NewRecorder()
 			want := "new-csrf"
 			s.SetCSRF(w, r, want)
 			found := false
 			for _, cookie := range w.Result().Cookies() {
-				if cookie.Name == s.CSRFCookieName && cookie.Value == want {
+				if cookie.Name == s.Name+"_csrf" && cookie.Value == want {
 					found = true
 					break
 				}
@@ -162,7 +162,7 @@ func TestCookieStore_makeCookie(t *testing.T) {
 			w = httptest.NewRecorder()
 			s.ClearCSRF(w, r)
 			for _, cookie := range w.Result().Cookies() {
-				if cookie.Name == s.CSRFCookieName && cookie.Value == want {
+				if cookie.Name == s.Name+"_csrf" && cookie.Value == want {
 					t.Error("clear csrf failed")
 					break
 
@@ -229,7 +229,6 @@ func TestCookieStore_SaveSession(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			s := &CookieStore{
 				Name:           "_pomerium",
-				CSRFCookieName: "_pomerium_csrf",
 				CookieSecure:   true,
 				CookieHTTPOnly: true,
 				CookieDomain:   "pomerium.io",
@@ -335,3 +334,23 @@ func TestMockSessionStore(t *testing.T) {
 		})
 	}
 }
+
+func Test_splitDomain(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		s    string
+		want string
+	}{
+		{"httpbin.corp.example.com", "corp.example.com"},
+		{"some.httpbin.corp.example.com", "httpbin.corp.example.com"},
+		{"example.com", ""},
+		{"", ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.s, func(t *testing.T) {
+			if got := splitDomain(tt.s); got != tt.want {
+				t.Errorf("splitDomain() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

proxy/handlers.go

@@ -2,7 +2,6 @@ package proxy // import "github.com/pomerium/pomerium/proxy"
 
 import (
 	"encoding/base64"
-	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -18,11 +17,6 @@ import (
 	"github.com/pomerium/pomerium/internal/sessions"
 )
 
-var (
-	// ErrUserNotAuthorized is set when user is not authorized to access a resource.
-	ErrUserNotAuthorized = errors.New("user not authorized")
-)
-
 // StateParameter holds the redirect id along with the session id.
 type StateParameter struct {
 	SessionID   string `json:"session_id"`
@@ -38,7 +32,6 @@ func (p *Proxy) Handler() http.Handler {
 		return ok
 	}))
 	mux := http.NewServeMux()
-	mux.HandleFunc("/favicon.ico", p.Favicon)
 	mux.HandleFunc("/robots.txt", p.RobotsTxt)
 	mux.HandleFunc("/.pomerium/sign_out", p.SignOut)
 	mux.HandleFunc("/.pomerium/callback", p.OAuthCallback)
@@ -53,21 +46,10 @@ func (p *Proxy) RobotsTxt(w http.ResponseWriter, _ *http.Request) {
 	fmt.Fprintf(w, "User-agent: *\nDisallow: /")
 }
 
-// Favicon will proxy the request as usual if the user is already authenticated but responds
+// SignOut redirects the request to the sign out url. It's the responsibility
-// with a 404 otherwise, to avoid spurious and confusing authenticate attempts when a browser
+// of the authenticate service to revoke the remote session and clear
-// automatically requests the favicon on an error page.
+// the local session state.
-func (p *Proxy) Favicon(w http.ResponseWriter, r *http.Request) {
-	err := p.Authenticate(w, r)
-	if err != nil {
-		w.WriteHeader(http.StatusNotFound)
-		return
-	}
-	p.Proxy(w, r)
-}
-
-// SignOut redirects the request to the sign out url.
 func (p *Proxy) SignOut(w http.ResponseWriter, r *http.Request) {
-	p.sessionStore.ClearSession(w, r)
 	redirectURL := &url.URL{
 		Scheme: "https",
 		Host:   r.Host,
@@ -198,7 +180,6 @@ func (p *Proxy) OAuthCallback(w http.ResponseWriter, r *http.Request) {
 // Conditions should be few in number and have strong justifications.
 func (p *Proxy) shouldSkipAuthentication(r *http.Request) bool {
 	pol, foundPolicy := p.policy(r)
-
 	if isCORSPreflight(r) && foundPolicy && pol.CORSAllowPreflight {
 		log.FromRequest(r).Debug().Msg("proxy: skipping authentication for valid CORS preflight request")
 		return true
@@ -221,13 +202,12 @@ func isCORSPreflight(r *http.Request) bool {
 // or starting the authenticate service for validation if not.
 func (p *Proxy) Proxy(w http.ResponseWriter, r *http.Request) {
 	if !p.shouldSkipAuthentication(r) {
-		err := p.Authenticate(w, r)
+		session, err := p.sessionStore.LoadSession(r)
-		// If the authenticate is not successful we proceed to start the OAuth Flow with
-		// OAuthStart. If successful, we proceed to proxy to the configured upstream.
 		if err != nil {
 			switch err {
 			case http.ErrNoCookie, sessions.ErrLifetimeExpired, sessions.ErrInvalidSession:
-				log.FromRequest(r).Debug().Err(err).Msg("proxy: starting auth flow")
+				log.FromRequest(r).Debug().Err(err).Msg("proxy: invalid session")
+				p.sessionStore.ClearSession(w, r)
 				p.OAuthStart(w, r)
 				return
 			default:
@@ -236,18 +216,24 @@ func (p *Proxy) Proxy(w http.ResponseWriter, r *http.Request) {
 				return
 			}
 		}
-		// remove dupe session call
+
-		session, err := p.sessionStore.LoadSession(r)
+		err = p.authenticate(w, r, session)
 		if err != nil {
 			p.sessionStore.ClearSession(w, r)
+			log.Debug().Err(err).Msg("proxy: user unauthenticated")
+			httputil.ErrorResponse(w, r, "User unauthenticated", http.StatusForbidden)
 			return
 		}
 		authorized, err := p.AuthorizeClient.Authorize(r.Context(), r.Host, session)
-		if !authorized || err != nil {
+		if err != nil || !authorized {
 			log.FromRequest(r).Warn().Err(err).Msg("proxy: user unauthorized")
 			httputil.ErrorResponse(w, r, "Access unauthorized", http.StatusForbidden)
 			return
 		}
+		// append
+		r.Header.Set(HeaderUserID, session.User)
+		r.Header.Set(HeaderEmail, session.Email)
+		r.Header.Set(HeaderGroups, strings.Join(session.Groups, ","))
 	}
 
 	// We have validated the users request and now proxy their request to the provided upstream.
@@ -289,35 +275,22 @@ func (p *Proxy) Proxy(w http.ResponseWriter, r *http.Request) {
 
 // Authenticate authenticates a request by checking for a session cookie, and validating its expiration,
 // clearing the session cookie if it's invalid and returning an error if necessary..
-func (p *Proxy) Authenticate(w http.ResponseWriter, r *http.Request) (err error) {
+func (p *Proxy) authenticate(w http.ResponseWriter, r *http.Request, session *sessions.SessionState) error {
-	session, err := p.sessionStore.LoadSession(r)
-	if err != nil {
-		p.sessionStore.ClearSession(w, r)
-		return err
-	}
-
 	if session.RefreshPeriodExpired() {
-		// AccessToken's usually expire after 60 or so minutes. If offline_access scope is set, a
+		session, err := p.AuthenticateClient.Refresh(r.Context(), session)
-		// refresh token (which doesn't change) can be used to request a new access-token. If access
-		// is revoked by identity provider, or no refresh token is set, request will return an error
-		session, err = p.AuthenticateClient.Refresh(r.Context(), session)
 		if err != nil {
-			p.sessionStore.ClearSession(w, r)
+			return fmt.Errorf("proxy: session refresh failed : %v", err)
-			log.FromRequest(r).Warn().Err(err).Msg("proxy: refresh failed")
-			return err
 		}
-	}
-
 		err = p.sessionStore.SaveSession(w, r, session)
 		if err != nil {
-		p.sessionStore.ClearSession(w, r)
+			return fmt.Errorf("proxy: refresh failed : %v", err)
-		return err
+		}
+	} else {
+		valid, err := p.AuthenticateClient.Validate(r.Context(), session.IDToken)
+		if err != nil || !valid {
+			return fmt.Errorf("proxy: session valid: %v : %v", valid, err)
+		}
 	}
-	// pass user & user-email details to client applications
-	r.Header.Set(HeaderUserID, session.User)
-	r.Header.Set(HeaderEmail, session.Email)
-	r.Header.Set(HeaderGroups, strings.Join(session.Groups, ","))
-	// This user has been OK'd. Allow the request!
 	return nil
 }
 

proxy/handlers_test.go

@@ -147,19 +147,6 @@ func TestProxy_GetSignInURL(t *testing.T) {
 	}
 }
 
-func TestProxy_Favicon(t *testing.T) {
-	proxy, err := New(testOptions())
-	if err != nil {
-		t.Fatal(err)
-	}
-	req := httptest.NewRequest("GET", "/favicon.ico", nil)
-	rr := httptest.NewRecorder()
-	proxy.Favicon(rr, req)
-	if status := rr.Code; status != http.StatusNotFound {
-		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusNotFound)
-	}
-}
-
 func TestProxy_Signout(t *testing.T) {
 	proxy, err := New(testOptions())
 	if err != nil {
@@ -349,14 +336,16 @@ func TestProxy_Proxy(t *testing.T) {
 		RefreshDeadline:  time.Now().Add(10 * time.Second),
 	}
 
-	opts := testOptions()
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-	optsCORS := testOptionsWithCORS()
+		fmt.Fprintln(w, "RVSI FILIVS CAISAR")
+	}))
+	defer ts.Close()
 
+	opts := testOptionsTestServer(ts.URL)
+	optsCORS := testOptionsWithCORS(ts.URL)
 	defaultHeaders, goodCORSHeaders, badCORSHeaders := http.Header{}, http.Header{}, http.Header{}
-
 	goodCORSHeaders.Set("origin", "anything")
 	goodCORSHeaders.Set("access-control-request-method", "anything")
-
 	// missing "Origin"
 	badCORSHeaders.Set("access-control-request-method", "anything")
 
@@ -372,16 +361,22 @@ func TestProxy_Proxy(t *testing.T) {
 		wantStatus    int
 	}{
 		// weirdly, we want 503 here because that means proxy is trying to route a domain (example.com) that we dont control. Weird. I know.
-		{"good", opts, http.MethodGet, defaultHeaders, "https://corp.example.notatld/test", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusBadGateway},
+		{"good", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusOK},
-		{"good cors preflight", optsCORS, http.MethodOptions, goodCORSHeaders, "https://corp.example.notatld/test", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden},
+		{"good cors preflight", optsCORS, http.MethodOptions, goodCORSHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusOK},
 		// same request as above, but with cors_allow_preflight=false in the policy
-		{"valid cors, but not allowed", opts, http.MethodOptions, goodCORSHeaders, "https://corp.example.com/test", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden},
+		{"valid cors, but not allowed", opts, http.MethodOptions, goodCORSHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden},
 		// cors allowed, but the request is missing proper headers
-		{"invalid cors headers", optsCORS, http.MethodOptions, badCORSHeaders, "https://corp.example.com/test", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden},
+		{"invalid cors headers", optsCORS, http.MethodOptions, badCORSHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden},
-		{"unexpected error", opts, http.MethodGet, defaultHeaders, "https://corp.example.com/test", &sessions.MockSessionStore{LoadError: errors.New("ok")}, clients.MockAuthenticate{}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusInternalServerError},
+		{"unexpected error", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{LoadError: errors.New("ok")}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusInternalServerError},
 		// redirect to start auth process
-		{"unknown host", opts, http.MethodGet, defaultHeaders, "https://notcorp.example.com/test", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusNotFound},
+		{"unknown host", opts, http.MethodGet, defaultHeaders, "https://nothttpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusNotFound},
-		{"user forbidden", opts, http.MethodGet, defaultHeaders, "https://notcorp.example.com/test", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden},
+		{"user forbidden", opts, http.MethodGet, defaultHeaders, "https://nothttpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: false}, http.StatusForbidden},
+		// authenticate errors
+		{"no session error", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{LoadError: http.ErrNoCookie, Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusFound},
+		{"weird load session error", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{LoadError: errors.New("weird"), Session: goodSession}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusInternalServerError},
+		{"failed refreshed session", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: &sessions.SessionState{RefreshDeadline: time.Now().Add(-10 * time.Second)}}, clients.MockAuthenticate{RefreshError: errors.New("refresh error")}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusForbidden},
+		{"cannot resave refreshed session", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{SaveError: errors.New("weird"), Session: &sessions.SessionState{RefreshDeadline: time.Now().Add(-10 * time.Second)}}, clients.MockAuthenticate{ValidateResponse: true}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusForbidden},
+		{"authenticate validation error", opts, http.MethodGet, defaultHeaders, "https://httpbin.corp.example", &sessions.MockSessionStore{Session: goodSession}, clients.MockAuthenticate{ValidateResponse: false}, clients.MockAuthorize{AuthorizeResponse: true}, http.StatusForbidden},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -399,138 +394,13 @@ func TestProxy_Proxy(t *testing.T) {
 			w := httptest.NewRecorder()
 			p.Proxy(w, r)
 			if status := w.Code; status != tt.wantStatus {
-				t.Errorf("handler returned wrong status code: got %v want %v \n body %s", status, tt.wantStatus, w.Body.String())
+				t.Errorf("handler returned wrong status code: got %v want %v", status, tt.wantStatus)
+				t.Errorf("\n%+v", w.Body.String())
+				t.Errorf("\n%+v", opts)
+				t.Errorf("\n%+v", ts.URL)
+
 			}
 
 		})
 	}
 }
-
-func TestProxy_Authenticate(t *testing.T) {
-	configBlob := `[{"from":"corp.example.com","to":"example.com"}]` //valid yaml
-	policy := base64.URLEncoding.EncodeToString([]byte(configBlob))
-
-	goodSession := &sessions.SessionState{
-		User:             "user",
-		Email:            "email@email.com",
-		Groups:           []string{"group1"},
-		AccessToken:      "AccessToken",
-		RefreshToken:     "RefreshToken",
-		LifetimeDeadline: time.Now().Add(10 * time.Second),
-		RefreshDeadline:  time.Now().Add(10 * time.Second),
-	}
-	testAuth := clients.MockAuthenticate{
-		RedeemResponse: goodSession,
-	}
-
-	tests := []struct {
-		name          string
-		host          string
-		mux           string
-		session       sessions.SessionStore
-		authenticator clients.Authenticator
-		wantErr       bool
-	}{
-		{"cannot save session",
-			"https://corp.example.com/",
-			policy,
-			&sessions.MockSessionStore{Session: &sessions.SessionState{
-				User:             "user",
-				Email:            "email@email.com",
-				Groups:           []string{"group1"},
-				AccessToken:      "AccessToken",
-				RefreshToken:     "RefreshToken",
-				LifetimeDeadline: time.Now().Add(10 * time.Second),
-				RefreshDeadline:  time.Now().Add(10 * time.Second),
-			}, SaveError: errors.New("error")},
-			testAuth, true},
-
-		{"cannot load session",
-			"https://corp.example.com/",
-			policy,
-			&sessions.MockSessionStore{LoadError: errors.New("error")}, testAuth, true},
-		{"expired session",
-			"https://corp.example.com/",
-			policy,
-			&sessions.MockSessionStore{
-				Session: &sessions.SessionState{
-					User:             "user",
-					Email:            "email@email.com",
-					Groups:           []string{"group1"},
-					AccessToken:      "AccessToken",
-					RefreshToken:     "RefreshToken",
-					LifetimeDeadline: time.Now().Add(10 * time.Second),
-					RefreshDeadline:  time.Now().Add(-10 * time.Second),
-				}},
-			clients.MockAuthenticate{
-				RefreshError: errors.New("error"),
-				RefreshResponse: &sessions.SessionState{
-					User:             "user",
-					Email:            "email@email.com",
-					Groups:           []string{"group1"},
-					AccessToken:      "AccessToken",
-					RefreshToken:     "RefreshToken",
-					LifetimeDeadline: time.Now().Add(10 * time.Second),
-					RefreshDeadline:  time.Now().Add(-10 * time.Second),
-				}}, true},
-		{"bad refresh authenticator",
-			"https://corp.example.com/",
-			policy,
-			&sessions.MockSessionStore{
-				Session: &sessions.SessionState{
-					User:             "user",
-					Email:            "email@email.com",
-					Groups:           []string{"group1"},
-					AccessToken:      "AccessToken",
-					RefreshToken:     "RefreshToken",
-					LifetimeDeadline: time.Now().Add(10 * time.Second),
-					RefreshDeadline:  time.Now().Add(-10 * time.Second),
-				},
-			},
-			clients.MockAuthenticate{
-				RefreshError: errors.New("error"),
-				RefreshResponse: &sessions.SessionState{
-					User:             "user",
-					Email:            "email@email.com",
-					Groups:           []string{"group1"},
-					AccessToken:      "AccessToken",
-					RefreshToken:     "RefreshToken",
-					LifetimeDeadline: time.Now().Add(10 * time.Second),
-					RefreshDeadline:  time.Now().Add(-10 * time.Second),
-				}},
-			true},
-
-		{"good",
-			"https://corp.example.com/",
-			policy,
-			&sessions.MockSessionStore{Session: &sessions.SessionState{
-				User:             "user",
-				Email:            "email@email.com",
-				Groups:           []string{"group1"},
-				AccessToken:      "AccessToken",
-				RefreshToken:     "RefreshToken",
-				LifetimeDeadline: time.Now().Add(10 * time.Second),
-				RefreshDeadline:  time.Now().Add(10 * time.Second),
-			}}, testAuth, false},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			opts := testOptions()
-			opts.Policy = tt.mux
-			p, err := New(opts)
-			if err != nil {
-				t.Fatal(err)
-			}
-			p.sessionStore = tt.session
-			p.AuthenticateClient = tt.authenticator
-			p.cipher = mockCipher{}
-			r := httptest.NewRequest("GET", tt.host, nil)
-			w := httptest.NewRecorder()
-			fmt.Printf("%s", tt.name)
-
-			if err := p.Authenticate(w, r); (err != nil) != tt.wantErr {
-				t.Errorf("Proxy.Authenticate() error = %v, wantErr %v", err, tt.wantErr)
-			}
-		})
-	}
-}

proxy/proxy.go

@@ -140,7 +140,7 @@ func New(opts *config.Options) (*Proxy, error) {
 
 	cookieStore, err := sessions.NewCookieStore(
 		&sessions.CookieStoreOptions{
-			Name:           opts.ProxyCookieName,
+			Name:           opts.CookieName,
 			CookieDomain:   opts.CookieDomain,
 			CookieSecure:   opts.CookieSecure,
 			CookieHTTPOnly: opts.CookieHTTPOnly,
@@ -267,7 +267,7 @@ func NewReverseProxyHandler(o *config.Options, proxy *httputil.ReverseProxy, rou
 	up := &UpstreamProxy{
 		name:       route.Destination.Host,
 		handler:    proxy,
-		cookieName: o.ProxyCookieName,
+		cookieName: o.CookieName,
 	}
 	if len(o.SigningKey) != 0 {
 		decodedSigningKey, _ := base64.StdEncoding.DecodeString(o.SigningKey)

proxy/proxy_test.go

@@ -2,6 +2,7 @@ package proxy // import "github.com/pomerium/pomerium/proxy"
 
 import (
 	"encoding/base64"
+	"fmt"
 	"io/ioutil"
 	"net"
 	"net/http"
@@ -82,7 +83,7 @@ func TestNewReverseProxyHandler(t *testing.T) {
 func testOptions() *config.Options {
 	authenticateService, _ := url.Parse("https://authenticate.corp.beyondperimeter.com")
 	authorizeService, _ := url.Parse("https://authorize.corp.beyondperimeter.com")
-	configBlob := `[{"from":"corp.example.notatld","to":"example.notatld"}]` //valid yaml
+	configBlob := `[{"from":"corp.example.notatld","to":"example.notatld"}]`
 	policy := base64.URLEncoding.EncodeToString([]byte(configBlob))
 
 	opts := config.NewOptions()
@@ -91,13 +92,30 @@ func testOptions() *config.Options {
 	opts.AuthorizeURL = authorizeService
 	opts.SharedKey = "80ldlrU2d7w+wVpKNfevk6fmb8otEx6CqOfshj2LwhQ="
 	opts.CookieSecret = "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw="
-	opts.ProxyCookieName = "pomerium"
+	opts.CookieName = "pomerium"
 	return opts
 }
 
-func testOptionsWithCORS() *config.Options {
+func testOptionsTestServer(uri string) *config.Options {
-	configBlob := `[{"from":"corp.example.com","to":"example.com","cors_allow_preflight":true}]` //valid yaml
+	authenticateService, _ := url.Parse("https://authenticate.corp.beyondperimeter.com")
-	opts := testOptions()
+	authorizeService, _ := url.Parse("https://authorize.corp.beyondperimeter.com")
+	// RFC 2606
+	configBlob := fmt.Sprintf(`[{"from":"httpbin.corp.example","to":"%s"}]`, uri)
+	policy := base64.URLEncoding.EncodeToString([]byte(configBlob))
+
+	opts := config.NewOptions()
+	opts.Policy = policy
+	opts.AuthenticateURL = authenticateService
+	opts.AuthorizeURL = authorizeService
+	opts.SharedKey = "80ldlrU2d7w+wVpKNfevk6fmb8otEx6CqOfshj2LwhQ="
+	opts.CookieSecret = "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw="
+	opts.CookieName = "pomerium"
+	return opts
+}
+
+func testOptionsWithCORS(uri string) *config.Options {
+	configBlob := fmt.Sprintf(`[{"from":"httpbin.corp.example","to":"%s","cors_allow_preflight":true}]`, uri)
+	opts := testOptionsTestServer(uri)
 	opts.Policy = base64.URLEncoding.EncodeToString([]byte(configBlob))
 	return opts
 }