From d17ee019461493f52df06074ce95a6cb951ffdc4 Mon Sep 17 00:00:00 2001 From: DenisGNoonan Date: Mon, 29 Jan 2024 15:43:34 +0000 Subject: [PATCH] NONRTRIC-946: Confirm ApiProvDomId for PUT equals path param Issue-ID: NONRTRIC-946 Change-Id: I407821d5394dc88e8cfda049973d46f360ad59b1 Signed-off-by: DenisGNoonan --- .../providermanagement/providermanagement.go | 11 ++-- .../providermanagement/providermanagement_test.go | 6 +- .../internal/publishservice/publishservice.go | 15 +++++ .../internal/publishservice/publishservice_test.go | 76 +++++++++++++++++++--- 4 files changed, 91 insertions(+), 17 deletions(-) diff --git a/capifcore/internal/providermanagement/providermanagement.go b/capifcore/internal/providermanagement/providermanagement.go index 51abd74..ebdfa9a 100644 --- a/capifcore/internal/providermanagement/providermanagement.go +++ b/capifcore/internal/providermanagement/providermanagement.go @@ -26,12 +26,11 @@ import ( "path" "sync" - "github.com/labstack/echo/v4" + echo "github.com/labstack/echo/v4" + log "github.com/sirupsen/logrus" "oransc.org/nonrtric/capifcore/internal/common29122" provapi "oransc.org/nonrtric/capifcore/internal/providermanagementapi" - - log "github.com/sirupsen/logrus" ) //go:generate mockery --name ServiceRegister @@ -154,9 +153,9 @@ func (pm *ProviderManager) PutRegistrationsRegistrationId(ctx echo.Context, regi } // Additional validation for PUT - if updatedProvider.ApiProvDomId == nil { - errDetail := "APIProviderEnrolmentDetails missing required ApiProvDomId" - return sendCoreError(ctx, http.StatusNotFound, fmt.Sprintf(errMsg, errDetail)) + if (updatedProvider.ApiProvDomId == nil) || (*updatedProvider.ApiProvDomId != registrationId) { + errDetail := "APIProviderEnrolmentDetails ApiProvDomId doesn't match path parameter" + return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, errDetail)) } if err = pm.updateProvider(updatedProvider, registeredProvider); err != nil { diff --git a/capifcore/internal/providermanagement/providermanagement_test.go b/capifcore/internal/providermanagement/providermanagement_test.go index 7d52729..cbcf438 100644 --- a/capifcore/internal/providermanagement/providermanagement_test.go +++ b/capifcore/internal/providermanagement/providermanagement_test.go @@ -62,7 +62,7 @@ func TestFailedUpdateValidProviderWithNewFunction(t *testing.T) { // Modify the provider updatedProvider := getProvider() - // We omit to set updatedProvider.ApiProvDomId + // For this test case, we do not set updatedProvider.ApiProvDomId, so that we can test for a 400 error below. (*updatedProvider.ApiProvFuncs)[0].ApiProvFuncId = &funcIdAPF (*updatedProvider.ApiProvFuncs)[1].ApiProvFuncId = &funcIdAMF (*updatedProvider.ApiProvFuncs)[2].ApiProvFuncId = &funcIdAEF @@ -82,13 +82,13 @@ func TestFailedUpdateValidProviderWithNewFunction(t *testing.T) { updatedProvider.ApiProvFuncs = &testFuncs result := testutil.NewRequest().Put("/registrations/"+domainID).WithJsonBody(updatedProvider).Go(t, requestHandler) - assert.Equal(t, http.StatusNotFound, result.Code()) + assert.Equal(t, http.StatusBadRequest, result.Code()) var resultError common29122.ProblemDetails err := result.UnmarshalJsonToObject(&resultError) assert.NoError(t, err, "error unmarshaling response") - assert.Contains(t, *resultError.Cause, "APIProviderEnrolmentDetails missing required ApiProvDomId") + assert.Contains(t, *resultError.Cause, "APIProviderEnrolmentDetails ApiProvDomId doesn't match path parameter") assert.False(t, managerUnderTest.IsFunctionRegistered("AEF_id_new_func_as_AEF")) } diff --git a/capifcore/internal/publishservice/publishservice.go b/capifcore/internal/publishservice/publishservice.go index bb065d2..b4e38e7 100644 --- a/capifcore/internal/publishservice/publishservice.go +++ b/capifcore/internal/publishservice/publishservice.go @@ -257,21 +257,33 @@ func (ps *PublishService) PutApfIdServiceApisServiceApiId(ctx echo.Context, apfI ps.lock.Lock() defer ps.lock.Unlock() errMsg := "Unable to update service due to %s." + pos, publishedService, err := ps.checkIfServiceIsPublished(apfId, serviceApiId, ctx) if err != nil { return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, err)) } + updatedServiceDescription, err := getServiceFromRequest(ctx) if err != nil { return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, err)) } + + // Additional validation for PUT + if (updatedServiceDescription.ApiId == nil) || (*updatedServiceDescription.ApiId != serviceApiId) { + errDetail := "ServiceAPIDescription ApiId doesn't match path parameter" + return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, errDetail)) + } + err = ps.checkProfilesRegistered(apfId, *updatedServiceDescription.AefProfiles) if err != nil { return sendCoreError(ctx, http.StatusBadRequest, fmt.Sprintf(errMsg, err)) } + ps.updateDescription(pos, apfId, &updatedServiceDescription, &publishedService) + publishedService.AefProfiles = updatedServiceDescription.AefProfiles ps.publishedServices[apfId][pos] = publishedService + err = ctx.JSON(http.StatusOK, publishedService) if err != nil { // Something really bad happened, tell Echo that our handler failed @@ -279,6 +291,7 @@ func (ps *PublishService) PutApfIdServiceApisServiceApiId(ctx echo.Context, apfI } return nil } + func (ps *PublishService) checkIfServiceIsPublished(apfId string, serviceApiId string, ctx echo.Context) (int, publishapi.ServiceAPIDescription, error) { publishedServices, ok := ps.publishedServices[apfId] if !ok { @@ -292,6 +305,7 @@ func (ps *PublishService) checkIfServiceIsPublished(apfId string, serviceApiId s } return 0, publishapi.ServiceAPIDescription{}, fmt.Errorf("service must be published before updating it") } + func getServiceFromRequest(ctx echo.Context) (publishapi.ServiceAPIDescription, error) { var updatedServiceDescription publishapi.ServiceAPIDescription err := ctx.Bind(&updatedServiceDescription) @@ -300,6 +314,7 @@ func getServiceFromRequest(ctx echo.Context) (publishapi.ServiceAPIDescription, } return updatedServiceDescription, nil } + func (ps *PublishService) updateDescription(pos int, apfId string, updatedServiceDescription, publishedService *publishapi.ServiceAPIDescription) { if updatedServiceDescription.Description != nil { publishedService.Description = updatedServiceDescription.Description diff --git a/capifcore/internal/publishservice/publishservice_test.go b/capifcore/internal/publishservice/publishservice_test.go index 6882bc9..670f273 100644 --- a/capifcore/internal/publishservice/publishservice_test.go +++ b/capifcore/internal/publishservice/publishservice_test.go @@ -332,6 +332,72 @@ func TestUpdateDescription(t *testing.T) { } } +func TestFailedUpdateDescription(t *testing.T) { + apfId := "apfId" + serviceApiId := "serviceApiId" + // Trying to update a different serviceApiId will cause a 400 error + updatedServiceApiId := "updatedServiceApiId" + aefId := "aefId" + apiName := "apiName" + description := "description" + + serviceRegisterMock := serviceMocks.ServiceRegister{} + serviceRegisterMock.On("GetAefsForPublisher", apfId).Return([]string{aefId, "otherAefId", "aefIdNew"}) + serviceRegisterMock.On("IsPublishingFunctionRegistered", apfId).Return(true) + helmManagerMock := helmMocks.HelmManager{} + helmManagerMock.On("InstallHelmChart", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + serviceUnderTest, _, requestHandler := getEcho(&serviceRegisterMock, &helmManagerMock) + serviceDescription := getServiceAPIDescription(aefId, apiName, description) + serviceDescription.ApiId = &serviceApiId + serviceUnderTest.publishedServices[apfId] = []publishapi.ServiceAPIDescription{serviceDescription} + (*serviceDescription.AefProfiles)[0].AefId = aefId + + // Modify the service + updatedServiceDescription := getServiceAPIDescription(aefId, apiName, description) + updatedServiceDescription.ApiId = &updatedServiceApiId + (*updatedServiceDescription.AefProfiles)[0].AefId = aefId + newDescription := "new description" + updatedServiceDescription.Description = &newDescription + newDomainName := "new domainName" + (*updatedServiceDescription.AefProfiles)[0].DomainName = &newDomainName + + newProfileDomain := "new profile Domain name" + var protocol publishapi.Protocol = "HTTP_1_1" + + test := append(*updatedServiceDescription.AefProfiles, publishapi.AefProfile{ + AefId: "aefIdNew", + DomainName: &newProfileDomain, + Protocol: &protocol, + Versions: []publishapi.Version{ + { + ApiVersion: "v1", + Resources: &[]publishapi.Resource{ + { + CommType: "REQUEST_RESPONSE", + Operations: &[]publishapi.Operation{ + "POST", + }, + ResourceName: "app", + Uri: "app", + }, + }, + }, + }, + }, + ) + updatedServiceDescription.AefProfiles = &test + + result := testutil.NewRequest().Put("/"+apfId+"/service-apis/"+serviceApiId).WithJsonBody(updatedServiceDescription).Go(t, requestHandler) + assert.Equal(t, http.StatusBadRequest, result.Code()) + + var resultError common29122.ProblemDetails + err := result.UnmarshalJsonToObject(&resultError) + assert.NoError(t, err, "error unmarshaling response") + + assert.Contains(t, *resultError.Cause, "ServiceAPIDescription ApiId doesn't match path parameter") + assert.Equal(t, http.StatusBadRequest, *resultError.Status) +} + func TestUpdateValidServiceWithDeletedFunction(t *testing.T) { apfId := "apfId" serviceApiId := "serviceApiId" @@ -352,10 +418,7 @@ func TestUpdateValidServiceWithDeletedFunction(t *testing.T) { newProfileDomain := "new profile Domain name" var protocol publishapi.Protocol = "HTTP_1_1" - test := make([]publishapi.AefProfile, 1) - test = *serviceDescription.AefProfiles - test = append(test, publishapi.AefProfile{ - + test := append(*serviceDescription.AefProfiles, publishapi.AefProfile{ AefId: "aefIdNew", DomainName: &newProfileDomain, Protocol: &protocol, @@ -382,10 +445,7 @@ func TestUpdateValidServiceWithDeletedFunction(t *testing.T) { //Modify the service updatedServiceDescription := getServiceAPIDescription(aefId, apiName, description) updatedServiceDescription.ApiId = &serviceApiId - test1 := make([]publishapi.AefProfile, 1) - test1 = *updatedServiceDescription.AefProfiles - test1 = append(test1, publishapi.AefProfile{ - + test1 := append(*updatedServiceDescription.AefProfiles, publishapi.AefProfile{ AefId: "aefIdNew", DomainName: &newProfileDomain, Protocol: &protocol, -- 2.16.6