From a283fda6d65dabeb0b7e4a1fcae7758ca7c4b2c0 Mon Sep 17 00:00:00 2001 From: James Elliott Date: Wed, 26 Oct 2022 19:14:43 +1100 Subject: [PATCH] fix(oidc): handle authorization post requests (#4270) This fixes an issue where the authorization endpoint was not handling post requests as per the specification. It also fixes the missing CORS middleware on the authorization endpoint. --- .../handlers/handler_oidc_authorization.go | 4 ++-- internal/middlewares/cors.go | 18 +++++++++--------- internal/server/handlers.go | 16 +++++++++------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/internal/handlers/handler_oidc_authorization.go b/internal/handlers/handler_oidc_authorization.go index 7c24c7c8f..df919e074 100644 --- a/internal/handlers/handler_oidc_authorization.go +++ b/internal/handlers/handler_oidc_authorization.go @@ -13,10 +13,10 @@ import ( "github.com/authelia/authelia/v4/internal/oidc" ) -// OpenIDConnectAuthorizationGET handles GET requests to the OpenID Connect 1.0 Authorization endpoint. +// OpenIDConnectAuthorization handles GET/POST requests to the OpenID Connect 1.0 Authorization endpoint. // // https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationEndpoint -func OpenIDConnectAuthorizationGET(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r *http.Request) { +func OpenIDConnectAuthorization(ctx *middlewares.AutheliaCtx, rw http.ResponseWriter, r *http.Request) { var ( requester fosite.AuthorizeRequester responder fosite.AuthorizeResponder diff --git a/internal/middlewares/cors.go b/internal/middlewares/cors.go index d5a7234a4..8de1aaf84 100644 --- a/internal/middlewares/cors.go +++ b/internal/middlewares/cors.go @@ -197,19 +197,19 @@ type CORSPolicy struct { // HandleOPTIONS is an OPTIONS handler that just adds CORS headers, the Allow header, and sets the status code to 204 // without a body. This handler should generally not be used without using WithAllowedMethods. -func (p CORSPolicy) HandleOPTIONS(ctx *fasthttp.RequestCtx) { +func (p *CORSPolicy) HandleOPTIONS(ctx *fasthttp.RequestCtx) { p.handleOPTIONS(ctx) p.handle(ctx) } // HandleOnlyOPTIONS is an OPTIONS handler that just handles the Allow header, and sets the status code to 204 // without a body. This handler should generally not be used without using WithAllowedMethods. -func (p CORSPolicy) HandleOnlyOPTIONS(ctx *fasthttp.RequestCtx) { +func (p *CORSPolicy) HandleOnlyOPTIONS(ctx *fasthttp.RequestCtx) { p.handleOPTIONS(ctx) } // Middleware provides a middleware that adds the appropriate CORS headers for this CORSPolicyBuilder. -func (p CORSPolicy) Middleware(next fasthttp.RequestHandler) (handler fasthttp.RequestHandler) { +func (p *CORSPolicy) Middleware(next fasthttp.RequestHandler) (handler fasthttp.RequestHandler) { return func(ctx *fasthttp.RequestCtx) { p.handle(ctx) @@ -217,7 +217,7 @@ func (p CORSPolicy) Middleware(next fasthttp.RequestHandler) (handler fasthttp.R } } -func (p CORSPolicy) handle(ctx *fasthttp.RequestCtx) { +func (p *CORSPolicy) handle(ctx *fasthttp.RequestCtx) { if !p.enabled { return } @@ -229,7 +229,7 @@ func (p CORSPolicy) handle(ctx *fasthttp.RequestCtx) { } } -func (p CORSPolicy) handleOPTIONS(ctx *fasthttp.RequestCtx) { +func (p *CORSPolicy) handleOPTIONS(ctx *fasthttp.RequestCtx) { ctx.Response.ResetBody() /* The OPTIONS method should not return a 204 as per the following specifications when read together: @@ -250,13 +250,13 @@ func (p CORSPolicy) handleOPTIONS(ctx *fasthttp.RequestCtx) { } } -func (p CORSPolicy) handleVary(ctx *fasthttp.RequestCtx) { +func (p *CORSPolicy) handleVary(ctx *fasthttp.RequestCtx) { if len(p.vary) != 0 { ctx.Response.Header.SetBytesKV(headerVary, p.vary) } } -func (p CORSPolicy) handleCORS(ctx *fasthttp.RequestCtx) { +func (p *CORSPolicy) handleCORS(ctx *fasthttp.RequestCtx) { var ( originURL *url.URL err error @@ -302,7 +302,7 @@ func (p CORSPolicy) handleCORS(ctx *fasthttp.RequestCtx) { p.handleAllowedMethods(ctx) } -func (p CORSPolicy) handleAllowedMethods(ctx *fasthttp.RequestCtx) { +func (p *CORSPolicy) handleAllowedMethods(ctx *fasthttp.RequestCtx) { switch len(p.methods) { case 0: // TODO: It may be beneficial to be able to control this automatic behaviour. @@ -314,7 +314,7 @@ func (p CORSPolicy) handleAllowedMethods(ctx *fasthttp.RequestCtx) { } } -func (p CORSPolicy) handleAllowedHeaders(ctx *fasthttp.RequestCtx) { +func (p *CORSPolicy) handleAllowedHeaders(ctx *fasthttp.RequestCtx) { switch len(p.headers) { case 0: // TODO: It may be beneficial to be able to control this automatic behaviour. diff --git a/internal/server/handlers.go b/internal/server/handlers.go index b899ebe06..11ae57153 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -261,21 +261,23 @@ func handleRouter(config schema.Configuration, providers middlewares.Providers) r.GET("/api/oidc/jwks", policyCORSPublicGET.Middleware(middlewareOIDC(handlers.JSONWebKeySetGET))) policyCORSAuthorization := middlewares.NewCORSPolicyBuilder(). - WithAllowedMethods("OPTIONS", "GET"). + WithAllowedMethods(fasthttp.MethodOptions, fasthttp.MethodGet, fasthttp.MethodPost). WithAllowedOrigins(allowedOrigins...). WithEnabled(utils.IsStringInSlice(oidc.EndpointAuthorization, config.IdentityProviders.OIDC.CORS.Endpoints)). Build() r.OPTIONS(oidc.EndpointPathAuthorization, policyCORSAuthorization.HandleOnlyOPTIONS) - r.GET(oidc.EndpointPathAuthorization, middlewareOIDC(middlewares.NewHTTPToAutheliaHandlerAdaptor(handlers.OpenIDConnectAuthorizationGET))) + r.GET(oidc.EndpointPathAuthorization, policyCORSAuthorization.Middleware(middlewareOIDC(middlewares.NewHTTPToAutheliaHandlerAdaptor(handlers.OpenIDConnectAuthorization)))) + r.POST(oidc.EndpointPathAuthorization, policyCORSAuthorization.Middleware(middlewareOIDC(middlewares.NewHTTPToAutheliaHandlerAdaptor(handlers.OpenIDConnectAuthorization)))) // TODO (james-d-elliott): Remove in GA. This is a legacy endpoint. r.OPTIONS("/api/oidc/authorize", policyCORSAuthorization.HandleOnlyOPTIONS) - r.GET("/api/oidc/authorize", middlewareOIDC(middlewares.NewHTTPToAutheliaHandlerAdaptor(handlers.OpenIDConnectAuthorizationGET))) + r.GET("/api/oidc/authorize", policyCORSAuthorization.Middleware(middlewareOIDC(middlewares.NewHTTPToAutheliaHandlerAdaptor(handlers.OpenIDConnectAuthorization)))) + r.POST("/api/oidc/authorize", policyCORSAuthorization.Middleware(middlewareOIDC(middlewares.NewHTTPToAutheliaHandlerAdaptor(handlers.OpenIDConnectAuthorization)))) policyCORSToken := middlewares.NewCORSPolicyBuilder(). WithAllowCredentials(true). - WithAllowedMethods("OPTIONS", "POST"). + WithAllowedMethods(fasthttp.MethodOptions, fasthttp.MethodPost). WithAllowedOrigins(allowedOrigins...). WithEnabled(utils.IsStringInSlice(oidc.EndpointToken, config.IdentityProviders.OIDC.CORS.Endpoints)). Build() @@ -285,7 +287,7 @@ func handleRouter(config schema.Configuration, providers middlewares.Providers) policyCORSUserinfo := middlewares.NewCORSPolicyBuilder(). WithAllowCredentials(true). - WithAllowedMethods("OPTIONS", "GET", "POST"). + WithAllowedMethods(fasthttp.MethodOptions, fasthttp.MethodGet, fasthttp.MethodPost). WithAllowedOrigins(allowedOrigins...). WithEnabled(utils.IsStringInSlice(oidc.EndpointUserinfo, config.IdentityProviders.OIDC.CORS.Endpoints)). Build() @@ -296,7 +298,7 @@ func handleRouter(config schema.Configuration, providers middlewares.Providers) policyCORSIntrospection := middlewares.NewCORSPolicyBuilder(). WithAllowCredentials(true). - WithAllowedMethods("OPTIONS", "POST"). + WithAllowedMethods(fasthttp.MethodOptions, fasthttp.MethodPost). WithAllowedOrigins(allowedOrigins...). WithEnabled(utils.IsStringInSlice(oidc.EndpointIntrospection, config.IdentityProviders.OIDC.CORS.Endpoints)). Build() @@ -310,7 +312,7 @@ func handleRouter(config schema.Configuration, providers middlewares.Providers) policyCORSRevocation := middlewares.NewCORSPolicyBuilder(). WithAllowCredentials(true). - WithAllowedMethods("OPTIONS", "POST"). + WithAllowedMethods(fasthttp.MethodOptions, fasthttp.MethodPost). WithAllowedOrigins(allowedOrigins...). WithEnabled(utils.IsStringInSlice(oidc.EndpointRevocation, config.IdentityProviders.OIDC.CORS.Endpoints)). Build()