Skip to content

Commit

Permalink
Merge pull request #4396 from nickmango/bug/ecla-check
Browse files Browse the repository at this point in the history
[#4348] Bug/Ecla Check
  • Loading branch information
nickmango authored Jul 30, 2024
2 parents 2e175d6 + fd771bc commit 9bde61e
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 35 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions cla-backend-go/signatures/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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),
}

Expand All @@ -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,
Expand Down
34 changes: 26 additions & 8 deletions cla-backend-go/signatures/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
}
}
Expand Down
85 changes: 85 additions & 0 deletions cla-backend-go/signatures/service_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
1 change: 1 addition & 0 deletions cla-backend-go/v2/signatures/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions cla-backend-go/v2/signatures/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 9bde61e

Please sign in to comment.