Skip to content

Commit

Permalink
Merge pull request #1043 from oauth2-proxy/sign-in-page
Browse files Browse the repository at this point in the history 
Refactor Sign In Page rendering and capture all page rendering code in pagewriter package
  • Loading branch information
@JoelSpeed
JoelSpeed authored Feb 14, 2021
2 parents 1e3d854 + 425cff7 commit ce29b16
Show file tree
Hide file tree
Showing 11 changed files with 467 additions and 159 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

## Changes since v7.0.1

- [#1043](https://github.com/oauth2-proxy/oauth2-proxy/pull/1043) Refactor Sign In Page rendering and capture all page rendering code in pagewriter package (@JoelSpeed)
- [#1029](https://github.com/oauth2-proxy/oauth2-proxy/pull/1029) Refactor error page rendering and allow debug messages on error (@JoelSpeed)
- [#1028](https://github.com/oauth2-proxy/oauth2-proxy/pull/1028) Refactor templates, update theme and provide styled error pages (@JoelSpeed)
- [#1039](https://github.com/oauth2-proxy/oauth2-proxy/pull/1039) Ensure errors in tests are logged to the GinkgoWriter (@JoelSpeed)
Expand Down
175 changes: 72 additions & 103 deletions oauthproxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"encoding/json"
"errors"
"fmt"
"html/template"
"net"
"net/http"
"net/url"
Expand All @@ -18,7 +17,7 @@ import (
middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/app"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/app/pagewriter"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/authentication/basic"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/cookies"
"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption"
Expand Down Expand Up @@ -76,39 +75,33 @@ type OAuthProxy struct {
AuthOnlyPath string
UserInfoPath string

allowedRoutes []allowedRoute
redirectURL *url.URL // the url to receive requests at
whitelistDomains []string
provider providers.Provider
providerNameOverride string
sessionStore sessionsapi.SessionStore
ProxyPrefix string
SignInMessage string
basicAuthValidator basic.Validator
displayHtpasswdForm bool
serveMux http.Handler
SetXAuthRequest bool
PassBasicAuth bool
SetBasicAuth bool
SkipProviderButton bool
PassUserHeaders bool
BasicAuthPassword string
PassAccessToken bool
SetAuthorization bool
PassAuthorization bool
PreferEmailToUser bool
skipAuthPreflight bool
skipJwtBearerTokens bool
templates *template.Template
realClientIPParser ipapi.RealClientIPParser
trustedIPs *ip.NetSet
Banner string
Footer string
allowedRoutes []allowedRoute
redirectURL *url.URL // the url to receive requests at
whitelistDomains []string
provider providers.Provider
sessionStore sessionsapi.SessionStore
ProxyPrefix string
basicAuthValidator basic.Validator
serveMux http.Handler
SetXAuthRequest bool
PassBasicAuth bool
SetBasicAuth bool
SkipProviderButton bool
PassUserHeaders bool
BasicAuthPassword string
PassAccessToken bool
SetAuthorization bool
PassAuthorization bool
PreferEmailToUser bool
skipAuthPreflight bool
skipJwtBearerTokens bool
realClientIPParser ipapi.RealClientIPParser
trustedIPs *ip.NetSet

sessionChain alice.Chain
headersChain alice.Chain
preAuthChain alice.Chain
errorPage *app.ErrorPage
pageWriter pagewriter.Writer
}

// NewOAuthProxy creates a new instance of OAuthProxy from the options provided
Expand All @@ -118,20 +111,31 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
return nil, fmt.Errorf("error initialising session store: %v", err)
}

templates, err := app.LoadTemplates(opts.Templates.Path)
if err != nil {
return nil, fmt.Errorf("error loading templates: %v", err)
var basicAuthValidator basic.Validator
if opts.HtpasswdFile != "" {
logger.Printf("using htpasswd file: %s", opts.HtpasswdFile)
var err error
basicAuthValidator, err = basic.NewHTPasswdValidator(opts.HtpasswdFile)
if err != nil {
return nil, fmt.Errorf("could not load htpasswdfile: %v", err)
}
}

errorPage := &app.ErrorPage{
Template: templates.Lookup("error.html"),
ProxyPrefix: opts.ProxyPrefix,
Footer: opts.Templates.Footer,
Version: VERSION,
Debug: opts.Templates.Debug,
pageWriter, err := pagewriter.NewWriter(pagewriter.Opts{
TemplatesPath: opts.Templates.Path,
ProxyPrefix: opts.ProxyPrefix,
Footer: opts.Templates.Footer,
Version: VERSION,
Debug: opts.Templates.Debug,
ProviderName: buildProviderName(opts.GetProvider(), opts.ProviderName),
SignInMessage: buildSignInMessage(opts),
DisplayLoginForm: basicAuthValidator != nil && opts.Templates.DisplayLoginForm,
})
if err != nil {
return nil, fmt.Errorf("error initialising page writer: %v", err)
}

upstreamProxy, err := upstream.NewProxy(opts.UpstreamServers, opts.GetSignatureData(), errorPage.ProxyErrorHandler)
upstreamProxy, err := upstream.NewProxy(opts.UpstreamServers, opts.GetSignatureData(), pageWriter.ProxyErrorHandler)
if err != nil {
return nil, fmt.Errorf("error initialising upstream proxy: %v", err)
}
Expand Down Expand Up @@ -164,16 +168,6 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr
}
}

var basicAuthValidator basic.Validator
if opts.HtpasswdFile != "" {
logger.Printf("using htpasswd file: %s", opts.HtpasswdFile)
var err error
basicAuthValidator, err = basic.NewHTPasswdValidator(opts.HtpasswdFile)
if err != nil {
return nil, fmt.Errorf("could not load htpasswdfile: %v", err)
}
}

allowedRoutes, err := buildRoutesAllowlist(opts)
if err != nil {
return nil, err
Expand Down Expand Up @@ -210,30 +204,24 @@ 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,
provider: opts.GetProvider(),
providerNameOverride: opts.ProviderName,
sessionStore: sessionStore,
serveMux: upstreamProxy,
redirectURL: redirectURL,
allowedRoutes: allowedRoutes,
whitelistDomains: opts.WhitelistDomains,
skipAuthPreflight: opts.SkipAuthPreflight,
skipJwtBearerTokens: opts.SkipJwtBearerTokens,
realClientIPParser: opts.GetRealClientIPParser(),
SkipProviderButton: opts.SkipProviderButton,
templates: templates,
trustedIPs: trustedIPs,
Banner: opts.Templates.Banner,
Footer: opts.Templates.Footer,
SignInMessage: buildSignInMessage(opts),

basicAuthValidator: basicAuthValidator,
displayHtpasswdForm: basicAuthValidator != nil && opts.Templates.DisplayLoginForm,
sessionChain: sessionChain,
headersChain: headersChain,
preAuthChain: preAuthChain,
errorPage: errorPage,
ProxyPrefix: opts.ProxyPrefix,
provider: opts.GetProvider(),
sessionStore: sessionStore,
serveMux: upstreamProxy,
redirectURL: redirectURL,
allowedRoutes: allowedRoutes,
whitelistDomains: opts.WhitelistDomains,
skipAuthPreflight: opts.SkipAuthPreflight,
skipJwtBearerTokens: opts.SkipJwtBearerTokens,
realClientIPParser: opts.GetRealClientIPParser(),
SkipProviderButton: opts.SkipProviderButton,
trustedIPs: trustedIPs,

basicAuthValidator: basicAuthValidator,
sessionChain: sessionChain,
headersChain: headersChain,
preAuthChain: preAuthChain,
pageWriter: pageWriter,
}, nil
}

Expand Down Expand Up @@ -331,6 +319,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)
Expand Down Expand Up @@ -533,7 +528,7 @@ func (p *OAuthProxy) ErrorPage(rw http.ResponseWriter, req *http.Request, code i
redirectURL = "/"
}

p.errorPage.Render(rw, code, redirectURL, appError, messages...)
p.pageWriter.WriteErrorPage(rw, code, redirectURL, appError, messages...)
}

// IsAllowedRequest is used to check if auth should be skipped for this request
Expand Down Expand Up @@ -594,33 +589,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code
redirectURL = "/"
}

// 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: 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())
}
p.pageWriter.WriteSignInPage(rw, redirectURL)
}

// ManualSignIn handles basic auth logins to the proxy
Expand Down
48 changes: 24 additions & 24 deletions pkg/app/error_page.go → pkg/app/pagewriter/error_page.go
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package app
package pagewriter

import (
"fmt"
Expand All @@ -17,30 +17,30 @@ var errorMessages = map[int]string{
http.StatusUnauthorized: "You need to be logged in to access this resource.",
}

// ErrorPage is used to render error pages.
type ErrorPage struct {
// Template is the error page HTML template.
Template *template.Template
// errorPageWriter is used to render error pages.
type errorPageWriter struct {
// template is the error page HTML template.
template *template.Template

// ProxyPrefix is the prefix under which OAuth2 Proxy pages are served.
ProxyPrefix string
// proxyPrefix is the prefix under which OAuth2 Proxy pages are served.
proxyPrefix string

// Footer is the footer to be displayed at the bottom of the page.
// footer is the footer to be displayed at the bottom of the page.
// If not set, a default footer will be used.
Footer string
footer string

// Version is the OAuth2 Proxy version to be used in the default footer.
Version string
// version is the OAuth2 Proxy version to be used in the default footer.
version string

// Debug determines whether errors pages should be rendered with detailed
// debug determines whether errors pages should be rendered with detailed
// errors.
Debug bool
debug bool
}

// Render writes an error page to the given response writer.
// WriteErrorPage writes an error page to the given response writer.
// It uses the passed redirectURL to give users the option to go back to where
// they originally came from or try signing in again.
func (e *ErrorPage) Render(rw http.ResponseWriter, status int, redirectURL string, appError string, messages ...interface{}) {
func (e *errorPageWriter) WriteErrorPage(rw http.ResponseWriter, status int, redirectURL string, appError string, messages ...interface{}) {
rw.WriteHeader(status)

// We allow unescaped template.HTML since it is user configured options
Expand All @@ -56,14 +56,14 @@ func (e *ErrorPage) Render(rw http.ResponseWriter, status int, redirectURL strin
}{
Title: http.StatusText(status),
Message: e.getMessage(status, appError, messages...),
ProxyPrefix: e.ProxyPrefix,
ProxyPrefix: e.proxyPrefix,
StatusCode: status,
Redirect: redirectURL,
Footer: template.HTML(e.Footer),
Version: e.Version,
Footer: template.HTML(e.footer),
Version: e.version,
}

if err := e.Template.Execute(rw, data); err != nil {
if err := e.template.Execute(rw, data); err != nil {
logger.Printf("Error rendering error template: %v", err)
http.Error(rw, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
}
Expand All @@ -72,18 +72,18 @@ func (e *ErrorPage) Render(rw http.ResponseWriter, status int, redirectURL strin
// ProxyErrorHandler is used by the upstream ReverseProxy to render error pages
// when there are issues with upstream servers.
// It is expected to always render a bad gateway error.
func (e *ErrorPage) ProxyErrorHandler(rw http.ResponseWriter, req *http.Request, proxyErr error) {
func (e *errorPageWriter) ProxyErrorHandler(rw http.ResponseWriter, req *http.Request, proxyErr error) {
logger.Errorf("Error proxying to upstream server: %v", proxyErr)
e.Render(rw, http.StatusBadGateway, "", proxyErr.Error(), "There was a problem connecting to the upstream server.")
e.WriteErrorPage(rw, http.StatusBadGateway, "", proxyErr.Error(), "There was a problem connecting to the upstream server.")
}

// getMessage creates the message for the template parameters.
// If the ErrorPage.Debug is enabled, the application error takes precedence.
// If the errorPagewriter.Debug is enabled, the application error takes precedence.
// Otherwise, any messages will be used.
// The first message is expected to be a format string.
// If no messages are supplied, a default error message will be used.
func (e *ErrorPage) getMessage(status int, appError string, messages ...interface{}) string {
if e.Debug {
func (e *errorPageWriter) getMessage(status int, appError string, messages ...interface{}) string {
if e.debug {
return appError
}
if len(messages) > 0 {
Expand Down
Loading

0 comments on commit ce29b16

Please sign in to comment.