fix(backend): improve LoginID synchronization from custom metadata fields

- Centralize LoginID sync logic in syncLoginID helper - Support namespaced metadata in CreateUser, UpdateUser, and BulkCreateUsers - Ensure UpdateUser and UpdateMe always sync LoginID from configured field even if not in update request - Add phone number normalization consistency for custom LoginIDs - Add unit tests for namespaced metadata LoginID sync
2026-03-26 12:46:33 +09:00
parent 6337d975ea
commit 85b2049a61
3 changed files with 289 additions and 63 deletions
--- a/backend/internal/handler/user_handler_test.go
+++ b/backend/internal/handler/user_handler_test.go
@@ -87,6 +87,14 @@ func (m *MockTenantServiceForUser) GetTenantBySlug(ctx context.Context, slug str
 	return args.Get(0).(*domain.Tenant), args.Error(1)
 }

+func (m *MockTenantServiceForUser) ListManageableTenants(ctx context.Context, userID string) ([]domain.Tenant, error) {
+	args := m.Called(ctx, userID)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).([]domain.Tenant), args.Error(1)
+}
+
 // --- Tests ---

 func TestUserHandler_BulkCreateUsers(t *testing.T) {
@@ -353,3 +361,194 @@ func TestUserHandler_UpdateUser_AdminOnlyField(t *testing.T) {
 		assert.Contains(t, result["error"].(string), "field salary is admin only")
 	})
 }
+
+func TestUserHandler_UpdateUser_LoginIDSync(t *testing.T) {
+	t.Run("Success - Sync LoginID from namespaced metadata", func(t *testing.T) {
+		app := fiber.New()
+		mockKratos := new(MockKratosAdmin)
+		mockTenant := new(MockTenantServiceForUser)
+		h := &UserHandler{
+			KratosAdmin:   mockKratos,
+			TenantService: mockTenant,
+		}
+		app.Put("/users/:id", func(c *fiber.Ctx) error {
+			c.Locals("user_profile", &domain.UserProfileResponse{Role: domain.RoleSuperAdmin})
+			return h.UpdateUser(c)
+		})
+
+		tenantID := "t-123"
+		userID := "u-1"
+		mockKratos.On("GetIdentity", mock.Anything, userID).Return(&service.KratosIdentity{
+			ID: userID,
+			Traits: map[string]interface{}{
+				"email":       "user@test.com",
+				"companyCode": "test-tenant",
+				"tenant_id":   tenantID,
+			},
+		}, nil).Once()
+
+		mockTenant.On("GetTenantBySlug", mock.Anything, "test-tenant").Return(&domain.Tenant{
+			ID:   tenantID,
+			Slug: "test-tenant",
+			Config: domain.JSONMap{
+				"loginIdField": "emp_no",
+				"userSchema": []interface{}{
+					map[string]interface{}{"key": "emp_no", "label": "Employee No"},
+				},
+			},
+		}, nil) // Allow multiple calls for validation and sync
+
+		mockTenant.On("ListManageableTenants", mock.Anything, userID).Return([]domain.Tenant{}, nil).Once()
+
+		// Expect traits to include 'id' synced from 'emp_no'
+		mockKratos.On("UpdateIdentity", mock.Anything, userID, mock.MatchedBy(func(traits map[string]interface{}) bool {
+			return traits["id"] == "E1001"
+		}), mock.Anything).Return(&service.KratosIdentity{
+			ID: userID,
+			Traits: map[string]interface{}{
+				"id":    "E1001",
+				"email": "user@test.com",
+			},
+		}, nil).Once()
+
+		payload := map[string]interface{}{
+			"metadata": map[string]interface{}{
+				tenantID: map[string]interface{}{
+					"emp_no": "E1001",
+				},
+			},
+		}
+		body, _ := json.Marshal(payload)
+		req := httptest.NewRequest("PUT", "/users/"+userID, bytes.NewReader(body))
+		req.Header.Set("Content-Type", "application/json")
+
+		resp, _ := app.Test(req)
+		assert.Equal(t, 200, resp.StatusCode)
+		mockKratos.AssertExpectations(t)
+	})
+
+	t.Run("Success - Sync LoginID from existing traits when not in metadata", func(t *testing.T) {
+		app := fiber.New()
+		mockKratos := new(MockKratosAdmin)
+		mockTenant := new(MockTenantServiceForUser)
+		h := &UserHandler{
+			KratosAdmin:   mockKratos,
+			TenantService: mockTenant,
+		}
+		app.Put("/users/:id", func(c *fiber.Ctx) error {
+			c.Locals("user_profile", &domain.UserProfileResponse{Role: domain.RoleSuperAdmin})
+			return h.UpdateUser(c)
+		})
+
+		tenantID := "t-123"
+		userID := "u-2"
+		mockKratos.On("GetIdentity", mock.Anything, userID).Return(&service.KratosIdentity{
+			ID: userID,
+			Traits: map[string]interface{}{
+				"email":       "user2@test.com",
+				"companyCode": "test-tenant",
+				"tenant_id":   tenantID,
+				"id":          "old-id",
+				tenantID: map[string]interface{}{
+					"emp_no": "E2002",
+				},
+			},
+		}, nil).Once()
+
+		mockTenant.On("GetTenantBySlug", mock.Anything, "test-tenant").Return(&domain.Tenant{
+			ID:   tenantID,
+			Slug: "test-tenant",
+			Config: domain.JSONMap{
+				"loginIdField": "emp_no",
+			},
+		}, nil)
+
+		mockTenant.On("ListManageableTenants", mock.Anything, userID).Return([]domain.Tenant{}, nil).Once()
+
+		// Even if metadata is empty, it should sync from existing traits
+		mockKratos.On("UpdateIdentity", mock.Anything, userID, mock.MatchedBy(func(traits map[string]interface{}) bool {
+			return traits["id"] == "E2002"
+		}), mock.Anything).Return(&service.KratosIdentity{
+			ID: userID,
+			Traits: map[string]interface{}{
+				"id": "E2002",
+			},
+		}, nil).Once()
+
+		payload := map[string]interface{}{
+			"name": "New Name",
+		}
+		body, _ := json.Marshal(payload)
+		req := httptest.NewRequest("PUT", "/users/"+userID, bytes.NewReader(body))
+		req.Header.Set("Content-Type", "application/json")
+
+		resp, _ := app.Test(req)
+		assert.Equal(t, 200, resp.StatusCode)
+		mockKratos.AssertExpectations(t)
+	})
+}
+
+func TestUserHandler_CreateUser_LoginIDSync(t *testing.T) {
+	t.Run("Success - Sync LoginID from namespaced metadata", func(t *testing.T) {
+		app := fiber.New()
+		mockKratos := new(MockKratosAdmin)
+		mockOry := new(MockOryProvider)
+		mockTenant := new(MockTenantServiceForUser)
+		h := &UserHandler{
+			KratosAdmin:   mockKratos,
+			OryProvider:   mockOry,
+			TenantService: mockTenant,
+		}
+		app.Post("/users", h.CreateUser)
+
+		tenantID := "t-123"
+		mockTenant.On("GetTenantBySlug", mock.Anything, "test-tenant").Return(&domain.Tenant{
+			ID:   tenantID,
+			Slug: "test-tenant",
+			Config: domain.JSONMap{
+				"loginIdField": "emp_no",
+				"userSchema": []interface{}{
+					map[string]interface{}{"key": "emp_no", "label": "Employee No"},
+				},
+			},
+		}, nil)
+
+		mockOry.On("GetPasswordPolicy").Return(&domain.PasswordPolicy{MinLength: 8}, nil)
+
+		// Expect OryProvider.CreateUser to be called with attributes["id"] synced from metadata
+		mockOry.On("CreateUser", mock.MatchedBy(func(user *domain.BrokerUser) bool {
+			return user.LoginID == "E1001" && user.Attributes["id"] == "E1001"
+		}), mock.Anything).Return("u-1", nil).Once()
+
+		// Mock GetIdentity after creation
+		mockKratos.On("GetIdentity", mock.Anything, "u-1").Return(&service.KratosIdentity{
+			ID: "u-1",
+			Traits: map[string]interface{}{
+				"id":          "E1001",
+				"email":       "new@test.com",
+				"companyCode": "test-tenant",
+			},
+		}, nil).Once()
+
+		// Mock ListManageableTenants for mapIdentitySummary
+		mockTenant.On("ListManageableTenants", mock.Anything, "u-1").Return([]domain.Tenant{}, nil).Once()
+
+		payload := map[string]interface{}{
+			"email":       "new@test.com",
+			"name":        "New User",
+			"companyCode": "test-tenant",
+			"metadata": map[string]interface{}{
+				tenantID: map[string]interface{}{
+					"emp_no": "E1001",
+				},
+			},
+		}
+		body, _ := json.Marshal(payload)
+		req := httptest.NewRequest("POST", "/users", bytes.NewReader(body))
+		req.Header.Set("Content-Type", "application/json")
+
+		resp, _ := app.Test(req)
+		assert.Equal(t, 201, resp.StatusCode)
+		mockOry.AssertExpectations(t)
+	})
+}