From 33076cb9391559c534a827b390c32fedce794a66 Mon Sep 17 00:00:00 2001 From: ahmedomosanya Date: Thu, 24 Sep 2026 17:36:05 +0100 Subject: [PATCH] fix(company): return 404 when a CLA group's project is gone GET /v4/company/{companyID}/project/{projectSFID}/cla-managers answered 400 for every service error, including the project service reporting that the project no longer exists. Callers could not tell that case apart from a real bad request, so LFX Self Serve had no safe way to fall back to the CLA-group manager list. - Map a project-service GetProjectNotFound (wrapped or not) to the 404 the swagger already declares for this route; the permission check still runs first - Keep every other service error at 400 - Cover success, project-not-found, wrapped not-found, other failure, and forbidden-before-lookup in handler tests Refs linuxfoundation/lfx-self-serve#2953 Signed-off-by: ahmedomosanya --- cla-backend-go/v2/company/handlers.go | 8 ++ cla-backend-go/v2/company/handlers_test.go | 104 ++++++++++++++++++++- 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/cla-backend-go/v2/company/handlers.go b/cla-backend-go/v2/company/handlers.go index 7efea91f1..99bfd0a6a 100644 --- a/cla-backend-go/v2/company/handlers.go +++ b/cla-backend-go/v2/company/handlers.go @@ -24,6 +24,7 @@ import ( "github.com/linuxfoundation/easycla/cla-backend-go/gen/v2/restapi/operations/company" "github.com/linuxfoundation/easycla/cla-backend-go/utils" "github.com/linuxfoundation/easycla/cla-backend-go/v2/organization-service/client/organizations" + v2ProjectServiceClient "github.com/linuxfoundation/easycla/cla-backend-go/v2/project-service/client/project" ) // Configure sets up the middleware handlers @@ -181,6 +182,13 @@ func Configure(api *operations.EasyclaAPI, service Service, projectClaGroupRepo result, err := service.GetCompanyProjectCLAManagers(ctx, v2CompanyModel, params.ProjectSFID) if err != nil { + var projectNotFound *v2ProjectServiceClient.GetProjectNotFound + if errors.As(err, &projectNotFound) { + msg := fmt.Sprintf("project not found in the project service: %s", params.ProjectSFID) + log.WithFields(f).WithError(err).Warn(msg) + return company.NewGetCompanyProjectClaManagersNotFound().WithXRequestID(reqID).WithPayload(utils.ErrorResponseNotFound(reqID, msg)) + } + msg := "unable to load company project CLA managers" log.WithFields(f).WithError(err).Warn(msg) return company.NewGetCompanyProjectClaManagersBadRequest().WithXRequestID(reqID).WithPayload( diff --git a/cla-backend-go/v2/company/handlers_test.go b/cla-backend-go/v2/company/handlers_test.go index b050c8846..a5fbd9c0f 100644 --- a/cla-backend-go/v2/company/handlers_test.go +++ b/cla-backend-go/v2/company/handlers_test.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "testing" @@ -16,15 +17,37 @@ import ( "github.com/linuxfoundation/easycla/cla-backend-go/gen/v2/models" "github.com/linuxfoundation/easycla/cla-backend-go/gen/v2/restapi/operations" v2CompanyOps "github.com/linuxfoundation/easycla/cla-backend-go/gen/v2/restapi/operations/company" + v2ProjectServiceClient "github.com/linuxfoundation/easycla/cla-backend-go/v2/project-service/client/project" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +const ( + testCompanySFID = "0014100000Te0000AAE" + testOtherCompanySFID = "0014100000Te0000AAB" +) + type fakeCompanyService struct { Service result *models.CompanyClaGroups err error calls int + + company *models.Company + claManagersErr error + claManagerCalls int +} + +func (f *fakeCompanyService) GetCompanyByID(_ context.Context, _ string) (*models.Company, error) { + return f.company, nil +} + +func (f *fakeCompanyService) GetCompanyProjectCLAManagers(_ context.Context, _ *models.Company, _ string) (*models.CompanyClaManagers, error) { + f.claManagerCalls++ + if f.claManagersErr != nil { + return nil, f.claManagersErr + } + return &models.CompanyClaManagers{List: make([]*models.CompanyClaManager, 0)}, nil } func (f *fakeCompanyService) GetCompanyClaGroups(_ context.Context, companySFID string, _, _ *int64) (*models.CompanyClaGroups, error) { @@ -51,7 +74,7 @@ func respond(t *testing.T, api *operations.EasyclaAPI, companySFID string, authU } func TestGetCompanyClaGroupsHandler(t *testing.T) { - companySFID := "0014100000Te0000AAE" + companySFID := testCompanySFID testCases := []struct { name string @@ -118,3 +141,82 @@ func TestGetCompanyClaGroupsHandler(t *testing.T) { }) } } + +func respondProjectClaManagers(t *testing.T, api *operations.EasyclaAPI, companyID, projectSFID string, authUser *auth.User) int { + t.Helper() + require.NotNil(t, api.CompanyGetCompanyProjectClaManagersHandler) + responder := api.CompanyGetCompanyProjectClaManagersHandler.Handle(v2CompanyOps.GetCompanyProjectClaManagersParams{ + HTTPRequest: httptest.NewRequest(http.MethodGet, "/v4/company/"+companyID+"/project/"+projectSFID+"/cla-managers", nil), + CompanyID: companyID, + ProjectSFID: projectSFID, + }, authUser) + recorder := httptest.NewRecorder() + responder.WriteResponse(recorder, runtime.JSONProducer()) + return recorder.Code +} + +func TestGetCompanyProjectClaManagersHandler(t *testing.T) { + companyID := "company-uuid-1" + companySFID := testCompanySFID + projectSFID := "project-sfid-1" + orgUser := &auth.User{UserName: "org-user", ACL: auth.ACL{Allowed: true, Scopes: []auth.Scope{{Type: auth.Organization, ID: companySFID}}}} + + testCases := []struct { + name string + authUser *auth.User + serviceErr error + expectedStatus int + expectedCalls int + }{ + { + name: "success", + authUser: orgUser, + expectedStatus: http.StatusOK, + expectedCalls: 1, + }, + { + name: "project missing from the project service", + authUser: orgUser, + serviceErr: v2ProjectServiceClient.NewGetProjectNotFound(), + expectedStatus: http.StatusNotFound, + expectedCalls: 1, + }, + { + name: "wrapped project not found", + authUser: orgUser, + serviceErr: fmt.Errorf("loading CLA groups: %w", v2ProjectServiceClient.NewGetProjectNotFound()), + expectedStatus: http.StatusNotFound, + expectedCalls: 1, + }, + { + name: "any other service failure", + authUser: orgUser, + serviceErr: errors.New("dynamodb failure"), + expectedStatus: http.StatusBadRequest, + expectedCalls: 1, + }, + { + name: "permission check runs before the project lookup", + authUser: &auth.User{UserName: "other-user", ACL: auth.ACL{Allowed: true, Scopes: []auth.Scope{{Type: auth.Organization, ID: testOtherCompanySFID}}}}, + serviceErr: v2ProjectServiceClient.NewGetProjectNotFound(), + expectedStatus: http.StatusForbidden, + expectedCalls: 0, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + api := operations.NewEasyclaAPI(nil) + service := &fakeCompanyService{ + company: &models.Company{CompanyID: companyID, CompanyExternalID: companySFID}, + claManagersErr: tc.serviceErr, + } + Configure(api, service, nil, "") + + status := respondProjectClaManagers(t, api, companyID, projectSFID, tc.authUser) + + assert.Equal(t, tc.expectedStatus, status) + assert.Equal(t, tc.expectedCalls, service.claManagerCalls) + }) + } +}