Fix 5047

cla-backend-go/cmd/refresh_stored_username_test.go

@@ -0,0 +1,182 @@
+// Copyright The Linux Foundation and each contributor to CommunityBridge.
+// SPDX-License-Identifier: MIT
+
+package cmd
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/go-openapi/strfmt"
+	"github.com/linuxfoundation/easycla/cla-backend-go/gen/v1/models"
+	"github.com/linuxfoundation/easycla/cla-backend-go/user"
+)
+
+type stubUsersService struct {
+	called  bool
+	updates map[string]interface{}
+}
+
+func (s *stubUsersService) UpdateUser(userID string, updates map[string]interface{}) (*models.User, error) {
+	s.called = true
+	s.updates = updates
+	return &models.User{UserID: userID}, nil
+}
+func (s *stubUsersService) CreateUser(*models.User, *user.CLAUser) (*models.User, error) { return nil, nil }
+func (s *stubUsersService) Save(*models.UserUpdate, *user.CLAUser) (*models.User, error) { return nil, nil }
+func (s *stubUsersService) Delete(string, *user.CLAUser) error                          { return nil }
+func (s *stubUsersService) GetUser(string) (*models.User, error)                        { return nil, nil }
+func (s *stubUsersService) GetUserByLFUserName(string) (*models.User, error)            { return nil, nil }
+func (s *stubUsersService) GetUserByUserName(string, bool) (*models.User, error)        { return nil, nil }
+func (s *stubUsersService) GetUserByEmail(string) (*models.User, error)                 { return nil, nil }
+func (s *stubUsersService) GetUsersByEmail(string) ([]*models.User, error)              { return nil, nil }
+func (s *stubUsersService) GetUsersByLFEmail(string) ([]*models.User, error)            { return nil, nil }
+func (s *stubUsersService) GetUserByGitHubID(string) (*models.User, error)              { return nil, nil }
+func (s *stubUsersService) GetUserByGitHubUsername(string) (*models.User, error)        { return nil, nil }
+func (s *stubUsersService) GetUserByGitlabID(int) (*models.User, error)                 { return nil, nil }
+func (s *stubUsersService) GetUserByGitLabUsername(string) (*models.User, error)        { return nil, nil }
+func (s *stubUsersService) SearchUsers(string, string, bool) (*models.Users, error)     { return nil, nil }
+func (s *stubUsersService) UpdateUserCompanyID(string, string, string) error            { return nil }
+func (s *stubUsersService) ConvertUserModelToUserCompatModel(*models.User) (*models.UserCompat, error) {
+	return nil, nil
+}
+
+func makeStoredUser(name, email string) *models.User {
+	return &models.User{Username: name, LfEmail: strfmt.Email(email)}
+}
+func makeTestCLAUser(name, email string) *user.CLAUser {
+	return &user.CLAUser{Name: name, LFEmail: email}
+}
+
+func TestRefreshStoredUserName(t *testing.T) {
+	cases := []struct {
+		desc         string
+		stored       *models.User
+		claUser      *user.CLAUser
+		wantCalled   bool
+		wantNameKey  bool
+		wantEmailKey bool
+		wantEmailVal string
+	}{
+		{
+			desc:       "nil userModel → no-op",
+			stored:     nil,
+			claUser:    makeTestCLAUser("Alice", "alice@example.com"),
+			wantCalled: false,
+		},
+		{
+			desc:       "nil claUser → no-op",
+			stored:     makeStoredUser("Alice", "alice@example.com"),
+			claUser:    nil,
+			wantCalled: false,
+		},
+		{
+			desc:       "name unchanged, email unchanged → no-op",
+			stored:     makeStoredUser("Alice", "alice@example.com"),
+			claUser:    makeTestCLAUser("Alice", "alice@example.com"),
+			wantCalled: false,
+		},
+		{
+			desc:         "name changed only → only user_name updated",
+			stored:       makeStoredUser("Alice", "alice@example.com"),
+			claUser:      makeTestCLAUser("Alice Smith", "alice@example.com"),
+			wantCalled:   true,
+			wantNameKey:  true,
+			wantEmailKey: false,
+		},
+		{
+			desc:         "email changed only → only lf_email updated",
+			stored:       makeStoredUser("Alice", "alice-old@example.com"),
+			claUser:      makeTestCLAUser("Alice", "alice-new@example.com"),
+			wantCalled:   true,
+			wantNameKey:  false,
+			wantEmailKey: true,
+			wantEmailVal: "alice-new@example.com",
+		},
+		{
+			desc:         "both changed → both updated",
+			stored:       makeStoredUser("Alice", "alice-old@example.com"),
+			claUser:      makeTestCLAUser("Alice Smith", "alice-new@example.com"),
+			wantCalled:   true,
+			wantNameKey:  true,
+			wantEmailKey: true,
+			wantEmailVal: "alice-new@example.com",
+		},
+		{
+			desc:       "case-insensitive email match → no-op",
+			stored:     makeStoredUser("Alice", "Alice@Example.COM"),
+			claUser:    makeTestCLAUser("Alice", "alice@example.com"),
+			wantCalled: false,
+		},
+		{
+			desc:         "email uppercased in JWT → lowercased on write",
+			stored:       makeStoredUser("Alice", "old@example.com"),
+			claUser:      makeTestCLAUser("Alice", "NEW@Example.COM"),
+			wantCalled:   true,
+			wantEmailKey: true,
+			wantEmailVal: "new@example.com",
+		},
+		{
+			desc:         "empty name in claUser → name not touched, email still synced",
+			stored:       makeStoredUser("Alice", "old@example.com"),
+			claUser:      makeTestCLAUser("", "new@example.com"),
+			wantCalled:   true,
+			wantNameKey:  false,
+			wantEmailKey: true,
+		},
+		{
+			desc:         "empty email in claUser → email not touched, name still synced",
+			stored:       makeStoredUser("Alice", "old@example.com"),
+			claUser:      makeTestCLAUser("Alice Smith", ""),
+			wantCalled:   true,
+			wantNameKey:  true,
+			wantEmailKey: false,
+		},
+		{
+			desc:       "both empty in claUser → no-op",
+			stored:     makeStoredUser("Alice", "old@example.com"),
+			claUser:    makeTestCLAUser("", ""),
+			wantCalled: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.desc, func(t *testing.T) {
+			svc := &stubUsersService{}
+			result := refreshStoredUserName(svc, tc.stored, tc.claUser)
+
+			if svc.called != tc.wantCalled {
+				t.Fatalf("UpdateUser called=%v, want %v", svc.called, tc.wantCalled)
+			}
+			if !svc.called {
+				return
+			}
+			_, hasName := svc.updates["user_name"]
+			if hasName != tc.wantNameKey {
+				t.Errorf("user_name in updates=%v, want %v", hasName, tc.wantNameKey)
+			}
+			emailVal, hasEmail := svc.updates["lf_email"]
+			if hasEmail != tc.wantEmailKey {
+				t.Errorf("lf_email in updates=%v, want %v", hasEmail, tc.wantEmailKey)
+			}
+			if hasEmail {
+				got, ok := emailVal.(string)
+				if !ok {
+					t.Fatalf("lf_email value is not string: %T", emailVal)
+				}
+				if got != strings.ToLower(got) {
+					t.Errorf("lf_email not lowercased: %q", got)
+				}
+				if tc.wantEmailVal != "" && got != tc.wantEmailVal {
+					t.Errorf("lf_email=%q, want %q", got, tc.wantEmailVal)
+				}
+			}
+			if _, hasDate := svc.updates["date_modified"]; !hasDate {
+				t.Error("date_modified must always be present when updating")
+			}
+			if result == nil {
+				t.Error("result must not be nil on success")
+			}
+		})
+	}
+}

cla-backend-go/cmd/server.go

@@ -909,14 +909,27 @@ func responseLoggingMiddleware(next http.Handler) http.Handler {
 }
 
 func refreshStoredUserName(usersService users.Service, userModel *models.User, claUser *user.CLAUser) *models.User {
-	if userModel == nil || claUser == nil || claUser.Name == "" || userModel.Username == claUser.Name {
+	if userModel == nil || claUser == nil {
 		return userModel
 	}
 
-	updatedUser, err := usersService.UpdateUser(userModel.UserID, map[string]interface{}{
-		"user_name":     claUser.Name,
+	nameChanged := claUser.Name != "" && userModel.Username != claUser.Name
+	emailChanged := claUser.LFEmail != "" && !strings.EqualFold(string(userModel.LfEmail), claUser.LFEmail)
+	if !nameChanged && !emailChanged {
+		return userModel
+	}
+
+	updates := map[string]interface{}{
 		"date_modified": time.Now().UTC().Format(time.RFC3339),
-	})
+	}
+	if nameChanged {
+		updates["user_name"] = claUser.Name
+	}
+	if emailChanged {
+		updates["lf_email"] = strings.ToLower(claUser.LFEmail)
+	}
+
+	updatedUser, err := usersService.UpdateUser(userModel.UserID, updates)
 	if err != nil {
 		log.WithFields(logrus.Fields{
 			"functionName": "cmd.refreshStoredUserName",

cla-backend-go/company/repository.go

@@ -348,6 +348,12 @@ func (repo repository) GetCompany(ctx context.Context, companyID string) (*model
 		utils.XREQUESTID: ctx.Value(utils.XREQUESTID),
 		"companyID":      companyID,
 	}
+
+	if companyID == "" {
+		log.WithFields(f).Warn("companyID is empty, skipping DynamoDB lookup")
+		return nil, &utils.CompanyNotFound{Message: "company_id cannot be empty", CompanyID: companyID}
+	}
+
 	companyTableData, err := repo.dynamoDBClient.GetItem(&dynamodb.GetItemInput{
 		TableName: aws.String(repo.companyTableName),
 		Key: map[string]*dynamodb.AttributeValue{

cla-backend-go/emails/params.go

@@ -80,6 +80,10 @@ type CLAGroupTemplateParams struct {
 // GetProjectNameOrFoundation returns if the foundationName is set it gets back
 // the foundation Name otherwise the ProjectName is  returned
 func (claParams CLAGroupTemplateParams) GetProjectNameOrFoundation() string {
+	if len(claParams.Projects) == 0 {
+		log.Warnf("GetProjectNameOrFoundation called with empty Projects slice (CLAGroupName=%q); rendering as empty string", claParams.CLAGroupName)
+		return ""
+	}
 	project := claParams.Projects[0]
 	if claParams.ChildProjectCount == 1 {
 		return claParams.Projects[0].ExternalProjectName
@@ -96,6 +100,10 @@ func (claParams CLAGroupTemplateParams) GetProjectNameOrFoundation() string {
 // Project is used generally in v1 templates because the matching there was 1:1
 // it will returns the first element from the projects list
 func (claParams CLAGroupTemplateParams) Project() CLAProjectParams {
+	if len(claParams.Projects) == 0 {
+		log.Warnf("Project called with empty Projects slice (CLAGroupName=%q); rendering as zero CLAProjectParams", claParams.CLAGroupName)
+		return CLAProjectParams{}
+	}
 	return claParams.Projects[0]
 }
 

cla-backend-go/events/service.go

@@ -439,6 +439,15 @@ func (s *service) LogEventWithContext(ctx context.Context, args *LogEventArgs) {
 		return
 	}
 
+	// loadDetails resolves UserID from LfUsername via DDB lookup; if the user is
+	// not found in EasyCLA's user table (e.g. external contractor whose LFX profile
+	// predates their EasyCLA record) UserID stays empty and CreateEvent would fail
+	// with ErrUserIDRequired. Log at warn and skip instead of surfacing a level=error.
+	if args.UserID == "" {
+		log.WithFields(f).Warnf("skipping event %s: UserID could not be resolved for LfUsername=%s", args.EventType, args.LfUsername)
+		return
+	}
+
 	eventData, containsPII := args.EventData.GetEventDetailsString(args)
 	eventSummary, _ := args.EventData.GetEventSummaryString(args)
 	event := models.Event{

cla-backend-go/signatures/converters.go

@@ -57,12 +57,16 @@ func (repo repository) buildProjectSignatureModels(ctx context.Context, results
 			claType = utils.ClaTypeICLA
 		}
 
-		// Use the signedOn field if possible, for older signatures that are missing it, use the date created value as the default/fallback
+		// Use the signedOn field if possible, for older signatures that are missing it, use the date created value as the default/fallback.
+		// Both fields can be empty for legacy signatures predating the SignedOn column — leave signedOn as "" in that case
+		// rather than calling FormatTimeString("") and emitting a spurious warning.
 		signedOn := dbSignature.DateCreated
 		if dbSignature.SignedOn != "" {
 			signedOn = dbSignature.SignedOn
 		}
-		signedOn = utils.FormatTimeString(signedOn)
+		if signedOn != "" {
+			signedOn = utils.FormatTimeString(signedOn)
+		}
 
 		sig := &models.Signature{
 			SignatureID:                   dbSignature.SignatureID,