fix: oidc issuer path and strip path middleware (#2272)

* fix: oidc issuer path and strip path middleware

This ensures the server.path requests append the base_url to the oidc well-known issuer information and adjusts server.path configuration to only strip the configured path instead of the first level entirely regardless of its content.

* fix: only log the token error and general refactoring

* refactor: factorize base_url functions

* refactor(server): include all paths in startup logging

* refactor: factorize

* refactor: GetExternalRootURL -> ExternalRootURL

Co-authored-by: James Elliott <james-d-elliott@users.noreply.github.com>

internal/handlers/const.go

@@ -60,6 +60,8 @@ const (
 
 // OIDC constants.
 const (
+	pathOpenIDConnectWellKnown = "/.well-known/openid-configuration"
+
 	pathOpenIDConnectJWKs          = "/api/oidc/jwks"
 	pathOpenIDConnectAuthorization = "/api/oidc/authorize"
 	pathOpenIDConnectToken         = "/api/oidc/token" //nolint:gosec // This is not a hard coded credential, it's a path.

internal/handlers/handler_oidc_authorization.go

@@ -17,7 +17,7 @@ import (
 	"github.com/authelia/authelia/internal/utils"
 )
 
-func oidcAuthorize(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r *http.Request) {
+func oidcAuthorization(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r *http.Request) {
 	ar, err := ctx.Providers.OpenIDConnect.Fosite.NewAuthorizeRequest(ctx, r)
 	if err != nil {
 		logging.Logger().Errorf("Error occurred in NewAuthorizeRequest: %+v", err)
@@ -62,7 +62,7 @@ func oidcAuthorize(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r *http
 		return
 	}
 
-	issuer, err := ctx.ForwardedProtoHost()
+	issuer, err := ctx.ExternalRootURL()
 	if err != nil {
 		ctx.Logger.Errorf("Error occurred obtaining issuer: %+v", err)
 		ctx.Providers.OpenIDConnect.Fosite.WriteAuthorizeError(rw, ar, err)
@@ -145,7 +145,7 @@ func oidcAuthorizeHandleAuthorizationOrConsentInsufficient(
 	ctx *middlewares.AutheliaCtx, userSession session.UserSession, client *oidc.InternalClient, isAuthInsufficient bool,
 	rw http.ResponseWriter, r *http.Request,
 	ar fosite.AuthorizeRequester) {
-	forwardedProtoHost, err := ctx.ForwardedProtoHost()
+	issuer, err := ctx.ExternalRootURL()
 	if err != nil {
 		ctx.Logger.Errorf("%v", err)
 		http.Error(rw, err.Error(), http.StatusBadRequest)
@@ -153,7 +153,7 @@ func oidcAuthorizeHandleAuthorizationOrConsentInsufficient(
 		return
 	}
 
-	redirectURL := fmt.Sprintf("%s%s", forwardedProtoHost, string(ctx.Request.RequestURI()))
+	redirectURL := fmt.Sprintf("%s%s", issuer, string(ctx.Request.RequestURI()))
 
 	ctx.Logger.Debugf("User %s must consent with scopes %s",
 		userSession.Username, strings.Join(ar.GetRequestedScopes(), ", "))
@@ -175,17 +175,9 @@ func oidcAuthorizeHandleAuthorizationOrConsentInsufficient(
 		return
 	}
 
-	uri, err := ctx.ForwardedProtoHost()
-	if err != nil {
-		ctx.Logger.Errorf("%v", err)
-		http.Error(rw, err.Error(), http.StatusBadRequest)
-
-		return
-	}
-
 	if isAuthInsufficient {
-		http.Redirect(rw, r, uri, http.StatusFound)
+		http.Redirect(rw, r, issuer, http.StatusFound)
 	} else {
-		http.Redirect(rw, r, fmt.Sprintf("%s/consent", uri), http.StatusFound)
+		http.Redirect(rw, r, fmt.Sprintf("%s/consent", issuer), http.StatusFound)
 	}
 }

internal/handlers/handler_oidc_introspection.go

@@ -6,7 +6,7 @@ import (
 	"github.com/authelia/authelia/internal/middlewares"
 )
 
-func oidcIntrospect(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, req *http.Request) {
+func oidcIntrospection(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, req *http.Request) {
 	oidcSession := newOpenIDSession("")
 
 	ir, err := ctx.Providers.OpenIDConnect.Fosite.NewIntrospectionRequest(ctx, req, oidcSession)

internal/handlers/handler_oidc_revocation.go

@@ -6,7 +6,7 @@ import (
 	"github.com/authelia/authelia/internal/middlewares"
 )
 
-func oidcRevoke(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, req *http.Request) {
+func oidcRevocation(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, req *http.Request) {
 	err := ctx.Providers.OpenIDConnect.Fosite.NewRevocationRequest(ctx, req)
 
 	ctx.Providers.OpenIDConnect.Fosite.WriteRevocationResponse(rw, err)

internal/handlers/handler_oidc_token.go

@@ -11,10 +11,10 @@ import (
 func oidcToken(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, req *http.Request) {
 	oidcSession := newOpenIDSession("")
 
-	accessRequest, accessReqErr := ctx.Providers.OpenIDConnect.Fosite.NewAccessRequest(ctx, req, oidcSession)
+	accessRequest, err := ctx.Providers.OpenIDConnect.Fosite.NewAccessRequest(ctx, req, oidcSession)
-	if accessReqErr != nil {
+	if err != nil {
-		ctx.Logger.Errorf("Error occurred in NewAccessRequest: %+v", accessRequest)
+		ctx.Logger.Errorf("Error occurred in NewAccessRequest: %+v", err)
-		ctx.Providers.OpenIDConnect.Fosite.WriteAccessError(rw, accessRequest, accessReqErr)
+		ctx.Providers.OpenIDConnect.Fosite.WriteAccessError(rw, accessRequest, err)
 
 		return
 	}

internal/handlers/handler_oidc_wellknown.go

@@ -11,10 +11,9 @@ import (
 )
 
 func oidcWellKnown(ctx *middlewares.AutheliaCtx) {
-	// TODO (james-d-elliott): append the server.path here for path based installs. Also check other instances in OIDC.
+	issuer, err := ctx.ExternalRootURL()
-	issuer, err := ctx.ForwardedProtoHost()
 	if err != nil {
-		ctx.Logger.Errorf("Error occurred in ForwardedProtoHost: %+v", err)
+		ctx.Logger.Errorf("Error occurred determining OpenID Connect issuer details: %+v", err)
 		ctx.Response.SetStatusCode(fasthttp.StatusBadRequest)
 
 		return
@@ -84,7 +83,7 @@ func oidcWellKnown(ctx *middlewares.AutheliaCtx) {
 	ctx.SetContentType("application/json")
 
 	if err := json.NewEncoder(ctx).Encode(wellKnown); err != nil {
-		ctx.Logger.Errorf("Error occurred in json Encode: %+v", err)
+		ctx.Logger.Errorf("Error occurred in JSON encode: %+v", err)
 		// TODO: Determine if this is the appropriate error code here.
 		ctx.Response.SetStatusCode(fasthttp.StatusInternalServerError)
 

internal/handlers/oidc_register.go

@@ -9,7 +9,7 @@ import (
 // RegisterOIDC registers the handlers with the fasthttp *router.Router. TODO: Add paths for UserInfo, Flush, Logout.
 func RegisterOIDC(router *router.Router, middleware middlewares.RequestHandlerBridge) {
 	// TODO: Add OPTIONS handler.
-	router.GET("/.well-known/openid-configuration", middleware(oidcWellKnown))
+	router.GET(pathOpenIDConnectWellKnown, middleware(oidcWellKnown))
 
 	router.GET(pathOpenIDConnectConsent, middleware(oidcConsent))
 
@@ -17,16 +17,16 @@ func RegisterOIDC(router *router.Router, middleware middlewares.RequestHandlerBr
 
 	router.GET(pathOpenIDConnectJWKs, middleware(oidcJWKs))
 
-	router.GET(pathOpenIDConnectAuthorization, middleware(middlewares.NewHTTPToAutheliaHandlerAdaptor(oidcAuthorize)))
+	router.GET(pathOpenIDConnectAuthorization, middleware(middlewares.NewHTTPToAutheliaHandlerAdaptor(oidcAuthorization)))
 
 	// TODO: Add OPTIONS handler.
 	router.POST(pathOpenIDConnectToken, middleware(middlewares.NewHTTPToAutheliaHandlerAdaptor(oidcToken)))
 
-	router.POST(pathOpenIDConnectIntrospection, middleware(middlewares.NewHTTPToAutheliaHandlerAdaptor(oidcIntrospect)))
+	router.POST(pathOpenIDConnectIntrospection, middleware(middlewares.NewHTTPToAutheliaHandlerAdaptor(oidcIntrospection)))
 
 	router.GET(pathOpenIDConnectUserinfo, middleware(middlewares.NewHTTPToAutheliaHandlerAdaptor(oidcUserinfo)))
 	router.POST(pathOpenIDConnectUserinfo, middleware(middlewares.NewHTTPToAutheliaHandlerAdaptor(oidcUserinfo)))
 
 	// TODO: Add OPTIONS handler.
-	router.POST(pathOpenIDConnectRevocation, middleware(middlewares.NewHTTPToAutheliaHandlerAdaptor(oidcRevoke)))
+	router.POST(pathOpenIDConnectRevocation, middleware(middlewares.NewHTTPToAutheliaHandlerAdaptor(oidcRevocation)))
 }

internal/handlers/response.go

@@ -22,7 +22,7 @@ func handleOIDCWorkflowResponse(ctx *middlewares.AutheliaCtx) {
 		return
 	}
 
-	uri, err := ctx.ForwardedProtoHost()
+	uri, err := ctx.ExternalRootURL()
 	if err != nil {
 		ctx.Logger.Errorf("%v", err)
 		handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to get forward facing URI"), messageAuthenticationFailed)

internal/middlewares/authelia_context.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net"
 	"net/url"
+	"path"
 	"strings"
 
 	"github.com/asaskevich/govalidator"
@@ -113,22 +114,41 @@ func (c *AutheliaCtx) XForwardedURI() []byte {
 	return c.RequestCtx.Request.Header.Peek(headerXForwardedURI)
 }
 
-// ForwardedProtoHost gets the X-Forwarded-Proto and X-Forwarded-Host headers and forms them into a URL.
+// BasePath returns the base_url as per the path visited by the client.
-func (c AutheliaCtx) ForwardedProtoHost() (string, error) {
+func (c *AutheliaCtx) BasePath() (base string) {
-	XForwardedProto := c.XForwardedProto()
+	if baseURL := c.UserValue("base_url"); baseURL != nil {
+		return baseURL.(string)
+	}
 
-	if XForwardedProto == nil {
+	return base
+}
+
+// ExternalRootURL gets the X-Forwarded-Proto, X-Forwarded-Host headers and the BasePath and forms them into a URL.
+func (c *AutheliaCtx) ExternalRootURL() (string, error) {
+	protocol := c.XForwardedProto()
+	if protocol == nil {
 		return "", errMissingXForwardedProto
 	}
 
-	XForwardedHost := c.XForwardedHost()
+	host := c.XForwardedHost()
-
+	if host == nil {
-	if XForwardedHost == nil {
 		return "", errMissingXForwardedHost
 	}
 
-	return fmt.Sprintf("%s://%s", XForwardedProto,
+	externalRootURL := fmt.Sprintf("%s://%s", protocol, host)
-		XForwardedHost), nil
+
+	if base := c.BasePath(); base != "" {
+		externalBaseURL, err := url.Parse(externalRootURL)
+		if err != nil {
+			return "", err
+		}
+
+		externalBaseURL.Path = path.Join(externalBaseURL.Path, base)
+
+		return externalBaseURL.String(), nil
+	}
+
+	return externalRootURL, nil
 }
 
 // XOriginalURL return the content of the X-Original-URL header.

internal/middlewares/errors.go

@@ -2,12 +2,5 @@ package middlewares
 
 import "errors"
 
-// InternalError is the error message sent when there was an internal error but it should
-// be hidden to the end user. In that case the error should be in the server logs.
-const InternalError = "Internal error."
-
-// UnauthorizedError is the error message sent when the user is not authorized.
-const UnauthorizedError = "You're not authorized."
-
 var errMissingXForwardedHost = errors.New("Missing header X-Forwarded-Host")
 var errMissingXForwardedProto = errors.New("Missing header X-Forwarded-Proto")

internal/middlewares/identity_verification.go

@@ -51,13 +51,13 @@ func IdentityVerificationStart(args IdentityVerificationStartArgs) RequestHandle
 			return
 		}
 
-		uri, err := ctx.ForwardedProtoHost()
+		uri, err := ctx.ExternalRootURL()
 		if err != nil {
 			ctx.Error(err, messageOperationFailed)
 			return
 		}
 
-		link := fmt.Sprintf("%s%s%s?token=%s", uri, ctx.Configuration.Server.Path, args.TargetEndpoint, ss)
+		link := fmt.Sprintf("%s%s?token=%s", uri, args.TargetEndpoint, ss)
 
 		bufHTML := new(bytes.Buffer)
 

internal/middlewares/strip_path.go

@@ -1,20 +1,21 @@
 package middlewares
 
 import (
-	"bytes"
+	"strings"
 
 	"github.com/valyala/fasthttp"
 )
 
 // StripPathMiddleware strips the first level of a path.
-func StripPathMiddleware(next fasthttp.RequestHandler) fasthttp.RequestHandler {
+func StripPathMiddleware(path string, next fasthttp.RequestHandler) fasthttp.RequestHandler {
 	return func(ctx *fasthttp.RequestCtx) {
-		uri := ctx.Request.RequestURI()
+		uri := ctx.RequestURI()
-		n := bytes.IndexByte(uri[1:], '/')
 
-		if n >= 0 {
+		if strings.HasPrefix(string(uri), path) {
-			uri = uri[n+1:]
+			ctx.SetUserValue("base_url", path)
-			ctx.Request.SetRequestURI(string(uri))
+
+			newURI := strings.TrimPrefix(string(uri), path)
+			ctx.Request.SetRequestURI(newURI)
 		}
 
 		next(ctx)

internal/server/server.go

@@ -37,9 +37,9 @@ func registerRoutes(configuration schema.Configuration, providers middlewares.Pr
 	embeddedFS := fasthttpadaptor.NewFastHTTPHandler(http.FileServer(http.FS(embeddedPath)))
 	rootFiles := []string{"favicon.ico", "manifest.json", "robots.txt"}
 
-	serveIndexHandler := ServeTemplatedFile(embeddedAssets, indexFile, configuration.Server.Path, rememberMe, resetPassword, configuration.Session.Name, configuration.Theme)
+	serveIndexHandler := ServeTemplatedFile(embeddedAssets, indexFile, rememberMe, resetPassword, configuration.Session.Name, configuration.Theme)
-	serveSwaggerHandler := ServeTemplatedFile(swaggerAssets, indexFile, configuration.Server.Path, rememberMe, resetPassword, configuration.Session.Name, configuration.Theme)
+	serveSwaggerHandler := ServeTemplatedFile(swaggerAssets, indexFile, rememberMe, resetPassword, configuration.Session.Name, configuration.Theme)
-	serveSwaggerAPIHandler := ServeTemplatedFile(swaggerAssets, apiFile, configuration.Server.Path, rememberMe, resetPassword, configuration.Session.Name, configuration.Theme)
+	serveSwaggerAPIHandler := ServeTemplatedFile(swaggerAssets, apiFile, rememberMe, resetPassword, configuration.Session.Name, configuration.Theme)
 
 	r := router.New()
 	r.GET("/", serveIndexHandler)
@@ -143,7 +143,7 @@ func registerRoutes(configuration schema.Configuration, providers middlewares.Pr
 
 	handler := middlewares.LogRequestMiddleware(r.Handler)
 	if configuration.Server.Path != "" {
-		handler = middlewares.StripPathMiddleware(handler)
+		handler = middlewares.StripPathMiddleware(configuration.Server.Path, handler)
 	}
 
 	if providers.OpenIDConnect.Fosite != nil {
@@ -196,14 +196,23 @@ func Start(configuration schema.Configuration, providers middlewares.Providers)
 			logger.Fatalf("Could not configure healthcheck: %v", err)
 		}
 
-		logger.Infof("Listening for TLS connections on %s%s", addrPattern, configuration.Server.Path)
+		if configuration.Server.Path == "" {
+			logger.Infof("Listening for TLS connections on '%s' path '/'", addrPattern)
+		} else {
+			logger.Infof("Listening for TLS connections on '%s' paths '/' and '%s'", addrPattern, configuration.Server.Path)
+		}
+
 		logger.Fatal(server.ServeTLS(listener, configuration.Server.TLS.Certificate, configuration.Server.TLS.Key))
 	} else {
 		if err = writeHealthCheckEnv(configuration.Server.DisableHealthcheck, "http", configuration.Server.Host, configuration.Server.Path, configuration.Server.Port); err != nil {
 			logger.Fatalf("Could not configure healthcheck: %v", err)
 		}
 
-		logger.Infof("Listening for non-TLS connections on %s%s", addrPattern, configuration.Server.Path)
+		if configuration.Server.Path == "" {
+			logger.Infof("Listening for non-TLS connections on '%s' path '/'", addrPattern)
+		} else {
+			logger.Infof("Listening for non-TLS connections on '%s' paths '/' and '%s'", addrPattern, configuration.Server.Path)
+		}
 		logger.Fatal(server.Serve(listener))
 	}
 }

internal/server/template.go

@@ -18,7 +18,7 @@ var alphaNumericRunes = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUV
 // ServeTemplatedFile serves a templated version of a specified file,
 // this is utilised to pass information between the backend and frontend
 // and generate a nonce to support a restrictive CSP while using material-ui.
-func ServeTemplatedFile(publicDir, file, base, rememberMe, resetPassword, session, theme string) fasthttp.RequestHandler {
+func ServeTemplatedFile(publicDir, file, rememberMe, resetPassword, session, theme string) fasthttp.RequestHandler {
 	logger := logging.Logger()
 
 	f, err := assets.Open(publicDir + file)
@@ -37,6 +37,11 @@ func ServeTemplatedFile(publicDir, file, base, rememberMe, resetPassword, sessio
 	}
 
 	return func(ctx *fasthttp.RequestCtx) {
+		base := ""
+		if baseURL := ctx.UserValue("base_url"); baseURL != nil {
+			base = baseURL.(string)
+		}
+
 		nonce := utils.RandomString(32, alphaNumericRunes)
 
 		switch extension := filepath.Ext(file); extension {