From 3cce2158e9500d0ce7429ddcebe2be8bcd593e17 Mon Sep 17 00:00:00 2001 From: Amir Husayn Panahifar Date: Wed, 7 Oct 2026 23:13:22 +0330 Subject: [PATCH] fix(backend): disable one-registration-per-IP by default Shared NAT egress makes client IP unusable for uniqueness checks. Gate the behavior behind REGISTRATION_ENFORCE_IP (default false) for public/invite signup. --- .env.example | 2 ++ .env.prod.example | 2 ++ backend/internal/api/invites.go | 12 ++------ backend/internal/api/invites_test.go | 1 + backend/internal/api/register.go | 35 ++++++++++++++++------- backend/internal/api/register_test.go | 21 ++++++++++++++ backend/internal/config/config.go | 4 +++ backend/internal/store/invites.go | 32 +++++++++++---------- backend/internal/store/store.go | 33 ++++++++++----------- deploy/compose/compose.yaml | 1 + deploy/helm/pargar/templates/backend.yaml | 2 ++ deploy/helm/pargar/values.yaml | 2 ++ docs/configuration.md | 1 + docs/product.md | 2 +- docs/security.md | 6 +++- 15 files changed, 103 insertions(+), 53 deletions(-) diff --git a/.env.example b/.env.example index 5f90d93..cb557eb 100644 --- a/.env.example +++ b/.env.example @@ -23,6 +23,8 @@ FRONTEND_ORIGINS=https://localhost,https://127.0.0.1 # Keep false unless you intentionally want fail-open rate limits / captcha. DEV_MODE=false SEED_CURRICULUM=false +# One registration per client IP. Keep false behind NAT / shared egress (default). +REGISTRATION_ENFORCE_IP=false # METRICS_TOKEN= # TRUSTED_PROXY_CIDRS=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,127.0.0.0/8,::1/128 diff --git a/.env.prod.example b/.env.prod.example index 8b3ffda..71f52c6 100644 --- a/.env.prod.example +++ b/.env.prod.example @@ -23,6 +23,8 @@ FRONTEND_ORIGINS=https://app.example.com DEV_MODE=false SEED_CURRICULUM=false +# Keep false behind NAT until the edge forwards real client IPs. +REGISTRATION_ENFORCE_IP=false METRICS_TOKEN=replace-with-metrics-scrape-token # TRUSTED_PROXY_CIDRS=10.0.0.0/8,172.16.0.0/12,192.168.0.0/16 diff --git a/backend/internal/api/invites.go b/backend/internal/api/invites.go index 2e25d3c..7b5743d 100644 --- a/backend/internal/api/invites.go +++ b/backend/internal/api/invites.go @@ -130,16 +130,8 @@ func (s *Server) handleRegisterInvite(w http.ResponseWriter, r *http.Request) { return } - ip := clientAddress(r) - if ip == "" || ip == "unknown" { - writeErr(w, http.StatusBadRequest, "آدرس شبکه شناسایی نشد") - return - } - if taken, err := s.store.HasRegistrationIP(r.Context(), ip); err != nil { - writeErr(w, http.StatusInternalServerError, "بررسی محدودیت ثبت‌نام ممکن نشد") - return - } else if taken { - writeErr(w, http.StatusConflict, "از این شبکه قبلاً ثبت‌نام انجام شده است") + ip, ok := s.registrationClientIP(w, r) + if !ok { return } diff --git a/backend/internal/api/invites_test.go b/backend/internal/api/invites_test.go index 7cd2f26..866fcb8 100644 --- a/backend/internal/api/invites_test.go +++ b/backend/internal/api/invites_test.go @@ -130,6 +130,7 @@ func TestInviteRevoke(t *testing.T) { } func TestInviteDuplicatePhoneAndIP(t *testing.T) { + t.Setenv("REGISTRATION_ENFORCE_IP", "true") e := setup(t) inv := createInvite(t, e, 5, "", nil) token := inv["token"].(string) diff --git a/backend/internal/api/register.go b/backend/internal/api/register.go index 3e4c480..28817ac 100644 --- a/backend/internal/api/register.go +++ b/backend/internal/api/register.go @@ -27,6 +27,29 @@ func (s *Server) registrationRequireWhitelist(r *http.Request) bool { return s.settingTruthy(r, "registration_require_whitelist", true) } +// registrationClientIP returns the IP to bind to a registration when enforcement is on. +// When REGISTRATION_ENFORCE_IP is false (default), returns "" so NAT / shared egress is ignored. +func (s *Server) registrationClientIP(w http.ResponseWriter, r *http.Request) (string, bool) { + if s.cfg == nil || !s.cfg.RegistrationEnforceIP { + return "", true + } + ip := clientAddress(r) + if ip == "" || ip == "unknown" { + writeErr(w, http.StatusBadRequest, "آدرس شبکه شناسایی نشد") + return "", false + } + taken, err := s.store.HasRegistrationIP(r.Context(), ip) + if err != nil { + writeErr(w, http.StatusInternalServerError, "بررسی محدودیت ثبت‌نام ممکن نشد") + return "", false + } + if taken { + writeErr(w, http.StatusConflict, "از این شبکه قبلاً ثبت‌نام انجام شده است") + return "", false + } + return ip, true +} + func (s *Server) handleRegisterStatus(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, map[string]any{ "enabled": s.registrationEnabled(r), @@ -83,16 +106,8 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) { return } - ip := clientAddress(r) - if ip == "" || ip == "unknown" { - writeErr(w, http.StatusBadRequest, "آدرس شبکه شناسایی نشد") - return - } - if taken, err := s.store.HasRegistrationIP(r.Context(), ip); err != nil { - writeErr(w, http.StatusInternalServerError, "بررسی محدودیت ثبت‌نام ممکن نشد") - return - } else if taken { - writeErr(w, http.StatusConflict, "از این شبکه قبلاً ثبت‌نام انجام شده است") + ip, ok := s.registrationClientIP(w, r) + if !ok { return } diff --git a/backend/internal/api/register_test.go b/backend/internal/api/register_test.go index 0bcb869..4431e36 100644 --- a/backend/internal/api/register_test.go +++ b/backend/internal/api/register_test.go @@ -261,6 +261,7 @@ func TestRegisterDuplicatePhone(t *testing.T) { } func TestRegisterSameIPRejected(t *testing.T) { + t.Setenv("REGISTRATION_ENFORCE_IP", "true") e := setup(t) enableRegistration(t, e) whitelistPhones(t, e, "09121110007", "09121110008") @@ -278,7 +279,27 @@ func TestRegisterSameIPRejected(t *testing.T) { } } +func TestRegisterSameIPAllowedWhenEnforcementOff(t *testing.T) { + t.Setenv("REGISTRATION_ENFORCE_IP", "false") + e := setup(t) + enableRegistration(t, e) + whitelistPhones(t, e, "09121110007", "09121110008") + + headers := map[string]string{"X-Forwarded-For": "203.0.113.40"} + code, body := e.doHeaders(t, "POST", "/api/auth/register", "", headers, + registerBody(t, e, "09121110007", "reg-ipoff1@test.dev", "regipoff1")) + if code != http.StatusCreated { + t.Fatalf("first: %d %v", code, body) + } + code, body = e.doHeaders(t, "POST", "/api/auth/register", "", headers, + registerBody(t, e, "09121110008", "reg-ipoff2@test.dev", "regipoff2")) + if code != http.StatusCreated { + t.Fatalf("same IP must be allowed when REGISTRATION_ENFORCE_IP=false: %d %v", code, body) + } +} + func TestRegisterUsesForwardedIP(t *testing.T) { + t.Setenv("REGISTRATION_ENFORCE_IP", "true") e := setup(t) enableRegistration(t, e) whitelistPhones(t, e, "09121110009", "09121110010", "09121110011") diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index 770e6df..bda853a 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -46,6 +46,9 @@ type Config struct { PhysicalCertEnabled bool PhysicalCertPriceIRR int PhysicalCertWindowDays int + // RegistrationEnforceIP limits one successful registration per client IP. + // Default false: behind NAT / shared egress the peer IP is not a real client identity. + RegistrationEnforceIP bool } func getEnv(key, fallback string) string { @@ -140,6 +143,7 @@ func Load() *Config { PhysicalCertEnabled: getEnv("PHYSICAL_CERT_ENABLED", "false") == "true", PhysicalCertPriceIRR: getEnvInt("PHYSICAL_CERT_PRICE_IRR", 2_500_000), PhysicalCertWindowDays: getEnvInt("PHYSICAL_CERT_WINDOW_DAYS", 30), + RegistrationEnforceIP: getEnv("REGISTRATION_ENFORCE_IP", "false") == "true", } } diff --git a/backend/internal/store/invites.go b/backend/internal/store/invites.go index 23504ff..63746c1 100644 --- a/backend/internal/store/invites.go +++ b/backend/internal/store/invites.go @@ -344,16 +344,14 @@ func EvaluateInviteAvailability(inv *models.RegistrationInvite, now time.Time) e return nil } -// RegisterInviteStudent creates a student, consumes the IP, and atomically consumes one invite seat. +// RegisterInviteStudent creates a student, optionally records the client IP, and consumes one invite seat. +// Empty ip skips one-registration-per-IP enforcement (NAT / shared egress). func (s *Store) RegisterInviteStudent( ctx context.Context, inviteID int64, name, email, username, passwordHash, phone, securityQuestion, securityAnswerHash, ip string, ) (*models.User, error) { ip = strings.TrimSpace(ip) - if ip == "" { - return nil, errors.New("empty registration ip") - } tx, err := s.pool.Begin(ctx) if err != nil { return nil, err @@ -366,12 +364,14 @@ func (s *Store) RegisterInviteStudent( return nil, ErrPhoneBlacklisted } - var taken bool - if err := tx.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM registration_ips WHERE ip=$1)`, ip).Scan(&taken); err != nil { - return nil, err - } - if taken { - return nil, ErrRegistrationIPTaken + if ip != "" { + var taken bool + if err := tx.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM registration_ips WHERE ip=$1)`, ip).Scan(&taken); err != nil { + return nil, err + } + if taken { + return nil, ErrRegistrationIPTaken + } } tag, err := tx.Exec(ctx, ` @@ -436,12 +436,14 @@ func (s *Store) RegisterInviteStudent( ON CONFLICT (user_id) DO NOTHING`, u.ID); err != nil { return nil, err } - if _, err := tx.Exec(ctx, ` - INSERT INTO registration_ips (ip, user_id) VALUES ($1, $2)`, ip, u.ID); err != nil { - if isUniqueViolation(err) { - return nil, ErrRegistrationIPTaken + if ip != "" { + if _, err := tx.Exec(ctx, ` + INSERT INTO registration_ips (ip, user_id) VALUES ($1, $2)`, ip, u.ID); err != nil { + if isUniqueViolation(err) { + return nil, ErrRegistrationIPTaken + } + return nil, err } - return nil, err } if err := tx.Commit(ctx); err != nil { return nil, err diff --git a/backend/internal/store/store.go b/backend/internal/store/store.go index 7161d8b..fa5e28f 100644 --- a/backend/internal/store/store.go +++ b/backend/internal/store/store.go @@ -331,18 +331,15 @@ func (s *Store) HasRegistrationIP(ctx context.Context, ip string) (bool, error) return exists, err } -// RegisterPublicStudent creates a student and consumes the IP atomically. +// RegisterPublicStudent creates a student and optionally records the client IP. // When requireWhitelist is true, the phone must be an available whitelist entry. -// Concurrent same-IP registrations: one commits; the other rolls back on unique IP conflict. +// Empty ip skips one-registration-per-IP enforcement (NAT / shared egress). func (s *Store) RegisterPublicStudent( ctx context.Context, name, email, username, passwordHash, phone, securityQuestion, securityAnswerHash, ip string, requireWhitelist bool, ) (*models.User, error) { ip = strings.TrimSpace(ip) - if ip == "" { - return nil, errors.New("empty registration ip") - } tx, err := s.pool.Begin(ctx) if err != nil { return nil, err @@ -355,12 +352,14 @@ func (s *Store) RegisterPublicStudent( return nil, ErrPhoneBlacklisted } - var taken bool - if err := tx.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM registration_ips WHERE ip=$1)`, ip).Scan(&taken); err != nil { - return nil, err - } - if taken { - return nil, ErrRegistrationIPTaken + if ip != "" { + var taken bool + if err := tx.QueryRow(ctx, `SELECT EXISTS(SELECT 1 FROM registration_ips WHERE ip=$1)`, ip).Scan(&taken); err != nil { + return nil, err + } + if taken { + return nil, ErrRegistrationIPTaken + } } row := tx.QueryRow(ctx, ` @@ -380,12 +379,14 @@ func (s *Store) RegisterPublicStudent( ON CONFLICT (user_id) DO NOTHING`, u.ID); err != nil { return nil, err } - if _, err := tx.Exec(ctx, ` - INSERT INTO registration_ips (ip, user_id) VALUES ($1, $2)`, ip, u.ID); err != nil { - if isUniqueViolation(err) { - return nil, ErrRegistrationIPTaken + if ip != "" { + if _, err := tx.Exec(ctx, ` + INSERT INTO registration_ips (ip, user_id) VALUES ($1, $2)`, ip, u.ID); err != nil { + if isUniqueViolation(err) { + return nil, ErrRegistrationIPTaken + } + return nil, err } - return nil, err } if err := tx.Commit(ctx); err != nil { return nil, err diff --git a/deploy/compose/compose.yaml b/deploy/compose/compose.yaml index 8a1ac32..d1a191a 100644 --- a/deploy/compose/compose.yaml +++ b/deploy/compose/compose.yaml @@ -23,6 +23,7 @@ x-backend-env: &backend-env STORAGE_SECRET_KEY: ${RUSTFS_SECRET_KEY:?RUSTFS_SECRET_KEY must be set} STORAGE_BUCKET: ${STORAGE_BUCKET:-pargar} SEED_CURRICULUM: ${SEED_CURRICULUM:-false} + REGISTRATION_ENFORCE_IP: ${REGISTRATION_ENFORCE_IP:-false} ADMIN_NAME: ${ADMIN_NAME:-Admin} ADMIN_USERNAME: ${ADMIN_USERNAME:-admin} ADMIN_EMAIL: ${ADMIN_EMAIL:-admin@example.com} diff --git a/deploy/helm/pargar/templates/backend.yaml b/deploy/helm/pargar/templates/backend.yaml index 7e1e770..44f5c74 100644 --- a/deploy/helm/pargar/templates/backend.yaml +++ b/deploy/helm/pargar/templates/backend.yaml @@ -66,6 +66,8 @@ spec: value: {{ .Values.backend.env.devMode | quote }} - name: SEED_CURRICULUM value: {{ .Values.backend.env.seedCurriculum | quote }} + - name: REGISTRATION_ENFORCE_IP + value: {{ .Values.backend.env.registrationEnforceIP | quote }} - name: ADMIN_NAME value: {{ .Values.backend.admin.name | quote }} - name: ADMIN_USERNAME diff --git a/deploy/helm/pargar/values.yaml b/deploy/helm/pargar/values.yaml index fb300a9..1a6299c 100644 --- a/deploy/helm/pargar/values.yaml +++ b/deploy/helm/pargar/values.yaml @@ -44,6 +44,8 @@ backend: env: devMode: "false" seedCurriculum: "false" + # false behind NAT / shared egress (peer IP is not a real client). + registrationEnforceIP: "false" donationEnabled: "true" # Offline payment only — leave off until ops process is ready; enable in admin settings. physicalCertEnabled: "false" diff --git a/docs/configuration.md b/docs/configuration.md index c0c5a1b..afc9240 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -63,6 +63,7 @@ One of two patterns: |----------|-----------------|--------| | `DEV_MODE` | false | Open metrics without token, limited fail-open behavior | | `SEED_CURRICULUM` | false (empty in DevMode → true) | Seed lessons/events | +| `REGISTRATION_ENFORCE_IP` | false | One registration per client IP; keep off behind NAT | | `CAPTCHA_EXPOSE_ANSWER` | DevMode only | For tests; never in production | | `DONATION_ENABLED` | true | Support card | | `DONATION_NOTE` | Copy text | | diff --git a/docs/product.md b/docs/product.md index 173ba37..b13990c 100644 --- a/docs/product.md +++ b/docs/product.md @@ -28,7 +28,7 @@ - Whitelist and blacklist are **mutually exclusive**; import resolves conflicts. - A phone already tied to an account cannot re-enter either list as a fresh entry. -- One account per phone and per IP on registration paths. +- One account per phone. Optional one-account-per-IP (`REGISTRATION_ENFORCE_IP`, default off — unsuitable behind NAT). - Blacklist blocks all registration paths (public + invite) and keeps a failed-attempt history tab. ## Certificates diff --git a/docs/security.md b/docs/security.md index 269d51b..a26c40b 100644 --- a/docs/security.md +++ b/docs/security.md @@ -25,6 +25,10 @@ Public password recovery (`/api/auth/recovery/*` and `/recovery`) is disabled fo - Only peers in `TRUSTED_PROXY_CIDRS` are trusted for `X-Forwarded-*` / `X-Real-IP`. - CORS: `FRONTEND_ORIGINS` + `PUBLIC_URL` with `AllowCredentials: true`. +## Registration IP binding + +`REGISTRATION_ENFORCE_IP` (default **false**) optionally limits one successful public/invite registration per client IP. Leave it off when the app sits behind NAT or shared egress and cannot see real client addresses. Enable only after the edge forwards trustworthy `X-Forwarded-For` / `X-Real-IP` via `TRUSTED_PROXY_CIDRS`. + ## Rate limiting | Scope | Approximate behavior | @@ -34,7 +38,7 @@ Public password recovery (`/api/auth/recovery/*` and `/recovery`) is disabled fo | External chat export | ~10/min/IP | | Per-user uploads | Separately limited | -`/api/health` is excluded from the global ceiling. +`/api/health` is excluded from the global ceiling. Note: per-IP rate limits still use the peer/forwarded address — configure trusted proxies when you need per-client fairness behind NAT. ## Uploads -- 2.47.3