chore: refactor signup logic (#10915)

This PR is in preparation of, but independent of, an upcoming suggestion for a feature addition:

* The first commit moves a tiny bit of logic into a separate function to prepare for extension of that logic, avoiding duplication
* The second commit moves checking for disabled registrations earlier, which, I think, has merits in terms of performance and resilience (hopefully not significant, but who knows?)
* The third commit adds simple unit tests for SignUp() and SignUpPost() to avoid the long-ish roundtrip over integration tests
*  The forth commit introduces `ctx.Data["DisableRegistrationReason"]` for the signup template to use as the reason printed if `.DisableRegistration` to prepare for other reasons to be added

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10915
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Nils Goroll <nils.goroll@uplex.de>
Co-committed-by: Nils Goroll <nils.goroll@uplex.de>

routers/web/auth/auth.go

@@ -386,6 +386,16 @@ func SignOut(ctx *context.Context) {
 	ctx.JSONRedirect(setting.AppSubURL + "/")
 }
 
+// check if registration is allowed and set Data for template
+func registrationDisabled(ctx *context.Context) bool {
+	if setting.Service.DisableRegistration || setting.Service.AllowOnlyExternalRegistration {
+		ctx.Data["DisableRegistration"] = true
+		ctx.Data["DisableRegistrationReason"] = ctx.Locale.Tr("auth.disable_register_prompt")
+		return true
+	}
+	return false
+}
+
 // SignUp render the register page
 func SignUp(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("sign_up")
@@ -403,8 +413,7 @@ func SignUp(ctx *context.Context) {
 
 	ctx.Data["PageIsSignUp"] = true
 
-	// Show Disabled Registration message if DisableRegistration or AllowOnlyExternalRegistration options are true
-	ctx.Data["DisableRegistration"] = setting.Service.DisableRegistration || setting.Service.AllowOnlyExternalRegistration
+	registrationDisabled(ctx)
 
 	redirectTo := ctx.FormString("redirect_to")
 	if len(redirectTo) > 0 {
@@ -416,6 +425,11 @@ func SignUp(ctx *context.Context) {
 
 // SignUpPost response for sign up information submission
 func SignUpPost(ctx *context.Context) {
+	if registrationDisabled(ctx) {
+		ctx.Error(http.StatusForbidden)
+		return
+	}
+
 	form := web.GetForm(ctx).(*forms.RegisterForm)
 	ctx.Data["Title"] = ctx.Tr("sign_up")
 
@@ -432,12 +446,6 @@ func SignUpPost(ctx *context.Context) {
 
 	ctx.Data["PageIsSignUp"] = true
 
-	// Permission denied if DisableRegistration or AllowOnlyExternalRegistration options are true
-	if setting.Service.DisableRegistration || setting.Service.AllowOnlyExternalRegistration {
-		ctx.Error(http.StatusForbidden)
-		return
-	}
-
 	if ctx.HasError() {
 		ctx.HTML(http.StatusOK, tplSignUp)
 		return

routers/web/auth/auth_test.go

@@ -8,6 +8,8 @@ import (
 	"net/url"
 	"testing"
 
+	"forgejo.org/modules/setting"
+	"forgejo.org/modules/templates"
 	"forgejo.org/modules/test"
 	"forgejo.org/services/contexttest"
 
@@ -41,3 +43,29 @@ func TestUserLogin(t *testing.T) {
 	SignIn(ctx)
 	assert.Equal(t, "/", test.RedirectURL(resp))
 }
+
+// NB: Full signup test is in tests/integration/signup_test.go
+// this is to test disabled signup
+func TestSignUpDefault(t *testing.T) {
+	ctx, resp := contexttest.MockContext(t, "/user/sign_up",
+		contexttest.MockContextOption{Render: templates.HTMLRenderer()})
+	SignUp(ctx)
+	assert.Equal(t, http.StatusOK, resp.Code)
+	assert.Contains(t, resp.Body.String(), ctx.Locale.Tr("username"))
+}
+
+func TestSignUpDisabled(t *testing.T) {
+	ctx, resp := contexttest.MockContext(t, "/user/sign_up",
+		contexttest.MockContextOption{Render: templates.HTMLRenderer()})
+	defer test.MockVariableValue(&setting.Service.DisableRegistration, true)()
+	SignUp(ctx)
+	assert.Equal(t, http.StatusOK, resp.Code)
+	assert.Contains(t, resp.Body.String(), ctx.Locale.Tr("auth.disable_register_prompt"))
+}
+
+func TestSignUpPostDisabled(t *testing.T) {
+	ctx, resp := contexttest.MockContext(t, "/user/sign_up")
+	defer test.MockVariableValue(&setting.Service.DisableRegistration, true)()
+	SignUpPost(ctx)
+	assert.Equal(t, http.StatusForbidden, resp.Code)
+}

templates/user/auth/signup_inner.tmpl

@@ -12,7 +12,7 @@
 			{{template "base/alert" .}}
 			{{end}}
 			{{if .DisableRegistration}}
-				<p>{{ctx.Locale.Tr "auth.disable_register_prompt"}}</p>
+				<p>{{.DisableRegistrationReason}}</p>
 			{{else}}
 				<div class="required field {{if and (.Err_UserName) (or (not .LinkAccountMode) (and .LinkAccountMode .LinkAccountModeRegister))}}error{{end}}">
 					<label for="user_name">{{ctx.Locale.Tr "username"}}</label>