diff --git a/cla-backend-go/users/models.go b/cla-backend-go/users/models.go index 85a670720..3a0b8f52d 100644 --- a/cla-backend-go/users/models.go +++ b/cla-backend-go/users/models.go @@ -5,22 +5,22 @@ package users // DBUser data model type DBUser struct { - UserID string `json:"user_id"` - UserExternalID string `json:"user_external_id"` - LFEmail string `json:"lf_email"` - Admin bool `json:"admin"` - LFUsername string `json:"lf_username"` - DateCreated string `json:"date_created"` - DateModified string `json:"date_modified"` - UserName string `json:"user_name"` - Version string `json:"version"` - UserEmails UserEmails `json:"user_emails"` - UserGithubID string `json:"user_github_id"` - UserGithubUsername string `json:"user_github_username"` - UserGitlabID string `json:"user_gitlab_id"` - UserGitlabUsername string `json:"user_gitlab_username"` - UserCompanyID string `json:"user_company_id"` - Note string `json:"note"` + UserID string `json:"user_id"` + UserExternalID string `json:"user_external_id"` + LFEmail string `json:"lf_email"` + Admin bool `json:"admin"` + LFUsername string `json:"lf_username"` + DateCreated string `json:"date_created"` + DateModified string `json:"date_modified"` + UserName string `json:"user_name"` + Version string `json:"version"` + UserEmails []string `json:"user_emails"` + UserGithubID string `json:"user_github_id"` + UserGithubUsername string `json:"user_github_username"` + UserGitlabID string `json:"user_gitlab_id"` + UserGitlabUsername string `json:"user_gitlab_username"` + UserCompanyID string `json:"user_company_id"` + Note string `json:"note"` } type UserEmails struct { diff --git a/cla-backend-go/users/repository.go b/cla-backend-go/users/repository.go index 6900cece2..ccfa88e11 100644 --- a/cla-backend-go/users/repository.go +++ b/cla-backend-go/users/repository.go @@ -1254,11 +1254,6 @@ func (repo repository) UpdateUserCompanyID(userID, companyID, note string) error // convertDBUserModel translates a dyanamoDB data model into a service response model func convertDBUserModel(user DBUser) *models.User { - var emails []string - if user.UserEmails.SS != nil { - emails = user.UserEmails.SS - } - return &models.User{ UserID: user.UserID, UserExternalID: user.UserExternalID, @@ -1269,7 +1264,7 @@ func convertDBUserModel(user DBUser) *models.User { DateModified: user.DateModified, Username: user.UserName, Version: user.Version, - Emails: emails, + Emails: user.UserEmails, GithubID: user.UserGithubID, GithubUsername: user.UserGithubUsername, GitlabID: user.UserGitlabID, diff --git a/cla-backend-go/v2/signatures/service.go b/cla-backend-go/v2/signatures/service.go index 8d2eccbba..5d7b6a3e3 100644 --- a/cla-backend-go/v2/signatures/service.go +++ b/cla-backend-go/v2/signatures/service.go @@ -496,6 +496,8 @@ func (s *Service) IsUserAuthorized(ctx context.Context, lfid, claGroupId string) log.WithFields(f).Debug("user has signed ICLA") response.ICLA = true hasSigned = true + } else { + log.WithFields(f).Debug("user has not signed ICLA") } // fetch company @@ -505,13 +507,12 @@ func (s *Service) IsUserAuthorized(ctx context.Context, lfid, claGroupId string) } else { log.WithFields(f).Debug("fetching company") companyModel, err := s.v1CompanyService.GetCompany(ctx, user.CompanyID) - if err != nil { + if companyErr, ok := err.(*utils.CompanyNotFound); ok { + log.WithFields(f).WithError(companyErr).Debug("company not found") + response.CompanyAffiliation = false + } else if err != nil { log.WithFields(f).WithError(err).Debug("unable to fetch company") return nil, err - } - if companyModel == nil { - log.WithFields(f).Debug("company not found") - response.CompanyAffiliation = false } else { log.WithFields(f).Debug("company found") response.CompanyAffiliation = true diff --git a/cla-backend-go/v2/signatures/service_test.go b/cla-backend-go/v2/signatures/service_test.go index 54c42ad23..c5d62a4af 100644 --- a/cla-backend-go/v2/signatures/service_test.go +++ b/cla-backend-go/v2/signatures/service_test.go @@ -6,11 +6,11 @@ package signatures import ( "context" "errors" - "fmt" "testing" v1Models "github.com/communitybridge/easycla/cla-backend-go/gen/v1/models" "github.com/communitybridge/easycla/cla-backend-go/gen/v2/models" + "github.com/communitybridge/easycla/cla-backend-go/utils" // mock_signatures "github.com/communitybridge/easycla/cla-backend-go/v2/signatures/mock_v1_signatures" mock_company "github.com/communitybridge/easycla/cla-backend-go/company/mocks" @@ -24,6 +24,7 @@ import ( func TestService_IsUserAuthorized(t *testing.T) { type testCase struct { + name string lfid string projectID string userID string @@ -40,10 +41,13 @@ func TestService_IsUserAuthorized(t *testing.T) { expectedICLA bool expectedCCLA bool expectedCompanyAffiliation bool + getCompanyResult *v1Models.Company + getCompanyError error } cases := []testCase{ { + name: "claGroupRequiresICLA", lfid: "foobar_1", projectID: "project-123", userID: "user-123", @@ -66,8 +70,13 @@ func TestService_IsUserAuthorized(t *testing.T) { expectedICLA: true, expectedCCLA: true, expectedCompanyAffiliation: true, + getCompanyResult: &v1Models.Company{ + CompanyID: "company-123", + }, + getCompanyError: nil, }, { + name: "claGroupDoesNotRequireICLA", lfid: "foobar_2", projectID: "project-123", userID: "user-123", @@ -90,8 +99,13 @@ func TestService_IsUserAuthorized(t *testing.T) { expectedICLA: true, expectedCCLA: true, expectedCompanyAffiliation: true, + getCompanyResult: &v1Models.Company{ + CompanyID: "company-123", + }, + getCompanyError: nil, }, { + name: "icla signature found", lfid: "foobar_3", projectID: "project-123", userID: "user-123", @@ -114,8 +128,13 @@ func TestService_IsUserAuthorized(t *testing.T) { expectedICLA: true, expectedCCLA: false, expectedCompanyAffiliation: true, + getCompanyResult: &v1Models.Company{ + CompanyID: "company-123", + }, + getCompanyError: nil, }, { + name: "icla signature not found", lfid: "foobar_4", projectID: "project-123", userID: "user-123", @@ -136,8 +155,13 @@ func TestService_IsUserAuthorized(t *testing.T) { expectedICLA: false, expectedCCLA: true, expectedCompanyAffiliation: true, + getCompanyResult: &v1Models.Company{ + CompanyID: "company-123", + }, + getCompanyError: nil, }, { + name: "individual signature error", lfid: "foobar_5", projectID: "project-123", userID: "user-123", @@ -157,8 +181,13 @@ func TestService_IsUserAuthorized(t *testing.T) { expectedICLA: false, expectedCCLA: false, expectedCompanyAffiliation: true, + getCompanyResult: &v1Models.Company{ + CompanyID: "company-123", + }, + getCompanyError: nil, }, { + name: "user has not signed ccla and icla", lfid: "foobar_6", projectID: "project-123", userID: "user-123", @@ -171,11 +200,42 @@ func TestService_IsUserAuthorized(t *testing.T) { expectedICLA: false, expectedCCLA: false, expectedCompanyAffiliation: false, + getCompanyResult: &v1Models.Company{ + CompanyID: "company-123", + }, + getCompanyError: nil, + }, + { + name: "user has icla and has company id that does not exist", + lfid: "foobar_7", + projectID: "project-123", + userID: "user-123", + companyID: "company-123", + getUserByLFUsernameResult: &v1Models.User{ + UserID: "user-123", + CompanyID: "company-123", + }, + getUserByLFUsernameError: nil, + claGroupRequiresICLA: false, + expectedAuthorized: true, + expectedCCLARequiresICLA: false, + expectedICLA: true, + expectedCCLA: false, + expectedCompanyAffiliation: false, + getCompanyResult: nil, + getCompanyError: &utils.CompanyNotFound{ + Message: "no company matching company record", + CompanyID: "company-123", + }, + getIndividualSignatureResult: &v1Models.Signature{ + SignatureID: "signature-123", + }, + getIndividualSignatureError: nil, }, } for _, tc := range cases { - t.Run(fmt.Sprintf("LFID=%s ProjectID=%s", tc.lfid, tc.projectID), func(t *testing.T) { + t.Run(tc.name, func(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -204,12 +264,12 @@ func TestService_IsUserAuthorized(t *testing.T) { signed := true mockSignatureService.EXPECT().GetIndividualSignature(context.Background(), tc.projectID, tc.userID, &approved, &signed).Return(tc.getIndividualSignatureResult, tc.getIndividualSignatureError) - mockSignatureService.EXPECT().ProcessEmployeeSignature(context.Background(), gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.processEmployeeSignatureResult, tc.processEmployeeSignatureError) + if tc.getCompanyError == nil { + mockSignatureService.EXPECT().ProcessEmployeeSignature(context.Background(), gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.processEmployeeSignatureResult, tc.processEmployeeSignatureError) + } mockCompanyService := mock_company.NewMockIService(ctrl) - mockCompanyService.EXPECT().GetCompany(context.Background(), tc.companyID).Return(&v1Models.Company{ - CompanyID: tc.companyID, - }, nil) + mockCompanyService.EXPECT().GetCompany(context.Background(), tc.companyID).Return(tc.getCompanyResult, tc.getCompanyError) service := NewService(awsSession, "", mockProjectService, mockCompanyService, mockSignatureService, nil, nil, mockUserService, nil)