NONRTRIC-946: Confirm ApiProvDomId for PUT equals path param

Issue-ID: NONRTRIC-946
Change-Id: I407821d5394dc88e8cfda049973d46f360ad59b1
Signed-off-by: DenisGNoonan <denis.noonan@est.tech>

diff --git a/capifcore/internal/providermanagement/providermanagement.go b/capifcore/internal/providermanagement/providermanagement.go
index 51abd74..ebdfa9a 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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,