Fix 5047

cla-backend-go/cmd/refresh_stored_username_test.go

@@ -0,0 +1,213 @@
+// 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
+	calledWith string
+	updates    map[string]interface{}
+}
+
+func (s *stubUsersService) UpdateUser(userID string, updates map[string]interface{}) (*models.User, error) {
+	s.called = true
+	s.calledWith = userID
+	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
+}
+
+const testUserID = "user-id-123"
+
+func makeStoredUser(email string) *models.User {
+	return &models.User{UserID: testUserID, Username: "Alice", LfEmail: strfmt.Email(email)}
+}
+
+func makeTestCLAUser(name, email string) *user.CLAUser {
+	return &user.CLAUser{Name: name, LFEmail: email}
+}
+
+func TestRefreshStoredUserIdentity(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@example.com"),
+			claUser:    nil,
+			wantCalled: false,
+		},
+		{
+			desc:       "name unchanged, email unchanged → no-op",
+			stored:     makeStoredUser("alice@example.com"),
+			claUser:    makeTestCLAUser("Alice", "alice@example.com"),
+			wantCalled: false,
+		},
+		{
+			desc:         "name changed only → only user_name updated",
+			stored:       makeStoredUser("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-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-old@example.com"),
+			claUser:      makeTestCLAUser("Alice Smith", "alice-new@example.com"),
+			wantCalled:   true,
+			wantNameKey:  true,
+			wantEmailKey: true,
+			wantEmailVal: "alice-new@example.com",
+		},
+		{
+			desc:         "stored email differs only in case → normalize on write",
+			stored:       makeStoredUser("Alice@Example.COM"),
+			claUser:      makeTestCLAUser("Alice", "alice@example.com"),
+			wantCalled:   true,
+			wantEmailKey: true,
+			wantEmailVal: "alice@example.com",
+		},
+		{
+			desc:       "incoming email differs only in case from already-normalized stored → no-op",
+			stored:     makeStoredUser("alice@example.com"),
+			claUser:    makeTestCLAUser("Alice", "ALICE@EXAMPLE.COM"),
+			wantCalled: false,
+		},
+		{
+			desc:         "email uppercased in JWT → lowercased on write",
+			stored:       makeStoredUser("old@example.com"),
+			claUser:      makeTestCLAUser("Alice", "NEW@Example.COM"),
+			wantCalled:   true,
+			wantEmailKey: true,
+			wantEmailVal: "new@example.com",
+		},
+		{
+			desc:         "incoming email has whitespace → trimmed and lowercased",
+			stored:       makeStoredUser("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("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("old@example.com"),
+			claUser:      makeTestCLAUser("Alice Smith", ""),
+			wantCalled:   true,
+			wantNameKey:  true,
+			wantEmailKey: false,
+		},
+		{
+			desc:       "both empty in claUser → no-op",
+			stored:     makeStoredUser("old@example.com"),
+			claUser:    makeTestCLAUser("", ""),
+			wantCalled: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.desc, func(t *testing.T) {
+			svc := &stubUsersService{}
+			result := refreshStoredUserIdentity(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
+			}
+			if svc.calledWith != testUserID {
+				t.Errorf("UpdateUser userID=%q, want %q", svc.calledWith, testUserID)
+			}
+			_, 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/root.go

@@ -7,6 +7,7 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"testing"
 
 	ini "github.com/linuxfoundation/easycla/cla-backend-go/init"
 	log "github.com/linuxfoundation/easycla/cla-backend-go/logging"
@@ -52,6 +53,14 @@ func init() {
 	// when this action is called directly.
 	rootCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle")
 
+	// Skip AWS/SSM init when running under `go test`: ini.Init() fatals on
+	// missing DYNAMODB_AWS_REGION (and other env vars) which are not set in
+	// CI test environments, and unit tests in this package don't exercise
+	// that code path.
+	if testing.Testing() {
+		return
+	}
+
 	// Initialize our common stuff
 	ini.Init()
 }

cla-backend-go/cmd/server.go

@@ -908,21 +908,40 @@ 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 {
+func refreshStoredUserIdentity(usersService users.Service, userModel *models.User, claUser *user.CLAUser) *models.User {
+	if userModel == nil || claUser == nil {
 		return userModel
 	}
 
-	updatedUser, err := usersService.UpdateUser(userModel.UserID, map[string]interface{}{
-		"user_name":     claUser.Name,
+	// Normalize the incoming email before compare/persist: the lf-email-index
+	// query path (users/repository.go:GetUsersByLFEmail) lowercases the
+	// search key, so a mixed-case stored value would be orphaned from that
+	// index. Compare byte-wise against the stored value to detect that
+	// situation and rewrite to the normalized form.
+	normalizedIncomingEmail := strings.ToLower(strings.TrimSpace(claUser.LFEmail))
+	nameChanged := claUser.Name != "" && userModel.Username != claUser.Name
+	emailChanged := normalizedIncomingEmail != "" && string(userModel.LfEmail) != normalizedIncomingEmail
+	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"] = normalizedIncomingEmail
+	}
+
+	updatedUser, err := usersService.UpdateUser(userModel.UserID, updates)
 	if err != nil {
 		log.WithFields(logrus.Fields{
-			"functionName": "cmd.refreshStoredUserName",
+			"functionName": "cmd.refreshStoredUserIdentity",
 			"userID":       userModel.UserID,
 			"lfUsername":   claUser.LFUsername,
-		}).WithError(err).Warn("unable to refresh stored user_name from current identity claims")
+		}).WithError(err).Warn("unable to refresh stored user identity from current identity claims")
 		return userModel
 	}
 
@@ -973,7 +992,7 @@ func createUserFromRequest(authorizer auth.Authorizer, usersService users.Servic
 	}
 	// If found - refresh the stored display name and return
 	if userModel != nil {
-		userModel = refreshStoredUserName(usersService, userModel, claUser)
+		userModel = refreshStoredUserIdentity(usersService, userModel, claUser)
 		if !needToStoreUser {
 			return r
 		}
@@ -993,7 +1012,7 @@ func createUserFromRequest(authorizer auth.Authorizer, usersService users.Servic
 	}
 	// If found - refresh the stored display name and return
 	if userModel != nil {
-		userModel = refreshStoredUserName(usersService, userModel, claUser)
+		userModel = refreshStoredUserIdentity(usersService, userModel, claUser)
 		if !needToStoreUser {
 			return r
 		}

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,