From 05f6b36ece7d875e4e1e2e6ad515abac20b86493 Mon Sep 17 00:00:00 2001 From: dandds Date: Tue, 4 Feb 2020 10:16:02 -0500 Subject: [PATCH 01/21] Update SQL query to find pending portfolios. The query to find portfolios that are pending provisioning is updated to check for: - a period of performance that has started - a portfolio state machine that has an UNSTARTED or one of the CREATED states I left several TODOs to ensure that the orchestration functions correctly for portfolio. --- atst/domain/portfolios/portfolios.py | 22 ++++++------ atst/jobs.py | 2 +- atst/models/portfolio_state_machine.py | 3 ++ tests/domain/test_environments.py | 36 +------------------ tests/domain/test_portfolios.py | 49 ++++++++++++++++++++++---- tests/test_jobs.py | 16 +++++++-- tests/utils.py | 40 +++++++++++++++++++++ 7 files changed, 110 insertions(+), 58 deletions(-) diff --git a/atst/domain/portfolios/portfolios.py b/atst/domain/portfolios/portfolios.py index 1254ac71..b8663730 100644 --- a/atst/domain/portfolios/portfolios.py +++ b/atst/domain/portfolios/portfolios.py @@ -15,6 +15,8 @@ from atst.models import ( Permissions, PortfolioRole, PortfolioRoleStatus, + TaskOrder, + CLIN, ) from .query import PortfoliosQuery, PortfolioStateMachinesQuery @@ -144,7 +146,7 @@ class Portfolios(object): return db.session.query(Portfolio.id) @classmethod - def get_portfolios_pending_provisioning(cls) -> List[UUID]: + def get_portfolios_pending_provisioning(cls, now) -> List[UUID]: """ Any portfolio with a corresponding State Machine that is either: not started yet, @@ -153,22 +155,18 @@ class Portfolios(object): """ results = ( - cls.base_provision_query() + db.session.query(Portfolio.id) .join(PortfolioStateMachine) + .join(TaskOrder) + .join(CLIN) + .filter(Portfolio.deleted == False) + .filter(CLIN.start_date <= now) + .filter(CLIN.end_date > now) .filter( or_( PortfolioStateMachine.state == FSMStates.UNSTARTED, - PortfolioStateMachine.state == FSMStates.FAILED, - PortfolioStateMachine.state == FSMStates.TENANT_FAILED, + PortfolioStateMachine.state.like("%CREATED"), ) ) ) return [id_ for id_, in results] - - # db.session.query(PortfolioStateMachine).\ - # filter( - # or_( - # PortfolioStateMachine.state==FSMStates.UNSTARTED, - # PortfolioStateMachine.state==FSMStates.UNSTARTED, - # ) - # ).all() diff --git a/atst/jobs.py b/atst/jobs.py index 986b2004..ee820066 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -203,7 +203,7 @@ def dispatch_provision_portfolio(self): """ Iterate over portfolios with a corresponding State Machine that have not completed. """ - for portfolio_id in Portfolios.get_portfolios_pending_provisioning(): + for portfolio_id in Portfolios.get_portfolios_pending_provisioning(pendulum.now()): provision_portfolio.delay(portfolio_id=portfolio_id) diff --git a/atst/models/portfolio_state_machine.py b/atst/models/portfolio_state_machine.py index 14e9c01d..f5c1a461 100644 --- a/atst/models/portfolio_state_machine.py +++ b/atst/models/portfolio_state_machine.py @@ -175,11 +175,14 @@ class PortfolioStateMachine( app.logger.info(exc.json()) print(exc.json()) app.logger.info(payload_data) + # TODO: Ensure that failing the stage does not preclude a Celery retry self.fail_stage(stage) + # TODO: catch and handle general CSP exception here except (ConnectionException, UnknownServerException) as exc: app.logger.error( f"CSP api call. Caught exception for {self.__repr__()}.", exc_info=1, ) + # TODO: Ensure that failing the stage does not preclude a Celery retry self.fail_stage(stage) self.finish_stage(stage) diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index ff4b8605..9144c68a 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -1,5 +1,4 @@ import pytest -import pendulum from uuid import uuid4 from atst.domain.environments import Environments @@ -14,6 +13,7 @@ from tests.factories import ( EnvironmentRoleFactory, ApplicationRoleFactory, ) +from tests.utils import EnvQueryTest def test_create_environments(): @@ -119,40 +119,6 @@ def test_update_does_not_duplicate_names_within_application(): Environments.update(dupe_env, name) -class EnvQueryTest: - @property - def NOW(self): - return pendulum.now() - - @property - def YESTERDAY(self): - return self.NOW.subtract(days=1) - - @property - def TOMORROW(self): - return self.NOW.add(days=1) - - def create_portfolio_with_clins(self, start_and_end_dates, env_data=None): - env_data = env_data or {} - return PortfolioFactory.create( - applications=[ - { - "name": "Mos Eisley", - "description": "Where Han shot first", - "environments": [{"name": "thebar", **env_data}], - } - ], - task_orders=[ - { - "create_clins": [ - {"start_date": start_date, "end_date": end_date} - for (start_date, end_date) in start_and_end_dates - ] - } - ], - ) - - class TestGetEnvironmentsPendingCreate(EnvQueryTest): def test_with_expired_clins(self, session): self.create_portfolio_with_clins([(self.YESTERDAY, self.YESTERDAY)]) diff --git a/tests/domain/test_portfolios.py b/tests/domain/test_portfolios.py index ff8ccacb..1093253b 100644 --- a/tests/domain/test_portfolios.py +++ b/tests/domain/test_portfolios.py @@ -26,6 +26,7 @@ from tests.factories import ( PortfolioStateMachineFactory, get_all_portfolio_permission_sets, ) +from tests.utils import EnvQueryTest @pytest.fixture(scope="function") @@ -263,10 +264,44 @@ def test_create_state_machine(portfolio): assert fsm -def test_get_portfolios_pending_provisioning(session): - for x in range(5): - portfolio = PortfolioFactory.create() - sm = PortfolioStateMachineFactory.create(portfolio=portfolio) - if x == 2: - sm.state = FSMStates.COMPLETED - assert len(Portfolios.get_portfolios_pending_provisioning()) == 4 +class TestGetPortfoliosPendingCreate(EnvQueryTest): + def test_finds_unstarted(self): + for x in range(5): + if x == 2: + state = "COMPLETED" + else: + state = "UNSTARTED" + self.create_portfolio_with_clins( + [(self.YESTERDAY, self.TOMORROW)], state_machine_status=state + ) + assert len(Portfolios.get_portfolios_pending_provisioning(self.NOW)) == 4 + + def test_finds_created(self): + self.create_portfolio_with_clins( + [(self.YESTERDAY, self.TOMORROW)], state_machine_status="TENANT_CREATED" + ) + assert len(Portfolios.get_portfolios_pending_provisioning(self.NOW)) == 1 + + def test_does_not_find_failed(self): + self.create_portfolio_with_clins( + [(self.YESTERDAY, self.TOMORROW)], state_machine_status="TENANT_FAILED" + ) + assert len(Portfolios.get_portfolios_pending_provisioning(self.NOW)) == 0 + + def test_with_expired_clins(self): + self.create_portfolio_with_clins([(self.YESTERDAY, self.YESTERDAY)]) + assert len(Portfolios.get_portfolios_pending_provisioning(self.NOW)) == 0 + + def test_with_active_clins(self): + portfolio = self.create_portfolio_with_clins([(self.YESTERDAY, self.TOMORROW)]) + Portfolios.get_portfolios_pending_provisioning(self.NOW) == [portfolio.id] + + def test_with_future_clins(self): + self.create_portfolio_with_clins([(self.TOMORROW, self.TOMORROW)]) + assert len(Portfolios.get_portfolios_pending_provisioning(self.NOW)) == 0 + + def test_with_already_provisioned_env(self): + self.create_portfolio_with_clins( + [(self.YESTERDAY, self.TOMORROW)], env_data={"cloud_id": uuid4().hex} + ) + assert len(Portfolios.get_portfolios_pending_provisioning(self.NOW)) == 0 diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 54f4c5dc..f15e9da0 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -286,9 +286,19 @@ def test_create_environment_no_dupes(session, celery_app, celery_worker): assert environment.claimed_until == None -def test_dispatch_provision_portfolio( - csp, session, portfolio, celery_app, celery_worker, monkeypatch -): +def test_dispatch_provision_portfolio(csp, monkeypatch): + portfolio = PortfolioFactory.create( + task_orders=[ + { + "create_clins": [ + { + "start_date": pendulum.now().subtract(days=1), + "end_date": pendulum.now().add(days=1), + } + ] + } + ], + ) sm = PortfolioStateMachineFactory.create(portfolio=portfolio) mock = Mock() monkeypatch.setattr("atst.jobs.provision_portfolio", mock) diff --git a/tests/utils.py b/tests/utils.py index 66bf2b18..665910af 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -5,9 +5,12 @@ from unittest.mock import Mock from OpenSSL import crypto from cryptography.hazmat.backends import default_backend from flask import template_rendered +import pendulum from atst.utils.notification_sender import NotificationSender +import tests.factories as factories + @contextmanager def captured_templates(app): @@ -62,3 +65,40 @@ def make_crl_list(x509_obj, x509_path): issuer = x509_obj.issuer.public_bytes(default_backend()) filename = os.path.basename(x509_path) return [(filename, issuer.hex())] + + +class EnvQueryTest: + @property + def NOW(self): + return pendulum.now() + + @property + def YESTERDAY(self): + return self.NOW.subtract(days=1) + + @property + def TOMORROW(self): + return self.NOW.add(days=1) + + def create_portfolio_with_clins( + self, start_and_end_dates, env_data=None, state_machine_status=None + ): + env_data = env_data or {} + return factories.PortfolioFactory.create( + state=state_machine_status, + applications=[ + { + "name": "Mos Eisley", + "description": "Where Han shot first", + "environments": [{"name": "thebar", **env_data}], + } + ], + task_orders=[ + { + "create_clins": [ + {"start_date": start_date, "end_date": end_date} + for (start_date, end_date) in start_and_end_dates + ] + } + ], + ) From 1378fcfc15b2e7a4bc0fc28bf252259204fbbe95 Mon Sep 17 00:00:00 2001 From: Philip Kalinsky Date: Tue, 28 Jan 2020 15:06:48 -0500 Subject: [PATCH 02/21] merge conflict --- atst/domain/csp/cloud/azure_cloud_provider.py | 35 ++++++++++-------- atst/domain/csp/cloud/mock_cloud_provider.py | 37 +++++-------------- atst/domain/csp/cloud/models.py | 8 ++++ atst/domain/environments.py | 2 +- atst/jobs.py | 36 +++++++----------- tests/domain/cloud/test_azure_csp.py | 10 +++-- 6 files changed, 57 insertions(+), 71 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index d5ef5204..908474ab 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -24,6 +24,8 @@ from .models import ( BillingProfileTenantAccessCSPResult, BillingProfileVerificationCSPPayload, BillingProfileVerificationCSPResult, + EnvironmentCSPPayload, + EnvironmentCSPResult, KeyVaultCredentials, ManagementGroupCSPResponse, ProductPurchaseCSPPayload, @@ -135,24 +137,25 @@ class AzureCloudProvider(CloudProviderInterface): exc_info=1, ) - def create_environment(self, auth_credentials: Dict, user, environment): - # since this operation would only occur within a tenant, should we source the tenant - # via lookup from environment once we've created the portfolio csp data schema - # something like this: - # environment_tenant = environment.application.portfolio.csp_data.get('tenant_id', None) - # though we'd probably source the whole credentials for these calls from the portfolio csp - # data, as it would have to be where we store the creds for the at-at user within the portfolio tenant - # credentials = self._get_credential_obj(environment.application.portfolio.csp_data.get_creds()) - credentials = self._get_credential_obj(self._root_creds) - display_name = f"{environment.application.name}_{environment.name}_{environment.id}" # proposed format - management_group_id = "?" # management group id chained from environment - parent_id = "?" # from environment.application - - management_group = self._create_management_group( - credentials, management_group_id, display_name, parent_id, + def create_environment(self, payload: EnvironmentCSPPayload): + creds = self._source_creds(payload.tenant_id) + credentials = self._get_credential_obj( + { + "client_id": creds.root_sp_client_id, + "secret_key": creds.root_sp_key, + "tenant_id": creds.root_tenant_id, + }, + resource=AZURE_MANAGEMENT_API, ) - return ManagementGroupCSPResponse(**management_group) + response = self._create_management_group( + credentials, + payload.management_group_name, + payload.display_name, + payload.parent_id, + ) + + return EnvironmentCSPResult(**response) def create_atat_admin_user( self, auth_credentials: Dict, csp_environment_id: str diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index ec730a3b..5b9a8b34 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -35,6 +35,8 @@ from .models import ( SubscriptionCreationCSPResult, SubscriptionVerificationCSPPayload, SuscriptionVerificationCSPResult, + EnvironmentCSPPayload, + EnvironmentCSPResult, TaskOrderBillingCreationCSPPayload, TaskOrderBillingCreationCSPResult, TaskOrderBillingVerificationCSPPayload, @@ -91,34 +93,6 @@ class MockCloudProvider(CloudProviderInterface): def get_secret(self, secret_key: str, default=dict()): return default - def create_environment(self, auth_credentials, user, environment): - self._authorize(auth_credentials) - - self._delay(1, 5) - self._maybe_raise(self.NETWORK_FAILURE_PCT, self.NETWORK_EXCEPTION) - self._maybe_raise(self.SERVER_FAILURE_PCT, self.SERVER_EXCEPTION) - self._maybe_raise( - self.ENV_CREATE_FAILURE_PCT, - EnvironmentCreationException( - environment.id, "Could not create environment." - ), - ) - - csp_environment_id = self._id() - - self._delay(1, 5) - self._maybe_raise(self.NETWORK_FAILURE_PCT, self.NETWORK_EXCEPTION) - self._maybe_raise(self.SERVER_FAILURE_PCT, self.SERVER_EXCEPTION) - self._maybe_raise( - self.ATAT_ADMIN_CREATE_FAILURE_PCT, - BaselineProvisionException( - csp_environment_id, "Could not create environment baseline." - ), - ) - self._maybe_raise(self.UNAUTHORIZED_RATE, self.AUTHORIZATION_EXCEPTION) - - return csp_environment_id - def create_subscription(self, payload: SubscriptionCreationCSPPayload): return self.create_subscription_creation(payload) @@ -482,6 +456,13 @@ class MockCloudProvider(CloudProviderInterface): return UserCSPResult(id=str(uuid4())) + def create_environment(self, payload: EnvironmentCSPPayload): + self._maybe_raise(self.UNAUTHORIZED_RATE, GeneralCSPException) + + return EnvironmentCSPResult( + id=f"{AZURE_MGMNT_PATH}{payload.management_group_name}" + ) + def get_credentials(self, scope="portfolio", tenant_id=None): return self.root_creds() diff --git a/atst/domain/csp/cloud/models.py b/atst/domain/csp/cloud/models.py index 188c2cc7..05f3c866 100644 --- a/atst/domain/csp/cloud/models.py +++ b/atst/domain/csp/cloud/models.py @@ -358,6 +358,14 @@ class ApplicationCSPResult(ManagementGroupCSPResponse): pass +class EnvironmentCSPPayload(ManagementGroupCSPPayload): + pass + + +class EnvironmentCSPResult(ManagementGroupCSPResponse): + pass + + class KeyVaultCredentials(BaseModel): root_sp_client_id: Optional[str] root_sp_key: Optional[str] diff --git a/atst/domain/environments.py b/atst/domain/environments.py index b8a59485..78aa2af3 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -124,7 +124,7 @@ class Environments(object): Any environment with an active CLIN that doesn't yet have a `cloud_id`. """ results = ( - cls.base_provision_query(now).filter(Environment.cloud_id == None).all() + cls.base_provision_query(now).filter(Application.cloud_id != None).all() ) return [id_ for id_, in results] diff --git a/atst/jobs.py b/atst/jobs.py index 986b2004..c2e2d03e 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -12,7 +12,11 @@ from atst.domain.portfolios import Portfolios from atst.domain.application_roles import ApplicationRoles from atst.models.utils import claim_for_update, claim_many_for_update from atst.utils.localization import translate -from atst.domain.csp.cloud.models import ApplicationCSPPayload, UserCSPPayload +from atst.domain.csp.cloud.models import ( + ApplicationCSPPayload, + EnvironmentCSPPayload, + UserCSPPayload, +) class RecordFailure(celery.Task): @@ -109,34 +113,20 @@ def do_create_environment(csp: CloudProviderInterface, environment_id=None): with claim_for_update(environment) as environment: if environment.cloud_id is not None: - # TODO: Return value for this? return - user = environment.creator + csp_details = environment.application.portfolio.csp_data + parent_id = environment.application.cloud_id + tenant_id = csp_details.get("tenant_id") + payload = EnvironmentCSPPayload( + tenant_id=tenant_id, display_name=application.name, parent_id=parent_id + ) - # we'll need to do some checking in this job for cases where it's retrying - # when a failure occured after some successful steps - # (e.g. if environment.cloud_id is not None, then we can skip first step) - - # credentials either from a given user or pulled from config? - # if using global creds, do we need to log what user authorized action? - atat_root_creds = csp.root_creds() - - # user is needed because baseline root account in the environment will - # be assigned to the requesting user, open question how to handle duplicate - # email addresses across new environments - csp_environment_id = csp.create_environment(atat_root_creds, user, environment) - environment.cloud_id = csp_environment_id + env_result = csp.create_environment(payload) + environment.cloud_id = env_result.id db.session.add(environment) db.session.commit() - body = render_email( - "emails/application/environment_ready.txt", {"environment": environment} - ) - app.mailer.send( - [environment.creator.email], translate("email.environment_ready"), body - ) - def do_create_atat_admin_user(csp: CloudProviderInterface, environment_id=None): environment = Environments.get(environment_id) diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index eef5620e..639997b3 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -20,6 +20,8 @@ from atst.domain.csp.cloud.models import ( BillingProfileTenantAccessCSPResult, BillingProfileVerificationCSPPayload, BillingProfileVerificationCSPResult, + EnvironmentCSPPayload, + EnvironmentCSPResult, ProductPurchaseCSPPayload, ProductPurchaseCSPResult, ProductPurchaseVerificationCSPPayload, @@ -57,12 +59,14 @@ def mock_management_group_create(mock_azure, spec_dict): def test_create_environment_succeeds(mock_azure: AzureCloudProvider): environment = EnvironmentFactory.create() - mock_management_group_create(mock_azure, {"id": "Test Id"}) - result = mock_azure.create_environment( - AUTH_CREDENTIALS, environment.creator, environment + mock_azure = mock_get_secret(mock_azure, lambda *a, **k: json.dumps(MOCK_CREDS)) + + payload = EnvironmentCSPPayload( + tenant_id="1234", display_name=environment.name, parent_id=str(uuid4()) ) + result = mock_azure.create_environment(payload) assert result.id == "Test Id" From 21d48c55cf1467b3730582c3fd82232392bbfef2 Mon Sep 17 00:00:00 2001 From: tomdds Date: Fri, 31 Jan 2020 16:52:44 -0500 Subject: [PATCH 03/21] Use correct config value for create_environment resource --- atst/domain/csp/cloud/azure_cloud_provider.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 908474ab..ce6d45ee 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -145,7 +145,7 @@ class AzureCloudProvider(CloudProviderInterface): "secret_key": creds.root_sp_key, "tenant_id": creds.root_tenant_id, }, - resource=AZURE_MANAGEMENT_API, + resource=self.sdk.cloud.endpoints.resource_manager, ) response = self._create_management_group( @@ -155,6 +155,8 @@ class AzureCloudProvider(CloudProviderInterface): payload.parent_id, ) + if + return EnvironmentCSPResult(**response) def create_atat_admin_user( From bcb552134b48dc3e8bd5cb4ba043fa71d1f35e16 Mon Sep 17 00:00:00 2001 From: tomdds Date: Mon, 3 Feb 2020 15:01:42 -0500 Subject: [PATCH 04/21] Update tests for environment management group provisioning --- atst/jobs.py | 2 +- tests/domain/cloud/test_mock_csp.py | 15 ++++++++++++--- tests/test_jobs.py | 4 ++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/atst/jobs.py b/atst/jobs.py index c2e2d03e..6a91b03e 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -119,7 +119,7 @@ def do_create_environment(csp: CloudProviderInterface, environment_id=None): parent_id = environment.application.cloud_id tenant_id = csp_details.get("tenant_id") payload = EnvironmentCSPPayload( - tenant_id=tenant_id, display_name=application.name, parent_id=parent_id + tenant_id=tenant_id, display_name=environment.name, parent_id=parent_id ) env_result = csp.create_environment(payload) diff --git a/tests/domain/cloud/test_mock_csp.py b/tests/domain/cloud/test_mock_csp.py index b3a8d551..86727b38 100644 --- a/tests/domain/cloud/test_mock_csp.py +++ b/tests/domain/cloud/test_mock_csp.py @@ -1,6 +1,7 @@ import pytest from atst.domain.csp import MockCloudProvider +from atst.domain.csp.cloud.models import EnvironmentCSPPayload, EnvironmentCSPResult from tests.factories import EnvironmentFactory, EnvironmentRoleFactory, UserFactory @@ -14,9 +15,17 @@ def mock_csp(): def test_create_environment(mock_csp: MockCloudProvider): environment = EnvironmentFactory.create() - user = UserFactory.create() - environment_id = mock_csp.create_environment(CREDENTIALS, user, environment) - assert isinstance(environment_id, str) + environment.application.cloud_id = "parent_id" + environment.application.portfolio.csp_data = {"tenant_id": "fake"} + payload = EnvironmentCSPPayload( + **dict( + tenant_id=environment.application.portfolio.csp_data.get("tenant_id"), + display_name=environment.name, + parent_id=environment.application.cloud_id, + ) + ) + result = mock_csp.create_environment(payload) + assert isinstance(result, EnvironmentCSPResult) def test_create_admin_user(mock_csp: MockCloudProvider): diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 54f4c5dc..de17eb6a 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -94,6 +94,10 @@ tomorrow = now.add(days=1) def test_create_environment_job(session, csp): environment = EnvironmentFactory.create() + environment.application.cloud_id = "parentId" + environment.application.portfolio.csp_data = {"tenant_id": "fake"} + session.add(environment) + session.commit() do_create_environment(csp, environment.id) session.refresh(environment) From b364a1d2a6ed59b07716000e1502f2f096d0962c Mon Sep 17 00:00:00 2001 From: dandds Date: Sun, 2 Feb 2020 13:58:41 -0500 Subject: [PATCH 05/21] Celery wrapper for creating a user. --- atst/domain/csp/cloud/mock_cloud_provider.py | 5 +++++ atst/jobs.py | 21 ++++++++++---------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index 5b9a8b34..96648b52 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -463,6 +463,11 @@ class MockCloudProvider(CloudProviderInterface): id=f"{AZURE_MGMNT_PATH}{payload.management_group_name}" ) + def create_user(self, payload: UserCSPPayload): + self._maybe_raise(self.UNAUTHORIZED_RATE, GeneralCSPException) + + return UserCSPResult(id=str(uuid4())) + def get_credentials(self, scope="portfolio", tenant_id=None): return self.root_creds() diff --git a/atst/jobs.py b/atst/jobs.py index 6a91b03e..0ce664da 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -1,22 +1,23 @@ -from flask import current_app as app import pendulum +from flask import current_app as app from atst.database import db -from atst.queue import celery -from atst.models import JobFailure -from atst.domain.csp.cloud.exceptions import GeneralCSPException -from atst.domain.csp.cloud import CloudProviderInterface -from atst.domain.applications import Applications -from atst.domain.environments import Environments -from atst.domain.portfolios import Portfolios from atst.domain.application_roles import ApplicationRoles -from atst.models.utils import claim_for_update, claim_many_for_update -from atst.utils.localization import translate +from atst.domain.applications import Applications +from atst.domain.csp.cloud import CloudProviderInterface +from atst.domain.csp.cloud.exceptions import GeneralCSPException from atst.domain.csp.cloud.models import ( ApplicationCSPPayload, EnvironmentCSPPayload, UserCSPPayload, ) +from atst.domain.environment_roles import EnvironmentRoles +from atst.domain.environments import Environments +from atst.domain.portfolios import Portfolios +from atst.models import JobFailure +from atst.models.utils import claim_for_update, claim_many_for_update +from atst.queue import celery +from atst.utils.localization import translate class RecordFailure(celery.Task): From 17ec944ad5abb71f8c2fb367f90b85ff3ca0ebf9 Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 14:34:59 -0500 Subject: [PATCH 06/21] Update environment queries to look for app provisioning completion as well --- atst/domain/environments.py | 5 ++++- atst/jobs.py | 2 -- tests/domain/test_environments.py | 23 +++++++++++++++++------ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 78aa2af3..3abc4b18 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -124,7 +124,10 @@ class Environments(object): Any environment with an active CLIN that doesn't yet have a `cloud_id`. """ results = ( - cls.base_provision_query(now).filter(Application.cloud_id != None).all() + cls.base_provision_query(now) + .filter(Application.cloud_id != None) + .filter(Environment.cloud_id == None) + .all() ) return [id_ for id_, in results] diff --git a/atst/jobs.py b/atst/jobs.py index 0ce664da..ae0655fe 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -11,13 +11,11 @@ from atst.domain.csp.cloud.models import ( EnvironmentCSPPayload, UserCSPPayload, ) -from atst.domain.environment_roles import EnvironmentRoles from atst.domain.environments import Environments from atst.domain.portfolios import Portfolios from atst.models import JobFailure from atst.models.utils import claim_for_update, claim_many_for_update from atst.queue import celery -from atst.utils.localization import translate class RecordFailure(celery.Task): diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index ff4b8605..72f07651 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -132,15 +132,19 @@ class EnvQueryTest: def TOMORROW(self): return self.NOW.add(days=1) - def create_portfolio_with_clins(self, start_and_end_dates, env_data=None): + def create_portfolio_with_clins( + self, start_and_end_dates, env_data=None, app_data=None + ): env_data = env_data or {} + app_data = app_data or {} return PortfolioFactory.create( applications=[ { "name": "Mos Eisley", "description": "Where Han shot first", "environments": [{"name": "thebar", **env_data}], - } + **app_data, + }, ], task_orders=[ { @@ -168,9 +172,17 @@ class TestGetEnvironmentsPendingCreate(EnvQueryTest): self.create_portfolio_with_clins([(self.TOMORROW, self.TOMORROW)]) assert len(Environments.get_environments_pending_creation(self.NOW)) == 0 + def test_with_already_provisioned_app(self, session): + self.create_portfolio_with_clins( + [(self.YESTERDAY, self.TOMORROW)], app_data={"cloud_id": uuid4().hex} + ) + assert len(Environments.get_environments_pending_creation(self.NOW)) == 1 + def test_with_already_provisioned_env(self, session): self.create_portfolio_with_clins( - [(self.YESTERDAY, self.TOMORROW)], env_data={"cloud_id": uuid4().hex} + [(self.YESTERDAY, self.TOMORROW)], + env_data={"cloud_id": uuid4().hex}, + app_data={"cloud_id": uuid4().hex}, ) assert len(Environments.get_environments_pending_creation(self.NOW)) == 0 @@ -187,11 +199,10 @@ class TestGetEnvironmentsPendingAtatUserCreation(EnvQueryTest): def test_with_unprovisioned_environment(self): self.create_portfolio_with_clins( - [(self.YESTERDAY, self.TOMORROW)], - {"cloud_id": uuid4().hex, "root_user_info": None}, + [(self.YESTERDAY, self.TOMORROW)], app_data={"cloud_id": uuid4().hex}, ) assert ( - len(Environments.get_environments_pending_atat_user_creation(self.NOW)) == 1 + len(Environments.get_environments_pending_atat_user_creation(self.NOW)) == 0 ) def test_with_unprovisioned_expired_clins_environment(self): From 1a7db62dac87ccc42126ab5ab86693c3f7eecbbb Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 15:16:18 -0500 Subject: [PATCH 07/21] Remove Environment level user provisioning --- ..._remove_root_user_info_from_environment.py | 28 +++++++++++++ atst/domain/environment_roles.py | 6 ++- atst/domain/environments.py | 12 ------ atst/jobs.py | 29 -------------- atst/models/environment.py | 19 +++------ atst/queue.py | 4 -- tests/domain/test_environment_roles.py | 6 +-- tests/domain/test_environments.py | 28 ------------- tests/models/test_environments.py | 14 +------ tests/test_jobs.py | 39 +------------------ 10 files changed, 42 insertions(+), 143 deletions(-) create mode 100644 alembic/versions/0039308c6351_remove_root_user_info_from_environment.py diff --git a/alembic/versions/0039308c6351_remove_root_user_info_from_environment.py b/alembic/versions/0039308c6351_remove_root_user_info_from_environment.py new file mode 100644 index 00000000..e9f3fa8a --- /dev/null +++ b/alembic/versions/0039308c6351_remove_root_user_info_from_environment.py @@ -0,0 +1,28 @@ +"""Remove root_user_info from Environment + +Revision ID: 0039308c6351 +Revises: 17da2a475429 +Create Date: 2020-02-04 14:37:06.814645 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '0039308c6351' # pragma: allowlist secret +down_revision = '17da2a475429' # pragma: allowlist secret +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('environments', 'root_user_info') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('environments', sa.Column('root_user_info', postgresql.JSONB(astext_type=sa.Text()), autoincrement=False, nullable=True)) + # ### end Alembic commands ### diff --git a/atst/domain/environment_roles.py b/atst/domain/environment_roles.py index ef8a4b8e..a022b932 100644 --- a/atst/domain/environment_roles.py +++ b/atst/domain/environment_roles.py @@ -107,8 +107,10 @@ class EnvironmentRoles(object): environment_role = EnvironmentRoles.get_by_id(environment_role_id) if environment_role.csp_user_id and not environment_role.environment.is_pending: - credentials = environment_role.environment.csp_credentials - app.csp.cloud.disable_user(credentials, environment_role.csp_user_id) + tenant_id = environment_role.environment.application.portfolio.csp_data.get( + "tenant_id" + ) + app.csp.cloud.disable_user(tenant_id, environment_role.csp_user_id) environment_role.status = EnvironmentRole.Status.DISABLED db.session.add(environment_role) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 3abc4b18..f945f3f8 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -130,15 +130,3 @@ class Environments(object): .all() ) return [id_ for id_, in results] - - @classmethod - def get_environments_pending_atat_user_creation(cls, now) -> List[UUID]: - """ - Any environment with an active CLIN that has a cloud_id but no `root_user_info`. - """ - results = ( - cls.base_provision_query(now) - .filter(Environment.cloud_id != None) - .filter(Environment.root_user_info == None) - ).all() - return [id_ for id_, in results] diff --git a/atst/jobs.py b/atst/jobs.py index ae0655fe..3453f81b 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -127,20 +127,6 @@ def do_create_environment(csp: CloudProviderInterface, environment_id=None): db.session.commit() -def do_create_atat_admin_user(csp: CloudProviderInterface, environment_id=None): - environment = Environments.get(environment_id) - - with claim_for_update(environment) as environment: - atat_root_creds = csp.root_creds() - - atat_remote_root_user = csp.create_atat_admin_user( - atat_root_creds, environment.cloud_id - ) - environment.root_user_info = atat_remote_root_user - db.session.add(environment) - db.session.commit() - - def render_email(template_path, context): return app.jinja_env.get_template(template_path).render(context) @@ -180,13 +166,6 @@ def create_environment(self, environment_id=None): do_work(do_create_environment, self, app.csp.cloud, environment_id=environment_id) -@celery.task(bind=True, base=RecordFailure) -def create_atat_admin_user(self, environment_id=None): - do_work( - do_create_atat_admin_user, self, app.csp.cloud, environment_id=environment_id - ) - - @celery.task(bind=True) def dispatch_provision_portfolio(self): """ @@ -214,11 +193,3 @@ def dispatch_create_environment(self): pendulum.now() ): create_environment.delay(environment_id=environment_id) - - -@celery.task(bind=True) -def dispatch_create_atat_admin_user(self): - for environment_id in Environments.get_environments_pending_atat_user_creation( - pendulum.now() - ): - create_atat_admin_user.delay(environment_id=environment_id) diff --git a/atst/models/environment.py b/atst/models/environment.py index b9d16fe0..8408c874 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -1,11 +1,11 @@ -from sqlalchemy import Column, ForeignKey, String, UniqueConstraint -from sqlalchemy.orm import relationship -from sqlalchemy.dialects.postgresql import JSONB from enum import Enum -from atst.models.base import Base +from sqlalchemy import Column, ForeignKey, String, UniqueConstraint +from sqlalchemy.orm import relationship + import atst.models.mixins as mixins import atst.models.types as types +from atst.models.base import Base class Environment( @@ -30,7 +30,6 @@ class Environment( creator = relationship("User") cloud_id = Column(String) - root_user_info = Column(JSONB(none_as_null=True)) roles = relationship( "EnvironmentRole", @@ -70,7 +69,7 @@ class Environment( @property def provisioning_status(self) -> ProvisioningStatus: - if self.cloud_id is None or self.root_user_info is None: + if self.cloud_id is None: return self.ProvisioningStatus.PENDING else: return self.ProvisioningStatus.COMPLETED @@ -91,11 +90,3 @@ class Environment( @property def history(self): return self.get_changes() - - @property - def csp_credentials(self): - return ( - self.root_user_info.get("credentials") - if self.root_user_info is not None - else None - ) diff --git a/atst/queue.py b/atst/queue.py index 3f97f88c..0ea910fb 100644 --- a/atst/queue.py +++ b/atst/queue.py @@ -19,10 +19,6 @@ def update_celery(celery, app): "task": "atst.jobs.dispatch_create_environment", "schedule": 60, }, - "beat-dispatch_create_atat_admin_user": { - "task": "atst.jobs.dispatch_create_atat_admin_user", - "schedule": 60, - }, "beat-dispatch_create_user": { "task": "atst.jobs.dispatch_create_user", "schedule": 60, diff --git a/tests/domain/test_environment_roles.py b/tests/domain/test_environment_roles.py index 216f072e..77f6033a 100644 --- a/tests/domain/test_environment_roles.py +++ b/tests/domain/test_environment_roles.py @@ -99,7 +99,6 @@ def test_disable_checks_env_provisioning_status(session): assert env_role1.disabled environment.cloud_id = "cloud-id" - environment.root_user_info = {"credentials": "credentials"} session.add(environment) session.commit() session.refresh(environment) @@ -111,9 +110,8 @@ def test_disable_checks_env_provisioning_status(session): def test_disable_checks_env_role_provisioning_status(): - environment = EnvironmentFactory.create( - cloud_id="cloud-id", root_user_info={"credentials": "credentials"} - ) + environment = EnvironmentFactory.create(cloud_id="cloud-id") + environment.application.portfolio.csp_data = {"tenant_id": uuid4().hex} env_role1 = EnvironmentRoleFactory.create(environment=environment) assert not env_role1.csp_user_id env_role1 = EnvironmentRoles.disable(env_role1.id) diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 72f07651..a86c6527 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -185,31 +185,3 @@ class TestGetEnvironmentsPendingCreate(EnvQueryTest): app_data={"cloud_id": uuid4().hex}, ) assert len(Environments.get_environments_pending_creation(self.NOW)) == 0 - - -class TestGetEnvironmentsPendingAtatUserCreation(EnvQueryTest): - def test_with_provisioned_environment(self): - self.create_portfolio_with_clins( - [(self.YESTERDAY, self.TOMORROW)], - {"cloud_id": uuid4().hex, "root_user_info": {}}, - ) - assert ( - len(Environments.get_environments_pending_atat_user_creation(self.NOW)) == 0 - ) - - def test_with_unprovisioned_environment(self): - self.create_portfolio_with_clins( - [(self.YESTERDAY, self.TOMORROW)], app_data={"cloud_id": uuid4().hex}, - ) - assert ( - len(Environments.get_environments_pending_atat_user_creation(self.NOW)) == 0 - ) - - def test_with_unprovisioned_expired_clins_environment(self): - self.create_portfolio_with_clins( - [(self.YESTERDAY, self.YESTERDAY)], - {"cloud_id": uuid4().hex, "root_user_info": None}, - ) - assert ( - len(Environments.get_environments_pending_atat_user_creation(self.NOW)) == 0 - ) diff --git a/tests/models/test_environments.py b/tests/models/test_environments.py index 4360cc93..8f8ac626 100644 --- a/tests/models/test_environments.py +++ b/tests/models/test_environments.py @@ -54,18 +54,8 @@ def test_audit_event_for_environment_deletion(session): @pytest.mark.parametrize( "env_data,expected_status", [ - [ - {"cloud_id": None, "root_user_info": None}, - Environment.ProvisioningStatus.PENDING, - ], - [ - {"cloud_id": 1, "root_user_info": None}, - Environment.ProvisioningStatus.PENDING, - ], - [ - {"cloud_id": 1, "root_user_info": {}}, - Environment.ProvisioningStatus.COMPLETED, - ], + [{"cloud_id": None,}, Environment.ProvisioningStatus.PENDING], + [{"cloud_id": 1}, Environment.ProvisioningStatus.COMPLETED], ], ) def test_environment_provisioning_status(env_data, expected_status): diff --git a/tests/test_jobs.py b/tests/test_jobs.py index de17eb6a..a1a821e6 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -12,14 +12,12 @@ from atst.jobs import ( dispatch_create_environment, dispatch_create_application, dispatch_create_user, - dispatch_create_atat_admin_user, dispatch_provision_portfolio, create_environment, do_create_user, do_provision_portfolio, do_create_environment, do_create_application, - do_create_atat_admin_user, ) from tests.factories import ( EnvironmentFactory, @@ -153,14 +151,6 @@ def test_create_user_job(session, csp): assert app_role.cloud_id -def test_create_atat_admin_user(csp, session): - environment = EnvironmentFactory.create(cloud_id="something") - do_create_atat_admin_user(csp, environment.id) - session.refresh(environment) - - assert environment.root_user_info - - def test_dispatch_create_environment(session, monkeypatch): # Given that I have a portfolio with an active CLIN and two environments, # one of which is deleted @@ -231,36 +221,9 @@ def test_dispatch_create_user(monkeypatch): mock.delay.assert_called_once_with(application_role_ids=[app_role.id]) -def test_dispatch_create_atat_admin_user(session, monkeypatch): - portfolio = PortfolioFactory.create( - applications=[ - {"environments": [{"cloud_id": uuid4().hex, "root_user_info": None}]} - ], - task_orders=[ - { - "create_clins": [ - { - "start_date": pendulum.now().subtract(days=1), - "end_date": pendulum.now().add(days=1), - } - ] - } - ], - ) - mock = Mock() - monkeypatch.setattr("atst.jobs.create_atat_admin_user", mock) - environment = portfolio.applications[0].environments[0] - - dispatch_create_atat_admin_user.run() - - mock.delay.assert_called_once_with(environment_id=environment.id) - - def test_create_environment_no_dupes(session, celery_app, celery_worker): portfolio = PortfolioFactory.create( - applications=[ - {"environments": [{"cloud_id": uuid4().hex, "root_user_info": {}}]} - ], + applications=[{"environments": [{"cloud_id": uuid4().hex}]}], task_orders=[ { "create_clins": [ From ca4feaa40375e1daa09b7f3e83a97af0e62c53ff Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 15:39:03 -0500 Subject: [PATCH 08/21] post-rebase fixes --- atst/domain/csp/cloud/azure_cloud_provider.py | 3 --- atst/domain/csp/cloud/mock_cloud_provider.py | 2 -- tests/domain/cloud/test_azure_csp.py | 2 +- tests/test_jobs.py | 2 +- 4 files changed, 2 insertions(+), 7 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index ce6d45ee..79f352d5 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -27,7 +27,6 @@ from .models import ( EnvironmentCSPPayload, EnvironmentCSPResult, KeyVaultCredentials, - ManagementGroupCSPResponse, ProductPurchaseCSPPayload, ProductPurchaseCSPResult, ProductPurchaseVerificationCSPPayload, @@ -155,8 +154,6 @@ class AzureCloudProvider(CloudProviderInterface): payload.parent_id, ) - if - return EnvironmentCSPResult(**response) def create_atat_admin_user( diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index 96648b52..4955f327 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -4,9 +4,7 @@ from .cloud_provider_interface import CloudProviderInterface from .exceptions import ( AuthenticationException, AuthorizationException, - BaselineProvisionException, ConnectionException, - EnvironmentCreationException, GeneralCSPException, UnknownServerException, UserProvisioningException, diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index 639997b3..2b55d1a5 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -61,7 +61,7 @@ def test_create_environment_succeeds(mock_azure: AzureCloudProvider): environment = EnvironmentFactory.create() mock_management_group_create(mock_azure, {"id": "Test Id"}) - mock_azure = mock_get_secret(mock_azure, lambda *a, **k: json.dumps(MOCK_CREDS)) + mock_azure = mock_get_secret(mock_azure) payload = EnvironmentCSPPayload( tenant_id="1234", display_name=environment.name, parent_id=str(uuid4()) diff --git a/tests/test_jobs.py b/tests/test_jobs.py index a1a821e6..8e3d5b1d 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -155,7 +155,7 @@ def test_dispatch_create_environment(session, monkeypatch): # Given that I have a portfolio with an active CLIN and two environments, # one of which is deleted portfolio = PortfolioFactory.create( - applications=[{"environments": [{}, {}]}], + applications=[{"environments": [{}, {}], "cloud_id": uuid4().hex}], task_orders=[ { "create_clins": [ From 41b2fff7746a76257d95fa1b765c0cccb5afe838 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 3 Feb 2020 15:51:38 -0500 Subject: [PATCH 09/21] Add success flash message for revoked portfolio invite and update flash message and translations to be generalized --- atst/routes/applications/settings.py | 5 +++-- atst/routes/portfolios/invitations.py | 8 +++++++- atst/utils/flash.py | 10 +++++----- translations.yaml | 6 +++--- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 8b744b04..8807c7f1 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -467,9 +467,10 @@ def revoke_invite(application_id, application_role_id): if invite.is_pending: ApplicationInvitations.revoke(invite.token) flash( - "application_invite_revoked", + "invite_revoked", + resource="Application", user_name=app_role.user_name, - application_name=g.application.name, + resource_name=g.application.name, ) else: flash( diff --git a/atst/routes/portfolios/invitations.py b/atst/routes/portfolios/invitations.py index 9ec56aa3..80375e75 100644 --- a/atst/routes/portfolios/invitations.py +++ b/atst/routes/portfolios/invitations.py @@ -37,8 +37,14 @@ def accept_invitation(portfolio_token): ) @user_can(Permissions.EDIT_PORTFOLIO_USERS, message="revoke invitation") def revoke_invitation(portfolio_id, portfolio_token): - PortfolioInvitations.revoke(portfolio_token) + invite = PortfolioInvitations.revoke(portfolio_token) + flash( + "invite_revoked", + resource="Portfolio", + user_name=invite.user_name, + resource_name=g.portfolio.name, + ) return redirect( url_for( "portfolios.admin", diff --git a/atst/utils/flash.py b/atst/utils/flash.py index ea85f1ef..b7ca0cb9 100644 --- a/atst/utils/flash.py +++ b/atst/utils/flash.py @@ -33,11 +33,6 @@ MESSAGES = { "message": "flash.application_invite.resent.message", "category": "success", }, - "application_invite_revoked": { - "title": "flash.application_invite.revoked.title", - "message": "flash.application_invite.revoked.message", - "category": "success", - }, "application_member_removed": { "title": "flash.application_member.removed.title", "message": "flash.application_member.removed.message", @@ -103,6 +98,11 @@ MESSAGES = { "message": None, "category": "warning", }, + "invite_revoked": { + "title": "flash.invite_revoked.title", + "message": "flash.invite_revoked.message", + "category": "success", + }, "logged_out": { "title": "flash.logged_out.title", "message": "flash.logged_out.message", diff --git a/translations.yaml b/translations.yaml index 44dd2a92..c16f89c6 100644 --- a/translations.yaml +++ b/translations.yaml @@ -128,9 +128,6 @@ flash: message: There was an error processing the invitation for {user_name} from {application_name} resent: message: "{email} has been sent an invitation to access this Application" - revoked: - title: Application invitation revoked - message: You have successfully revoked the invite for {user_name} from {application_name} application_member: removed: title: Team member removed from application @@ -166,6 +163,9 @@ flash: errors: title: There were some errors message: Please see below. + invite_revoked: + title: "{resource} invitation revoked" + message: "You have successfully revoked the invite for {user_name} from {resource_name}" login_required_message: After you log in, you will be redirected to your destination page. login_required_title: Log in required logged_out: From ff842f505177a7a0894134cc4fc1bfa8781f4601 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Tue, 4 Feb 2020 14:16:12 -0500 Subject: [PATCH 10/21] Add cloud method to get reporting data Adds a method to `azure_cloud_provider` to query the Cost Management API for usage data per invoice. For now, this query is relatively static. We're always calling the API at the billing invoice section scope, with the widest timeframe possible (one year), and with the same requested dataset. As the scope of the application's reporting needs changes, this function may change to be more general and/or revert back to the SDK. --- atst/domain/csp/cloud/azure_cloud_provider.py | 40 ++++++++++ atst/domain/csp/cloud/mock_cloud_provider.py | 25 ++++++ atst/domain/csp/cloud/models.py | 31 ++++++++ tests/domain/cloud/test_azure_csp.py | 78 +++++++++++++++++++ 4 files changed, 174 insertions(+) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index d5ef5204..c54ac7d8 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -24,6 +24,7 @@ from .models import ( BillingProfileTenantAccessCSPResult, BillingProfileVerificationCSPPayload, BillingProfileVerificationCSPResult, + CostManagementQueryCSPResult, KeyVaultCredentials, ManagementGroupCSPResponse, ProductPurchaseCSPPayload, @@ -32,6 +33,7 @@ from .models import ( ProductPurchaseVerificationCSPResult, PrincipalAdminRoleCSPPayload, PrincipalAdminRoleCSPResult, + ReportingCSPPayload, TaskOrderBillingCreationCSPPayload, TaskOrderBillingCreationCSPResult, TaskOrderBillingVerificationCSPPayload, @@ -1070,3 +1072,41 @@ class AzureCloudProvider(CloudProviderInterface): hashed = sha256_hex(tenant_id) raw_creds = self.get_secret(hashed) return KeyVaultCredentials(**json.loads(raw_creds)) + + def get_reporting_data(self, payload: ReportingCSPPayload): + """ + Queries the Cost Management API for an invoice section's raw reporting data + + We query at the invoiceSection scope. The full scope path is passed in + with the payload at the `invoice_section_id` key. + """ + creds = self._source_tenant_creds(payload.tenant_id) + token = self._get_sp_token( + payload.tenant_id, creds.tenant_sp_client_id, creds.tenant_sp_key + ) + + if not token: + raise AuthenticationException("Could not retrieve tenant access token") + + headers = {"Authorization": f"Bearer {token}"} + + request_body = { + "type": "Usage", + "timeframe": "Custom", + "timePeriod": {"from": payload.from_date, "to": payload.to_date,}, + "dataset": { + "granularity": "Daily", + "aggregation": {"totalCost": {"name": "PreTaxCost", "function": "Sum"}}, + "grouping": [{"type": "Dimension", "name": "InvoiceId"}], + }, + } + cost_mgmt_url = ( + f"/providers/Microsoft.CostManagement/query?api-version=2019-11-01" + ) + result = self.sdk.requests.post( + f"{self.sdk.cloud.endpoints.resource_manager}{payload.invoice_section_id}{cost_mgmt_url}", + json=request_body, + headers=headers, + ) + if result.ok: + return CostManagementQueryCSPResult(**result.json()) diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index ec730a3b..9ae78e65 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -25,12 +25,15 @@ from .models import ( BillingProfileTenantAccessCSPResult, BillingProfileVerificationCSPPayload, BillingProfileVerificationCSPResult, + CostManagementQueryCSPResult, + CostManagementQueryProperties, ProductPurchaseCSPPayload, ProductPurchaseCSPResult, ProductPurchaseVerificationCSPPayload, ProductPurchaseVerificationCSPResult, PrincipalAdminRoleCSPPayload, PrincipalAdminRoleCSPResult, + ReportingCSPPayload, SubscriptionCreationCSPPayload, SubscriptionCreationCSPResult, SubscriptionVerificationCSPPayload, @@ -487,3 +490,25 @@ class MockCloudProvider(CloudProviderInterface): def update_tenant_creds(self, tenant_id, secret): return secret + + def get_reporting_data(self, payload: ReportingCSPPayload): + self._maybe_raise(self.NETWORK_FAILURE_PCT, self.NETWORK_EXCEPTION) + self._maybe_raise(self.SERVER_FAILURE_PCT, self.SERVER_EXCEPTION) + self._maybe_raise(self.UNAUTHORIZED_RATE, self.AUTHORIZATION_EXCEPTION) + object_id = str(uuid4()) + + properties = CostManagementQueryProperties( + **dict( + columns=[ + {"name": "PreTaxCost", "type": "Number"}, + {"name": "UsageDate", "type": "Number"}, + {"name": "InvoiceId", "type": "String"}, + {"name": "Currency", "type": "String"}, + ], + rows=[], + ) + ) + + return CostManagementQueryCSPResult( + **dict(name=object_id, properties=properties,) + ) diff --git a/atst/domain/csp/cloud/models.py b/atst/domain/csp/cloud/models.py index 188c2cc7..c31c0704 100644 --- a/atst/domain/csp/cloud/models.py +++ b/atst/domain/csp/cloud/models.py @@ -499,3 +499,34 @@ class UserCSPPayload(BaseCSPPayload): class UserCSPResult(AliasModel): id: str + + +class QueryColumn(AliasModel): + name: str + type: str + + +class CostManagementQueryProperties(AliasModel): + columns: List[QueryColumn] + rows: List[Optional[list]] + + +class CostManagementQueryCSPResult(AliasModel): + name: str + properties: CostManagementQueryProperties + + +class ReportingCSPPayload(BaseCSPPayload): + invoice_section_id: str + from_date: str + to_date: str + + @root_validator(pre=True) + def extract_invoice_section(cls, values): + try: + values["invoice_section_id"] = values["billing_profile_properties"][ + "invoice_sections" + ][0]["invoice_section_id"] + return values + except (KeyError, IndexError): + raise ValueError("Invoice section ID not present in payload") diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index eef5620e..2209a27a 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -5,6 +5,8 @@ from uuid import uuid4 import pytest from tests.factories import ApplicationFactory, EnvironmentFactory from tests.mock_azure import AUTH_CREDENTIALS, mock_azure +import pendulum +import pydantic from atst.domain.csp.cloud import AzureCloudProvider from atst.domain.csp.cloud.models import ( @@ -20,10 +22,12 @@ from atst.domain.csp.cloud.models import ( BillingProfileTenantAccessCSPResult, BillingProfileVerificationCSPPayload, BillingProfileVerificationCSPResult, + CostManagementQueryCSPResult, ProductPurchaseCSPPayload, ProductPurchaseCSPResult, ProductPurchaseVerificationCSPPayload, ProductPurchaseVerificationCSPResult, + ReportingCSPPayload, SubscriptionCreationCSPPayload, SubscriptionCreationCSPResult, SubscriptionVerificationCSPPayload, @@ -718,3 +722,77 @@ def test_create_subscription_verification(mock_azure: AzureCloudProvider): payload ) assert result.subscription_id == "60fbbb72-0516-4253-ab18-c92432ba3230" + + +def test_get_reporting_data(mock_azure: AzureCloudProvider): + mock_result = Mock() + mock_result.json.return_value = { + "eTag": None, + "id": "providers/Microsoft.Billing/billingAccounts/52865e4c-52e8-5a6c-da6b-c58f0814f06f:7ea5de9d-b8ce-4901-b1c5-d864320c7b03_2019-05-31/billingProfiles/XQDJ-6LB4-BG7-TGB/invoiceSections/P73M-XC7J-PJA-TGB/providers/Microsoft.CostManagement/query/e82d0cda-2ffb-4476-a98a-425c83c216f9", + "location": None, + "name": "e82d0cda-2ffb-4476-a98a-425c83c216f9", + "properties": { + "columns": [ + {"name": "PreTaxCost", "type": "Number"}, + {"name": "UsageDate", "type": "Number"}, + {"name": "InvoiceId", "type": "String"}, + {"name": "Currency", "type": "String"}, + ], + "nextLink": None, + "rows": [], + }, + "sku": None, + "type": "Microsoft.CostManagement/query", + } + mock_result.ok = True + mock_azure.sdk.requests.post.return_value = mock_result + mock_azure = mock_get_secret(mock_azure) + + # Subset of a profile's CSP data that we care about for reporting + csp_data = { + "tenant_id": "6d2d2d6c-a6d6-41e1-8bb1-73d11475f8f4", + "billing_profile_properties": { + "invoice_sections": [ + { + "invoice_section_id": "providers/Microsoft.Billing/billingAccounts/52865e4c-52e8-5a6c-da6b-c58f0814f06f:7ea5de9d-b8ce-4901-b1c5-d864320c7b03_2019-05-31/billingProfiles/XQDJ-6LB4-BG7-TGB/invoiceSections/P73M-XC7J-PJA-TGB", + } + ], + }, + } + + data: CostManagementQueryCSPResult = mock_azure.get_reporting_data( + ReportingCSPPayload( + from_date=pendulum.now().subtract(years=1).add(days=1).format("YYYY-MM-DD"), + to_date=pendulum.now().format("YYYY-MM-DD"), + **csp_data, + ) + ) + + assert isinstance(data, CostManagementQueryCSPResult) + assert data.name == "e82d0cda-2ffb-4476-a98a-425c83c216f9" + assert len(data.properties.columns) == 4 + + +def test_get_reporting_data_malformed_payload(mock_azure: AzureCloudProvider): + mock_result = Mock() + mock_result.ok = True + mock_azure.sdk.requests.post.return_value = mock_result + mock_azure = mock_get_secret(mock_azure) + + # Malformed csp_data payloads that should throw pydantic validation errors + index_error = { + "tenant_id": "6d2d2d6c-a6d6-41e1-8bb1-73d11475f8f4", + "billing_profile_properties": {"invoice_sections": [],}, + } + key_error = { + "tenant_id": "6d2d2d6c-a6d6-41e1-8bb1-73d11475f8f4", + "billing_profile_properties": {"invoice_sections": [{}],}, + } + + for malformed_payload in [key_error, index_error]: + with pytest.raises(pydantic.ValidationError): + assert mock_azure.get_reporting_data( + ReportingCSPPayload( + from_date="foo", to_date="bar", **malformed_payload, + ) + ) From 8e2870b62f77359a88cb831c332a45cc5dd4b59b Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 4 Feb 2020 12:02:04 -0500 Subject: [PATCH 11/21] Add max width to CTA footer in the TO builder form - Created a new mixin (ExpandSidenavMixin) that sets the defaultVisible prop that can be used in both the root component and the SidenavToggler. This makes it so that the Root knows whether or not the sidenav is collapsed or expanded, so then child components can use this to calculate margins/paddings/offsets/etc. - Added new classes for the .action-group-footer that set the elements padding based on whether or not the sidenav is expanded. - Added a nested div (.action-group-footer--container) inside .action-group-footer which sets the max width --- js/components/sidenav_toggler.js | 21 +++++---------- js/index.js | 12 +++++++++ js/mixins/expand_sidenav.js | 15 +++++++++++ styles/elements/_action_group.scss | 35 +++++++++++++++++-------- templates/task_orders/builder_base.html | 10 ++++--- 5 files changed, 63 insertions(+), 30 deletions(-) create mode 100644 js/mixins/expand_sidenav.js diff --git a/js/components/sidenav_toggler.js b/js/components/sidenav_toggler.js index faba4c3b..11717849 100644 --- a/js/components/sidenav_toggler.js +++ b/js/components/sidenav_toggler.js @@ -1,30 +1,21 @@ +import ExpandSidenavMixin from '../mixins/expand_sidenav' import ToggleMixin from '../mixins/toggle' -const cookieName = 'expandSidenav' - export default { name: 'sidenav-toggler', - mixins: [ToggleMixin], + mixins: [ExpandSidenavMixin, ToggleMixin], - props: { - defaultVisible: { - type: Boolean, - default: function() { - if (document.cookie.match(cookieName)) { - return !!document.cookie.match(cookieName + ' *= *true') - } else { - return true - } - }, - }, + mounted: function() { + this.$parent.$emit('sidenavToggle', this.isVisible) }, methods: { toggle: function(e) { e.preventDefault() this.isVisible = !this.isVisible - document.cookie = cookieName + '=' + this.isVisible + '; path=/' + document.cookie = this.cookieName + '=' + this.isVisible + '; path=/' + this.$parent.$emit('sidenavToggle', this.isVisible) }, }, } diff --git a/js/index.js b/js/index.js index fb5cdd6e..6495268b 100644 --- a/js/index.js +++ b/js/index.js @@ -32,12 +32,14 @@ import ToForm from './components/forms/to_form' import ClinFields from './components/clin_fields' import PopDateRange from './components/pop_date_range' import ToggleMenu from './components/toggle_menu' +import ExpandSidenav from './mixins/expand_sidenav' Vue.config.productionTip = false Vue.use(VTooltip) Vue.mixin(Modal) +Vue.mixin(ExpandSidenav) const app = new Vue({ el: '#app-root', @@ -67,6 +69,12 @@ const app = new Vue({ ToggleMenu, }, + data: function() { + return { + sidenavExpanded: this.defaultVisible, + } + }, + mounted: function() { this.$on('modalOpen', data => { if (data['isOpen']) { @@ -105,6 +113,10 @@ const app = new Vue({ } }) }) + + this.$on('sidenavToggle', data => { + this.sidenavExpanded = data + }) }, delimiters: ['!{', '}'], diff --git a/js/mixins/expand_sidenav.js b/js/mixins/expand_sidenav.js new file mode 100644 index 00000000..7553b7d4 --- /dev/null +++ b/js/mixins/expand_sidenav.js @@ -0,0 +1,15 @@ +export default { + props: { + cookieName: 'expandSidenav', + defaultVisible: { + type: Boolean, + default: function() { + if (document.cookie.match(this.cookieName)) { + return !!document.cookie.match(this.cookieName + ' *= *true') + } else { + return true + } + }, + }, + }, +} diff --git a/styles/elements/_action_group.scss b/styles/elements/_action_group.scss index fe375f67..c2d11049 100644 --- a/styles/elements/_action_group.scss +++ b/styles/elements/_action_group.scss @@ -32,22 +32,35 @@ } .action-group-footer { - @extend .action-group; - - &:last-child { - margin-bottom: 0; - } - margin-top: 0; - margin-bottom: 0; padding-top: $gap; padding-bottom: $gap; - + padding-right: $gap * 4; position: fixed; bottom: $footer-height; + left: 0; background: white; - right: 0; - padding-right: $gap * 4; border-top: 1px solid $color-gray-lighter; - width: 100%; z-index: 1; + width: 100%; + + &.action-group-footer--expand-offset { + padding-left: $sidenav-expanded-width; + } + + &.action-group-footer--collapse-offset { + padding-left: $sidenav-collapsed-width; + } + + .action-group-footer--container { + @extend .action-group; + + margin-top: 0; + margin-bottom: 0; + margin-left: $large-spacing; + max-width: $max-panel-width; + + &:last-child { + margin-bottom: 0; + } + } } diff --git a/templates/task_orders/builder_base.html b/templates/task_orders/builder_base.html index 9ee8dd0c..66e84d53 100644 --- a/templates/task_orders/builder_base.html +++ b/templates/task_orders/builder_base.html @@ -31,7 +31,10 @@
{% block to_builder_form_field %}{% endblock %}
- +
+
{% block next_button %} {%- endif %} {% endif %} - {{ "common.cancel" | translate }} - - +
+
{% endblock %} From eeb174af351efc0a0c4e10f9dcdfcbd2d1d4cc55 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 4 Feb 2020 13:42:00 -0500 Subject: [PATCH 12/21] Set max width for alerts and delete unused classes --- styles/components/_alerts.scss | 36 +--------------------------------- 1 file changed, 1 insertion(+), 35 deletions(-) diff --git a/styles/components/_alerts.scss b/styles/components/_alerts.scss index be326807..eb62a756 100644 --- a/styles/components/_alerts.scss +++ b/styles/components/_alerts.scss @@ -11,6 +11,7 @@ .usa-alert { padding-bottom: 2.4rem; + max-width: $max-panel-width; } @mixin alert { @@ -97,38 +98,3 @@ } } } - -.alert { - @include alert; - @include alert-level("info"); - - &.alert--success { - @include alert-level("success"); - - .alert__actions { - .icon-link { - @include icon-link-color($color-green, $color-white); - } - } - } - - &.alert--warning { - @include alert-level("warning"); - - .alert__actions { - .icon-link { - @include icon-link-color($color-gold-dark, $color-white); - } - } - } - - &.alert--error { - @include alert-level("error"); - - .alert__actions { - .icon-link { - @include icon-link-color($color-red, $color-white); - } - } - } -} From a6e7dfda133e56370d018609f9fc07a972586e1f Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 4 Feb 2020 13:51:28 -0500 Subject: [PATCH 13/21] Apply .action-group-footer updates to the New application builder --- templates/applications/new/step_1.html | 20 ++++++++++-------- templates/applications/new/step_2.html | 28 +++++++++++++++----------- templates/applications/new/step_3.html | 26 ++++++++++++++---------- 3 files changed, 43 insertions(+), 31 deletions(-) diff --git a/templates/applications/new/step_1.html b/templates/applications/new/step_1.html index d06ddee0..3841bf96 100644 --- a/templates/applications/new/step_1.html +++ b/templates/applications/new/step_1.html @@ -39,14 +39,18 @@ - - {% block next_button %} - {{ SaveButton(text=('portfolios.applications.new.step_1_button_text' | translate)) }} - {% endblock %} - - Cancel - - +
+
+ {% block next_button %} + {{ SaveButton(text=('portfolios.applications.new.step_1_button_text' | translate)) }} + {% endblock %} + + Cancel + +
+
diff --git a/templates/applications/new/step_2.html b/templates/applications/new/step_2.html index 462c0f46..2cd5cf98 100644 --- a/templates/applications/new/step_2.html +++ b/templates/applications/new/step_2.html @@ -58,20 +58,24 @@ {{ Icon("plus") }} + + + +
+
+ {% block next_button %} + {{ SaveButton(text=('portfolios.applications.new.step_2_button_text' | translate)) }} + {% endblock %} + + Previous + + + Cancel +
- - - {% block next_button %} - {{ SaveButton(text=('portfolios.applications.new.step_2_button_text' | translate)) }} - {% endblock %} - - Previous - - - Cancel - - diff --git a/templates/applications/new/step_3.html b/templates/applications/new/step_3.html index d88a704c..35af10fa 100644 --- a/templates/applications/new/step_3.html +++ b/templates/applications/new/step_3.html @@ -25,16 +25,20 @@ action_update="applications.update_new_application_step_3") }} - - - {{ "portfolios.applications.new.step_3_button_text" | translate }} - - - {{ "common.previous" | translate }} - - - {{ "common.cancel" | translate }} - - +
+
+ + {{ "portfolios.applications.new.step_3_button_text" | translate }} + + + {{ "common.previous" | translate }} + + + {{ "common.cancel" | translate }} + +
+
{% endblock %} From 68c7a70082540c4edf29df9ff76cdb95dded3733 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 4 Feb 2020 14:21:08 -0500 Subject: [PATCH 14/21] Set max width on error page and include Last login on error page - Updated error_base template so that it contained the Root Vue component, which was the reason that the last login was not previously displaying - Deleted unused css - Created css variable max-page-width for use on the error page, topbar, and other full width elements --- styles/components/_error_page.scss | 20 ++++++----------- styles/components/_topbar.scss | 2 +- styles/core/_variables.scss | 1 + templates/error.html | 2 ++ templates/error_base.html | 35 +++++++++++++++--------------- 5 files changed, 29 insertions(+), 31 deletions(-) diff --git a/styles/components/_error_page.scss b/styles/components/_error_page.scss index 19ae7531..683b527f 100644 --- a/styles/components/_error_page.scss +++ b/styles/components/_error_page.scss @@ -1,8 +1,13 @@ .error-page { - max-width: 475px; - margin: auto; + max-width: $max-page-width; .panel { + box-shadow: none; + background-color: unset; + border: none; + max-width: 475px; + margin: auto; + &__heading { text-align: center; padding: $gap 0; @@ -15,17 +20,6 @@ margin-bottom: $gap; } } - - &__body { - padding: $gap * 2; - margin: 0; - - hr { - width: 80%; - margin: auto; - margin-bottom: $gap * 3; - } - } } .icon { diff --git a/styles/components/_topbar.scss b/styles/components/_topbar.scss index 6d84f426..feca57b6 100644 --- a/styles/components/_topbar.scss +++ b/styles/components/_topbar.scss @@ -12,7 +12,7 @@ flex-direction: row; align-items: stretch; justify-content: space-between; - max-width: 1190px; + max-width: $max-page-width; a { color: $color-white; diff --git a/styles/core/_variables.scss b/styles/core/_variables.scss index 12657ca4..372fa868 100644 --- a/styles/core/_variables.scss +++ b/styles/core/_variables.scss @@ -19,6 +19,7 @@ $sidenav-collapsed-width: 10rem; $max-panel-width: 90rem; $home-pg-icon-width: 6rem; $large-spacing: 4rem; +$max-page-width: $max-panel-width + $sidenav-expanded-width + $large-spacing; /* * USWDS Variables diff --git a/templates/error.html b/templates/error.html index 449c9a88..45ff12a3 100644 --- a/templates/error.html +++ b/templates/error.html @@ -5,6 +5,7 @@ {% block content %}
+
{{ Icon('cloud', classes="icon--red icon--large")}}
@@ -17,6 +18,7 @@ {%- endif %}

+
{% endblock %} diff --git a/templates/error_base.html b/templates/error_base.html index 92be8e60..b5751e81 100644 --- a/templates/error_base.html +++ b/templates/error_base.html @@ -10,29 +10,30 @@ +
+ {% block template_vars %}{% endblock %} - {% block template_vars %}{% endblock %} + {% include 'components/usa_header.html' %} - {% include 'components/usa_header.html' %} + {% include 'navigation/topbar.html' %} - {% include 'navigation/topbar.html' %} +
-
+
+ {% block sidenav %}{% endblock %} -
- {% block sidenav %}{% endblock %} - - {% block content %} - these are not the droids you are looking for - {% endblock %} + {% block content %} + these are not the droids you are looking for + {% endblock %} +
+ + {% include 'footer.html' %} + + {% block modal %}{% endblock %} + {% assets "js_all" %} + + {% endassets %}
- - {% include 'footer.html' %} - - {% block modal %}{% endblock %} - {% assets "js_all" %} - - {% endassets %} From e7487aa1149a3fd6ec8dec34532068b3ef73531b Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 4 Feb 2020 14:43:59 -0500 Subject: [PATCH 15/21] Set max width on ccpo admin pages --- styles/atat.scss | 1 + styles/sections/_ccpo.scss | 3 + templates/ccpo/add_user.html | 32 ++++---- templates/ccpo/confirm_user.html | 56 +++++++------ templates/ccpo/users.html | 136 ++++++++++++++++--------------- 5 files changed, 119 insertions(+), 109 deletions(-) create mode 100644 styles/sections/_ccpo.scss diff --git a/styles/atat.scss b/styles/atat.scss index 72c7af40..8eb73473 100644 --- a/styles/atat.scss +++ b/styles/atat.scss @@ -47,3 +47,4 @@ @import "sections/application_edit"; @import "sections/reports"; @import "sections/task_order"; +@import "sections/ccpo"; diff --git a/styles/sections/_ccpo.scss b/styles/sections/_ccpo.scss new file mode 100644 index 00000000..4033ac2e --- /dev/null +++ b/styles/sections/_ccpo.scss @@ -0,0 +1,3 @@ +.ccpo-panel-container { + max-width: $max-panel-width; +} diff --git a/templates/ccpo/add_user.html b/templates/ccpo/add_user.html index e8544a2b..eccd27a9 100644 --- a/templates/ccpo/add_user.html +++ b/templates/ccpo/add_user.html @@ -4,21 +4,23 @@ {% from "components/text_input.html" import TextInput %} {% block content %} - -
- {{ form.csrf_token }} -

{{ "ccpo.form.add_user_title" | translate }}

-
-
- {{ TextInput(form.dod_id, validation='dodId', optional=False) }} -
-
-
- {{ SaveButton(text="common.next"|translate, element="input", additional_classes="action-group__action", form="add-ccpo-user-form") }} - {{ "common.cancel" | translate }} +
+ + + {{ form.csrf_token }} +

{{ "ccpo.form.add_user_title" | translate }}

+
+
+ {{ TextInput(form.dod_id, validation='dodId', optional=False) }} +
+
+
+ {{ SaveButton(text="common.next"|translate, element="input", additional_classes="action-group__action", form="add-ccpo-user-form") }} + {{ "common.cancel" | translate }} +
-
- - + + +
{% endblock %} diff --git a/templates/ccpo/confirm_user.html b/templates/ccpo/confirm_user.html index dfe30bca..a45abd3c 100644 --- a/templates/ccpo/confirm_user.html +++ b/templates/ccpo/confirm_user.html @@ -3,31 +3,33 @@ {% from "components/text_input.html" import TextInput %} {% block content %} - {% if new_user %} -

{{ 'ccpo.form.confirm_user_title' | translate }}

-
- {{ form.csrf_token }} - -
-

- {{ "ccpo.form.confirm_user_text" | translate }} -

-

- {{ new_user.full_name }} -

-

- {{ new_user.email }} -

-
-
- - - {{ "common.cancel" | translate }} - -
-
- {% endif %} +
+ {% if new_user %} +

{{ 'ccpo.form.confirm_user_title' | translate }}

+
+ {{ form.csrf_token }} + +
+

+ {{ "ccpo.form.confirm_user_text" | translate }} +

+

+ {{ new_user.full_name }} +

+

+ {{ new_user.email }} +

+
+
+ + + {{ "common.cancel" | translate }} + +
+
+ {% endif %} +
{% endblock %} diff --git a/templates/ccpo/users.html b/templates/ccpo/users.html index c5c8cc3b..15e5d9fe 100644 --- a/templates/ccpo/users.html +++ b/templates/ccpo/users.html @@ -6,78 +6,80 @@ {% from "components/modal.html" import Modal %} {% block content %} -
-
- {{ "ccpo.users_title" | translate }} -
+
+
+
+ {{ "ccpo.users_title" | translate }} +
- {% include "fragments/flash.html" %} - - - - - - - - {% if user_can(permissions.DELETE_CCPO_USER) %} - - {% endif %} - - - - {% for user, form in users_info %} - {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} - {% set disable_button_class = 'button-danger-outline' %} - {% if user == g.current_user %} - {% set disable_button_class = "usa-button-disabled" %} - {% endif %} + {% include "fragments/flash.html" %} +
{{ "common.name" | translate }}{{ "common.email" | translate }}{{ "common.dod_id" | translate }}
+ - - - + + + {% if user_can(permissions.DELETE_CCPO_USER) %} - + {% endif %} - {% endfor %} - -
{{ user.full_name }}{{ user.email }}{{ user.dod_id }}{{ "common.name" | translate }}{{ "common.email" | translate }}{{ "common.dod_id" | translate }} - - {{ "common.disable" | translate }} - -
+ + + {% for user, form in users_info %} + {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} + {% set disable_button_class = 'button-danger-outline' %} + {% if user == g.current_user %} + {% set disable_button_class = "usa-button-disabled" %} + {% endif %} + + + {{ user.full_name }} + {{ user.email }} + {{ user.dod_id }} + {% if user_can(permissions.DELETE_CCPO_USER) %} + + + {{ "common.disable" | translate }} + + + {% endif %} + + {% endfor %} + + +
+ + {% if user_can(permissions.CREATE_CCPO_USER) %} + + {{ "ccpo.add_user" | translate }} {{ Icon("plus") }} + + {% endif %} + + {% if user_can(permissions.DELETE_CCPO_USER) %} + {% for user, form in users_info %} + {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} + {% call Modal(name=modal_id) %} +

Disable CCPO User

+
+ {{ + Alert( + title=("components.modal.destructive_title" | translate), + message=("ccpo.disable_user.alert_message" | translate({"user_name": user.full_name})), + level="warning" + ) + }} + {{ + DeleteConfirmation( + modal_id=modal_id, + delete_text='Remove Access', + delete_action=(url_for('ccpo.remove_access', user_id=user.id)), + form=form, + confirmation_text='remove' + ) + }} + {% endcall %} + {% endfor %} + {% endif %}
- - {% if user_can(permissions.CREATE_CCPO_USER) %} - - {{ "ccpo.add_user" | translate }} {{ Icon("plus") }} - - {% endif %} - - {% if user_can(permissions.DELETE_CCPO_USER) %} - {% for user, form in users_info %} - {% set modal_id = "disable_ccpo_user_{}".format(user.dod_id) %} - {% call Modal(name=modal_id) %} -

Disable CCPO User

-
- {{ - Alert( - title=("components.modal.destructive_title" | translate), - message=("ccpo.disable_user.alert_message" | translate({"user_name": user.full_name})), - level="warning" - ) - }} - {{ - DeleteConfirmation( - modal_id=modal_id, - delete_text='Remove Access', - delete_action=(url_for('ccpo.remove_access', user_id=user.id)), - form=form, - confirmation_text='remove' - ) - }} - {% endcall %} - {% endfor %} - {% endif %} {% endblock %} From 6cf39ca1e4a14a02c61e6ade42848c3fc8e12e84 Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Tue, 4 Feb 2020 15:31:48 -0500 Subject: [PATCH 16/21] Set max width on new portfolio form --- templates/portfolios/new/step_1.html | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/templates/portfolios/new/step_1.html b/templates/portfolios/new/step_1.html index 3305d924..940becee 100644 --- a/templates/portfolios/new/step_1.html +++ b/templates/portfolios/new/step_1.html @@ -15,6 +15,7 @@

{{ "portfolios.header" | translate }}

{{ 'portfolios.new.title' | translate }}

+
{{ StickyCTA(text="portfolios.new.cta_step_1"|translate, context=("portfolios.new.sticky_header_context"|translate({"step": "1"}) )) }}
@@ -38,13 +39,18 @@ {{ "forms.portfolio.defense_component.help_text" | translate | safe }}
-
- {% block next_button %} - {{ SaveButton(text=('portfolios.new.save' | translate), form="portfolio-create", element="input") }} - {% endblock %} - - Cancel - +
+
+ {% block next_button %} + {{ SaveButton(text=('portfolios.new.save' | translate), form="portfolio-create", element="input") }} + {% endblock %} + + Cancel + +
+
From 9d5918d6188744ff2e32a62c1fa998b55075818b Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 16:42:18 -0500 Subject: [PATCH 17/21] Add exception for problems with secrets --- atst/domain/csp/cloud/azure_cloud_provider.py | 27 ++++++++++++------- atst/domain/csp/cloud/exceptions.py | 14 ++++++++++ 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 79f352d5..6db3aba3 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -6,12 +6,8 @@ from uuid import uuid4 from atst.utils import sha256_hex from .cloud_provider_interface import CloudProviderInterface -from .exceptions import AuthenticationException, UserProvisioningException +from .exceptions import AuthenticationException, UserProvisioningException, SecretException from .models import ( - SubscriptionCreationCSPPayload, - SubscriptionCreationCSPResult, - SubscriptionVerificationCSPPayload, - SuscriptionVerificationCSPResult, AdminRoleDefinitionCSPPayload, AdminRoleDefinitionCSPResult, ApplicationCSPPayload, @@ -27,12 +23,16 @@ from .models import ( EnvironmentCSPPayload, EnvironmentCSPResult, KeyVaultCredentials, + PrincipalAdminRoleCSPPayload, + PrincipalAdminRoleCSPResult, ProductPurchaseCSPPayload, ProductPurchaseCSPResult, ProductPurchaseVerificationCSPPayload, ProductPurchaseVerificationCSPResult, - PrincipalAdminRoleCSPPayload, - PrincipalAdminRoleCSPResult, + SubscriptionCreationCSPPayload, + SubscriptionCreationCSPResult, + SubscriptionVerificationCSPPayload, + SuscriptionVerificationCSPResult, TaskOrderBillingCreationCSPPayload, TaskOrderBillingCreationCSPResult, TaskOrderBillingVerificationCSPPayload, @@ -54,7 +54,6 @@ from .models import ( ) from .policy import AzurePolicyManager - # This needs to be a fully pathed role definition identifier, not just a UUID # TODO: Extract these from sdk msrestazure.azure_cloud import AZURE_PUBLIC_CLOUD AZURE_SKU_ID = "0001" # probably a static sku specific to ATAT/JEDI @@ -117,11 +116,15 @@ class AzureCloudProvider(CloudProviderInterface): ) try: return secret_client.set_secret(secret_key, secret_value) - except self.exceptions.HttpResponseError: + except self.sdk.exceptions.HttpResponseError as exc: app.logger.error( f"Could not SET secret in Azure keyvault for key {secret_key}.", exc_info=1, ) + raise SecretException( + f"Could not SET secret in Azure keyvault for key {secret_key}.", + exc.message, + ) def get_secret(self, secret_key): credential = self._get_client_secret_credential_obj() @@ -130,11 +133,15 @@ class AzureCloudProvider(CloudProviderInterface): ) try: return secret_client.get_secret(secret_key).value - except self.exceptions.HttpResponseError: + except self.sdk.exceptions.HttpResponseError: app.logger.error( f"Could not GET secret in Azure keyvault for key {secret_key}.", exc_info=1, ) + raise SecretException( + f"Could not GET secret in Azure keyvault for key {secret_key}.", + exc.message, + ) def create_environment(self, payload: EnvironmentCSPPayload): creds = self._source_creds(payload.tenant_id) diff --git a/atst/domain/csp/cloud/exceptions.py b/atst/domain/csp/cloud/exceptions.py index 49b05fb4..3480180f 100644 --- a/atst/domain/csp/cloud/exceptions.py +++ b/atst/domain/csp/cloud/exceptions.py @@ -118,3 +118,17 @@ class BaselineProvisionException(GeneralCSPException): return "Could not complete baseline provisioning for environment ({}): {}".format( self.env_identifier, self.reason ) + + +class SecretException(GeneralCSPException): + """A problem occurred with setting or getting secrets""" + + def __init__(self, tenant_id, reason): + self.tenant_id = tenant_id + self.reason = reason + + @property + def message(self): + return "Could not get or set secret for ({}): {}".format( + self.tenant_id, self.reason + ) From 4d11f7217e1ea24dc951c7fcb14b1368e8014037 Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 16:42:38 -0500 Subject: [PATCH 18/21] Add missing test for create_principal_admin_role --- atst/domain/csp/cloud/azure_cloud_provider.py | 6 ++- tests/domain/cloud/test_azure_csp.py | 39 +++++++++++++++++-- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 6db3aba3..dc5d92c7 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -6,7 +6,11 @@ from uuid import uuid4 from atst.utils import sha256_hex from .cloud_provider_interface import CloudProviderInterface -from .exceptions import AuthenticationException, UserProvisioningException, SecretException +from .exceptions import ( + AuthenticationException, + UserProvisioningException, + SecretException, +) from .models import ( AdminRoleDefinitionCSPPayload, AdminRoleDefinitionCSPResult, diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index 2b55d1a5..2c946f2b 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -22,6 +22,8 @@ from atst.domain.csp.cloud.models import ( BillingProfileVerificationCSPResult, EnvironmentCSPPayload, EnvironmentCSPResult, + PrincipalAdminRoleCSPPayload, + PrincipalAdminRoleCSPResult, ProductPurchaseCSPPayload, ProductPurchaseCSPResult, ProductPurchaseVerificationCSPPayload, @@ -577,10 +579,10 @@ def test_create_tenant_principal_credential(mock_azure: AzureCloudProvider): def test_create_admin_role_definition(mock_azure: AzureCloudProvider): with patch.object( AzureCloudProvider, - "_get_elevated_management_token", - wraps=mock_azure._get_elevated_management_token, - ) as get_elevated_management_token: - get_elevated_management_token.return_value = "my fake token" + "_get_tenant_admin_token", + wraps=mock_azure._get_tenant_admin_token, + ) as get_tenant_admin_token: + get_tenant_admin_token.return_value = "my fake token" mock_result = Mock() mock_result.ok = True @@ -661,6 +663,35 @@ def test_create_tenant_principal_ownership(mock_azure: AzureCloudProvider): assert result.principal_owner_assignment_id == "id" +def test_create_principal_admin_role(mock_azure: AzureCloudProvider): + with patch.object( + AzureCloudProvider, + "_get_tenant_admin_token", + wraps=mock_azure._get_tenant_admin_token, + ) as get_tenant_admin_token: + get_tenant_admin_token.return_value = "my fake token" + + mock_result = Mock() + mock_result.ok = True + mock_result.json.return_value = {"id": "id"} + + mock_azure.sdk.requests.post.return_value = mock_result + + payload = PrincipalAdminRoleCSPPayload( + **{ + "tenant_id": uuid4().hex, + "principal_id": "6d2d2d6c-a6d6-41e1-8bb1-73d11475f8f4", + "admin_role_def_id": uuid4().hex, + } + ) + + result: PrincipalAdminRoleCSPResult = mock_azure.create_principal_admin_role( + payload + ) + + assert result.principal_assignment_id == "id" + + def test_create_subscription_creation(mock_azure: AzureCloudProvider): with patch.object( AzureCloudProvider, From 350e648bebebbe869b9f55183ac71b6b9850edf5 Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 17:39:31 -0500 Subject: [PATCH 19/21] Added tests for get and set secret --- atst/domain/csp/cloud/azure_cloud_provider.py | 11 ----- tests/domain/cloud/test_azure_csp.py | 45 +++++++++++++++++++ 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 22551ed2..0874e0d2 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -813,17 +813,6 @@ class AzureCloudProvider(CloudProviderInterface): return self._ok() - def create_billing_alerts(self, TBD): - # TODO: Add azure-mgmt-consumption for Budget and Notification entities/operations - # TODO: Determine how to auth against that API using the SDK, doesn't seeem possible at the moment - # TODO: billing alerts are registered as Notifications on Budget objects, which have start/end dates - # TODO: determine what the keys in the Notifications dict are supposed to be - # we may need to rotate budget objects when new TOs/CLINs are reported? - - # we likely only want the budget ID, can be updated or replaced? - response = {"id": "id"} - return self._ok({"budget_id": response["id"]}) - def _get_management_service_principal(self): # we really should be using graph.microsoft.com, but i'm getting # "expired token" errors for that diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index c58944b9..8744ca5c 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -172,6 +172,27 @@ def test_create_tenant(mock_azure: AzureCloudProvider): assert body.tenant_id == "60ff9d34-82bf-4f21-b565-308ef0533435" +def test_create_tenant_fails(mock_azure: AzureCloudProvider): + mock_result = Mock() + mock_result.json.return_value = {"error": "body"} + mock_result.status_code = 403 + mock_azure.sdk.requests.post.return_value = mock_result + payload = TenantCSPPayload( + **dict( + user_id="admin", + password="JediJan13$coot", # pragma: allowlist secret + domain_name="jediccpospawnedtenant2", + first_name="Tedry", + last_name="Tenet", + country_code="US", + password_recovery_email_address="thomas@promptworks.com", + ) + ) + mock_azure = mock_get_secret(mock_azure) + result = mock_azure.create_tenant(payload) + assert result.get("status") == "error" + + def test_create_billing_profile_creation(mock_azure: AzureCloudProvider): mock_azure.sdk.adal.AuthenticationContext.return_value.context.acquire_token_with_client_credentials.return_value = { "accessToken": "TOKEN" @@ -831,3 +852,27 @@ def test_get_reporting_data_malformed_payload(mock_azure: AzureCloudProvider): from_date="foo", to_date="bar", **malformed_payload, ) ) + +def test_get_secret(mock_azure: AzureCloudProvider): + with patch.object( + AzureCloudProvider, + "_get_client_secret_credential_obj", + wraps=mock_azure._get_client_secret_credential_obj, + ) as _get_client_secret_credential_obj: + _get_client_secret_credential_obj.return_value = {} + + mock_azure.sdk.secrets.SecretClient.return_value.get_secret.return_value.value = "my secret" + + assert mock_azure.get_secret("secret key") == "my secret" + +def test_set_secret(mock_azure: AzureCloudProvider): + with patch.object( + AzureCloudProvider, + "_get_client_secret_credential_obj", + wraps=mock_azure._get_client_secret_credential_obj, + ) as _get_client_secret_credential_obj: + _get_client_secret_credential_obj.return_value = {} + + mock_azure.sdk.secrets.SecretClient.return_value.set_secret.return_value = "my secret" + + assert mock_azure.set_secret("secret key", "secret_value") == "my secret" \ No newline at end of file From aaa4b401b57be998f2945a05ece634c43c820066 Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 17:42:08 -0500 Subject: [PATCH 20/21] Fix LGTM issues --- atst/domain/csp/cloud/cloud_provider_interface.py | 2 +- atst/domain/csp/cloud/mock_cloud_provider.py | 5 ----- tests/domain/cloud/test_azure_csp.py | 12 +++++++++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/atst/domain/csp/cloud/cloud_provider_interface.py b/atst/domain/csp/cloud/cloud_provider_interface.py index d173396a..ac652b5c 100644 --- a/atst/domain/csp/cloud/cloud_provider_interface.py +++ b/atst/domain/csp/cloud/cloud_provider_interface.py @@ -11,7 +11,7 @@ class CloudProviderInterface: def root_creds(self) -> Dict: raise NotImplementedError() - def create_environment(self, auth_credentials: Dict, user, environment) -> str: + def create_environment(self, payload): """Create a new environment in the CSP. Arguments: diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index 4e4532d3..bb2f4941 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -452,11 +452,6 @@ class MockCloudProvider(CloudProviderInterface): id=f"{AZURE_MGMNT_PATH}{payload.management_group_name}" ) - def create_user(self, payload: UserCSPPayload): - self._maybe_raise(self.UNAUTHORIZED_RATE, GeneralCSPException) - - return UserCSPResult(id=str(uuid4())) - def create_environment(self, payload: EnvironmentCSPPayload): self._maybe_raise(self.UNAUTHORIZED_RATE, GeneralCSPException) diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index 8744ca5c..7ef1e67b 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -853,6 +853,7 @@ def test_get_reporting_data_malformed_payload(mock_azure: AzureCloudProvider): ) ) + def test_get_secret(mock_azure: AzureCloudProvider): with patch.object( AzureCloudProvider, @@ -861,10 +862,13 @@ def test_get_secret(mock_azure: AzureCloudProvider): ) as _get_client_secret_credential_obj: _get_client_secret_credential_obj.return_value = {} - mock_azure.sdk.secrets.SecretClient.return_value.get_secret.return_value.value = "my secret" + mock_azure.sdk.secrets.SecretClient.return_value.get_secret.return_value.value = ( + "my secret" + ) assert mock_azure.get_secret("secret key") == "my secret" + def test_set_secret(mock_azure: AzureCloudProvider): with patch.object( AzureCloudProvider, @@ -873,6 +877,8 @@ def test_set_secret(mock_azure: AzureCloudProvider): ) as _get_client_secret_credential_obj: _get_client_secret_credential_obj.return_value = {} - mock_azure.sdk.secrets.SecretClient.return_value.set_secret.return_value = "my secret" + mock_azure.sdk.secrets.SecretClient.return_value.set_secret.return_value = ( + "my secret" + ) - assert mock_azure.set_secret("secret key", "secret_value") == "my secret" \ No newline at end of file + assert mock_azure.set_secret("secret key", "secret_value") == "my secret" From 13aca270ca378e25b50e6107116080648ef1fe62 Mon Sep 17 00:00:00 2001 From: tomdds Date: Wed, 5 Feb 2020 14:48:54 -0500 Subject: [PATCH 21/21] Remove unused code in both the cloud interfaces and environment models. Also add tests for some untested code in the cloud interface. --- atst/domain/csp/cloud/azure_cloud_provider.py | 43 +-------- .../csp/cloud/cloud_provider_interface.py | 27 ------ atst/domain/csp/cloud/mock_cloud_provider.py | 17 ---- atst/domain/environment_roles.py | 2 +- atst/domain/environments.py | 2 +- atst/models/environment.py | 17 ---- atst/routes/applications/settings.py | 1 - tests/domain/cloud/test_azure_csp.py | 88 +++++++++++++++---- tests/domain/cloud/test_mock_csp.py | 11 --- tests/domain/test_environment_roles.py | 4 +- tests/models/test_environments.py | 12 --- 11 files changed, 79 insertions(+), 145 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 0874e0d2..1d921fde 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -153,9 +153,9 @@ class AzureCloudProvider(CloudProviderInterface): creds = self._source_creds(payload.tenant_id) credentials = self._get_credential_obj( { - "client_id": creds.root_sp_client_id, - "secret_key": creds.root_sp_key, - "tenant_id": creds.root_tenant_id, + "client_id": creds.tenant_sp_client_id, + "secret_key": creds.tenant_sp_key, + "tenant_id": creds.tenant_id, }, resource=self.sdk.cloud.endpoints.resource_manager, ) @@ -169,43 +169,6 @@ class AzureCloudProvider(CloudProviderInterface): return EnvironmentCSPResult(**response) - def create_atat_admin_user( - self, auth_credentials: Dict, csp_environment_id: str - ) -> Dict: - root_creds = self._root_creds - credentials = self._get_credential_obj(root_creds) - - sub_client = self.sdk.subscription.SubscriptionClient(credentials) - subscription = sub_client.subscriptions.get(csp_environment_id) - - managment_principal = self._get_management_service_principal() - - auth_client = self.sdk.authorization.AuthorizationManagementClient( - credentials, - # TODO: Determine which subscription this needs to point at - # Once we're in a multi-sub environment - subscription.id, - ) - - # Create role assignment for - role_assignment_id = str(uuid4()) - role_assignment_create_params = auth_client.role_assignments.models.RoleAssignmentCreateParameters( - role_definition_id=REMOTE_ROOT_ROLE_DEF_ID, - principal_id=managment_principal.id, - ) - - auth_client.role_assignments.create( - scope=f"/subscriptions/{subscription.id}/", - role_assignment_name=role_assignment_id, - parameters=role_assignment_create_params, - ) - - return { - "csp_user_id": managment_principal.object_id, - "credentials": managment_principal.password_credentials, - "role_name": role_assignment_id, - } - def create_application(self, payload: ApplicationCSPPayload): creds = self._source_creds(payload.tenant_id) credentials = self._get_credential_obj( diff --git a/atst/domain/csp/cloud/cloud_provider_interface.py b/atst/domain/csp/cloud/cloud_provider_interface.py index ac652b5c..88b55f96 100644 --- a/atst/domain/csp/cloud/cloud_provider_interface.py +++ b/atst/domain/csp/cloud/cloud_provider_interface.py @@ -31,33 +31,6 @@ class CloudProviderInterface: """ raise NotImplementedError() - def create_atat_admin_user( - self, auth_credentials: Dict, csp_environment_id: str - ) -> Dict: - """Creates a new, programmatic user in the CSP. Grants this user full permissions to administer - the CSP. - - Arguments: - auth_credentials -- Object containing CSP account credentials - csp_environment_id -- ID of the CSP Environment the admin user should be created in - - Returns: - object: Object representing new remote admin user, including credentials - Something like: - { - "user_id": string, - "credentials": dict, # structure TBD based on csp - } - - Raises: - AuthenticationException: Problem with the credentials - AuthorizationException: Credentials not authorized for current action(s) - ConnectionException: Issue with the CSP API connection - UnknownServerException: Unknown issue on the CSP side - UserProvisioningException: Problem creating the root user - """ - raise NotImplementedError() - def create_or_update_user( self, auth_credentials: Dict, user_info, csp_role_id: str ) -> str: diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index bb2f4941..7ec0636f 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -117,23 +117,6 @@ class MockCloudProvider(CloudProviderInterface): subscription_id="subscriptions/60fbbb72-0516-4253-ab18-c92432ba3230" ) - def create_atat_admin_user(self, auth_credentials, csp_environment_id): - self._authorize(auth_credentials) - - self._delay(1, 5) - self._maybe_raise(self.NETWORK_FAILURE_PCT, self.NETWORK_EXCEPTION) - self._maybe_raise(self.SERVER_FAILURE_PCT, self.SERVER_EXCEPTION) - self._maybe_raise( - self.ATAT_ADMIN_CREATE_FAILURE_PCT, - UserProvisioningException( - csp_environment_id, "atat_admin", "Could not create admin user." - ), - ) - - self._maybe_raise(self.UNAUTHORIZED_RATE, self.AUTHORIZATION_EXCEPTION) - - return {"id": self._id(), "credentials": self._auth_credentials} - def create_tenant(self, payload: TenantCSPPayload): """ payload is an instance of TenantCSPPayload data class diff --git a/atst/domain/environment_roles.py b/atst/domain/environment_roles.py index a022b932..f0b600c6 100644 --- a/atst/domain/environment_roles.py +++ b/atst/domain/environment_roles.py @@ -106,7 +106,7 @@ class EnvironmentRoles(object): def disable(cls, environment_role_id): environment_role = EnvironmentRoles.get_by_id(environment_role_id) - if environment_role.csp_user_id and not environment_role.environment.is_pending: + if environment_role.csp_user_id and not environment_role.environment.cloud_id: tenant_id = environment_role.environment.application.portfolio.csp_data.get( "tenant_id" ) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index f945f3f8..b43ae495 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -126,7 +126,7 @@ class Environments(object): results = ( cls.base_provision_query(now) .filter(Application.cloud_id != None) - .filter(Environment.cloud_id == None) + .filter(Environment.cloud_id.is_(None)) .all() ) return [id_ for id_, in results] diff --git a/atst/models/environment.py b/atst/models/environment.py index 8408c874..cd202fd0 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -1,5 +1,3 @@ -from enum import Enum - from sqlalchemy import Column, ForeignKey, String, UniqueConstraint from sqlalchemy.orm import relationship @@ -43,10 +41,6 @@ class Environment( ), ) - class ProvisioningStatus(Enum): - PENDING = "pending" - COMPLETED = "completed" - @property def users(self): return {r.application_role.user for r in self.roles} @@ -67,17 +61,6 @@ class Environment( def portfolio_id(self): return self.application.portfolio_id - @property - def provisioning_status(self) -> ProvisioningStatus: - if self.cloud_id is None: - return self.ProvisioningStatus.PENDING - else: - return self.ProvisioningStatus.COMPLETED - - @property - def is_pending(self): - return self.provisioning_status == self.ProvisioningStatus.PENDING - def __repr__(self): return "".format( self.name, diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 8b744b04..d1ca1d44 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -39,7 +39,6 @@ def get_environments_obj_for_app(application): { "id": env.id, "name": env.name, - "pending": env.is_pending, "edit_form": EditEnvironmentForm(obj=env), "member_count": len(env.roles), "members": sorted( diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index 7ef1e67b..3a25f849 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -2,11 +2,11 @@ import json from unittest.mock import Mock, patch from uuid import uuid4 +import pendulum +import pydantic import pytest from tests.factories import ApplicationFactory, EnvironmentFactory from tests.mock_azure import AUTH_CREDENTIALS, mock_azure -import pendulum -import pydantic from atst.domain.csp.cloud import AzureCloudProvider from atst.domain.csp.cloud.models import ( @@ -52,6 +52,7 @@ from atst.domain.csp.cloud.models import ( TenantPrincipalCSPResult, TenantPrincipalOwnershipCSPPayload, TenantPrincipalOwnershipCSPResult, + UserCSPPayload, ) BILLING_ACCOUNT_NAME = "52865e4c-52e8-5a6c-da6b-c58f0814f06f:7ea5de9d-b8ce-4901-b1c5-d864320c7b03_2019-05-31" @@ -107,20 +108,6 @@ def test_create_application_succeeds(mock_azure: AzureCloudProvider): assert result.id == "Test Id" -def test_create_atat_admin_user_succeeds(mock_azure: AzureCloudProvider): - environment_id = str(uuid4()) - - csp_user_id = str(uuid4) - - mock_azure.sdk.graphrbac.GraphRbacManagementClient.return_value.service_principals.create.return_value.object_id = ( - csp_user_id - ) - - result = mock_azure.create_atat_admin_user(AUTH_CREDENTIALS, environment_id) - - assert result.get("csp_user_id") == csp_user_id - - def test_create_policy_definition_succeeds(mock_azure: AzureCloudProvider): subscription_id = str(uuid4()) management_group_id = str(uuid4()) @@ -882,3 +869,72 @@ def test_set_secret(mock_azure: AzureCloudProvider): ) assert mock_azure.set_secret("secret key", "secret_value") == "my secret" + + +def test_create_active_directory_user(mock_azure: AzureCloudProvider): + mock_result = Mock() + mock_result.ok = True + mock_result.json.return_value = {"id": "id"} + mock_azure.sdk.requests.post.return_value = mock_result + + payload = UserCSPPayload( + tenant_id=uuid4().hex, + display_name="Test Testerson", + tenant_host_name="testtenant", + email="test@testerson.test", + password="asdfghjkl", # pragma: allowlist secret + ) + + result = mock_azure._create_active_directory_user("token", payload) + + assert result.id == "id" + + +def test_update_active_directory_user_email(mock_azure: AzureCloudProvider): + mock_result = Mock() + mock_result.ok = True + mock_azure.sdk.requests.patch.return_value = mock_result + + payload = UserCSPPayload( + tenant_id=uuid4().hex, + display_name="Test Testerson", + tenant_host_name="testtenant", + email="test@testerson.test", + password="asdfghjkl", # pragma: allowlist secret + ) + + result = mock_azure._update_active_directory_user_email( + "token", uuid4().hex, payload + ) + + assert result + + +def test_create_user(mock_azure: AzureCloudProvider): + with patch.object( + AzureCloudProvider, + "_get_tenant_principal_token", + wraps=mock_azure._get_tenant_principal_token, + ) as _get_tenant_principal_token: + _get_tenant_principal_token.return_value = "token" + + mock_result_create = Mock() + mock_result_create.ok = True + mock_result_create.json.return_value = {"id": "id"} + mock_azure.sdk.requests.post.return_value = mock_result_create + + mock_result_update = Mock() + mock_result_update.ok = True + mock_azure.sdk.requests.patch.return_value = mock_result_update + + payload = UserCSPPayload( + tenant_id=uuid4().hex, + display_name="Test Testerson", + tenant_host_name="testtenant", + email="test@testerson.test", + password="asdfghjkl", # pragma: allowlist secret + ) + + result = mock_azure.create_user(payload) + + assert result.id == "id" diff --git a/tests/domain/cloud/test_mock_csp.py b/tests/domain/cloud/test_mock_csp.py index 86727b38..c65e11e1 100644 --- a/tests/domain/cloud/test_mock_csp.py +++ b/tests/domain/cloud/test_mock_csp.py @@ -28,17 +28,6 @@ def test_create_environment(mock_csp: MockCloudProvider): assert isinstance(result, EnvironmentCSPResult) -def test_create_admin_user(mock_csp: MockCloudProvider): - admin_user = mock_csp.create_atat_admin_user(CREDENTIALS, "env_id") - assert isinstance(admin_user["id"], str) - assert isinstance(admin_user["credentials"], dict) - - -def test_create_environment_baseline(mock_csp: MockCloudProvider): - baseline = mock_csp.create_atat_admin_user(CREDENTIALS, "env_id") - assert isinstance(baseline, dict) - - def test_create_or_update_user(mock_csp: MockCloudProvider): env_role = EnvironmentRoleFactory.create() csp_user_id = mock_csp.create_or_update_user(CREDENTIALS, env_role, "csp_role_id") diff --git a/tests/domain/test_environment_roles.py b/tests/domain/test_environment_roles.py index 77f6033a..e91a837d 100644 --- a/tests/domain/test_environment_roles.py +++ b/tests/domain/test_environment_roles.py @@ -93,7 +93,7 @@ def test_disable_completed(application_role, environment): def test_disable_checks_env_provisioning_status(session): environment = EnvironmentFactory.create() - assert environment.is_pending + assert not environment.cloud_id env_role1 = EnvironmentRoleFactory.create(environment=environment) env_role1 = EnvironmentRoles.disable(env_role1.id) assert env_role1.disabled @@ -103,7 +103,7 @@ def test_disable_checks_env_provisioning_status(session): session.commit() session.refresh(environment) - assert not environment.is_pending + assert environment.cloud_id env_role2 = EnvironmentRoleFactory.create(environment=environment) env_role2 = EnvironmentRoles.disable(env_role2.id) assert env_role2.disabled diff --git a/tests/models/test_environments.py b/tests/models/test_environments.py index 8f8ac626..ed55d737 100644 --- a/tests/models/test_environments.py +++ b/tests/models/test_environments.py @@ -51,18 +51,6 @@ def test_audit_event_for_environment_deletion(session): assert after -@pytest.mark.parametrize( - "env_data,expected_status", - [ - [{"cloud_id": None,}, Environment.ProvisioningStatus.PENDING], - [{"cloud_id": 1}, Environment.ProvisioningStatus.COMPLETED], - ], -) -def test_environment_provisioning_status(env_data, expected_status): - environment = EnvironmentFactory.create(**env_data) - assert environment.provisioning_status == expected_status - - def test_environment_roles_do_not_include_deleted(): member_list = [ {"role_name": CSPRole.ADMIN},