Skip to content

Commit

Permalink
avoid failing the request if sign fail (#756)
Browse files Browse the repository at this point in the history
* avoid failing the request if sign fail

* avoid failing the request if sign fail

* avoid failing the request if sign fail

* avoid failing the request if sign fail
  • Loading branch information
I065450 authored Apr 3, 2022
1 parent 67c0d85 commit e5239e9
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 9 deletions.
8 changes: 4 additions & 4 deletions api/osb/context_signature_plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ func (s *ContextSignaturePlugin) signContext(req *web.Request, next web.Handler)
err := json.Unmarshal(req.Body, &reqBodyMap)
if err != nil {
log.C(req.Context()).Errorf("failed to unmarshal context: %v", err)
return nil, err
return next.Handle(req)
}
if _, found := reqBodyMap["context"]; !found {
errorMsg := "context not found on request body"
log.C(req.Context()).Error(errorMsg)
return nil, fmt.Errorf(errorMsg)
return next.Handle(req)
}
contextMap := reqBodyMap["context"].(map[string]interface{})

Expand All @@ -79,13 +79,13 @@ func (s *ContextSignaturePlugin) signContext(req *web.Request, next web.Handler)
err = s.contextSigner.Sign(req.Context(), contextMap)
if err != nil {
log.C(req.Context()).Errorf("failed to sign request context: %v", err)
return nil, err
return next.Handle(req)
}

reqBody, err := json.Marshal(reqBodyMap)
if err != nil {
log.C(req.Context()).Errorf("failed to marshal request body: %v", err)
return nil, err
return next.Handle(req)
}
req.Body = reqBody

Expand Down
1 change: 0 additions & 1 deletion storage/interceptors/smaap_service_binding_interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,6 @@ func (i *ServiceBindingInterceptor) prepareBindRequest(ctx context.Context, inst
err := i.contextSigner.Sign(ctx, context)
if err != nil {
log.C(ctx).Errorf("failed to sign context: %v", err)
return nil, err
}
}

Expand Down
2 changes: 0 additions & 2 deletions storage/interceptors/smaap_service_instance_interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,6 @@ func (i *ServiceInstanceInterceptor) prepareProvisionRequest(ctx context.Context
err = i.contextSigner.Sign(ctx, instanceContext)
if err != nil {
log.C(ctx).Errorf("failed to sign context: %v", err)
return nil, err
}
}

Expand Down Expand Up @@ -877,7 +876,6 @@ func (i *ServiceInstanceInterceptor) prepareUpdateInstanceRequest(ctx context.Co
err := i.contextSigner.Sign(ctx, instanceContext)
if err != nil {
log.C(ctx).Errorf("failed to sign context: %v", err)
return nil, err
}
}

Expand Down
38 changes: 38 additions & 0 deletions test/common/context_signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,29 @@ func GetVerifyContextHandlerFunc(publicKeyStr string) func(http.ResponseWriter,
}
}

func GetVerifyContextInvalidKeyHandlerFunc() func(http.ResponseWriter, *http.Request) {
return func(rw http.ResponseWriter, r *http.Request) {
defer GinkgoRecover()
bytes, err := util.BodyToBytes(r.Body)
if err != nil {
Expect(err).ToNot(HaveOccurred())
}
signatureBytes := gjson.GetBytes(bytes, "context.signature")
Expect(signatureBytes.Exists()).To(Equal(false), "context should not have a signature field")

responseStatus := http.StatusCreated
if r.Method == http.MethodGet || r.Method == http.MethodPatch {
responseStatus = http.StatusOK
}
if err := util.WriteJSON(rw, responseStatus, Object{}); err != nil {
rw.WriteHeader(http.StatusInternalServerError)
if _, errWrite := rw.Write([]byte(err.Error())); errWrite != nil {
Expect(errWrite).ToNot(HaveOccurred())
}
}
}
}

func VerifySignatureNotPersisted(ctx *TestContext, objType types.ObjectType, id string) {
obj, err := ctx.SMRepository.Get(context.TODO(), objType, query.ByField(query.EqualsOperator, "id", id))
Expect(err).ToNot(HaveOccurred(), "failed to get "+objType.String()+" from db")
Expand Down Expand Up @@ -168,3 +191,18 @@ func ProvisionInstanceAndVerifySignature(ctx *TestContext, brokerServer *BrokerS

return instanceID
}

func ProvisionInstanceWithoutSignature(ctx *TestContext, brokerServer *BrokerServer, provisionFunc func() string) string {
brokerServer.ServiceInstanceHandler = GetVerifyContextInvalidKeyHandlerFunc()

instanceID := provisionFunc()

VerifySignatureNotPersisted(ctx, types.ServiceInstanceType, instanceID)

ctx.SMWithOAuthForTenant.GET(web.ServiceInstancesURL + "/" + instanceID).
Expect().Status(http.StatusOK).
JSON().
Object().Value("context").Object().NotContainsKey("signature")

return instanceID
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package context_signature

import (
"fmt"
"github.com/Peripli/service-manager/pkg/env"
"github.com/Peripli/service-manager/pkg/types"
"github.com/Peripli/service-manager/pkg/web"
"github.com/Peripli/service-manager/test/common"
Expand Down Expand Up @@ -59,7 +60,6 @@ var _ = Describe("context signature verification tests", func() {
})
})
})

Context("SMAAP", func() {
var provisionFunc func() string
BeforeEach(func() {
Expand Down Expand Up @@ -101,4 +101,26 @@ var _ = Describe("context signature verification tests", func() {
})
})
})

Context("when the private key is invalid", func() {
instanceID := "signed-ctx-instance-invalid"
var provisionFunc func() string
BeforeEach(func() {
ctx = common.NewTestContextBuilderWithSecurity().WithEnvPostExtensions(func(e env.Environment, servers map[string]common.FakeServer) {
e.Set("api.osb_rsa_private_key", "invalidKey")
}).Build()
registerServiceBroker()
})

It("should not fail the request,osb request sent without the signature", func() {
provisionFunc = common.GetOsbProvisionFunc(ctx, instanceID, osbURL, catalogServiceID, catalogPlanID)
common.ProvisionInstanceWithoutSignature(ctx, brokerServer, provisionFunc)
})
It("should not fail the request,SMAAP request sent without the signature", func() {
provisionFunc = common.GetSMAAPProvisionInstanceFunc(ctx, "false", planID)
common.ProvisionInstanceWithoutSignature(ctx, brokerServer, provisionFunc)
})

})

})
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ var _ = BeforeSuite(func() {
e.Set("api.osb_rsa_public_key", publicKeyStr)
e.Set("api.osb_successor_rsa_public_key", publicSuccessorKeyStr)
}).Build()
registerServiceBroker()
})

func registerServiceBroker() {
UUID, err := uuid.NewV4()
Expect(err).ToNot(HaveOccurred())
catalogPlanID = UUID.String()
Expand All @@ -67,7 +71,7 @@ var _ = BeforeSuite(func() {
serviceID = serviceOfferings.Object().Value("id").String().Raw()
servicePlans := ctx.SMWithBasic.ListWithQuery(web.ServicePlansURL, "fieldQuery="+fmt.Sprintf("service_offering_id eq '%s'", serviceID))
planID = servicePlans.Element(0).Object().Value("id").String().Raw()
})
}

var _ = AfterSuite(func() {
ctx.Cleanup()
Expand Down

0 comments on commit e5239e9

Please sign in to comment.