proxy: use internal/httputil for error handling (#36)

- General formatting and comment cleanup.
- Inject pomerium version at compiletime via template package.

authenticate/authenticate.go

@@ -47,15 +47,12 @@ type Options struct {
 
 	SessionLifetimeTTL time.Duration `envconfig:"SESSION_LIFETIME_TTL"`
 
-	// Authentication provider configuration vars
+	// Authentication provider configuration variables as specified by RFC6749
 	// See: https://openid.net/specs/openid-connect-basic-1_0.html#RFC6749
 	ClientID     string   `envconfig:"IDP_CLIENT_ID"`
 	ClientSecret string   `envconfig:"IDP_CLIENT_SECRET"`
 	Provider     string   `envconfig:"IDP_PROVIDER"`
 	ProviderURL  string   `envconfig:"IDP_PROVIDER_URL"`
-	// Scopes is an optional setting corresponding to OAuth 2.0 specification's access scopes
-	// issuing an Access Token. Named providers are already set with good defaults.
-	// Most likely only overrides if using the generic OIDC provider.
 	Scopes       []string `envconfig:"IDP_SCOPE"`
 }
 

authenticate/handlers.go

@@ -52,7 +52,7 @@ func (p *Authenticate) Handler() http.Handler {
 		middleware.ValidateSignature(p.SharedKey),
 		middleware.ValidateRedirectURI(p.ProxyRootDomains))
 
-	validateClientSecret := stdMiddleware.Append(middleware.ValidateClientSecret(p.SharedKey))
+	validateClientSecretMiddleware := stdMiddleware.Append(middleware.ValidateClientSecret(p.SharedKey))
 
 	mux := http.NewServeMux()
 	mux.Handle("/robots.txt", stdMiddleware.ThenFunc(p.RobotsTxt))
@@ -62,10 +62,10 @@ func (p *Authenticate) Handler() http.Handler {
 	// authenticate-server endpoints
 	mux.Handle("/sign_in", validateSignatureMiddleware.ThenFunc(p.SignIn))
 	mux.Handle("/sign_out", validateSignatureMiddleware.ThenFunc(p.SignOut))          // "GET", "POST"
-	mux.Handle("/profile", validateClientSecret.ThenFunc(p.GetProfile))      // GET
+	mux.Handle("/profile", validateClientSecretMiddleware.ThenFunc(p.GetProfile))     // GET
-	mux.Handle("/validate", validateClientSecret.ThenFunc(p.ValidateToken))  // GET
+	mux.Handle("/validate", validateClientSecretMiddleware.ThenFunc(p.ValidateToken)) // GET
-	mux.Handle("/redeem", validateClientSecret.ThenFunc(p.Redeem))           // POST
+	mux.Handle("/redeem", validateClientSecretMiddleware.ThenFunc(p.Redeem))          // POST
-	mux.Handle("/refresh", validateClientSecret.ThenFunc(p.Refresh))         //POST
+	mux.Handle("/refresh", validateClientSecretMiddleware.ThenFunc(p.Refresh))        //POST
 
 	return mux
 }
@@ -431,7 +431,7 @@ func (p *Authenticate) getOAuthCallback(w http.ResponseWriter, r *http.Request)
 	// - for p.provider.ValidateGroup see providers/google.go#ValidateGroup for more info
 	if !p.Validator(session.Email) {
 		log.FromRequest(r).Error().Err(err).Str("email", session.Email).Msg("invalid email permissions denied")
-		return "", httputil.HTTPError{Code: http.StatusForbidden, Message: "Invalid Account"}
+		return "", httputil.HTTPError{Code: http.StatusForbidden, Message: "You don't have access"}
 	}
 	log.FromRequest(r).Info().Str("email", session.Email).Msg("authentication complete")
 	err = p.sessionStore.SaveSession(w, r, session)

internal/httputil/errors.go

@@ -8,7 +8,6 @@ import (
 	"net/http"
 
 	"github.com/pomerium/pomerium/internal/templates"
-	"github.com/pomerium/pomerium/internal/version"
 )
 
 var (
@@ -51,12 +50,10 @@ func ErrorResponse(rw http.ResponseWriter, req *http.Request, message string, co
 			Code    int
 			Title   string
 			Message string
-			Version string
 		}{
 			Code:    code,
 			Title:   title,
 			Message: message,
-			Version: version.FullVersion(),
 		}
 		templates.New().ExecuteTemplate(rw, "error.html", t)
 	}

internal/templates/templates.go

@@ -1,12 +1,15 @@
 package templates // import "github.com/pomerium/pomerium/internal/templates"
 
 import (
+	"fmt"
 	"html/template"
+
+	"github.com/pomerium/pomerium/internal/version"
 )
 
 // New loads html and style resources directly. Panics on failure.
 func New() *template.Template {
-	t := template.New("authenticate-templates")
+	t := template.New("pomerium-templates")
 	template.Must(t.Parse(`
 {{define "header.html"}}
 <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no">
@@ -92,8 +95,7 @@ footer {
 }
 </style>
 {{end}}`))
-
+	t = template.Must(t.Parse(fmt.Sprintf(`{{define "footer.html"}}Secured by <b>pomerium</b> %s {{end}}`, version.FullVersion())))
-	t = template.Must(t.Parse(`{{define "footer.html"}}Secured by <b>pomerium</b> {{end}}`))
 
 	t = template.Must(t.Parse(`
 {{define "sign_in_message.html"}}
@@ -134,7 +136,7 @@ footer {
             </form>
         </div>
 
-        <footer>{{template "footer.html"}} </br> {{.Version}} </footer>
+        <footer>{{template "footer.html"}} </footer>
     </div>
 </body>
 </html>
@@ -159,7 +161,7 @@ footer {
           <span class="details">HTTP {{.Code}}</span>
         </p>
     </div>
-        <footer>{{template "footer.html"}} </br> {{.Version}} </footer>
+        <footer>{{template "footer.html"}} </footer>
     </div>
 </body>
 </html>{{end}}`))
@@ -190,7 +192,7 @@ footer {
               <button type="submit">Sign out</button>
             </form>
     	</div>
-    	<footer>{{template "footer.html"}} </br> {{.Version}}</footer>
+    	<footer>{{template "footer.html"}}</footer>
     </div>
 </body>
 </html>

proxy/handlers.go

@@ -73,10 +73,9 @@ 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
+// Favicon will proxy the request as usual if the user is already authenticated but responds
-// but responds with a 404 otherwise, to avoid spurious and confusing
+// with a 404 otherwise, to avoid spurious and confusing authentication attempts when a browser
-// authentication attempts when a browser automatically requests the favicon on
+// automatically requests the favicon on an error page.
-// an error page.
 func (p *Proxy) Favicon(w http.ResponseWriter, r *http.Request) {
 	err := p.Authenticate(w, r)
 	if err != nil {
@@ -98,23 +97,6 @@ func (p *Proxy) SignOut(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, fullURL.String(), http.StatusFound)
 }
 
-// ErrorPage renders an error page with a given status code, title, and message.
-func (p *Proxy) ErrorPage(w http.ResponseWriter, r *http.Request, code int, title string, message string) {
-	w.WriteHeader(code)
-	t := struct {
-		Code    int
-		Title   string
-		Message string
-		Version string
-	}{
-		Code:    code,
-		Title:   title,
-		Message: message,
-		Version: version.FullVersion(),
-	}
-	p.templates.ExecuteTemplate(w, "error.html", t)
-}
-
 // OAuthStart begins the authentication flow, encrypting the redirect url
 // in a request to the provider's sign in endpoint.
 func (p *Proxy) OAuthStart(w http.ResponseWriter, r *http.Request) {
@@ -134,7 +116,7 @@ func (p *Proxy) OAuthStart(w http.ResponseWriter, r *http.Request) {
 	encryptedCSRF, err := p.cipher.Marshal(state)
 	if err != nil {
 		log.FromRequest(r).Error().Err(err).Msg("failed to marshal csrf")
-		p.ErrorPage(w, r, http.StatusInternalServerError, "Internal Error", err.Error())
+		httputil.ErrorResponse(w, r, err.Error(), http.StatusInternalServerError)
 		return
 	}
 	p.csrfStore.SetCSRF(w, r, encryptedCSRF)
@@ -144,7 +126,7 @@ func (p *Proxy) OAuthStart(w http.ResponseWriter, r *http.Request) {
 	encryptedState, err := p.cipher.Marshal(state)
 	if err != nil {
 		log.FromRequest(r).Error().Err(err).Msg("failed to encrypt cookie")
-		p.ErrorPage(w, r, http.StatusInternalServerError, "Internal Error", err.Error())
+		httputil.ErrorResponse(w, r, err.Error(), http.StatusInternalServerError)
 		return
 	}
 
@@ -153,9 +135,8 @@ func (p *Proxy) OAuthStart(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, signinURL.String(), http.StatusFound)
 }
 
-// OAuthCallback validates the cookie sent back from the provider, then validates
+// OAuthCallback validates the cookie sent back from the provider, then validates he user
-// the user information, and if authorized, redirects the user back to the original
+// information, and if authorized, redirects the user back to the original application.
-// application.
 func (p *Proxy) OAuthCallback(w http.ResponseWriter, r *http.Request) {
 	// We receive the callback from the SSO Authenticator. This request will either contain an
 	// error, or it will contain a `code`; the code can be used to fetch an access token, and
@@ -164,12 +145,12 @@ func (p *Proxy) OAuthCallback(w http.ResponseWriter, r *http.Request) {
 	err := r.ParseForm()
 	if err != nil {
 		log.FromRequest(r).Error().Err(err).Msg("failed parsing request form")
-		p.ErrorPage(w, r, http.StatusInternalServerError, "Internal Error", err.Error())
+		httputil.ErrorResponse(w, r, err.Error(), http.StatusInternalServerError)
 		return
 	}
 	errorString := r.Form.Get("error")
 	if errorString != "" {
-		p.ErrorPage(w, r, http.StatusForbidden, "Permission Denied", errorString)
+		httputil.ErrorResponse(w, r, errorString, http.StatusForbidden)
 		return
 	}
 
@@ -177,7 +158,7 @@ func (p *Proxy) OAuthCallback(w http.ResponseWriter, r *http.Request) {
 	session, err := p.redeemCode(r.Host, r.Form.Get("code"))
 	if err != nil {
 		log.FromRequest(r).Error().Err(err).Msg("error redeeming authorization code")
-		p.ErrorPage(w, r, http.StatusInternalServerError, "Internal Error", "Internal Error")
+		httputil.ErrorResponse(w, r, "Internal error", http.StatusInternalServerError)
 		return
 	}
 
@@ -186,14 +167,14 @@ func (p *Proxy) OAuthCallback(w http.ResponseWriter, r *http.Request) {
 	err = p.cipher.Unmarshal(encryptedState, stateParameter)
 	if err != nil {
 		log.FromRequest(r).Error().Err(err).Msg("could not unmarshal state")
-		p.ErrorPage(w, r, http.StatusInternalServerError, "Internal Error", "Internal Error")
+		httputil.ErrorResponse(w, r, "Internal error", http.StatusInternalServerError)
 		return
 	}
 
 	c, err := p.csrfStore.GetCSRF(r)
 	if err != nil {
 		log.FromRequest(r).Error().Err(err).Msg("failed parsing csrf cookie")
-		p.ErrorPage(w, r, http.StatusBadRequest, "Bad Request", err.Error())
+		httputil.ErrorResponse(w, r, err.Error(), http.StatusBadRequest)
 		return
 	}
 	p.csrfStore.ClearCSRF(w, r)
@@ -203,19 +184,19 @@ func (p *Proxy) OAuthCallback(w http.ResponseWriter, r *http.Request) {
 	err = p.cipher.Unmarshal(encryptedCSRF, csrfParameter)
 	if err != nil {
 		log.FromRequest(r).Error().Err(err).Msg("couldn't unmarshal CSRF")
-		p.ErrorPage(w, r, http.StatusInternalServerError, "Internal Error", "Internal Error")
+		httputil.ErrorResponse(w, r, "Internal error", http.StatusInternalServerError)
 		return
 	}
 
 	if encryptedState == encryptedCSRF {
 		log.FromRequest(r).Error().Msg("encrypted state and CSRF should not be equal")
-		p.ErrorPage(w, r, http.StatusBadRequest, "Bad Request", "Bad Request")
+		httputil.ErrorResponse(w, r, "Bad request", http.StatusBadRequest)
 		return
 	}
 
 	if !reflect.DeepEqual(stateParameter, csrfParameter) {
 		log.FromRequest(r).Error().Msg("state and CSRF should be equal")
-		p.ErrorPage(w, r, http.StatusBadRequest, "Bad Request", "Bad Request")
+		httputil.ErrorResponse(w, r, "Bad request", http.StatusBadRequest)
 		return
 	}
 
@@ -223,7 +204,7 @@ func (p *Proxy) OAuthCallback(w http.ResponseWriter, r *http.Request) {
 	err = p.sessionStore.SaveSession(w, r, session)
 	if err != nil {
 		log.FromRequest(r).Error().Msg("error saving session")
-		p.ErrorPage(w, r, http.StatusInternalServerError, "Internal Error", "Internal Error")
+		httputil.ErrorResponse(w, r, "Error saving session", http.StatusInternalServerError)
 		return
 	}
 
@@ -252,7 +233,7 @@ func (p *Proxy) Proxy(w http.ResponseWriter, r *http.Request) {
 		case ErrUserNotAuthorized:
 			//todo(bdd) : custom forbidden page with details and troubleshooting info
 			log.FromRequest(r).Debug().Err(err).Msg("proxy: user access forbidden")
-			p.ErrorPage(w, r, http.StatusForbidden, "Forbidden", "You don't have access")
+			httputil.ErrorResponse(w, r, "You don't have access", http.StatusForbidden)
 			return
 		case http.ErrNoCookie, sessions.ErrLifetimeExpired, sessions.ErrInvalidSession:
 			log.FromRequest(r).Debug().Err(err).Msg("proxy: starting auth flow")
@@ -260,7 +241,7 @@ func (p *Proxy) Proxy(w http.ResponseWriter, r *http.Request) {
 			return
 		default:
 			log.FromRequest(r).Error().Err(err).Msg("proxy: unexpected error")
-			p.ErrorPage(w, r, http.StatusInternalServerError, "Internal Error", "An unexpected error occurred")
+			httputil.ErrorResponse(w, r, "An unexpected error occurred", http.StatusInternalServerError)
 			return
 		}
 	}

proxy/handlers_test.go

@@ -8,13 +8,13 @@ import (
 )
 
 func TestProxy_RobotsTxt(t *testing.T) {
-	auth := Proxy{}
+	proxy := Proxy{}
 	req, err := http.NewRequest("GET", "/robots.txt", nil)
 	if err != nil {
 		t.Fatal(err)
 	}
 	rr := httptest.NewRecorder()
-	handler := http.HandlerFunc(auth.RobotsTxt)
+	handler := http.HandlerFunc(proxy.RobotsTxt)
 	handler.ServeHTTP(rr, req)
 	if status := rr.Code; status != http.StatusOK {
 		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusOK)
@@ -22,6 +22,5 @@ func TestProxy_RobotsTxt(t *testing.T) {
 	expected := fmt.Sprintf("User-agent: *\nDisallow: /")
 	if rr.Body.String() != expected {
 		t.Errorf("handler returned wrong body: got %v want %v", rr.Body.String(), expected)
-
 	}
 }

proxy/proxy.go

@@ -301,7 +301,7 @@ func NewReverseProxyHandler(opts *Options, reverseProxy *httputil.ReverseProxy,
 }
 
 // urlParse adds a scheme if none-exists, addressesing a quirk in how
-// one may expect url.Parse to function when a "naked" domain is sent.
+// one may expect url.Parse to function when given scheme-less domain is provided.
 //
 // see: https://github.com/golang/go/issues/12585
 // see: https://golang.org/pkg/net/url/#Parse