diff --git a/pkg/authorize/handler.go b/pkg/authorize/handler.go index 7ee8d96..c5e7e6d 100644 --- a/pkg/authorize/handler.go +++ b/pkg/authorize/handler.go @@ -66,7 +66,7 @@ func HandleAuthorize(w http.ResponseWriter, r *http.Request) { // Validate redirect_uri format first — if invalid we cannot redirect back safely if !utils.IsValidRedirectURI(request.RedirectURI) { - renderError(w, r, "Invalid redirect_uri") + view.RenderError(w, r, http.StatusBadRequest,"Invalid redirect_uri") return } @@ -74,13 +74,13 @@ func HandleAuthorize(w http.ResponseWriter, r *http.Request) { registeredClient, err := client.ClientByClientID(request.ClientID) if err != nil { slog.Warn("authorize: unknown client_id", "request_id", reqid.Get(r.Context()), "client_id", request.ClientID, "ip", utils.GetClientIP(r)) - renderError(w, r, "Unknown client_id") + view.RenderError(w, r, http.StatusBadRequest,"Unknown client_id") return } if !registeredClient.IsActive { slog.Warn("authorize: inactive client", "request_id", reqid.Get(r.Context()), "client_id", request.ClientID, "ip", utils.GetClientIP(r)) - renderError(w, r, "Client is inactive") + view.RenderError(w, r, http.StatusBadRequest,"Client is inactive") return } @@ -88,7 +88,7 @@ func HandleAuthorize(w http.ResponseWriter, r *http.Request) { // if invalid, do not redirect — render an error page instead to avoid open redirector. if !client.IsValidRedirectURI(registeredClient, request.RedirectURI) { slog.Warn("authorize: invalid redirect_uri for client", "request_id", reqid.Get(r.Context()), "client_id", request.ClientID, "redirect_uri", request.RedirectURI) - renderError(w, r, "Redirect URI not allowed for this client") + view.RenderError(w, r, http.StatusBadRequest,"Redirect URI not allowed for this client") return } @@ -364,26 +364,3 @@ func parseMaxAge(s string) int64 { return v } -// renderError renders a branded error page without any login form fields. -// Use this for fatal errors where redirecting or submitting credentials makes no sense. -func renderError(w http.ResponseWriter, r *http.Request, errorMsg string) { - cfg := config.Get() - tmpl, err := view.ParseTemplate("error") - if err != nil { - slog.Error("authorize: failed to parse error template", "error", err) - http.Error(w, "Internal Server Error", http.StatusInternalServerError) - return - } - - data := map[string]any{ - "Error": errorMsg, - "ThemeTitle": cfg.Theme.Title, - "ThemeLogoUrl": cfg.Theme.LogoUrl, - } - view.InjectNonce(r, data) - - w.WriteHeader(http.StatusBadRequest) - if err = tmpl.ExecuteTemplate(w, "layout", data); err != nil { - http.Error(w, "Template Execution Error", http.StatusInternalServerError) - } -} diff --git a/pkg/cli/start.go b/pkg/cli/start.go index 4a3ea93..69ef02f 100644 --- a/pkg/cli/start.go +++ b/pkg/cli/start.go @@ -136,7 +136,7 @@ func RunStart(c *cli.Context) error { mux.Handle("GET "+oauth+"/authorize", rateLimited(csrfProtected(authorize.HandleAuthorize))) mux.Handle("POST "+oauth+"/authorize", rateLimitedFunc(authorize.HandleAuthorize)) - mux.Handle("POST "+oauth+"/login", rateLimited(csrfProtected(login.HandleLoginUser))) + mux.Handle(oauth+"/login", rateLimited(csrfProtected(login.HandleLoginUser))) mux.Handle(oauth+"/consent", csrfProtected(consent.HandleConsent)) mux.Handle(oauth+"/mfa", rateLimited(csrfProtected(mfa.HandleMfa))) mux.Handle(oauth+"/mfa/", rateLimited(csrfProtected(mfa.HandleMfa))) diff --git a/pkg/login/handler.go b/pkg/login/handler.go index 63b6072..09d13e1 100644 --- a/pkg/login/handler.go +++ b/pkg/login/handler.go @@ -22,6 +22,7 @@ import ( "github.com/eugenioenko/autentico/pkg/trusteddevice" "github.com/eugenioenko/autentico/pkg/user" "github.com/eugenioenko/autentico/pkg/utils" + "github.com/eugenioenko/autentico/view" ) const mfaChallengeExpiration = 10 * time.Minute @@ -42,7 +43,7 @@ const mfaChallengeExpiration = 10 * time.Minute // Failure 500 model.ApiError func HandleLoginUser(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost { - utils.WriteErrorResponse(w, http.StatusBadRequest, "invalid_request", "Only POST method is allowed") + view.RenderError(w, r, http.StatusMethodNotAllowed, "This page can only be accessed through the login flow.") return } @@ -299,3 +300,4 @@ func redirectToLogin(w http.ResponseWriter, r *http.Request, req LoginRequest, l redirectURL := config.GetBootstrap().AppOAuthPath + "/authorize?" + params.Encode() http.Redirect(w, r, redirectURL, http.StatusFound) } + diff --git a/pkg/signup/handler.go b/pkg/signup/handler.go index c3bb379..f13e344 100644 --- a/pkg/signup/handler.go +++ b/pkg/signup/handler.go @@ -36,17 +36,12 @@ import ( // Param state formData string false "OAuth2 state" // Success 302 "Redirect back to client with code" func HandleSignup(w http.ResponseWriter, r *http.Request) { - if !config.Get().AuthAllowSelfSignup { - http.NotFound(w, r) + if !config.Get().AuthAllowSelfSignup || r.Method != http.MethodPost { + view.RenderError(w, r, http.StatusMethodNotAllowed, "This page can only be accessed through the signup flow.") return } - switch r.Method { - case http.MethodPost: - handleSignupPost(w, r) - default: - utils.WriteErrorResponse(w, http.StatusMethodNotAllowed, "invalid_request", "Method not allowed") - } + handleSignupPost(w, r) } func handleSignupPost(w http.ResponseWriter, r *http.Request) { @@ -303,3 +298,4 @@ func redirectSignupError(w http.ResponseWriter, r *http.Request, params SignupPa q.Set("code_challenge_method", params.CodeChallengeMethod) http.Redirect(w, r, config.GetBootstrap().AppOAuthPath+"/authorize?"+q.Encode(), http.StatusFound) } + diff --git a/pkg/signup/handler_test.go b/pkg/signup/handler_test.go index e99bee1..6ea554c 100644 --- a/pkg/signup/handler_test.go +++ b/pkg/signup/handler_test.go @@ -17,7 +17,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestHandleSignup_DisabledReturns404(t *testing.T) { +func TestHandleSignup_DisabledReturns405(t *testing.T) { testutils.WithTestDB(t) // Seed 'onboarded' as true to simulate an already-setup system _ = appsettings.SetSetting("onboarded", "true") @@ -30,7 +30,7 @@ func TestHandleSignup_DisabledReturns404(t *testing.T) { HandleSignup(rr, req) - assert.Equal(t, http.StatusNotFound, rr.Code) + assert.Equal(t, http.StatusMethodNotAllowed, rr.Code) } func TestHandleSignup_WrongMethod(t *testing.T) { @@ -309,7 +309,7 @@ func TestHandleSignupPost_Disabled(t *testing.T) { rr := httptest.NewRecorder() HandleSignup(rr, req) - assert.Equal(t, http.StatusNotFound, rr.Code) + assert.Equal(t, http.StatusMethodNotAllowed, rr.Code) } func TestHandleSignup_Post_RequireEmailVerification_ShowsVerifyPage(t *testing.T) { diff --git a/tests/e2e/signup_test.go b/tests/e2e/signup_test.go index 1ea612a..fe899af 100644 --- a/tests/e2e/signup_test.go +++ b/tests/e2e/signup_test.go @@ -22,7 +22,7 @@ func TestSelfSignup_Disabled(t *testing.T) { require.NoError(t, err) defer func() { _ = resp.Body.Close() }() - assert.Equal(t, http.StatusNotFound, resp.StatusCode) + assert.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode) } func TestSelfSignup_RendersForm(t *testing.T) { diff --git a/view/template.go b/view/template.go index 8d1a630..8981b8d 100644 --- a/view/template.go +++ b/view/template.go @@ -42,6 +42,27 @@ func InjectNonce(r *http.Request, data map[string]any) { data["CspNonce"] = cspnonce.Get(r.Context()) } +// RenderError renders a branded error page using the error template. +// Use this for user-facing errors on browser endpoints that should not return JSON. +func RenderError(w http.ResponseWriter, r *http.Request, status int, errorMsg string) { + cfg := config.Get() + tmpl, err := ParseTemplate("error") + if err != nil { + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } + data := map[string]any{ + "Error": errorMsg, + "ThemeTitle": cfg.Theme.Title, + "ThemeLogoUrl": cfg.Theme.LogoUrl, + } + InjectNonce(r, data) + w.WriteHeader(status) + if err = tmpl.ExecuteTemplate(w, "layout", data); err != nil { + http.Error(w, "Template Execution Error", http.StatusInternalServerError) + } +} + // StaticHandler returns an http.Handler that serves files from view/static/. // Mount it with http.StripPrefix so the handler receives bare file names. func StaticHandler() http.Handler {