[#4348] Bug/Ecla Check

cla-backend-go/signatures/repository.go

@@ -2368,14 +2368,14 @@ func (repo repository) GetProjectCompanyEmployeeSignature(ctx context.Context, c
 	}
 
 	// This is the keys we want to match
-	condition := expression.Key("signature_user_ccla_company_id").Equal(expression.Value(companyModel.CompanyID)).And(
-		expression.Key("signature_project_id").Equal(expression.Value(claGroupModel.ProjectID)))
+	condition := expression.Key("signature_reference_id").Equal(expression.Value(employeeUserModel.UserID))
 
 	var filterAdded bool
 	var filter expression.ConditionBuilder
 
 	// Check for approved signatures
-	filter = addAndCondition(filter, expression.Name("signature_reference_id").Equal(expression.Value(employeeUserModel.UserID)), &filterAdded)
+	filter = addAndCondition(filter, expression.Name("signature_user_ccla_company_id").Equal(expression.Value(companyModel.CompanyID)), &filterAdded)
+	filter = addAndCondition(filter, expression.Name("signature_project_id").Equal(expression.Value(claGroupModel.ProjectID)), &filterAdded)
 
 	log.WithFields(f).Debugf("running employee signature query on table: %s", repo.signatureTableName)
 	expr, err := expression.NewBuilder().WithKeyCondition(condition).WithFilter(filter).WithProjection(buildProjection()).Build()
@@ -2394,7 +2394,7 @@ func (repo repository) GetProjectCompanyEmployeeSignature(ctx context.Context, c
 		FilterExpression:          expr.Filter(),
 		ProjectionExpression:      expr.Projection(),
 		TableName:                 aws.String(repo.signatureTableName),
-		IndexName:                 aws.String("signature-user-ccla-company-index"), // Name of a secondary index to scan
+		IndexName:                 aws.String("reference-signature-index"), // Name of a secondary index to scan
 		Limit:                     aws.Int64(10),
 	}
 
@@ -2406,7 +2406,9 @@ func (repo repository) GetProjectCompanyEmployeeSignature(ctx context.Context, c
 		errorChannel <- errQuery
 		return
 	}
+
 	if results == nil || len(results.Items) == 0 {
+		log.WithFields(f).Debug("No ecla records found!")
 		resultChannel <- &EmployeeModel{
 			Signature: nil,
 			User:      employeeUserModel,

cla-backend-go/signatures/service.go

@@ -77,6 +77,7 @@ type SignatureService interface {
 	UpdateEnvelopeDetails(ctx context.Context, signatureID, envelopeID string, signURL *string) (*models.Signature, error)
 	// handleGitHubStatusUpdate(ctx context.Context, employeeUserModel *models.User) error
 	ProcessEmployeeSignature(ctx context.Context, companyModel *models.Company, claGroupModel *models.ClaGroup, user *models.User) (*bool, error)
+	UserIsApproved(ctx context.Context, user *models.User, cclaSignature *models.Signature) (bool, error)
 }
 
 type service struct {
@@ -1199,7 +1200,7 @@ func (s service) HasUserSigned(ctx context.Context, user *models.User, projectID
 
 func (s service) ProcessEmployeeSignature(ctx context.Context, companyModel *models.Company, claGroupModel *models.ClaGroup, user *models.User) (*bool, error) {
 	f := logrus.Fields{
-		"functionName":   "v2.signatures.service.processEmployeeSignature",
+		"functionName":   "v2.signatures.service.ProcessEmployeeSignature",
 		utils.XREQUESTID: ctx.Value(utils.XREQUESTID),
 		"companyID":      companyModel.CompanyID,
 		"projectID":      claGroupModel.ProjectID,
@@ -1227,7 +1228,8 @@ func (s service) ProcessEmployeeSignature(ctx context.Context, companyModel *mod
 		if result != nil {
 			employeeSignature := result.Signature
 			if employeeSignature != nil {
-				log.WithFields(f).Debugf("ECLA Signature check - located employee acknowledgement - signature id: %s", employeeSignature.SignatureID)
+				// log.WithFields(f).Debugf("ECLA Signature check - located employee acknowledgement - signature id: %s", employeeSignature.SignatureID)
+				log.WithFields(f).Debugf("ecla signature check -  :%+v", employeeSignature)
 
 				// Get corporate ccla signature of company to access the approval list
 				cclaSignature, cclaErr := s.GetCorporateSignature(ctx, projectID, companyID, &approved, &signed)
@@ -1237,7 +1239,8 @@ func (s service) ProcessEmployeeSignature(ctx context.Context, companyModel *mod
 				}
 
 				if cclaSignature != nil {
-					userApproved, approvedErr := s.userIsApproved(ctx, user, cclaSignature)
+					log.WithFields(f).Debug("found ccla signature")
+					userApproved, approvedErr := s.UserIsApproved(ctx, user, cclaSignature)
 					if approvedErr != nil {
 						log.WithFields(f).WithError(approvedErr).Warnf("problem determining if user: %s is approved for project: %s", user.UserID, projectID)
 						return &hasSigned, approvedErr
@@ -1247,6 +1250,8 @@ func (s service) ProcessEmployeeSignature(ctx context.Context, companyModel *mod
 					if userApproved {
 						log.WithFields(f).Debugf("user: %s is in the approval list for signature: %s", user.UserID, employeeSignature.SignatureID)
 						hasSigned = true
+					} else {
+						log.WithFields(f).Debugf("user: %s is not in the approval list for signature: %s", user.UserID, employeeSignature.SignatureID)
 					}
 				}
 			} else {
@@ -1264,14 +1269,24 @@ func (s service) ProcessEmployeeSignature(ctx context.Context, companyModel *mod
 
 }
 
-func (s service) userIsApproved(ctx context.Context, user *models.User, cclaSignature *models.Signature) (bool, error) {
-	emails := append(user.Emails, string(user.LfEmail))
-
+func (s service) UserIsApproved(ctx context.Context, user *models.User, cclaSignature *models.Signature) (bool, error) {
+	// add lf email to emails
 	f := logrus.Fields{
-		"functionName": "v1.signatures.service.userIsApproved",
+		"functionName": "v1.signatures.service.UserIsApproved",
+	}
+
+	emails := user.Emails
+
+	if user.LfEmail != "" {
+		log.WithFields(f).Debugf("adding lf email: %s to emails", user.LfEmail)
+		emails = append(emails, string(user.LfEmail))
+		// remove duplicates
+		log.WithFields(f).Debug("removing duplicates")
+		emails = utils.RemoveDuplicates(emails)
 	}
 
 	// check GitHub username approval list
+	log.WithFields(f).Debug("checking if user is in the approval list")
 	gitHubUsernameApprovalList := cclaSignature.GithubUsernameApprovalList
 	if len(gitHubUsernameApprovalList) > 0 {
 		for _, gitHubUsername := range gitHubUsernameApprovalList {
@@ -1297,9 +1312,12 @@ func (s service) userIsApproved(ctx context.Context, user *models.User, cclaSign
 
 	// check email email approval list
 	emailApprovalList := cclaSignature.EmailApprovalList
+	log.WithFields(f).Debugf("checking if user is in the email approval list: %+v with emails :%v", emailApprovalList, emails)
 	if len(emailApprovalList) > 0 {
 		for _, email := range emails {
-			if strings.EqualFold(email, strings.TrimSpace(user.LfUsername)) {
+			log.WithFields(f).Debugf("checking email: %s", email)
+			if utils.StringInSlice(email, emailApprovalList) {
+				log.WithFields(f).Debugf("found matching email: %s in the email approval list", email)
 				return true, nil
 			}
 		}

cla-backend-go/signatures/service_test.go

@@ -0,0 +1,85 @@
+// Copyright The Linux Foundation and each contributor to CommunityBridge.
+// SPDX-License-Identifier: MIT
+
+package signatures
+
+import (
+	"context"
+	"testing"
+
+	v1Models "github.com/communitybridge/easycla/cla-backend-go/gen/v1/models"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestUserIsApproved(t *testing.T) {
+	ctx := context.Background()
+
+	testCases := []struct {
+		name               string
+		user               *v1Models.User
+		cclaSignature      *v1Models.Signature
+		expectedIsApproved bool
+	}{
+		{
+			name: "User in GitHub username approval list",
+			user: &v1Models.User{
+				GithubUsername: "approved-user",
+			},
+			cclaSignature: &v1Models.Signature{
+				GithubUsernameApprovalList: []string{"approved-user"},
+			},
+			expectedIsApproved: true,
+		},
+		{
+			name: "User not in GitHub username approval list",
+			user: &v1Models.User{
+				GithubUsername: "unapproved-user",
+			},
+			cclaSignature: &v1Models.Signature{
+				GithubUsernameApprovalList: []string{"approved-user"},
+			},
+			expectedIsApproved: false,
+		},
+		{
+			name: "User in Email approval list",
+			user: &v1Models.User{
+				Emails: []string{"[email protected]"},
+			},
+			cclaSignature: &v1Models.Signature{
+				EmailApprovalList: []string{"[email protected]"},
+			},
+			expectedIsApproved: true,
+		},
+		{
+			name: "User not in Email approval list",
+			user: &v1Models.User{
+				Emails: []string{"[email protected]"},
+			},
+			cclaSignature: &v1Models.Signature{
+				EmailApprovalList: []string{"[email protected]"},
+			},
+			expectedIsApproved: false,
+		},
+		{
+			name: "User in Domain approval list",
+			user: &v1Models.User{
+				Emails: []string{"[email protected]"},
+			},
+			cclaSignature: &v1Models.Signature{
+				DomainApprovalList: []string{"samsung.com"},
+			},
+			expectedIsApproved: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			service := NewService(nil, nil, nil, nil, false, nil, nil, nil, nil, "", "", "")
+
+			isApproved, err := service.UserIsApproved(ctx, tc.user, tc.cclaSignature)
+
+			assert.Nil(t, err)
+			assert.Equal(t, tc.expectedIsApproved, isApproved)
+		})
+	}
+}

cla-backend-go/v2/signatures/service.go

@@ -522,6 +522,7 @@ func (s *Service) IsUserAuthorized(ctx context.Context, lfid, claGroupId string)
 				log.WithFields(f).WithError(err).Debug("unable to process ecla")
 				return nil, err
 			}
+			log.WithFields(f).Debugf("ecla value: %b", ecla)
 			if ecla != nil && *ecla {
 				log.WithFields(f).Debug("user has signed ECLA")
 				hasSigned = true

cla-backend-go/v2/signatures/service_test.go

@@ -16,8 +16,8 @@ import (
 	mock_company "github.com/communitybridge/easycla/cla-backend-go/company/mocks"
 	ini "github.com/communitybridge/easycla/cla-backend-go/init"
 	mock_project "github.com/communitybridge/easycla/cla-backend-go/project/mocks"
+	mock_v1_signatures "github.com/communitybridge/easycla/cla-backend-go/signatures/mocks"
 	mock_users "github.com/communitybridge/easycla/cla-backend-go/v2/signatures/mock_users"
-	mock_v1_signatures "github.com/communitybridge/easycla/cla-backend-go/v2/signatures/mock_v1_signatures"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
 )
@@ -257,7 +257,6 @@ func TestService_IsUserAuthorized(t *testing.T) {
 			mockUserService.EXPECT().GetUserByLFUserName(tc.lfid).Return(tc.getUserByLFUsernameResult, tc.getUserByLFUsernameError)
 
 			if tc.getUserByLFUsernameResult != nil {
-
 				mockSignatureService := mock_v1_signatures.NewMockSignatureService(ctrl)
 
 				approved := true