From 75cc6737bd542157ce34f16abe011d2dc35051c4 Mon Sep 17 00:00:00 2001 From: chan Date: Fri, 27 Mar 2026 11:19:28 +0900 Subject: [PATCH] feat: add robust login ID collision prevention and UI validation (#440) - Add `ValidateLoginID` to enforce ID collision and security rules (prevents phone number collision, email format usage, and reserved words). - Add `POST /api/v1/auth/signup/check-login-id` endpoint for real-time ID availability checks. - Add `checkLoginIDAvailability` API call to userfront's `AuthProxyService`. - Implement "Check Duplication" button and error/success messaging for the Login ID field in the signup screen. - Add "000000" magic code bypass for `VerifySignupCode` in non-production environments to streamline testing. --- backend/cmd/server/main.go | 1 + backend/internal/domain/auth_models.go | 5 ++ backend/internal/domain/user.go | 61 ++++++++++++++++++ backend/internal/domain/user_validate_test.go | 40 ++++++++++++ backend/internal/handler/auth_handler.go | 48 +++++++++++++- .../internal/handler/auth_handler_otp_test.go | 2 +- .../handler/auth_handler_signup_test.go | 1 + backend/internal/handler/user_handler.go | 29 +++++++-- .../lib/core/services/auth_proxy_service.dart | 21 +++++++ .../auth/presentation/signup_screen.dart | 63 +++++++++++++++++-- 10 files changed, 257 insertions(+), 14 deletions(-) create mode 100644 backend/internal/domain/user_validate_test.go diff --git a/backend/cmd/server/main.go b/backend/cmd/server/main.go index bc5da0e3..83f976f0 100644 --- a/backend/cmd/server/main.go +++ b/backend/cmd/server/main.go @@ -549,6 +549,7 @@ func main() { // Signup Routes signup := auth.Group("/signup") signup.Post("/check-email", authHandler.CheckEmail) + signup.Post("/check-login-id", authHandler.CheckLoginID) signup.Post("/send-email-code", authHandler.SendSignupEmailCode) signup.Post("/send-sms-code", authHandler.SendSignupSmsCode) signup.Post("/verify-code", authHandler.VerifySignupCode) diff --git a/backend/internal/domain/auth_models.go b/backend/internal/domain/auth_models.go index 5358c738..002ff21a 100644 --- a/backend/internal/domain/auth_models.go +++ b/backend/internal/domain/auth_models.go @@ -112,3 +112,8 @@ type PasswordChangeRequest struct { CurrentPassword string `json:"currentPassword"` NewPassword string `json:"newPassword"` } + +type CheckLoginIDRequest struct { + LoginID string `json:"loginId"` + CompanyCode string `json:"companyCode,omitempty"` +} diff --git a/backend/internal/domain/user.go b/backend/internal/domain/user.go index 411f3aa7..771d7529 100644 --- a/backend/internal/domain/user.go +++ b/backend/internal/domain/user.go @@ -1,6 +1,7 @@ package domain import ( + "fmt" "strings" "time" @@ -61,3 +62,63 @@ func (u *User) BeforeCreate(tx *gorm.DB) (err error) { } return } + +// ValidateLoginID checks if the loginID violates any collision, length, or security rules. +func ValidateLoginID(loginID, email, phone string) error { + loginID = strings.TrimSpace(loginID) + if loginID == "" { + return nil + } + + if len(loginID) < 4 || len(loginID) > 30 { + return fmt.Errorf("ID must be between 4 and 30 characters") + } + + if strings.Contains(loginID, "@") { + return fmt.Errorf("ID cannot be an email format") + } + + if email != "" && strings.EqualFold(loginID, email) { + return fmt.Errorf("ID cannot be the same as the email address") + } + + if phone != "" { + normalizedPhone := strings.ReplaceAll(phone, "-", "") + normalizedPhone = strings.ReplaceAll(normalizedPhone, " ", "") + if strings.HasPrefix(normalizedPhone, "010") { + normalizedPhone = "+82" + normalizedPhone[1:] + } else if strings.HasPrefix(normalizedPhone, "82") { + normalizedPhone = "+" + normalizedPhone + } + + if loginID == phone || loginID == normalizedPhone { + return fmt.Errorf("ID cannot be the same as the phone number") + } + } + + isPureNumber := true + loginIDDigits := strings.ReplaceAll(loginID, "-", "") + loginIDDigits = strings.ReplaceAll(loginIDDigits, " ", "") + for _, c := range loginIDDigits { + if (c < '0' || c > '9') && c != '+' { + isPureNumber = false + break + } + } + + if isPureNumber && len(loginIDDigits) >= 10 && len(loginIDDigits) <= 12 { + if strings.HasPrefix(loginIDDigits, "010") || strings.HasPrefix(loginIDDigits, "82") || strings.HasPrefix(loginIDDigits, "+82") { + return fmt.Errorf("ID cannot be a phone number format") + } + } + + reserved := []string{"admin", "system", "root", "master", "superuser", "guest", "operator"} + lowerID := strings.ToLower(loginID) + for _, r := range reserved { + if lowerID == r { + return fmt.Errorf("reserved ID cannot be used") + } + } + + return nil +} diff --git a/backend/internal/domain/user_validate_test.go b/backend/internal/domain/user_validate_test.go new file mode 100644 index 00000000..9bf977d0 --- /dev/null +++ b/backend/internal/domain/user_validate_test.go @@ -0,0 +1,40 @@ +package domain + +import ( + "testing" +) + +func TestValidateLoginID(t *testing.T) { + tests := []struct { + name string + loginID string + email string + phone string + wantErr bool + }{ + {"Empty", "", "test@email.com", "01012345678", false}, + {"Valid alphanumeric", "user123", "test@email.com", "01012345678", false}, + {"Too short", "us", "test@email.com", "01012345678", true}, + {"Too long", "thisisaverylongloginidthatiswayoverthirtycharacters", "test@email.com", "01012345678", true}, + {"Email format", "user@domain.com", "test@email.com", "01012345678", true}, + {"Exact email match", "Test@Email.Com", "test@email.com", "01012345678", true}, + {"Phone number match", "010-1234-5678", "test@email.com", "01012345678", true}, + {"Phone number match +82", "+821012345678", "test@email.com", "01012345678", true}, + {"Phone number match digits", "01012345678", "test@email.com", "01012345678", true}, + {"Phone format (11 digits)", "01098765432", "test@email.com", "01012345678", true}, + {"Valid pure digits (employee ID)", "20230001", "test@email.com", "01012345678", false}, + {"Valid pure digits long", "123456789", "test@email.com", "01012345678", false}, + {"Valid pure digits 10 chars", "1234567890", "test@email.com", "01012345678", false}, + {"Reserved word admin", "ADMIN", "test@email.com", "01012345678", true}, + {"Reserved word root", "root", "test@email.com", "01012345678", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateLoginID(tt.loginID, tt.email, tt.phone) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateLoginID() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/backend/internal/handler/auth_handler.go b/backend/internal/handler/auth_handler.go index d5634dd5..6a79a336 100644 --- a/backend/internal/handler/auth_handler.go +++ b/backend/internal/handler/auth_handler.go @@ -194,6 +194,35 @@ func (h *AuthHandler) CheckEmail(c *fiber.Ctx) error { return c.JSON(fiber.Map{"available": true}) } +// CheckLoginID - 로그인 ID 사용 가능 여부를 확인합니다. +func (h *AuthHandler) CheckLoginID(c *fiber.Ctx) error { + var req domain.CheckLoginIDRequest + if err := c.BodyParser(&req); err != nil { + return errorJSON(c, fiber.StatusBadRequest, "Invalid request") + } + + if h.IdpProvider == nil { + return errorJSON(c, fiber.StatusServiceUnavailable, "Identity provider unavailable") + } + + // Basic validation via our ValidateLoginID helper (without email/phone since we just check format & collision with reserved words) + if err := domain.ValidateLoginID(req.LoginID, "", ""); err != nil { + return c.JSON(fiber.Map{"available": false, "message": err.Error()}) + } + + // We don't prepend companyCode to Kratos lookup if traits.id is unique globally + // Assuming Kratos traits.id handles unique constraints per tenant or globally based on schema + exists, err := h.IdpProvider.UserExists(req.LoginID) + if err != nil { + return errorJSON(c, fiber.StatusServiceUnavailable, "Identity provider unavailable") + } + + if exists { + return c.JSON(fiber.Map{"available": false, "message": "ID already registered"}) + } + return c.JSON(fiber.Map{"available": true}) +} + // SendSignupEmailCode - Sends verification code to email func (h *AuthHandler) SendSignupEmailCode(c *fiber.Ctx) error { var req domain.SendSignupCodeRequest @@ -329,8 +358,9 @@ func (h *AuthHandler) VerifySignupCode(c *fiber.Ctx) error { return errorJSON(c, fiber.StatusTooManyRequests, "Too many failed attempts") } - // Check Code match - if state.Code != req.Code { + // Check Code match (Allow magic code 000000 in non-production environments) + isMagicCodeAllowed := service.IsDryRunAllowed() && req.Code == "000000" + if state.Code != req.Code && !isMagicCodeAllowed { state.FailCount++ h.saveSignupState(key, state, signupStateExpiration) return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{ @@ -465,9 +495,14 @@ func (h *AuthHandler) Signup(c *fiber.Ctx) error { } } + finalLoginID := extractTraitString(attributes, "id") + if err := domain.ValidateLoginID(finalLoginID, req.Email, normalizedPhone); err != nil { + return errorJSON(c, fiber.StatusBadRequest, err.Error()) + } + brokerUser := &domain.BrokerUser{ Email: req.Email, - LoginID: extractTraitString(attributes, "id"), + LoginID: finalLoginID, Name: req.Name, PhoneNumber: normalizedPhone, Attributes: attributes, @@ -5315,6 +5350,13 @@ func (h *AuthHandler) UpdateMe(c *fiber.Ctx) error { } } + finalLoginID := extractTraitString(traits, "id") + userEmail := extractTraitString(traits, "email") + userPhone := extractTraitString(traits, "phone") + if err := domain.ValidateLoginID(finalLoginID, userEmail, userPhone); err != nil { + return errorJSON(c, fiber.StatusBadRequest, err.Error()) + } + if err := h.updateKratosIdentity(identityID, traits); err != nil { slog.Error("Failed to update profile in Kratos", "error", err) return errorJSON(c, fiber.StatusInternalServerError, "프로필 업데이트에 실패했습니다.") diff --git a/backend/internal/handler/auth_handler_otp_test.go b/backend/internal/handler/auth_handler_otp_test.go index 09f6a191..7a5f8967 100644 --- a/backend/internal/handler/auth_handler_otp_test.go +++ b/backend/internal/handler/auth_handler_otp_test.go @@ -99,7 +99,7 @@ func TestVerifySignupCode_Invalid(t *testing.T) { verifyBody := map[string]string{ "type": "email", "target": "user@test.com", - "code": "000000", // wrong code + "code": "222222", // wrong code } body, _ := json.Marshal(verifyBody) req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/signup/verify", bytes.NewReader(body)) diff --git a/backend/internal/handler/auth_handler_signup_test.go b/backend/internal/handler/auth_handler_signup_test.go index b8c92275..bbd3367a 100644 --- a/backend/internal/handler/auth_handler_signup_test.go +++ b/backend/internal/handler/auth_handler_signup_test.go @@ -136,6 +136,7 @@ func TestSignup_CompanyCodeValidation(t *testing.T) { validTenant := &domain.Tenant{ID: "t1", Slug: "valid-slug", Status: domain.TenantStatusActive} mockTenantSvc.On("GetTenantByDomain", mock.Anything, "gmail.com").Return(nil, nil) mockTenantSvc.On("GetTenantBySlug", mock.Anything, "valid-slug").Return(validTenant, nil) + mockTenantSvc.On("GetTenant", mock.Anything, "t1").Return(validTenant, nil) mockIdp.On("CreateUser", mock.Anything, mock.Anything).Return("user-id", nil) mockRedis.On("Delete", mock.Anything).Return(nil) diff --git a/backend/internal/handler/user_handler.go b/backend/internal/handler/user_handler.go index 224f8ad0..d0c21069 100644 --- a/backend/internal/handler/user_handler.go +++ b/backend/internal/handler/user_handler.go @@ -352,9 +352,14 @@ func (h *UserHandler) CreateUser(c *fiber.Ctx) error { } } + finalLoginID := extractTraitString(attributes, "id") + if err := domain.ValidateLoginID(finalLoginID, email, normalizePhoneNumber(req.Phone)); err != nil { + return errorJSON(c, fiber.StatusBadRequest, err.Error()) + } + brokerUser := &domain.BrokerUser{ Email: email, - LoginID: extractTraitString(attributes, "id"), + LoginID: finalLoginID, Name: name, PhoneNumber: normalizePhoneNumber(req.Phone), Attributes: attributes, @@ -571,11 +576,20 @@ func (h *UserHandler) BulkCreateUsers(c *fiber.Ctx) error { } } + finalLoginID := extractTraitString(attributes, "id") + userEmail := email + userPhone := normalizePhoneNumber(item.Phone) + + if err := domain.ValidateLoginID(finalLoginID, userEmail, userPhone); err != nil { + results = append(results, bulkUserResult{Email: email, Success: false, Message: err.Error()}) + continue + } + identityID, err := h.OryProvider.CreateUser(&domain.BrokerUser{ - Email: email, - LoginID: extractTraitString(attributes, "id"), + Email: userEmail, + LoginID: finalLoginID, Name: item.Name, - PhoneNumber: normalizePhoneNumber(item.Phone), + PhoneNumber: userPhone, Attributes: attributes, }, password) if err != nil { @@ -1189,6 +1203,13 @@ func (h *UserHandler) UpdateUser(c *fiber.Ctx) error { } } + finalLoginID := extractTraitString(traits, "id") + userEmail := extractTraitString(traits, "email") + userPhone := extractTraitString(traits, "phone") + if err := domain.ValidateLoginID(finalLoginID, userEmail, userPhone); err != nil { + return errorJSON(c, fiber.StatusBadRequest, err.Error()) + } + state := normalizeKratosState(req.Status) slog.Info("[UpdateUser] Calling Kratos UpdateIdentity", "userID", userID, "traits", traits, "state", state) diff --git a/userfront/lib/core/services/auth_proxy_service.dart b/userfront/lib/core/services/auth_proxy_service.dart index 2171de51..534813b2 100644 --- a/userfront/lib/core/services/auth_proxy_service.dart +++ b/userfront/lib/core/services/auth_proxy_service.dart @@ -893,6 +893,27 @@ class AuthProxyService { return false; } + static Future> checkLoginIDAvailability(String loginId, {String? companyCode}) async { + final url = Uri.parse('$_baseUrl/api/v1/auth/signup/check-login-id'); + final bodyData = {'loginId': loginId}; + if (companyCode != null && companyCode.isNotEmpty) { + bodyData['companyCode'] = companyCode; + } + final response = await http.post( + url, + headers: {'Content-Type': 'application/json'}, + body: jsonEncode(bodyData), + ); + + if (response.statusCode == 200) { + final data = jsonDecode(response.body); + return {'available': data['available'] ?? false, 'message': data['message']}; + } else { + final data = jsonDecode(response.body); + return {'available': false, 'message': data['message'] ?? 'Failed to check ID'}; + } + } + static Future sendSignupCode(String target, String type) async { final path = type == 'email' ? 'send-email-code' : 'send-sms-code'; final url = Uri.parse('$_baseUrl/api/v1/auth/signup/$path'); diff --git a/userfront/lib/features/auth/presentation/signup_screen.dart b/userfront/lib/features/auth/presentation/signup_screen.dart index 57b5b5da..330a8f12 100644 --- a/userfront/lib/features/auth/presentation/signup_screen.dart +++ b/userfront/lib/features/auth/presentation/signup_screen.dart @@ -59,6 +59,8 @@ class _SignupScreenState extends State { String? _phoneError; String? _passwordError; String? _confirmPasswordError; + String? _loginIdError; + String? _loginIdSuccess; // Timers Timer? _emailTimer; @@ -1428,12 +1430,61 @@ class _SignupScreenState extends State { title: '로그인 ID (선택)', description: '이메일/전화번호 외에 별도의 식별자로 로그인할 때 사용합니다.', isDesktop: isDesktop, - child: TextFormField( - controller: _loginIdController, - decoration: const InputDecoration( - labelText: '사번 또는 아이디', - border: OutlineInputBorder(), - ), + child: Column( + crossAxisAlignment: CrossAxisAlignment.stretch, + children: [ + TextFormField( + controller: _loginIdController, + onChanged: (val) { + setState(() { + _loginIdError = null; + _loginIdSuccess = null; + }); + }, + decoration: InputDecoration( + labelText: '사번 또는 아이디', + border: const OutlineInputBorder(), + errorText: _loginIdError, + suffixIcon: TextButton( + onPressed: _isLoading ? null : () async { + final loginId = _loginIdController.text.trim(); + if (loginId.isEmpty) { + setState(() => _loginIdError = 'ID를 입력해주세요.'); + return; + } + setState(() { + _isLoading = true; + _loginIdError = null; + _loginIdSuccess = null; + }); + try { + final result = await AuthProxyService.checkLoginIDAvailability(loginId, companyCode: _affiliationType == 'AFFILIATE' ? _companyCode : null); + setState(() { + if (result['available'] == true) { + _loginIdSuccess = '사용 가능한 ID입니다.'; + } else { + _loginIdError = result['message'] ?? '사용할 수 없는 ID입니다.'; + } + }); + } catch (e) { + setState(() => _loginIdError = e.toString().replaceAll('Exception: ', '')); + } finally { + if (mounted) setState(() => _isLoading = false); + } + }, + child: const Text('중복 확인'), + ), + ), + ), + if (_loginIdSuccess != null) + Padding( + padding: const EdgeInsets.only(top: 8.0, left: 12.0), + child: Text( + _loginIdSuccess!, + style: const TextStyle(color: Colors.green, fontSize: 12), + ), + ), + ], ), ), const SizedBox(height: 18),