From c033c3c919591ded7054e72e5d5473672f63e6e1 Mon Sep 17 00:00:00 2001
From: Jeet Nihalani <jeetend@amazon.com>
Date: Wed, 4 Aug 2021 20:34:40 -0600
Subject: [PATCH 1/3] feat: Disable CIDR feature when AppStream is enabled

---
 .../base-raas-ui/src/helpers/settings.js      |   3 +-
 .../parts/ScEnvironmentButtons.js             |   4 +-
 .../setup/CreateInternalEnvForm.js            |   4 +-
 .../ec2-linux-instance.cfn.yml                |  41 +++--
 .../ec2-windows-instance.cfn.yml              |  21 ++-
 .../sagemaker-notebook-instance.cfn.yml       |   6 +-
 .../environment-sc-cidr-service.test.js       |  41 +++++
 .../__tests__/environment-sc-service.test.js  | 162 +++++++++++++++++-
 .../environment-sc-cidr-service.js            |   8 +
 .../service-catalog/environment-sc-service.js |  31 +++-
 .../src/models/forms/CfnParamsForm.js         |  15 +-
 .../forms/__tests__/CfnParamsForm.test.js     | 134 +++++++++++++++
 openapi.yaml                                  |   1 +
 13 files changed, 426 insertions(+), 45 deletions(-)
 create mode 100644 addons/addon-environment-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/__tests__/CfnParamsForm.test.js

diff --git a/addons/addon-base-raas-ui/packages/base-raas-ui/src/helpers/settings.js b/addons/addon-base-raas-ui/packages/base-raas-ui/src/helpers/settings.js
index 05ff41c9c2..b430859426 100644
--- a/addons/addon-base-raas-ui/packages/base-raas-ui/src/helpers/settings.js
+++ b/addons/addon-base-raas-ui/packages/base-raas-ui/src/helpers/settings.js
@@ -1,5 +1,6 @@
 /* eslint-disable import/prefer-default-export */
 const enableBuiltInWorkspaces = process.env.REACT_APP_ENABLE_BUILT_IN_WORKSPACES === 'true';
 const enableEgressStore = process.env.REACT_APP_ENABLE_EGRESS_STORE;
+const isAppStreamEnabled = process.env.REACT_APP_IS_APP_STREAM_ENABLED === 'true';
 
-export { enableBuiltInWorkspaces, enableEgressStore };
+export { enableBuiltInWorkspaces, enableEgressStore, isAppStreamEnabled };
diff --git a/addons/addon-base-raas-ui/packages/base-raas-ui/src/parts/environments-sc/parts/ScEnvironmentButtons.js b/addons/addon-base-raas-ui/packages/base-raas-ui/src/parts/environments-sc/parts/ScEnvironmentButtons.js
index 77a5ef2120..5452a3f428 100644
--- a/addons/addon-base-raas-ui/packages/base-raas-ui/src/parts/environments-sc/parts/ScEnvironmentButtons.js
+++ b/addons/addon-base-raas-ui/packages/base-raas-ui/src/parts/environments-sc/parts/ScEnvironmentButtons.js
@@ -9,7 +9,7 @@ import { displayError } from '@aws-ee/base-ui/dist/helpers/notification';
 
 import ScEnvironmentConnections from './ScEnvironmentConnections';
 import ScEnvironmentUpdateCidrs from './ScEnvironmentUpdateCidrs';
-import { enableEgressStore } from '../../../helpers/settings';
+import { enableEgressStore, isAppStreamEnabled } from '../../../helpers/settings';
 import ScEnvironmentEgressStoreDetail from './ScEnvironmentEgressStoreDetail';
 
 const PROCESSING_STATUS_CODE = 'PROCESSING';
@@ -212,7 +212,7 @@ class ScEnvironmentButtons extends React.Component {
               View Detail
             </Button>
           )}
-          {state.canTerminate && !state.key.includes('FAILED') && (
+          {!isAppStreamEnabled && state.canTerminate && !state.key.includes('FAILED') && (
             <Button
               floated="left"
               basic
diff --git a/addons/addon-base-raas-ui/packages/base-raas-ui/src/parts/environments-sc/setup/CreateInternalEnvForm.js b/addons/addon-base-raas-ui/packages/base-raas-ui/src/parts/environments-sc/setup/CreateInternalEnvForm.js
index e05370d7d4..427920c01e 100644
--- a/addons/addon-base-raas-ui/packages/base-raas-ui/src/parts/environments-sc/setup/CreateInternalEnvForm.js
+++ b/addons/addon-base-raas-ui/packages/base-raas-ui/src/parts/environments-sc/setup/CreateInternalEnvForm.js
@@ -28,7 +28,7 @@ class CreateInternalEnvForm extends React.Component {
     runInAction(() => {
       this.form = getCreateInternalEnvForm({
         projectIdOptions: this.getProjectIdOptions(),
-        cidr: this.props.defaultCidr,
+        cidr: this.isAppStreamEnabled ? undefined : this.props.defaultCidr,
       });
     });
   }
@@ -148,7 +148,7 @@ class CreateInternalEnvForm extends React.Component {
 
   renderForm() {
     const form = this.form;
-    const askForCidr = !_.isUndefined(this.props.defaultCidr);
+    const askForCidr = !_.isUndefined(this.props.defaultCidr) && !this.isAppStreamEnabled;
     const configurations = this.configurations;
 
     // we show the AppStream configuration warning when the feature is enabled,
diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/ec2-linux-instance.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/ec2-linux-instance.cfn.yml
index 82d0d04de8..6b14636a22 100644
--- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/ec2-linux-instance.cfn.yml
+++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/ec2-linux-instance.cfn.yml
@@ -23,6 +23,7 @@ Parameters:
   AccessFromCIDRBlock:
     Type: String
     Description: The CIDR used to access the ec2 instances.
+    Default: 10.0.0.0/19
   S3Mounts:
     Type: String
     Description: A JSON array of objects with name, bucket, and prefix properties used to mount data
@@ -137,10 +138,13 @@ Resources:
           FromPort: 0
           ToPort: 65535
           CidrIp: 0.0.0.0/0
-        - IpProtocol: icmp
-          FromPort: -1
-          ToPort: -1
-          CidrIp: !Ref AccessFromCIDRBlock
+        - !If
+          - AppStreamEnabled
+          - !Ref "AWS::NoValue"
+          - IpProtocol: icmp
+            FromPort: -1
+            ToPort: -1
+            CidrIp: !Ref AccessFromCIDRBlock
         - !If
           - AppStreamEnabled
           - DestinationSecurityGroupId:
@@ -148,24 +152,29 @@ Resources:
             IpProtocol: '-1'
           - !Ref "AWS::NoValue"
       SecurityGroupIngress:
-        - IpProtocol: tcp
-          FromPort: 22
-          ToPort: 22
-          CidrIp: !Ref AccessFromCIDRBlock
-        - IpProtocol: tcp
-          FromPort: 80
-          ToPort: 80
-          CidrIp: !Ref AccessFromCIDRBlock
-        - IpProtocol: tcp
-          FromPort: 443
-          ToPort: 443
-          CidrIp: !Ref AccessFromCIDRBlock
         - !If
           - AppStreamEnabled
           - SourceSecurityGroupId:
               Fn::ImportValue: !Sub "${SolutionNamespace}-SwbAppStreamSG"
             IpProtocol: '-1'
+          - IpProtocol: tcp
+            FromPort: 22
+            ToPort: 22
+            CidrIp: !Ref AccessFromCIDRBlock
+        - !If
+          - AppStreamEnabled
+          - !Ref "AWS::NoValue"
+          - IpProtocol: tcp
+            FromPort: 80
+            ToPort: 80
+            CidrIp: !Ref AccessFromCIDRBlock
+        - !If
+          - AppStreamEnabled
           - !Ref "AWS::NoValue"
+          - IpProtocol: tcp
+            FromPort: 443
+            ToPort: 443
+            CidrIp: !Ref AccessFromCIDRBlock
       Tags:
         - Key: Name
           Value: !Join ['-', [Ref: Namespace, 'ec2-sg']]
diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/ec2-windows-instance.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/ec2-windows-instance.cfn.yml
index 053a9ed8b1..2a2b83c93c 100644
--- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/ec2-windows-instance.cfn.yml
+++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/ec2-windows-instance.cfn.yml
@@ -26,6 +26,7 @@ Parameters:
   AccessFromCIDRBlock:
     Type: String
     Description: The CIDR used to access the ec2 instances.
+    Default: 10.0.0.0/19
   S3Mounts:
     Type: String
     Description: A JSON array of objects with name, bucket, and prefix properties used to mount data
@@ -159,10 +160,13 @@ Resources:
           FromPort: 0
           ToPort: 65535
           CidrIp: 0.0.0.0/0
-        - IpProtocol: icmp
-          FromPort: -1
-          ToPort: -1
-          CidrIp: !Ref AccessFromCIDRBlock
+        - !If
+          - AppStreamEnabled
+          - !Ref "AWS::NoValue"
+          - IpProtocol: icmp
+            FromPort: -1
+            ToPort: -1
+            CidrIp: !Ref AccessFromCIDRBlock
         - !If
           - AppStreamEnabled
           - DestinationSecurityGroupId:
@@ -170,16 +174,15 @@ Resources:
             IpProtocol: '-1'
           - !Ref "AWS::NoValue"
       SecurityGroupIngress:
-        - IpProtocol: tcp
-          FromPort: 3389
-          ToPort: 3389
-          CidrIp: !Ref AccessFromCIDRBlock
         - !If
           - AppStreamEnabled
           - SourceSecurityGroupId:
               Fn::ImportValue: !Sub "${SolutionNamespace}-SwbAppStreamSG"
             IpProtocol: '-1'
-          - !Ref "AWS::NoValue"
+          - IpProtocol: tcp
+            FromPort: 3389
+            ToPort: 3389
+            CidrIp: !Ref AccessFromCIDRBlock
       Tags:
         - Key: Name
           Value: !Join ['-', [Ref: Namespace, 'ec2-sg']]
diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/sagemaker-notebook-instance.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/sagemaker-notebook-instance.cfn.yml
index 9cde311963..2852be1431 100644
--- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/sagemaker-notebook-instance.cfn.yml
+++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/sagemaker-notebook-instance.cfn.yml
@@ -21,7 +21,8 @@ Parameters:
     Type: AWS::EC2::Subnet::Id
   AccessFromCIDRBlock:
     Type: String
-    Description: The CIDR used to access sagemaker.
+    Description: The CIDR used to access sagemaker. This parameter is only required when AppStream is disabled
+    Default: 10.0.0.0/19
   S3Mounts:
     Type: String
     Description: A JSON array of objects with name, bucket and prefix properties used to mount data
@@ -60,6 +61,9 @@ Resources:
       VpcId:
         Ref: VPC
       SecurityGroupIngress:
+        !If
+        - AppStreamEnabled
+        - !Ref 'AWS::NoValue'
         - IpProtocol: tcp
           FromPort: 443
           ToPort: 443
diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-cidr-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-cidr-service.test.js
index cdb03afc11..1dd77ddc2b 100644
--- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-cidr-service.test.js
+++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-cidr-service.test.js
@@ -46,6 +46,7 @@ describe('EnvironmentScCidrService', () => {
   let service = null;
   let environmentScService = null;
   let lockService = null;
+  let settings = null;
 
   beforeEach(async () => {
     const container = new ServicesContainer();
@@ -69,6 +70,13 @@ describe('EnvironmentScCidrService', () => {
     service = await container.find('environmentScCidrService');
     lockService = await container.find('lockService');
     environmentScService = await container.find('environmentScService');
+    settings = await container.find('settings');
+    settings.optionalBoolean = jest.fn(key => {
+      if (key === 'isAppStreamEnabled') {
+        return false;
+      }
+      return undefined;
+    });
 
     // Skip authorization by default
     service.assertAuthorized = jest.fn();
@@ -79,6 +87,39 @@ describe('EnvironmentScCidrService', () => {
   });
 
   describe('Validation checks', () => {
+    it('should fail validation check since AppStream is enabled', async () => {
+      // BUILD
+      settings.optionalBoolean = jest.fn(key => {
+        if (key === 'isAppStreamEnabled') {
+          return true;
+        }
+        return undefined;
+      });
+      const requestContext = {};
+      const params = {
+        id: 'testId',
+        updateRequest: [
+          {
+            protocol: 'tcp',
+            fromPort: 123,
+            toPort: 123,
+            cidrBlocks: ['123.123.123.123/32'],
+          },
+        ],
+      };
+
+      // OPERATE
+      try {
+        await service.update(requestContext, params);
+        expect.hasAssertions();
+      } catch (err) {
+        expect(service.boom.is(err, 'badRequest')).toBe(true);
+        expect(err.message).toContain('CIDR operation unavailable when AppStream is enabled');
+        expect(settings.optionalBoolean).toHaveBeenCalledTimes(1);
+        expect(settings.optionalBoolean).toHaveBeenCalledWith('isAppStreamEnabled', false);
+      }
+    });
+
     it('should fail because the updateRequest contains additional properties', async () => {
       // BUILD
       const params = {
diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js
index 989d400f2d..696b9399fb 100644
--- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js
+++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js
@@ -73,6 +73,7 @@ describe('EnvironmentSCService', () => {
   let awsAccountsService = null;
   let aws = null;
   let storageGatewayService = null;
+  let settings = null;
   const error = { code: 'ConditionalCheckFailedException' };
   beforeEach(async () => {
     const container = new ServicesContainer();
@@ -106,6 +107,7 @@ describe('EnvironmentSCService', () => {
     wfService = await container.find('workflowTriggerService');
     aws = await container.find('aws');
     storageGatewayService = await container.find('storageGatewayService');
+    settings = await container.find('settings');
 
     // Skip authorization by default
     service.assertAuthorized = jest.fn();
@@ -121,9 +123,87 @@ describe('EnvironmentSCService', () => {
       return { roleArn: 'cfnExecutionRole', externalId: 'roleExternalId' };
     });
     service._fromRawToDbObject = jest.fn(x => x);
+    settings.optionalBoolean = jest.fn((key, defaultBoolean) => {
+      if (key === 'isAppStreamEnabled') {
+        return false;
+      }
+      return defaultBoolean;
+    });
   });
 
   describe('create function', () => {
+    it('should fail create since CIDR info is included when AppStream is enabled', async () => {
+      // BUILD
+      settings.optionalBoolean = jest.fn((key, defaultBoolean) => {
+        if (key === 'isAppStreamEnabled') {
+          return true;
+        }
+        return defaultBoolean;
+      });
+      const requestContext = {
+        principal: {
+          isExternalUser: false,
+        },
+      };
+      const newEnv = {
+        name: 'exampleName',
+        envTypeId: 'exampleETI',
+        envTypeConfigId: 'exampleETCI',
+        cidr: '0.0.0.0/32',
+      };
+
+      // OPERATE
+      try {
+        await service.create(requestContext, newEnv);
+        expect.hasAssertions();
+      } catch (err) {
+        expect(service.boom.is(err, 'badRequest')).toBe(true);
+        expect(err.message).toBe('Cannot specify CIDR when AppStream is enabled');
+        expect(settings.optionalBoolean).toHaveBeenCalledTimes(1);
+        expect(settings.optionalBoolean).toHaveBeenCalledWith('isAppStreamEnabled', false);
+      }
+    });
+
+    it('should succeed create if environment has undefined cidr and AppStream is enabled', async () => {
+      // BUILD
+      settings.optionalBoolean = jest.fn((key, defaultBoolean) => {
+        if (key === 'isAppStreamEnabled') {
+          return true;
+        }
+        return defaultBoolean;
+      });
+      projectService.mustFind = jest.fn(() => {
+        return { indexId: 'testIndex', isAppStreamConfigured: true };
+      });
+      const requestContext = {
+        principal: {
+          isExternalUser: false,
+        },
+      };
+      const newEnv = {
+        name: 'exampleName',
+        envTypeId: 'exampleETI',
+        envTypeConfigId: 'exampleETCI',
+        cidr: undefined,
+      };
+      service.audit = jest.fn();
+      wfService.triggerWorkflow = jest.fn();
+
+      // OPERATE
+      await service.create(requestContext, newEnv);
+
+      // CHECK
+      expect(service.audit).toHaveBeenCalledWith(
+        requestContext,
+        expect.objectContaining({ action: 'create-environment-sc' }),
+      );
+      expect(wfService.triggerWorkflow).toHaveBeenCalled();
+      expect(settings.optionalBoolean).toHaveBeenCalledTimes(2);
+      expect(projectService.mustFind).toHaveBeenCalledTimes(1);
+      expect(settings.optionalBoolean).toHaveBeenCalledWith('isAppStreamEnabled', false);
+      expect('cidr' in newEnv).toBe(false);
+    });
+
     it('should fail because the user is external', async () => {
       // BUILD
       const requestContext = {
@@ -280,7 +360,7 @@ describe('EnvironmentSCService', () => {
       await service.mustFind(requestContext, { id: 'oldId' });
 
       // CHECK
-      expect(service.find).toHaveBeenCalledWith(requestContext, { id: 'oldId', fields: [], fetchCidr: true });
+      expect(service.find).toHaveBeenCalledWith(requestContext, { id: 'oldId', fields: [] });
     });
 
     it('verify mustFind with fetchCidr', async () => {
@@ -308,6 +388,86 @@ describe('EnvironmentSCService', () => {
       expect(service.find).toHaveBeenCalledWith(requestContext, { id: 'oldId', fields: [], fetchCidr: false });
     });
 
+    it('verify error is thrown when AppStream is enabled and fetchCidr is true', async () => {
+      // BUILD
+      settings.optionalBoolean = jest.fn((key, defaultBoolean) => {
+        if (key === 'isAppStreamEnabled') {
+          return true;
+        }
+        return defaultBoolean;
+      });
+      const uid = 'u-12345';
+      const requestContext = { principalIdentifier: { uid } };
+
+      const env = {
+        status: 'COMPLETED',
+        id: 'oldId',
+        name: 'exampleName',
+        envTypeId: 'exampleETI',
+        envTypeConfigId: 'exampleETCI',
+        updatedBy: {
+          username: 'user',
+        },
+        studyIds: ['study1', 'study2'],
+      };
+      service.audit = jest.fn();
+      service.getSecurityGroupDetails = jest.fn();
+      dbService.table.get.mockReturnValueOnce(env);
+
+      // OPERATE
+      try {
+        await service.find(requestContext, { id: 'oldId', fetchCidr: true });
+        expect.hasAssertions();
+      } catch (err) {
+        // CHECK
+        expect(service.boom.is(err, 'badRequest')).toBe(true);
+        expect(err.message).toBe('CIDR operation unavailable when AppStream is enabled');
+        expect(service.getSecurityGroupDetails).not.toHaveBeenCalled();
+        expect(settings.optionalBoolean).toHaveBeenCalledTimes(1);
+        expect(settings.optionalBoolean).toHaveBeenCalledWith('isAppStreamEnabled', false);
+      }
+    });
+
+    it('verify getSecurityGroupDetails not called when AppStream is enabled and fetchCidr is undefined', async () => {
+      // BUILD
+      settings.optionalBoolean = jest.fn((key, defaultBoolean) => {
+        if (key === 'isAppStreamEnabled') {
+          return true;
+        }
+        return defaultBoolean;
+      });
+      const uid = 'u-12345';
+      const requestContext = { principalIdentifier: { uid } };
+
+      const env = {
+        status: 'COMPLETED',
+        id: 'oldId',
+        name: 'exampleName',
+        envTypeId: 'exampleETI',
+        envTypeConfigId: 'exampleETCI',
+        updatedBy: {
+          username: 'user',
+        },
+        studyIds: ['study1', 'study2'],
+      };
+      service.audit = jest.fn();
+      service.getSecurityGroupDetails = jest.fn();
+      dbService.table.get.mockReturnValueOnce(env);
+
+      // OPERATE
+      await service.find(requestContext, { id: 'oldId' });
+
+      // CHECK
+      expect(service.getSecurityGroupDetails).not.toHaveBeenCalled();
+      expect(service.assertAuthorized).toHaveBeenCalledWith(
+        requestContext,
+        expect.objectContaining({ action: 'get-sc', conditions: [service._allowAuthorized] }),
+        env,
+      );
+      expect(settings.optionalBoolean).toHaveBeenCalledTimes(1);
+      expect(settings.optionalBoolean).toHaveBeenCalledWith('isAppStreamEnabled', false);
+    });
+
     it('verify getSecurityGroupDetails not called when fetchCidr is false', async () => {
       // BUILD
       const uid = 'u-12345';
diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-cidr-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-cidr-service.js
index e063d6fb10..9ff2c79c88 100644
--- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-cidr-service.js
+++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-cidr-service.js
@@ -19,6 +19,10 @@ const Service = require('@aws-ee/base-services-container/lib/service');
 
 const cidrUpdateSchema = require('../../schema/update-environment-sc-cidr');
 
+const settingKeys = {
+  isAppStreamEnabled: 'isAppStreamEnabled',
+};
+
 class EnvironmentScCidrService extends Service {
   constructor() {
     super();
@@ -42,6 +46,10 @@ class EnvironmentScCidrService extends Service {
   }
 
   checkRequest(updateRequest) {
+    const isAppStreamEnabled = this.settings.optionalBoolean(settingKeys.isAppStreamEnabled, false);
+    if (isAppStreamEnabled) {
+      throw this.boom.badRequest(`CIDR operation unavailable when AppStream is enabled`, true);
+    }
     const erroneousInputs = [];
     const ipv6Format = [];
     const protPortCombos = {};
diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js
index a609d7a974..9109369044 100644
--- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js
+++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js
@@ -69,7 +69,6 @@ class EnvironmentScService extends Service {
     await super.init();
     const [dbService, environmentAuthzService] = await this.service(['dbService', 'environmentAuthzService']);
     const table = this.settings.get(settingKeys.tableName);
-    this.isAppStreamEnabled = this.settings.get(settingKeys.isAppStreamEnabled) === 'true';
 
     this._getter = () => dbService.helper.getter().table(table);
     this._query = () => dbService.helper.query().table(table);
@@ -97,7 +96,7 @@ class EnvironmentScService extends Service {
         return environments.filter(env => isCurrentUser(requestContext, { uid: env.createdBy }));
       });
 
-    if (this.isAppStreamEnabled) {
+    if (this.isAppStreamEnabled()) {
       envs = await this.filterAppStreamProjectEnvs(requestContext, envs);
     }
 
@@ -328,7 +327,13 @@ class EnvironmentScService extends Service {
     return _.filter(envs, env => !_.includes(filterStatus, env.status) && !env.status.includes('FAILED'));
   }
 
-  async find(requestContext, { id, fields = [], fetchCidr = true }) {
+  async find(requestContext, { id, fields = [], fetchCidr }) {
+    // Define fetchCidr flag based on isAppStreamEnabled config rather than defaulting it to true
+    const isAppStreamEnabled = this.isAppStreamEnabled();
+    const computedFetchCidr = fetchCidr === undefined ? !isAppStreamEnabled : fetchCidr;
+    if (computedFetchCidr && isAppStreamEnabled) {
+      throw this.boom.badRequest(`CIDR operation unavailable when AppStream is enabled`, true);
+    }
     // Make sure 'createdBy' is always returned as that's required for authorizing the 'get' action
     // If empty "fields" is specified then it means the caller is asking for all fields. No need to append 'createdBy'
     // in that case.
@@ -357,7 +362,7 @@ class EnvironmentScService extends Service {
         ],
         env.status,
       ) &&
-      fetchCidr
+      computedFetchCidr
     ) {
       const { currentIngressRules } = await this.getSecurityGroupDetails(requestContext, env);
       env.cidr = currentIngressRules;
@@ -366,7 +371,7 @@ class EnvironmentScService extends Service {
     return toReturn;
   }
 
-  async mustFind(requestContext, { id, fields = [], fetchCidr = true }) {
+  async mustFind(requestContext, { id, fields = [], fetchCidr }) {
     const result = await this.find(requestContext, { id, fields, fetchCidr });
     if (!result) throw this.boom.notFound(`environment with id "${id}" does not exist`, true);
     return result;
@@ -390,6 +395,14 @@ class EnvironmentScService extends Service {
     // Validate input
     await validationService.ensureValid(environment, createSchema);
 
+    // If the AppStream feature is enabled, verify that update request doesn't include a cidr
+    if (this.isAppStreamEnabled()) {
+      if (environment.cidr) {
+        throw this.boom.badRequest('Cannot specify CIDR when AppStream is enabled', true);
+      }
+      delete environment.cidr;
+    }
+
     // Make sure the user has permissions to create the environment
     // The following will result in checking permissions by calling the condition function "this._allowAuthorized" first
     await this.assertAuthorized(
@@ -409,7 +422,7 @@ class EnvironmentScService extends Service {
     });
 
     // If the AppStream feature is enabled, verify the project linked to the environment has it configured
-    if (this.isAppStreamEnabled && !isAppStreamConfigured)
+    if (this.isAppStreamEnabled() && !isAppStreamConfigured)
       throw this.boom.badRequest('Please select an AppStream-configured project', true);
 
     // Save environment to db and trigger the workflow
@@ -469,7 +482,7 @@ class EnvironmentScService extends Service {
 
   async verifyAppStreamConfig(requestContext, projectId) {
     // If the AppStream feature is enabled, verify the project linked to the environment has it configured
-    if (this.isAppStreamEnabled) {
+    if (this.isAppStreamEnabled()) {
       const projectService = await this.service('projectService');
       // The isAppStreamConfigured attribute value will be returned by project service. indexId field is enough for filtering
       const { isAppStreamConfigured } = await projectService.mustFind(requestContext, {
@@ -1027,6 +1040,10 @@ class EnvironmentScService extends Service {
     return { currentIngressRules: returnVal, securityGroupId };
   }
 
+  isAppStreamEnabled() {
+    return this.settings.optionalBoolean(settingKeys.isAppStreamEnabled, false);
+  }
+
   /**
    * Method assumes the environment management role in the AWS account where the specified environment is running and
    * constructs an instance of the specified AWS client SDK with the temporary credentials obtained by assuming the role.
diff --git a/addons/addon-environment-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/CfnParamsForm.js b/addons/addon-environment-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/CfnParamsForm.js
index 3569d67bb3..5b99e97397 100644
--- a/addons/addon-environment-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/CfnParamsForm.js
+++ b/addons/addon-environment-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/CfnParamsForm.js
@@ -33,15 +33,18 @@ import { createForm } from '@aws-ee/base-ui/dist/helpers/form';
  * @param existingParamValues Array containing key/value pairs for existing values for the params. Has the shape [{key,value}]
  */
 function getCfnParamsForm(cfnParams, existingParamValues) {
+  const isAppStreamEnabled = process.env.REACT_APP_IS_APP_STREAM_ENABLED === 'true';
   const fields = {};
   _.forEach(cfnParams, ({ ParameterKey, Description, DefaultValue }) => {
     const existingValue = _.get(_.find(existingParamValues, { key: ParameterKey }), 'value') || DefaultValue;
-    fields[ParameterKey] = {
-      label: ParameterKey,
-      extra: { explain: Description },
-      value: existingValue,
-      rules: 'required',
-    };
+    if (!isAppStreamEnabled || (isAppStreamEnabled && !(ParameterKey === 'AccessFromCIDRBlock'))) {
+      fields[ParameterKey] = {
+        label: ParameterKey,
+        extra: { explain: Description },
+        value: existingValue,
+        rules: 'required',
+      };
+    }
   });
   return createForm(fields);
 }
diff --git a/addons/addon-environment-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/__tests__/CfnParamsForm.test.js b/addons/addon-environment-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/__tests__/CfnParamsForm.test.js
new file mode 100644
index 0000000000..760a2082e7
--- /dev/null
+++ b/addons/addon-environment-sc-ui/packages/environment-type-mgmt-ui/src/models/forms/__tests__/CfnParamsForm.test.js
@@ -0,0 +1,134 @@
+/*
+ *  Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License").
+ *  You may not use this file except in compliance with the License.
+ *  A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ *  or in the "license" file accompanying this file. This file is distributed
+ *  on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ *  express or implied. See the License for the specific language governing
+ *  permissions and limitations under the License.
+ */
+
+import { getCfnParamsForm } from '../CfnParamsForm';
+
+jest.mock('@aws-ee/base-ui/dist/helpers/form');
+const formMock = require('@aws-ee/base-ui/dist/helpers/form');
+
+describe('CfnParamsForm', () => {
+  const param1 = { ParameterKey: 'key1', Description: 'desc1', DefaultValue: 'default1' };
+  const param2 = { ParameterKey: 'key2', Description: 'desc2', DefaultValue: 'default2' };
+  let defaultParams = null;
+  let defaultFields = null;
+  const expectedForm = 'testForm';
+
+  beforeEach(async () => {
+    defaultParams = [param1, param2];
+    defaultFields = {
+      key1: {
+        label: param1.ParameterKey,
+        extra: { explain: param1.Description },
+        value: param1.DefaultValue,
+        rules: 'required',
+      },
+      key2: {
+        label: param2.ParameterKey,
+        extra: { explain: param2.Description },
+        value: param2.DefaultValue,
+        rules: 'required',
+      },
+    };
+
+    formMock.createForm = jest.fn(() => {
+      return expectedForm;
+    });
+  });
+
+  afterEach(async () => {
+    formMock.createForm.mockReset();
+  });
+
+  it('should return all fields when AppStream is enabled and no CIDR config present', async () => {
+    // BUILD
+    process.env.REACT_APP_IS_APP_STREAM_ENABLED = true;
+
+    // OPERATE
+    const returnedForm = getCfnParamsForm(defaultParams, []);
+
+    // CHECK
+    expect(returnedForm).toBe(expectedForm);
+    expect(formMock.createForm).toHaveBeenCalledTimes(1);
+    expect(formMock.createForm).toHaveBeenCalledWith(defaultFields);
+  });
+
+  it('should return all fields when AppStream is disabled and no CIDR config present', async () => {
+    // BUILD
+    process.env.REACT_APP_IS_APP_STREAM_ENABLED = false;
+
+    // OPERATE
+    const returnedForm = getCfnParamsForm(defaultParams, []);
+
+    // CHECK
+    expect(returnedForm).toBe(expectedForm);
+    expect(formMock.createForm).toHaveBeenCalledTimes(1);
+    expect(formMock.createForm).toHaveBeenCalledWith(defaultFields);
+  });
+
+  it('should return default values correctly', async () => {
+    // BUILD
+    const overwriteParam2 = { ParameterKey: 'key2', Description: 'desc2', DefaultValue: 'newDefault2' };
+    const params = [param1, overwriteParam2];
+    const newFields = JSON.parse(JSON.stringify(defaultFields));
+    newFields.key2.value = overwriteParam2.DefaultValue;
+
+    // OPERATE
+    const returnedForm = getCfnParamsForm(params, []);
+
+    // CHECK
+    expect(returnedForm).toBe(expectedForm);
+    expect(formMock.createForm).toHaveBeenCalledTimes(1);
+    expect(formMock.createForm).toHaveBeenCalledWith(newFields);
+  });
+
+  it('should not return cidr field when AppStream is enabled and CIDR config present', async () => {
+    // BUILD
+    process.env.REACT_APP_IS_APP_STREAM_ENABLED = true;
+    const param3 = { ParameterKey: 'AccessFromCIDRBlock', Description: 'cidr', DefaultValue: '0.0.0.0/32' };
+    const paramsWithCidr = [...defaultParams];
+    paramsWithCidr.push(param3);
+
+    // OPERATE
+    const returnedForm = getCfnParamsForm(paramsWithCidr, []);
+
+    // CHECK
+    expect(returnedForm).toBe(expectedForm);
+    expect(formMock.createForm).toHaveBeenCalledTimes(1);
+    expect(formMock.createForm).toHaveBeenCalledWith(defaultFields);
+  });
+
+  it('should return all fields including cidr field when AppStream is disabled and CIDR config present', async () => {
+    // BUILD
+    process.env.REACT_APP_IS_APP_STREAM_ENABLED = false;
+    const param3 = { ParameterKey: 'AccessFromCIDRBlock', Description: 'cidr', DefaultValue: '0.0.0.0/32' };
+    const paramsWithCidr = [...defaultParams];
+    paramsWithCidr.push(param3);
+    const fieldsWithCidr = JSON.parse(JSON.stringify(defaultFields));
+    fieldsWithCidr.AccessFromCIDRBlock = {
+      label: param3.ParameterKey,
+      extra: { explain: param3.Description },
+      value: param3.DefaultValue,
+      rules: 'required',
+    };
+
+    // OPERATE
+    const returnedForm = getCfnParamsForm(paramsWithCidr, []);
+
+    // CHECK
+    expect(returnedForm).toBe(expectedForm);
+    expect(formMock.createForm).toHaveBeenCalledTimes(1);
+    expect(formMock.createForm).toHaveBeenCalledWith(fieldsWithCidr);
+  });
+});
diff --git a/openapi.yaml b/openapi.yaml
index 3f339febfc..ebd7a8af33 100644
--- a/openapi.yaml
+++ b/openapi.yaml
@@ -3414,6 +3414,7 @@ components:
           type: string
           pattern: '^(?:([0-9]{1,3}\.){3}[0-9]{1,3}(\/([0-9]|[1-2][0-9]|3[0-2]))?)?$'
           example: '192.255.255.255/32'
+          description: optional field only to be specified if AppStream feature is disabled
         studyIds:
           type: array
           example: ['test_study1']

From b0fa089d959174af4910bf0a3c97144de58d2600 Mon Sep 17 00:00:00 2001
From: Jeet Nihalani <jeetend@amazon.com>
Date: Thu, 5 Aug 2021 10:48:42 -0600
Subject: [PATCH 2/3] chore: changed optionalBoolean to getBoolean

---
 .../environment-sc-cidr-service.test.js       | 12 +++----
 .../__tests__/environment-sc-service.test.js  | 36 +++++++++----------
 .../environment-sc-cidr-service.js            |  2 +-
 .../service-catalog/environment-sc-service.js |  2 +-
 4 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-cidr-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-cidr-service.test.js
index 1dd77ddc2b..301444dba8 100644
--- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-cidr-service.test.js
+++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-cidr-service.test.js
@@ -71,11 +71,11 @@ describe('EnvironmentScCidrService', () => {
     lockService = await container.find('lockService');
     environmentScService = await container.find('environmentScService');
     settings = await container.find('settings');
-    settings.optionalBoolean = jest.fn(key => {
+    settings.getBoolean = jest.fn(key => {
       if (key === 'isAppStreamEnabled') {
         return false;
       }
-      return undefined;
+      throw Error(`${key} not found`);
     });
 
     // Skip authorization by default
@@ -89,11 +89,11 @@ describe('EnvironmentScCidrService', () => {
   describe('Validation checks', () => {
     it('should fail validation check since AppStream is enabled', async () => {
       // BUILD
-      settings.optionalBoolean = jest.fn(key => {
+      settings.getBoolean = jest.fn(key => {
         if (key === 'isAppStreamEnabled') {
           return true;
         }
-        return undefined;
+        throw Error(`${key} not found`);
       });
       const requestContext = {};
       const params = {
@@ -115,8 +115,8 @@ describe('EnvironmentScCidrService', () => {
       } catch (err) {
         expect(service.boom.is(err, 'badRequest')).toBe(true);
         expect(err.message).toContain('CIDR operation unavailable when AppStream is enabled');
-        expect(settings.optionalBoolean).toHaveBeenCalledTimes(1);
-        expect(settings.optionalBoolean).toHaveBeenCalledWith('isAppStreamEnabled', false);
+        expect(settings.getBoolean).toHaveBeenCalledTimes(1);
+        expect(settings.getBoolean).toHaveBeenCalledWith('isAppStreamEnabled');
       }
     });
 
diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js
index 696b9399fb..451d8f2f4a 100644
--- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js
+++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js
@@ -123,22 +123,22 @@ describe('EnvironmentSCService', () => {
       return { roleArn: 'cfnExecutionRole', externalId: 'roleExternalId' };
     });
     service._fromRawToDbObject = jest.fn(x => x);
-    settings.optionalBoolean = jest.fn((key, defaultBoolean) => {
+    settings.getBoolean = jest.fn(key => {
       if (key === 'isAppStreamEnabled') {
         return false;
       }
-      return defaultBoolean;
+      throw Error(`${key} not found`);
     });
   });
 
   describe('create function', () => {
     it('should fail create since CIDR info is included when AppStream is enabled', async () => {
       // BUILD
-      settings.optionalBoolean = jest.fn((key, defaultBoolean) => {
+      settings.getBoolean = jest.fn(key => {
         if (key === 'isAppStreamEnabled') {
           return true;
         }
-        return defaultBoolean;
+        throw Error(`${key} not found`);
       });
       const requestContext = {
         principal: {
@@ -159,18 +159,18 @@ describe('EnvironmentSCService', () => {
       } catch (err) {
         expect(service.boom.is(err, 'badRequest')).toBe(true);
         expect(err.message).toBe('Cannot specify CIDR when AppStream is enabled');
-        expect(settings.optionalBoolean).toHaveBeenCalledTimes(1);
-        expect(settings.optionalBoolean).toHaveBeenCalledWith('isAppStreamEnabled', false);
+        expect(settings.getBoolean).toHaveBeenCalledTimes(1);
+        expect(settings.getBoolean).toHaveBeenCalledWith('isAppStreamEnabled');
       }
     });
 
     it('should succeed create if environment has undefined cidr and AppStream is enabled', async () => {
       // BUILD
-      settings.optionalBoolean = jest.fn((key, defaultBoolean) => {
+      settings.getBoolean = jest.fn(key => {
         if (key === 'isAppStreamEnabled') {
           return true;
         }
-        return defaultBoolean;
+        throw Error(`${key} not found`);
       });
       projectService.mustFind = jest.fn(() => {
         return { indexId: 'testIndex', isAppStreamConfigured: true };
@@ -198,9 +198,9 @@ describe('EnvironmentSCService', () => {
         expect.objectContaining({ action: 'create-environment-sc' }),
       );
       expect(wfService.triggerWorkflow).toHaveBeenCalled();
-      expect(settings.optionalBoolean).toHaveBeenCalledTimes(2);
+      expect(settings.getBoolean).toHaveBeenCalledTimes(2);
       expect(projectService.mustFind).toHaveBeenCalledTimes(1);
-      expect(settings.optionalBoolean).toHaveBeenCalledWith('isAppStreamEnabled', false);
+      expect(settings.getBoolean).toHaveBeenCalledWith('isAppStreamEnabled');
       expect('cidr' in newEnv).toBe(false);
     });
 
@@ -390,11 +390,11 @@ describe('EnvironmentSCService', () => {
 
     it('verify error is thrown when AppStream is enabled and fetchCidr is true', async () => {
       // BUILD
-      settings.optionalBoolean = jest.fn((key, defaultBoolean) => {
+      settings.getBoolean = jest.fn(key => {
         if (key === 'isAppStreamEnabled') {
           return true;
         }
-        return defaultBoolean;
+        throw Error(`${key} not found`);
       });
       const uid = 'u-12345';
       const requestContext = { principalIdentifier: { uid } };
@@ -423,18 +423,18 @@ describe('EnvironmentSCService', () => {
         expect(service.boom.is(err, 'badRequest')).toBe(true);
         expect(err.message).toBe('CIDR operation unavailable when AppStream is enabled');
         expect(service.getSecurityGroupDetails).not.toHaveBeenCalled();
-        expect(settings.optionalBoolean).toHaveBeenCalledTimes(1);
-        expect(settings.optionalBoolean).toHaveBeenCalledWith('isAppStreamEnabled', false);
+        expect(settings.getBoolean).toHaveBeenCalledTimes(1);
+        expect(settings.getBoolean).toHaveBeenCalledWith('isAppStreamEnabled');
       }
     });
 
     it('verify getSecurityGroupDetails not called when AppStream is enabled and fetchCidr is undefined', async () => {
       // BUILD
-      settings.optionalBoolean = jest.fn((key, defaultBoolean) => {
+      settings.getBoolean = jest.fn(key => {
         if (key === 'isAppStreamEnabled') {
           return true;
         }
-        return defaultBoolean;
+        throw Error(`${key} not found`);
       });
       const uid = 'u-12345';
       const requestContext = { principalIdentifier: { uid } };
@@ -464,8 +464,8 @@ describe('EnvironmentSCService', () => {
         expect.objectContaining({ action: 'get-sc', conditions: [service._allowAuthorized] }),
         env,
       );
-      expect(settings.optionalBoolean).toHaveBeenCalledTimes(1);
-      expect(settings.optionalBoolean).toHaveBeenCalledWith('isAppStreamEnabled', false);
+      expect(settings.getBoolean).toHaveBeenCalledTimes(1);
+      expect(settings.getBoolean).toHaveBeenCalledWith('isAppStreamEnabled');
     });
 
     it('verify getSecurityGroupDetails not called when fetchCidr is false', async () => {
diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-cidr-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-cidr-service.js
index 9ff2c79c88..7a36cc39b4 100644
--- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-cidr-service.js
+++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-cidr-service.js
@@ -46,7 +46,7 @@ class EnvironmentScCidrService extends Service {
   }
 
   checkRequest(updateRequest) {
-    const isAppStreamEnabled = this.settings.optionalBoolean(settingKeys.isAppStreamEnabled, false);
+    const isAppStreamEnabled = this.settings.getBoolean(settingKeys.isAppStreamEnabled);
     if (isAppStreamEnabled) {
       throw this.boom.badRequest(`CIDR operation unavailable when AppStream is enabled`, true);
     }
diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js
index 9109369044..2c7983e387 100644
--- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js
+++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js
@@ -1041,7 +1041,7 @@ class EnvironmentScService extends Service {
   }
 
   isAppStreamEnabled() {
-    return this.settings.optionalBoolean(settingKeys.isAppStreamEnabled, false);
+    return this.settings.getBoolean(settingKeys.isAppStreamEnabled);
   }
 
   /**

From 71357519f881c6a3ca25f33adf41bb71cc43e10c Mon Sep 17 00:00:00 2001
From: Jeet Nihalani <jeetend@amazon.com>
Date: Thu, 5 Aug 2021 17:16:26 -0600
Subject: [PATCH 3/3] fix: Update CIDR fetching based on updated CFN template

---
 .../sagemaker-notebook-instance.cfn.yml       | 14 +--
 .../__tests__/environment-sc-service.test.js  | 85 +++++++++++++++++++
 .../service-catalog/environment-sc-service.js | 16 ++--
 3 files changed, 102 insertions(+), 13 deletions(-)

diff --git a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/sagemaker-notebook-instance.cfn.yml b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/sagemaker-notebook-instance.cfn.yml
index 2852be1431..f7ae80bfec 100644
--- a/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/sagemaker-notebook-instance.cfn.yml
+++ b/addons/addon-base-raas/packages/base-raas-cfn-templates/src/templates/service-catalog/sagemaker-notebook-instance.cfn.yml
@@ -61,13 +61,13 @@ Resources:
       VpcId:
         Ref: VPC
       SecurityGroupIngress:
-        !If
-        - AppStreamEnabled
-        - !Ref 'AWS::NoValue'
-        - IpProtocol: tcp
-          FromPort: 443
-          ToPort: 443
-          CidrIp: !Ref AccessFromCIDRBlock
+        - !If
+          - AppStreamEnabled
+          - !Ref "AWS::NoValue"
+          - IpProtocol: tcp
+            FromPort: 443
+            ToPort: 443
+            CidrIp: !Ref AccessFromCIDRBlock
 
   PreSignedURLBoundary:
     Type: AWS::IAM::ManagedPolicy
diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js
index 451d8f2f4a..e21c03ac02 100644
--- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js
+++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/__tests__/environment-sc-service.test.js
@@ -1438,6 +1438,91 @@ describe('EnvironmentSCService', () => {
   });
 
   describe('getSecurityGroupDetails function', () => {
+    it('should send filtered security group rules as expected for AppStream template', async () => {
+      // BUILD
+      const requestContext = {};
+      const stackArn = 'sampleCloudFormationStackArn';
+      const environment = {
+        outputs: [{ OutputKey: 'CloudformationStackARN', OutputValue: `<AwsAccountRoot>/${stackArn}` }],
+        status: 'COMPLETED',
+      };
+      const origSecurityGroupId = 'sampleSecurityGroupId';
+      const stackResources = {
+        StackResourceSummaries: [{ LogicalResourceId: 'SecurityGroup', PhysicalResourceId: origSecurityGroupId }],
+      };
+      const templateDetails = {
+        TemplateBody: YAML.dump({
+          Resources: {
+            SecurityGroup: {
+              Properties: {
+                SecurityGroupIngress: [
+                  {
+                    'Fn::If': [
+                      'AppStreamEnabled',
+                      {
+                        CidrIp: {
+                          Ref: 'AccessFromCIDRBlock',
+                        },
+                        IpProtocol: 'tcp',
+                        FromPort: 1,
+                        ToPort: 1,
+                      },
+                      {
+                        IpProtocol: 'tcp',
+                        FromPort: 123,
+                        ToPort: 123,
+                        CidrIp: {
+                          Ref: 'AccessFromCIDRBlock',
+                        },
+                      },
+                    ],
+                  },
+                ],
+              },
+            },
+          },
+        }),
+      };
+      const workspaceIngressRules = [
+        {
+          IpProtocol: 'tcp',
+          FromPort: 123,
+          ToPort: 123,
+          IpRanges: [{ CidrIp: '123.123.123.123/32' }],
+        },
+        {
+          IpProtocol: 'tcp',
+          FromPort: 1,
+          ToPort: 1,
+          IpRanges: [{ CidrIp: '123.123.123.123/32' }],
+        },
+      ];
+      service.getCfnDetails = jest.fn(() => {
+        return { stackResources, templateDetails };
+      });
+      service.getWorkspaceSecurityGroup = jest.fn(() => {
+        return { securityGroupResponse: { SecurityGroups: [{ IpPermissions: workspaceIngressRules }] } };
+      });
+      const expectedOutcome = [
+        {
+          protocol: 'tcp',
+          fromPort: 123,
+          toPort: 123,
+          cidrBlocks: ['123.123.123.123/32'],
+        },
+      ];
+
+      // OPERATE
+      const { currentIngressRules, securityGroupId } = await service.getSecurityGroupDetails(
+        requestContext,
+        environment,
+      );
+
+      // CHECK
+      expect(currentIngressRules).toMatchObject(expectedOutcome);
+      expect(securityGroupId).toEqual(origSecurityGroupId);
+    });
+
     it('should send filtered security group rules as expected', async () => {
       // BUILD
       const requestContext = {};
diff --git a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js
index 2c7983e387..3e536523a0 100644
--- a/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js
+++ b/addons/addon-base-raas/packages/base-raas-services/lib/environment/service-catalog/environment-sc-service.js
@@ -1020,19 +1020,23 @@ class EnvironmentScService extends Service {
 
     // Only send back details of groups configured by the SC CFN stack
     const returnVal = _.map(cfnTemplateIngressRules, cfnRule => {
+      let ruleToUse = cfnRule;
+      if ('Fn::If' in cfnRule && cfnRule['Fn::If'][0] === 'AppStreamEnabled') {
+        ruleToUse = cfnRule['Fn::If'][2];
+      }
       const matchingRule = _.find(
         workspaceIngressRules,
         workspaceRule =>
-          cfnRule.FromPort === workspaceRule.FromPort &&
-          cfnRule.ToPort === workspaceRule.ToPort &&
-          cfnRule.IpProtocol === workspaceRule.IpProtocol,
+          ruleToUse.FromPort === workspaceRule.FromPort &&
+          ruleToUse.ToPort === workspaceRule.ToPort &&
+          ruleToUse.IpProtocol === workspaceRule.IpProtocol,
       );
       const currentCidrRanges = matchingRule ? _.map(matchingRule.IpRanges, ipRange => ipRange.CidrIp) : [];
 
       return {
-        fromPort: cfnRule.FromPort,
-        toPort: cfnRule.ToPort,
-        protocol: cfnRule.IpProtocol,
+        fromPort: ruleToUse.FromPort,
+        toPort: ruleToUse.ToPort,
+        protocol: ruleToUse.IpProtocol,
         cidrBlocks: currentCidrRanges,
       };
     });