From 41c2717f151e78706a07ef776ad77e2605f66b57 Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Mon, 12 Jun 2017 13:40:07 -0700 Subject: [PATCH] Add auth check for DeleteDestination (#220) * Add auth check for DeleteDestination * Use tenancy in deployment name for auth check --- common/auth_util.go | 21 +++++++++-------- common/auth_util_test.go | 38 +++++++++++++++---------------- services/frontendhost/frontend.go | 17 +++++++------- 3 files changed, 40 insertions(+), 36 deletions(-) diff --git a/common/auth_util.go b/common/auth_util.go index 3e74e75f..2e52eb38 100644 --- a/common/auth_util.go +++ b/common/auth_util.go @@ -27,7 +27,7 @@ import ( const ( resourceURNTemplateCreateDestination = "urn:cherami:dst:%v:%v" - resourceURNTemplateReadDestination = "urn:cherami:dst:%v:%v" + resourceURNTemplateOperateDestination = "urn:cherami:dst:%v:%v" resourceURNTemplateCreateConsumerGroup = "urn:cherami:cg:%v:%v" ) @@ -40,21 +40,19 @@ func GetResourceURNCreateDestination(scommon SCommon, dstPath *string) string { } else { dstPathString = getPathRootName(dstPath) } - deploymentName := scommon.GetConfig().GetDeploymentName() - return fmt.Sprintf(resourceURNTemplateCreateDestination, strings.ToLower(deploymentName), strings.ToLower(dstPathString)) + return fmt.Sprintf(resourceURNTemplateCreateDestination, getTenancyLowerCase(scommon), strings.ToLower(dstPathString)) } -// GetResourceURNReadDestination returns the resource URN to read destination, e.g. urn:cherami:dst:zone1_prod:/dst_prefix/dst1 +// GetResourceURNOperateDestination returns the resource URN to operate destination (read, delete), e.g. urn:cherami:dst:zone1_prod:/dst_prefix/dst1 // We use URN (Uniform Resource Name) like this: https://www.ietf.org/rfc/rfc2141.txt -func GetResourceURNReadDestination(scommon SCommon, dstPath *string) string { +func GetResourceURNOperateDestination(scommon SCommon, dstPath *string) string { var dstPathString string if dstPath == nil { dstPathString = "" } else { dstPathString = *dstPath } - deploymentName := scommon.GetConfig().GetDeploymentName() - return fmt.Sprintf(resourceURNTemplateReadDestination, strings.ToLower(deploymentName), strings.ToLower(dstPathString)) + return fmt.Sprintf(resourceURNTemplateOperateDestination, getTenancyLowerCase(scommon), strings.ToLower(dstPathString)) } // GetResourceURNCreateConsumerGroup returns the resource URN to create consumer group, e.g. urn:cherami:dst:zone1_prod:/cg_prefix @@ -66,8 +64,7 @@ func GetResourceURNCreateConsumerGroup(scommon SCommon, cgPath *string) string { } else { cgPathString = getPathRootName(cgPath) } - deploymentName := scommon.GetConfig().GetDeploymentName() - return fmt.Sprintf(resourceURNTemplateCreateConsumerGroup, strings.ToLower(deploymentName), strings.ToLower(cgPathString)) + return fmt.Sprintf(resourceURNTemplateCreateConsumerGroup, getTenancyLowerCase(scommon), strings.ToLower(cgPathString)) } func getPathRootName(path *string) string { @@ -83,3 +80,9 @@ func getPathRootName(path *string) string { return parts[0] } + +func getTenancyLowerCase(scommon SCommon) string { + deploymentName := scommon.GetConfig().GetDeploymentName() + parts := strings.Split(deploymentName, "_") + return strings.ToLower(parts[0]) +} diff --git a/common/auth_util_test.go b/common/auth_util_test.go index b3d7f652..b5964231 100644 --- a/common/auth_util_test.go +++ b/common/auth_util_test.go @@ -66,35 +66,35 @@ func (s *AuthUtilSuite) TestGetResourceURNCreateDestination() { s.Equal("urn:cherami:dst:zone1:/", GetResourceURNCreateDestination(mockService, StringPtr("//"))) config.deploymentName = "Zone2_ABC" - s.Equal("urn:cherami:dst:zone2_abc:/dst1", GetResourceURNCreateDestination(mockService, StringPtr("/Dst1"))) - s.Equal("urn:cherami:dst:zone2_abc:/root2", GetResourceURNCreateDestination(mockService, StringPtr("/Root2/Dst2"))) + s.Equal("urn:cherami:dst:zone2:/dst1", GetResourceURNCreateDestination(mockService, StringPtr("/Dst1"))) + s.Equal("urn:cherami:dst:zone2:/root2", GetResourceURNCreateDestination(mockService, StringPtr("/Root2/Dst2"))) - s.Equal("urn:cherami:dst:zone2_abc:dst2", GetResourceURNCreateDestination(mockService, StringPtr("Dst2"))) - s.Equal("urn:cherami:dst:zone2_abc:root2", GetResourceURNCreateDestination(mockService, StringPtr("Root2/Dst2"))) + s.Equal("urn:cherami:dst:zone2:dst2", GetResourceURNCreateDestination(mockService, StringPtr("Dst2"))) + s.Equal("urn:cherami:dst:zone2:root2", GetResourceURNCreateDestination(mockService, StringPtr("Root2/Dst2"))) } -func (s *AuthUtilSuite) TestGetResourceURNReadDestination() { +func (s *AuthUtilSuite) TestGetResourceURNOperateDestination() { mockService := new(MockService) config := &serviceConfig{} mockService.On("GetConfig").Return(config) - s.Equal("urn:cherami:dst::", GetResourceURNReadDestination(mockService, nil)) - s.Equal("urn:cherami:dst::", GetResourceURNReadDestination(mockService, StringPtr(""))) + s.Equal("urn:cherami:dst::", GetResourceURNOperateDestination(mockService, nil)) + s.Equal("urn:cherami:dst::", GetResourceURNOperateDestination(mockService, StringPtr(""))) config.deploymentName = "zone1" - s.Equal("urn:cherami:dst:zone1:", GetResourceURNReadDestination(mockService, nil)) - s.Equal("urn:cherami:dst:zone1:", GetResourceURNReadDestination(mockService, StringPtr(""))) - s.Equal("urn:cherami:dst:zone1:/", GetResourceURNReadDestination(mockService, StringPtr("/"))) - s.Equal("urn:cherami:dst:zone1://", GetResourceURNReadDestination(mockService, StringPtr("//"))) + s.Equal("urn:cherami:dst:zone1:", GetResourceURNOperateDestination(mockService, nil)) + s.Equal("urn:cherami:dst:zone1:", GetResourceURNOperateDestination(mockService, StringPtr(""))) + s.Equal("urn:cherami:dst:zone1:/", GetResourceURNOperateDestination(mockService, StringPtr("/"))) + s.Equal("urn:cherami:dst:zone1://", GetResourceURNOperateDestination(mockService, StringPtr("//"))) config.deploymentName = "Zone2_ABC" - s.Equal("urn:cherami:dst:zone2_abc:/dst1", GetResourceURNReadDestination(mockService, StringPtr("/Dst1"))) - s.Equal("urn:cherami:dst:zone2_abc:/root2/dst2", GetResourceURNReadDestination(mockService, StringPtr("/Root2/Dst2"))) + s.Equal("urn:cherami:dst:zone2:/dst1", GetResourceURNOperateDestination(mockService, StringPtr("/Dst1"))) + s.Equal("urn:cherami:dst:zone2:/root2/dst2", GetResourceURNOperateDestination(mockService, StringPtr("/Root2/Dst2"))) - s.Equal("urn:cherami:dst:zone2_abc:dst2", GetResourceURNReadDestination(mockService, StringPtr("Dst2"))) - s.Equal("urn:cherami:dst:zone2_abc:root2/dst2", GetResourceURNReadDestination(mockService, StringPtr("Root2/Dst2"))) + s.Equal("urn:cherami:dst:zone2:dst2", GetResourceURNOperateDestination(mockService, StringPtr("Dst2"))) + s.Equal("urn:cherami:dst:zone2:root2/dst2", GetResourceURNOperateDestination(mockService, StringPtr("Root2/Dst2"))) } func (s *AuthUtilSuite) TestGetResourceURNCreateConsumerGroup() { @@ -114,9 +114,9 @@ func (s *AuthUtilSuite) TestGetResourceURNCreateConsumerGroup() { s.Equal("urn:cherami:cg:zone1:/", GetResourceURNCreateConsumerGroup(mockService, StringPtr("//"))) config.deploymentName = "Zone2_ABC" - s.Equal("urn:cherami:cg:zone2_abc:/dst1", GetResourceURNCreateConsumerGroup(mockService, StringPtr("/Dst1"))) - s.Equal("urn:cherami:cg:zone2_abc:/root2", GetResourceURNCreateConsumerGroup(mockService, StringPtr("/Root2/Dst2"))) + s.Equal("urn:cherami:cg:zone2:/dst1", GetResourceURNCreateConsumerGroup(mockService, StringPtr("/Dst1"))) + s.Equal("urn:cherami:cg:zone2:/root2", GetResourceURNCreateConsumerGroup(mockService, StringPtr("/Root2/Dst2"))) - s.Equal("urn:cherami:cg:zone2_abc:dst2", GetResourceURNCreateConsumerGroup(mockService, StringPtr("Dst2"))) - s.Equal("urn:cherami:cg:zone2_abc:root2", GetResourceURNCreateConsumerGroup(mockService, StringPtr("Root2/Dst2"))) + s.Equal("urn:cherami:cg:zone2:dst2", GetResourceURNCreateConsumerGroup(mockService, StringPtr("Dst2"))) + s.Equal("urn:cherami:cg:zone2:root2", GetResourceURNCreateConsumerGroup(mockService, StringPtr("Root2/Dst2"))) } diff --git a/services/frontendhost/frontend.go b/services/frontendhost/frontend.go index 2b02f4dd..30c61d06 100644 --- a/services/frontendhost/frontend.go +++ b/services/frontendhost/frontend.go @@ -669,16 +669,17 @@ func (h *Frontend) DeleteDestination(ctx thrift.Context, deleteRequest *c.Delete return } - // Disallow delete destination for non-test destinations without a password - // TODO: remove when appropriate authentication is in place + lclLg := h.logger.WithField(common.TagDstPth, common.FmtDstPth(deleteRequest.GetPath())) + + // To keep backward compatiblity, only check auth when no password is provided for DeleteDestination if !allowMutate { - err = &c.BadRequestError{Message: fmt.Sprintf("Contact Cherami team to delete this path: %v", deleteRequest.GetPath())} - h.logger.WithField(common.TagErr, err).Error("DeleteDestination failed") - return + authResource := common.GetResourceURNOperateDestination(h.SCommon, deleteRequest.Path) + err = h.checkAuth(ctx, authResource, common.OperationDelete, lclLg) + if err != nil { + return err + } } - lclLg := h.logger.WithField(common.TagDstPth, common.FmtDstPth(deleteRequest.GetPath())) - // Request to controller cClient, err := h.getControllerClient() if err != nil { @@ -1117,7 +1118,7 @@ func (h *Frontend) CreateConsumerGroup(ctx thrift.Context, createRequest *c.Crea }) // Check auth for read destination - authResource := common.GetResourceURNReadDestination(h.SCommon, createRequest.DestinationPath) + authResource := common.GetResourceURNOperateDestination(h.SCommon, createRequest.DestinationPath) err = h.checkAuth(ctx, authResource, common.OperationRead, lclLg) if err != nil { return nil, err