Move SignIn page rendering to app pkg

oauthproxy.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"html/template"
 	"net"
 	"net/http"
 	"net/url"
@@ -76,39 +75,34 @@ type OAuthProxy struct {
 	AuthOnlyPath      string
 	UserInfoPath      string
 
-	allowedRoutes        []allowedRoute
+	allowedRoutes       []allowedRoute
-	redirectURL          *url.URL // the url to receive requests at
+	redirectURL         *url.URL // the url to receive requests at
-	whitelistDomains     []string
+	whitelistDomains    []string
-	provider             providers.Provider
+	provider            providers.Provider
-	providerNameOverride string
+	sessionStore        sessionsapi.SessionStore
-	sessionStore         sessionsapi.SessionStore
+	ProxyPrefix         string
-	ProxyPrefix          string
+	basicAuthValidator  basic.Validator
-	SignInMessage        string
+	serveMux            http.Handler
-	basicAuthValidator   basic.Validator
+	SetXAuthRequest     bool
-	displayHtpasswdForm  bool
+	PassBasicAuth       bool
-	serveMux             http.Handler
+	SetBasicAuth        bool
-	SetXAuthRequest      bool
+	SkipProviderButton  bool
-	PassBasicAuth        bool
+	PassUserHeaders     bool
-	SetBasicAuth         bool
+	BasicAuthPassword   string
-	SkipProviderButton   bool
+	PassAccessToken     bool
-	PassUserHeaders      bool
+	SetAuthorization    bool
-	BasicAuthPassword    string
+	PassAuthorization   bool
-	PassAccessToken      bool
+	PreferEmailToUser   bool
-	SetAuthorization     bool
+	skipAuthPreflight   bool
-	PassAuthorization    bool
+	skipJwtBearerTokens bool
-	PreferEmailToUser    bool
+	realClientIPParser  ipapi.RealClientIPParser
-	skipAuthPreflight    bool
+	trustedIPs          *ip.NetSet
-	skipJwtBearerTokens  bool
-	templates            *template.Template
-	realClientIPParser   ipapi.RealClientIPParser
-	trustedIPs           *ip.NetSet
-	Banner               string
-	Footer               string
 
 	sessionChain alice.Chain
 	headersChain alice.Chain
 	preAuthChain alice.Chain
 	errorPage    *app.ErrorPage
+	signInPage   *app.SignInPage
 }
 
 // NewOAuthProxy creates a new instance of OAuthProxy from the options provided
@@ -174,6 +168,17 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
 		}
 	}
 
+	signInPage := &app.SignInPage{
+		Template:         templates.Lookup("sign_in.html"),
+		ErrorPage:        errorPage,
+		ProxyPrefix:      opts.ProxyPrefix,
+		ProviderName:     buildProviderName(opts.GetProvider(), opts.ProviderName),
+		SignInMessage:    buildSignInMessage(opts),
+		Footer:           opts.Templates.Footer,
+		Version:          VERSION,
+		DisplayLoginForm: basicAuthValidator != nil && opts.Templates.DisplayLoginForm,
+	}
+
 	allowedRoutes, err := buildRoutesAllowlist(opts)
 	if err != nil {
 		return nil, err
@@ -210,30 +215,25 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
 		AuthOnlyPath:      fmt.Sprintf("%s/auth", opts.ProxyPrefix),
 		UserInfoPath:      fmt.Sprintf("%s/userinfo", opts.ProxyPrefix),
 
-		ProxyPrefix:          opts.ProxyPrefix,
+		ProxyPrefix:         opts.ProxyPrefix,
-		provider:             opts.GetProvider(),
+		provider:            opts.GetProvider(),
-		providerNameOverride: opts.ProviderName,
+		sessionStore:        sessionStore,
-		sessionStore:         sessionStore,
+		serveMux:            upstreamProxy,
-		serveMux:             upstreamProxy,
+		redirectURL:         redirectURL,
-		redirectURL:          redirectURL,
+		allowedRoutes:       allowedRoutes,
-		allowedRoutes:        allowedRoutes,
+		whitelistDomains:    opts.WhitelistDomains,
-		whitelistDomains:     opts.WhitelistDomains,
+		skipAuthPreflight:   opts.SkipAuthPreflight,
-		skipAuthPreflight:    opts.SkipAuthPreflight,
+		skipJwtBearerTokens: opts.SkipJwtBearerTokens,
-		skipJwtBearerTokens:  opts.SkipJwtBearerTokens,
+		realClientIPParser:  opts.GetRealClientIPParser(),
-		realClientIPParser:   opts.GetRealClientIPParser(),
+		SkipProviderButton:  opts.SkipProviderButton,
-		SkipProviderButton:   opts.SkipProviderButton,
+		trustedIPs:          trustedIPs,
-		templates:            templates,
-		trustedIPs:           trustedIPs,
-		Banner:               opts.Templates.Banner,
-		Footer:               opts.Templates.Footer,
-		SignInMessage:        buildSignInMessage(opts),
 
-		basicAuthValidator:  basicAuthValidator,
+		basicAuthValidator: basicAuthValidator,
-		displayHtpasswdForm: basicAuthValidator != nil && opts.Templates.DisplayLoginForm,
+		sessionChain:       sessionChain,
-		sessionChain:        sessionChain,
+		headersChain:       headersChain,
-		headersChain:        headersChain,
+		preAuthChain:       preAuthChain,
-		preAuthChain:        preAuthChain,
+		errorPage:          errorPage,
-		errorPage:           errorPage,
+		signInPage:         signInPage,
 	}, nil
 }
 
@@ -331,6 +331,13 @@ func buildSignInMessage(opts *options.Options) string {
 	return msg
 }
 
+func buildProviderName(p providers.Provider, override string) string {
+	if override != "" {
+		return override
+	}
+	return p.Data().ProviderName
+}
+
 // buildRoutesAllowlist builds an []allowedRoute  list from either the legacy
 // SkipAuthRegex option (paths only support) or newer SkipAuthRoutes option
 // (method=path support)
@@ -594,33 +601,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code
 		redirectURL = "/"
 	}
 
-	// We allow unescaped template.HTML since it is user configured options
+	p.signInPage.Render(rw, redirectURL)
-	/* #nosec G203 */
-	t := struct {
-		ProviderName  string
-		SignInMessage template.HTML
-		CustomLogin   bool
-		Redirect      string
-		Version       string
-		ProxyPrefix   string
-		Footer        template.HTML
-	}{
-		ProviderName:  p.provider.Data().ProviderName,
-		SignInMessage: template.HTML(p.SignInMessage),
-		CustomLogin:   p.displayHtpasswdForm,
-		Redirect:      redirectURL,
-		Version:       VERSION,
-		ProxyPrefix:   p.ProxyPrefix,
-		Footer:        template.HTML(p.Footer),
-	}
-	if p.providerNameOverride != "" {
-		t.ProviderName = p.providerNameOverride
-	}
-	err = p.templates.ExecuteTemplate(rw, "sign_in.html", t)
-	if err != nil {
-		logger.Printf("Error rendering sign_in.html template: %v", err)
-		p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())
-	}
 }
 
 // ManualSignIn handles basic auth logins to the proxy

pkg/app/sign_in_page.go

@@ -0,0 +1,66 @@
+package app
+
+import (
+	"html/template"
+	"net/http"
+
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
+)
+
+// SignInPage is used to render sign-in pages.
+type SignInPage struct {
+	// Template is the sign-in page HTML template.
+	Template *template.Template
+
+	// ErrorPage is used to render an error if there are problems with rendering the sign-in page.
+	ErrorPage *ErrorPage
+
+	// ProxyPrefix is the prefix under which OAuth2 Proxy pages are served.
+	ProxyPrefix string
+
+	// ProviderName is the name of the provider that should be displayed on the login button.
+	ProviderName string
+
+	// SignInMessage is the messge displayed above the login button.
+	SignInMessage string
+
+	// Footer is the footer to be displayed at the bottom of the page.
+	// If not set, a default footer will be used.
+	Footer string
+
+	// Version is the OAuth2 Proxy version to be used in the default footer.
+	Version string
+
+	// DisplayLoginForm determines whether or not the basic auth password form is displayed on the sign-in page.
+	DisplayLoginForm bool
+}
+
+// Render writes the sign-in page to the given response writer.
+// It uses the redirectURL to be able to set the final destination for the user post login.
+func (s *SignInPage) Render(rw http.ResponseWriter, redirectURL string) {
+	// We allow unescaped template.HTML since it is user configured options
+	/* #nosec G203 */
+	t := struct {
+		ProviderName  string
+		SignInMessage template.HTML
+		CustomLogin   bool
+		Redirect      string
+		Version       string
+		ProxyPrefix   string
+		Footer        template.HTML
+	}{
+		ProviderName:  s.ProviderName,
+		SignInMessage: template.HTML(s.SignInMessage),
+		CustomLogin:   s.DisplayLoginForm,
+		Redirect:      redirectURL,
+		Version:       s.Version,
+		ProxyPrefix:   s.ProxyPrefix,
+		Footer:        template.HTML(s.Footer),
+	}
+
+	err := s.Template.Execute(rw, t)
+	if err != nil {
+		logger.Printf("Error rendering sign-in template: %v", err)
+		s.ErrorPage.Render(rw, http.StatusInternalServerError, redirectURL, err.Error())
+	}
+}

pkg/app/sign_in_page_test.go

@@ -0,0 +1,61 @@
+package app
+
+import (
+	"html/template"
+	"io/ioutil"
+	"net/http/httptest"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("SignIn Page", func() {
+	var signInPage *SignInPage
+
+	BeforeEach(func() {
+		errorTmpl, err := template.New("").Parse("{{.Title}}")
+		Expect(err).ToNot(HaveOccurred())
+		errorPage := &ErrorPage{
+			Template: errorTmpl,
+		}
+
+		tmpl, err := template.New("").Parse("{{.ProxyPrefix}} {{.ProviderName}} {{.SignInMessage}} {{.Footer}} {{.Version}} {{.Redirect}} {{.CustomLogin}}")
+		Expect(err).ToNot(HaveOccurred())
+
+		signInPage = &SignInPage{
+			Template:         tmpl,
+			ErrorPage:        errorPage,
+			ProxyPrefix:      "/prefix/",
+			ProviderName:     "My Provider",
+			SignInMessage:    "Sign In Here",
+			Footer:           "Custom Footer Text",
+			Version:          "v0.0.0-test",
+			DisplayLoginForm: true,
+		}
+	})
+
+	Context("Render", func() {
+		It("Writes the template to the response writer", func() {
+			recorder := httptest.NewRecorder()
+			signInPage.Render(recorder, "/redirect")
+
+			body, err := ioutil.ReadAll(recorder.Result().Body)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(string(body)).To(Equal("/prefix/ My Provider Sign In Here Custom Footer Text v0.0.0-test /redirect true"))
+		})
+
+		It("Writes an error if the template can't be rendered", func() {
+			// Overwrite the template with something bad
+			tmpl, err := template.New("").Parse("{{.Unknown}}")
+			Expect(err).ToNot(HaveOccurred())
+			signInPage.Template = tmpl
+
+			recorder := httptest.NewRecorder()
+			signInPage.Render(recorder, "/redirect")
+
+			body, err := ioutil.ReadAll(recorder.Result().Body)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(string(body)).To(Equal("Internal Server Error"))
+		})
+	})
+})