Repository: cloud-custodian/cloud-custodian

Issue:
aws - service catalog portfolios public sharing
**Is your feature request related to a problem? Please describe.**
We want to make sure that service catalog portfolio is not shared with external accounts.

**Describe the solution you'd like**
1. In servicecatalog.py create a `resource: catalog-portfolio`
2. Create a `cross-account` filter for `resource: catalog-portfolio`. The filter would use list_portfolio_access api(https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/servicecatalog.html#ServiceCatalog.Client.list_portfolio_access) which returns a list of account id.
3. The returned list of account_id would be matched with whitelisted (whitelist and whitelist_from) accounts in the policy

Sample Policy:
```
policies:
  - name: portfolio-cross-account
    resource: catalog-portfolio
    filters:
      - type: cross-account
        whitelist_from:
          expr: "abc.*.accountnumber"
          url: *accounts_url
```

**Describe alternatives you've considered**
A clear and concise description of any alternative solutions or features you've considered.

**Additional context**
Currently portfolios can only be shared with its own ORGANIZATION or ORGANIZATIONAL_UNIT which should not be a risk imo. But it can be shared with external accounts, hence the feature request


Discussion:
PratMis: Thanks for the feedback @howbazaar , I'll address them shortly
PratMis: Verified that a portfolio cannot be deleted if its shared with external AWS accounts with error shown below:
```botocore.errorfactory.ResourceInUseException: An error occurred (ResourceInUseException) when calling the DeletePortfolio operation: Portfolio port-hlgxpz7lc55iw is still shared with other accounts.```
I went ahead and combined the two tests to reduce extraneous data recorded. Let me know if there's anything else that needs to be changed. Thanks
PratMis: @kapilt are we okay to move forward with this? Please let me know if we need more changes. Thanks

Patch:
diff --git a/c7n/resources/resource_map.py b/c7n/resources/resource_map.py
index b53ec8935..ca92bd35f 100644
--- a/c7n/resources/resource_map.py
+++ b/c7n/resources/resource_map.py
@@ -13,6 +13,7 @@ ResourceMap = {
     "aws.cache-cluster": "c7n.resources.elasticache.ElastiCacheCluster",
     "aws.cache-snapshot": "c7n.resources.elasticache.ElastiCacheSnapshot",
     "aws.cache-subnet-group": "c7n.resources.elasticache.ElastiCacheSubnetGroup",
+    "aws.catalog-portfolio": "c7n.resources.servicecatalog.CatalogPortfolio",
     "aws.cfn": "c7n.resources.cfn.CloudFormation",
     "aws.cloud-directory": "c7n.resources.directory.CloudDirectory",
     "aws.cloudhsm-cluster": "c7n.resources.hsm.CloudHSMCluster",
diff --git a/c7n/resources/servicecatalog.py b/c7n/resources/servicecatalog.py
new file mode 100644
index 000000000..18c780a7e
--- /dev/null
+++ b/c7n/resources/servicecatalog.py
@@ -0,0 +1,172 @@
+# Copyright The Cloud Custodian Authors.
+# SPDX-License-Identifier: Apache-2.0
+
+
+from c7n.actions import BaseAction
+from c7n.filters import CrossAccountAccessFilter
+from c7n.query import ConfigSource, DescribeSource, QueryResourceManager, TypeInfo
+from c7n.manager import resources
+from c7n.exceptions import PolicyValidationError
+from c7n.tags import universal_augment
+from c7n.utils import local_session, type_schema
+
+
+class DescribePortfolio(DescribeSource):
+
+    def augment(self, resources):
+        return universal_augment(self.manager, super().augment(resources))
+
+
+@resources.register('catalog-portfolio')
+class CatalogPortfolio(QueryResourceManager):
+
+    class resource_type(TypeInfo):
+        service = 'servicecatalog'
+        enum_spec = ('list_portfolios', 'PortfolioDetails', None)
+        detail_spec = ('describe_portfolio', 'Id', 'Id', None)
+        arn = 'ARN'
+        id = 'Id'
+        name = 'DisplayName'
+        date = 'CreatedTime'
+        universal_taggable = object()
+        cfn_type = config_type = 'AWS::ServiceCatalog::Portfolio'
+
+    source_mapping = {
+        'describe': DescribePortfolio,
+        'config': ConfigSource
+    }
+
+
+@CatalogPortfolio.action_registry.register('delete')
+class CatalogPortfolioDeleteAction(BaseAction):
+    """Action to delete a Service Catalog Portfolio
+
+    :example:
+
+    .. code-block:: yaml
+
+        policies:
+          - name: service-catalog-portfolio-delete
+            resource: aws.catalog-portfolio
+            filters:
+              - type: cross-account
+            actions:
+              - delete
+    """
+
+    schema = type_schema('delete')
+    permissions = ('servicecatalog:DeletePortfolio',)
+
+    def process(self, portfolios):
+        client = local_session(self.manager.session_factory).client('servicecatalog')
+        for r in portfolios:
+            self.manager.retry(client.delete_portfolio, Id=r['Id'], ignore_err_codes=(
+                'ResourceNotFoundException',))
+
+
+@CatalogPortfolio.filter_registry.register('cross-account')
+class CatalogPortfolioCrossAccount(CrossAccountAccessFilter):
+    """Check for account ids that the service catalog portfolio is shared with
+
+    :example:
+
+    .. code-block:: yaml
+
+            policies:
+              - name: catalog-portfolio-cross-account
+                resource: aws.catalog-portfolio
+                filters:
+                  - type: cross-account
+    """
+
+    schema = type_schema(
+        'cross-account',
+        whitelist_accounts_from={'$ref': '#/definitions/filters_common/value_from'},
+        whitelist={'type': 'array', 'items': {'type': 'string'}})
+
+    permissions = ('servicecatalog:ListPortfolioAccess',)
+    annotation_key = 'c7n:CrossAccountViolations'
+
+    def check_access(self, client, accounts, resources):
+        results = []
+        shared_accounts = set()
+        for r in resources:
+            accounts = self.manager.retry(
+                client.list_portfolio_access, PortfolioId=r['Id'], ignore_err_codes=(
+                    'ResourceNotFoundException',))
+            if not accounts:
+                continue
+            shared_accounts = set(accounts.get('AccountIds'))
+            delta_accounts = shared_accounts.difference(accounts)
+            if delta_accounts:
+                r[self.annotation_key] = list(delta_accounts)
+                results.append(r)
+        return results
+
+    def process(self, resources, event=None):
+        results = []
+        client = local_session(self.manager.session_factory).client('servicecatalog')
+        accounts = self.get_accounts()
+        results.extend(self.check_access(client, accounts, resources))
+        return results
+
+
+@CatalogPortfolio.action_registry.register('remove-shared-accounts')
+class RemoveSharedAccounts(BaseAction):
+    """Action to delete Portfolio share with other accounts
+
+    :example:
+
+    .. code-block:: yaml
+
+            policies:
+              - name: catalog-portfolio-delete-share
+                resource: aws.catalog-portfolio
+                filters:
+                  - type: cross-account
+                actions:
+                  - type: remove-shared-accounts
+                    accounts: matched
+
+              - name: catalog-portfolio-delete-shared-account
+                resource: aws.catalog-portfolio
+                filters:
+                  - type: cross-account
+                actions:
+                  - type: remove-shared-accounts
+                    accounts: ['123456789123']
+    """
+
+    schema = type_schema(
+        'remove-shared-accounts',
+        accounts={'oneOf': [
+            {'enum': ['matched']},
+            {'type': 'array', 'items': {'type': 'string', 'pattern': '^[0-9]{12}$'}}]},
+        required=['accounts'])
+
+    permissions = ('servicecatalog:DeletePortfolioShare',)
+
+    def validate(self):
+        if self.data['accounts'] != 'matched':
+            return
+        found = False
+        for f in self.manager.iter_filters():
+            if isinstance(f, CatalogPortfolioCrossAccount):
+                found = True
+                break
+        if not found:
+            raise PolicyValidationError(
+                "policy:%s action:%s with matched requires cross-account filter" % (
+                    self.manager.ctx.policy.name, self.type))
+
+    def delete_shared_accounts(self, client, portfolio):
+        accounts = self.data.get('accounts')
+        if accounts == 'matched':
+            accounts = portfolio.get(CatalogPortfolioCrossAccount.annotation_key)
+        for account in accounts:
+            client.delete_portfolio_share(PortfolioId=portfolio['Id'], AccountId=account)
+
+    def process(self, portfolios):
+        client = local_session(self.manager.session_factory).client('servicecatalog')
+        for p in portfolios:
+            self.delete_shared_accounts(client, p)


Test patch:
diff --git a/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.DeletePortfolioShare_1.json b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.DeletePortfolioShare_1.json
new file mode 100644
index 000000000..5b2170a07
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.DeletePortfolioShare_1.json
@@ -0,0 +1,6 @@
+{
+    "status_code": 200,
+    "data": {
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.DeletePortfolio_1.json b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.DeletePortfolio_1.json
new file mode 100644
index 000000000..5b2170a07
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.DeletePortfolio_1.json
@@ -0,0 +1,6 @@
+{
+    "status_code": 200,
+    "data": {
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.DescribePortfolio_1.json b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.DescribePortfolio_1.json
new file mode 100644
index 000000000..c65807096
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.DescribePortfolio_1.json
@@ -0,0 +1,26 @@
+{
+    "status_code": 200,
+    "data": {
+        "PortfolioDetail": {
+            "Id": "port-3cfur4nmnvf4u",
+            "ARN": "arn:aws:catalog:us-east-1:644160558196:portfolio/port-3cfur4nmnvf4u",
+            "DisplayName": "c7n-test",
+            "Description": "Test deletion ",
+            "CreatedTime": {
+                "__class__": "datetime",
+                "year": 2020,
+                "month": 10,
+                "day": 10,
+                "hour": 13,
+                "minute": 34,
+                "second": 50,
+                "microsecond": 112000
+            },
+            "ProviderName": "pratyush"
+        },
+        "Tags": [],
+        "TagOptions": [],
+        "Budgets": [],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolioAccess_1.json b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolioAccess_1.json
new file mode 100644
index 000000000..bc501284c
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolioAccess_1.json
@@ -0,0 +1,9 @@
+{
+    "status_code": 200,
+    "data": {
+        "AccountIds": [
+            "644160558196"
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolioAccess_2.json b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolioAccess_2.json
new file mode 100644
index 000000000..bc501284c
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolioAccess_2.json
@@ -0,0 +1,9 @@
+{
+    "status_code": 200,
+    "data": {
+        "AccountIds": [
+            "644160558196"
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolios_1.json b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolios_1.json
new file mode 100644
index 000000000..a0e90b829
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolios_1.json
@@ -0,0 +1,25 @@
+{
+    "status_code": 200,
+    "data": {
+        "PortfolioDetails": [
+            {
+                "Id": "port-3cfur4nmnvf4u",
+                "ARN": "arn:aws:catalog:us-east-1:644160558196:portfolio/port-3cfur4nmnvf4u",
+                "DisplayName": "c7n-test",
+                "Description": "Test deletion ",
+                "CreatedTime": {
+                    "__class__": "datetime",
+                    "year": 2020,
+                    "month": 10,
+                    "day": 10,
+                    "hour": 13,
+                    "minute": 34,
+                    "second": 50,
+                    "microsecond": 112000
+                },
+                "ProviderName": "pratyush"
+            }
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolios_2.json b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolios_2.json
new file mode 100644
index 000000000..8ab12225d
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_cross_account_remove_delete/servicecatalog.ListPortfolios_2.json
@@ -0,0 +1,25 @@
+{
+    "status_code": 200,
+    "data": {
+        "PortfolioDetails": [
+            {
+                "Id": "port-srkytozjwbzpc",
+                "ARN": "arn:aws:catalog:us-east-1:644160558196:portfolio/port-srkytozjwbzpc",
+                "DisplayName": "dev",
+                "Description": "dev",
+                "CreatedTime": {
+                    "__class__": "datetime",
+                    "year": 2019,
+                    "month": 5,
+                    "day": 14,
+                    "hour": 11,
+                    "minute": 34,
+                    "second": 1,
+                    "microsecond": 554000
+                },
+                "ProviderName": "kapilt"
+            }
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_cross_account_remove_delete/tagging.GetResources_1.json b/tests/data/placebo/test_portfolio_cross_account_remove_delete/tagging.GetResources_1.json
new file mode 100644
index 000000000..8b704d185
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_cross_account_remove_delete/tagging.GetResources_1.json
@@ -0,0 +1,8 @@
+{
+    "status_code": 200,
+    "data": {
+        "PaginationToken": "",
+        "ResourceTagMappingList": [],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DeletePortfolioShare_1.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DeletePortfolioShare_1.json
new file mode 100644
index 000000000..5b2170a07
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DeletePortfolioShare_1.json
@@ -0,0 +1,6 @@
+{
+    "status_code": 200,
+    "data": {
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DeletePortfolioShare_2.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DeletePortfolioShare_2.json
new file mode 100644
index 000000000..5b2170a07
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DeletePortfolioShare_2.json
@@ -0,0 +1,6 @@
+{
+    "status_code": 200,
+    "data": {
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DescribePortfolio_1.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DescribePortfolio_1.json
new file mode 100644
index 000000000..4b550a864
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DescribePortfolio_1.json
@@ -0,0 +1,26 @@
+{
+    "status_code": 200,
+    "data": {
+        "PortfolioDetail": {
+            "Id": "port-hlgxpz7lc55iw",
+            "ARN": "arn:aws:catalog:us-east-1:644160558196:portfolio/port-hlgxpz7lc55iw",
+            "DisplayName": "c7n-test",
+            "Description": "test cross account filter",
+            "CreatedTime": {
+                "__class__": "datetime",
+                "year": 2020,
+                "month": 9,
+                "day": 29,
+                "hour": 21,
+                "minute": 8,
+                "second": 39,
+                "microsecond": 484000
+            },
+            "ProviderName": "pratyush"
+        },
+        "Tags": [],
+        "TagOptions": [],
+        "Budgets": [],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DescribePortfolio_2.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DescribePortfolio_2.json
new file mode 100644
index 000000000..b63c1178e
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.DescribePortfolio_2.json
@@ -0,0 +1,26 @@
+{
+    "status_code": 200,
+    "data": {
+        "PortfolioDetail": {
+            "Id": "port-srkytozjwbzpc",
+            "ARN": "arn:aws:catalog:us-east-1:644160558196:portfolio/port-srkytozjwbzpc",
+            "DisplayName": "dev",
+            "Description": "dev",
+            "CreatedTime": {
+                "__class__": "datetime",
+                "year": 2019,
+                "month": 5,
+                "day": 14,
+                "hour": 11,
+                "minute": 34,
+                "second": 1,
+                "microsecond": 554000
+            },
+            "ProviderName": "kapilt"
+        },
+        "Tags": [],
+        "TagOptions": [],
+        "Budgets": [],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_1.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_1.json
new file mode 100644
index 000000000..bc501284c
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_1.json
@@ -0,0 +1,9 @@
+{
+    "status_code": 200,
+    "data": {
+        "AccountIds": [
+            "644160558196"
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_2.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_2.json
new file mode 100644
index 000000000..bc501284c
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_2.json
@@ -0,0 +1,9 @@
+{
+    "status_code": 200,
+    "data": {
+        "AccountIds": [
+            "644160558196"
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_3.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_3.json
new file mode 100644
index 000000000..bc501284c
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_3.json
@@ -0,0 +1,9 @@
+{
+    "status_code": 200,
+    "data": {
+        "AccountIds": [
+            "644160558196"
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_4.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_4.json
new file mode 100644
index 000000000..bc501284c
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_4.json
@@ -0,0 +1,9 @@
+{
+    "status_code": 200,
+    "data": {
+        "AccountIds": [
+            "644160558196"
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_5.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_5.json
new file mode 100644
index 000000000..26ff4c68d
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_5.json
@@ -0,0 +1,7 @@
+{
+    "status_code": 200,
+    "data": {
+        "AccountIds": [],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_6.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_6.json
new file mode 100644
index 000000000..bc501284c
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolioAccess_6.json
@@ -0,0 +1,9 @@
+{
+    "status_code": 200,
+    "data": {
+        "AccountIds": [
+            "644160558196"
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolios_1.json b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolios_1.json
new file mode 100644
index 000000000..729ec91e7
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/servicecatalog.ListPortfolios_1.json
@@ -0,0 +1,42 @@
+{
+    "status_code": 200,
+    "data": {
+        "PortfolioDetails": [
+            {
+                "Id": "port-hlgxpz7lc55iw",
+                "ARN": "arn:aws:catalog:us-east-1:644160558196:portfolio/port-hlgxpz7lc55iw",
+                "DisplayName": "c7n-test",
+                "Description": "test cross account filter",
+                "CreatedTime": {
+                    "__class__": "datetime",
+                    "year": 2020,
+                    "month": 9,
+                    "day": 29,
+                    "hour": 21,
+                    "minute": 8,
+                    "second": 39,
+                    "microsecond": 484000
+                },
+                "ProviderName": "pratyush"
+            },
+            {
+                "Id": "port-srkytozjwbzpc",
+                "ARN": "arn:aws:catalog:us-east-1:644160558196:portfolio/port-srkytozjwbzpc",
+                "DisplayName": "dev",
+                "Description": "dev",
+                "CreatedTime": {
+                    "__class__": "datetime",
+                    "year": 2019,
+                    "month": 5,
+                    "day": 14,
+                    "hour": 11,
+                    "minute": 34,
+                    "second": 1,
+                    "microsecond": 554000
+                },
+                "ProviderName": "kapilt"
+            }
+        ],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/data/placebo/test_portfolio_remove_share_accountid/tagging.GetResources_1.json b/tests/data/placebo/test_portfolio_remove_share_accountid/tagging.GetResources_1.json
new file mode 100644
index 000000000..8b704d185
--- /dev/null
+++ b/tests/data/placebo/test_portfolio_remove_share_accountid/tagging.GetResources_1.json
@@ -0,0 +1,8 @@
+{
+    "status_code": 200,
+    "data": {
+        "PaginationToken": "",
+        "ResourceTagMappingList": [],
+        "ResponseMetadata": {}
+    }
+}
\ No newline at end of file
diff --git a/tests/test_policy.py b/tests/test_policy.py
index d054d706c..6f192d269 100644
--- a/tests/test_policy.py
+++ b/tests/test_policy.py
@@ -286,7 +286,6 @@ class PolicyMetaLint(BaseTest):
             'AWS::ApiGatewayV2::Api',
             'AWS::ServiceCatalog::CloudFormationProvisionedProduct',
             'AWS::ServiceCatalog::CloudFormationProduct',
-            'AWS::ServiceCatalog::Portfolio',
             'AWS::SSM::FileData',
             'AWS::SecretsManager::Secret'}
 
diff --git a/tests/test_servicecatalog.py b/tests/test_servicecatalog.py
new file mode 100644
index 000000000..2e04eb4c0
--- /dev/null
+++ b/tests/test_servicecatalog.py
@@ -0,0 +1,69 @@
+# Copyright 2020 Capital One Services, LLC
+# Copyright The Cloud Custodian Authors.
+# SPDX-License-Identifier: Apache-2.0
+
+from .common import BaseTest
+import time
+from c7n.exceptions import PolicyValidationError
+
+
+class TestServiceCatalog(BaseTest):
+
+    def test_portfolio_cross_account_remove_delete(self):
+        session_factory = self.replay_flight_data("test_portfolio_cross_account_remove_delete")
+        client = session_factory().client("servicecatalog")
+        accounts = client.list_portfolio_access(PortfolioId='port-3cfur4nmnvf4u').get('AccountIds')
+        self.assertEqual(len(accounts), 1)
+        p = self.load_policy(
+            {
+                "name": "servicecatalog-portfolio-cross-account",
+                "resource": "catalog-portfolio",
+                "filters": [{"type": "cross-account"}],
+                "actions": [
+                    {"type": "remove-shared-accounts", "accounts": "matched"},
+                    {"type": "delete"}
+                ],
+            },
+            session_factory=session_factory)
+        resources = p.run()
+        self.assertEqual(len(resources), 1)
+        self.assertEqual(resources[0]['Id'], 'port-3cfur4nmnvf4u')
+        if self.recording:
+            time.sleep(10)
+        portfolios = client.list_portfolios()
+        self.assertFalse('port-3cfur4nmnvf4u' in [p.get(
+            'Id') for p in portfolios.get('PortfolioDetails')])
+
+    def test_remove_accounts_validation_error(self):
+        self.assertRaises(
+            PolicyValidationError,
+            self.load_policy,
+            {
+                "name": "catalog-portfolio-delete-shared-accounts",
+                "resource": "aws.catalog-portfolio",
+                "actions": [{"type": "remove-shared-accounts", "accounts": "matched"}],
+            }
+        )
+
+    def test_portfolio_remove_share_accountid(self):
+        session_factory = self.replay_flight_data("test_portfolio_remove_share_accountid")
+        client = session_factory().client("servicecatalog")
+        self.assertTrue('644160558196' in client.list_portfolio_access(
+            PortfolioId='port-hlgxpz7lc55iw').get('AccountIds'))
+        self.assertTrue('644160558196' in client.list_portfolio_access(
+            PortfolioId='port-srkytozjwbzpc').get('AccountIds'))
+        p = self.load_policy(
+            {
+                "name": "servicecatalog-portfolio-cross-account",
+                "resource": "catalog-portfolio",
+                "filters": [{"type": "cross-account"}],
+                "actions": [{"type": "remove-shared-accounts", "accounts": ["644160558196"]}],
+            },
+            session_factory=session_factory)
+        resources = p.run()
+        self.assertEqual(len(resources), 2)
+        self.assertTrue(r['Id'] in ['port-hlgxpz7lc55iw', 'port-srkytozjwbzpc'] for r in resources)
+        self.assertFalse('644160558196' in client.list_portfolio_access(
+            PortfolioId='port-hlgxpz7lc55iw').get('AccountIds'))
+        self.assertTrue('644160558196' in client.list_portfolio_access(
+            PortfolioId='port-srkytozjwbzpc').get('AccountIds'))