Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: updating sso orchestrator self service api endpoints #1912

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ Change Log
Unreleased
----------
[4.6.4]
-------
chore: updating sso orchestrator self service api endpoints

[4.6.3]
-------
Expand Down
2 changes: 1 addition & 1 deletion enterprise/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
Your project description goes here.
"""

__version__ = "4.6.3"
__version__ = "4.6.4"
23 changes: 15 additions & 8 deletions enterprise/api/v1/views/enterprise_customer_sso_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from enterprise import models
from enterprise.api.utils import get_enterprise_customer_from_user_id
from enterprise.api.v1 import serializers
from enterprise.api_client.sso_orchestrator import SsoOrchestratorClientError
from enterprise.logging import getEnterpriseLogger
from enterprise.models import EnterpriseCustomer, EnterpriseCustomerSsoConfiguration, EnterpriseCustomerUser
from enterprise.tasks import send_sso_configured_email
Expand Down Expand Up @@ -146,6 +147,14 @@ def oauth_orchestration_complete(self, request, configuration_uuid, *args, **kwa
' not been marked as submitted.'
)

if error_msg := request.POST.get('error'):
LOGGER.error(
f'SSO configuration record {sso_configuration_record.pk} has failed to configure due to {error_msg}.'
)
sso_configuration_record.errored_at = localized_utcnow()
sso_configuration_record.save()
return Response(status=HTTP_200_OK)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we returning 200 success for an error state?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed, our system isn't failing here it's properly receiving the cancellation request from the orchestrator. I also don't want to blow up the orchestrator by returning a 400+ status code when our system did what it's supposed to


# Mark the configuration record as active IFF this the record has never been configured.
if not sso_configuration_record.configured_at:
sso_configuration_record.active = True
Expand Down Expand Up @@ -248,14 +257,12 @@ def create(self, request, *args, **kwargs):
request_data['entity_id'] = entity_id

try:
new_record = EnterpriseCustomerSsoConfiguration.objects.create(**request_data)
except TypeError as e:
LOGGER.error(f'{CONFIG_CREATE_ERROR}{e}')
return Response({'error': f'{CONFIG_CREATE_ERROR}{e}'}, status=HTTP_400_BAD_REQUEST)

# Wondering what to do here with error handling
# If we fail to submit for configuration (ie get a network error) should we rollback the created record?
new_record.submit_for_configuration()
with transaction.atomic():
new_record = EnterpriseCustomerSsoConfiguration.objects.create(**request_data)
new_record.submit_for_configuration()
except (TypeError, SsoOrchestratorClientError) as e:
LOGGER.error(f'{CONFIG_CREATE_ERROR} {e}')
return Response({'error': f'{CONFIG_CREATE_ERROR} {e}'}, status=HTTP_400_BAD_REQUEST)

return Response({'data': new_record.pk}, status=HTTP_201_CREATED)

Expand Down
23 changes: 23 additions & 0 deletions enterprise/migrations/0194_auto_20231013_1359.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django 3.2.22 on 2023-10-13 13:59

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('enterprise', '0193_auto_20231005_1708'),
]

operations = [
migrations.AddField(
model_name='enterprisecustomerssoconfiguration',
name='errored_at',
field=models.DateTimeField(blank=True, help_text='The date and time when the orchestrator encountered an error during configuration.', null=True),
),
migrations.AddField(
model_name='historicalenterprisecustomerssoconfiguration',
name='errored_at',
field=models.DateTimeField(blank=True, help_text='The date and time when the orchestrator encountered an error during configuration.', null=True),
),
]
10 changes: 10 additions & 0 deletions enterprise/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3947,6 +3947,14 @@ class Meta:
)
)

errored_at = models.DateTimeField(
blank=True,
null=True,
help_text=_(
"The date and time when the orchestrator encountered an error during configuration."
)
)

# ---------------------------- SAP Success Factors attribute mappings ---------------------------- #

odata_api_timeout_interval = models.PositiveIntegerField(
Expand Down Expand Up @@ -4051,6 +4059,8 @@ def is_pending_configuration(self):
if self.submitted_at:
if not self.configured_at:
return True
if self.errored_at and self.errored_at > self.submitted_at:
return False
if self.submitted_at > self.configured_at:
return True
return False
Expand Down
59 changes: 57 additions & 2 deletions tests/test_enterprise/api/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7228,13 +7228,13 @@ def post_new_sso_configuration(self, data):
)
return self.client.post(url, data=data)

def post_sso_configuration_complete(self, config_pk):
def post_sso_configuration_complete(self, config_pk, data=None):
"""Helper method to hit the configuration complete endpoint for sso configurations."""
url = settings.TEST_SERVER + reverse(
self.SSO_CONFIGURATION_COMPLETE_ENDPOINT,
kwargs={'configuration_uuid': config_pk}
)
return self.client.post(url)
return self.client.post(url, data=data)

def _get_existing_sso_record_url(self, config_pk):
"""Helper method to get the url for an existing sso configuration endpoint."""
Expand Down Expand Up @@ -7291,6 +7291,27 @@ def test_sso_configuration_oauth_orchestration_complete_not_found(self):
response = self.post_sso_configuration_complete(config_pk)
assert response.status_code == 404

@mock.patch("enterprise.api_client.braze.BrazeAPIClient.get_braze_client")
def test_sso_configuration_oauth_orchestration_complete_error(self, mock_braze_client):
"""
Verify that the endpoint is able to mark an sso config as errored.
"""
mock_braze_client.return_value.get_braze_client.return_value = mock.MagicMock()
self.set_jwt_cookie(ENTERPRISE_OPERATOR_ROLE, "*")
config_pk = uuid.uuid4()
enterprise_sso_orchestration_config = EnterpriseCustomerSsoConfigurationFactory(
uuid=config_pk,
enterprise_customer=self.enterprise_customer,
configured_at=None,
submitted_at=localized_utcnow(),
)
assert enterprise_sso_orchestration_config.is_pending_configuration()
response = self.post_sso_configuration_complete(config_pk, data={'error': 'test error'})
enterprise_sso_orchestration_config.refresh_from_db()
assert enterprise_sso_orchestration_config.configured_at is None
assert enterprise_sso_orchestration_config.errored_at is not None
assert response.status_code == status.HTTP_200_OK

@mock.patch("enterprise.api_client.braze.BrazeAPIClient.get_braze_client")
def test_sso_configuration_oauth_orchestration_complete(self, mock_braze_client):
"""
Expand Down Expand Up @@ -7561,6 +7582,40 @@ def test_sso_configuration_create_bad_data_format(self):
response = self.post_new_sso_configuration(data)
assert "somewhackyvalue" in response.json()['error']

@responses.activate
def test_sso_configuration_create_error_from_orchestrator(self):
"""
Test that the sso orchestration create endpoint will rollback a created object if the submission for
configuration fails.
"""
xml_metadata = """
<EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" entityID="https://example.com">
</EntityDescriptor>
"""
responses.add(
responses.GET,
"https://examples.com/metadata.xml",
body=xml_metadata,
)
responses.add(
responses.POST,
urljoin(get_sso_orchestrator_api_base_url(), get_sso_orchestrator_configure_path()),
json={'error': 'some error'},
status=400,
)
data = {
"metadata_url": "https://examples.com/metadata.xml",
"active": False,
"enterprise_customer": str(self.enterprise_customer.uuid),
"identity_provider": "cornerstone"
}
self.set_jwt_cookie(ENTERPRISE_ADMIN_ROLE, self.enterprise_customer.uuid)

response = self.post_new_sso_configuration(data)

assert response.status_code == 400
assert EnterpriseCustomerSsoConfiguration.objects.all().count() == 0

def test_sso_configuration_create_bad_xml_url(self):
"""
Test expected response when creating a new sso configuration with a bad xml url.
Expand Down
Loading