From 64152e6841edbb548d1a2c7eb0bb956ff1c98ef3 Mon Sep 17 00:00:00 2001 From: zubair-arbi Date: Tue, 29 Aug 2017 17:28:24 +0500 Subject: [PATCH] add error message in odata api log ENT-600 --- common/djangoapps/third_party_auth/saml.py | 22 ++++--- .../tests/specs/test_testshib.py | 60 ++++++++++++++++++- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/common/djangoapps/third_party_auth/saml.py b/common/djangoapps/third_party_auth/saml.py index 568e785920..1e5e0a7e82 100644 --- a/common/djangoapps/third_party_auth/saml.py +++ b/common/djangoapps/third_party_auth/saml.py @@ -290,27 +290,33 @@ class SapSuccessFactorsIdentityProvider(EdXSAMLIdentityProvider): # from the SAML assertion. return details username = details['username'] + fields = ','.join(self.field_mappings) + odata_api_url = '{root_url}User(userId=\'{user_id}\')?$select={fields}'.format( + root_url=self.odata_api_root_url, + user_id=username, + fields=fields, + ) try: client = self.get_odata_api_client(user_id=username) - fields = ','.join(self.field_mappings) response = client.get( - '{root_url}User(userId=\'{user_id}\')?$select={fields}'.format( - root_url=self.odata_api_root_url, - user_id=username, - fields=fields, - ), + odata_api_url, timeout=self.timeout, ) response.raise_for_status() response = response.json() - except requests.RequestException: + except requests.RequestException as err: # If there was an HTTP level error, log the error and return the details from the SAML assertion. log.warning( - 'Unable to retrieve user details with username %s from SAPSuccessFactors with company ID %s.', + 'Unable to retrieve user details with username %s from SAPSuccessFactors for company ID %s ' + 'with url "%s" and error message: %s', username, self.odata_company_id, + odata_api_url, + err.message, + exc_info=True, ) return details + return self.get_registration_fields(response) diff --git a/common/djangoapps/third_party_auth/tests/specs/test_testshib.py b/common/djangoapps/third_party_auth/tests/specs/test_testshib.py index d80607309d..5aa064e3c0 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_testshib.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_testshib.py @@ -6,12 +6,14 @@ import ddt import unittest import httpretty import json +import logging from mock import patch from freezegun import freeze_time from social_django.models import UserSocialAuth +from testfixtures import LogCapture from unittest import skip -from third_party_auth.saml import log as saml_log +from third_party_auth.saml import log as saml_log, SapSuccessFactorsIdentityProvider from third_party_auth.tasks import fetch_saml_metadata from third_party_auth.tests import testutil @@ -316,6 +318,26 @@ class SuccessFactorsIntegrationTest(SamlIntegrationTestUtilities, IntegrationTes httpretty.register_uri(httpretty.GET, ODATA_USER_URL, content_type='application/json', body=user_callback) + def _mock_odata_api_for_error(self, odata_api_root_url, username): + """ + Mock an error response when calling the OData API for user details. + """ + + def callback(request, uri, headers): # pylint: disable=unused-argument + """ + Return a 500 error when someone tries to call the URL. + """ + return 500, headers, 'Failure!' + + fields = ','.join(SapSuccessFactorsIdentityProvider.default_field_mapping.copy()) + url = '{root_url}User(userId=\'{user_id}\')?$select={fields}'.format( + root_url=odata_api_root_url, + user_id=username, + fields=fields, + ) + httpretty.register_uri(httpretty.GET, url, body=callback, content_type='application/json') + return url + def test_register_insufficient_sapsf_metadata(self): """ Configure the provider such that it doesn't have enough details to contact the SAP @@ -468,6 +490,42 @@ class SuccessFactorsIntegrationTest(SamlIntegrationTestUtilities, IntegrationTes self.USER_USERNAME = "myself" super(SuccessFactorsIntegrationTest, self).test_register() + def test_register_http_failure_in_odata(self): + """ + Ensure that if there's an HTTP failure while fetching user details from + SAP SuccessFactors OData API. + """ + # Because we're getting details from the assertion, fall back to the initial set of details. + self.USER_EMAIL = "myself@testshib.org" + self.USER_NAME = "Me Myself And I" + self.USER_USERNAME = "myself" + + odata_company_id = 'NCC1701D' + odata_api_root_url = 'http://api.successfactors.com/odata/v2/' + mocked_odata_ai_url = self._mock_odata_api_for_error(odata_api_root_url, self.USER_USERNAME) + self._configure_testshib_provider( + identity_provider_type='sap_success_factors', + metadata_source=TESTSHIB_METADATA_URL, + other_settings=json.dumps({ + 'sapsf_oauth_root_url': 'http://successfactors.com/oauth/', + 'sapsf_private_key': 'fake_private_key_here', + 'odata_api_root_url': odata_api_root_url, + 'odata_company_id': odata_company_id, + 'odata_client_id': 'TatVotSEiCMteSNWtSOnLanCtBGwNhGB', + }) + ) + with LogCapture(level=logging.WARNING) as log_capture: + super(SuccessFactorsIntegrationTest, self).test_register() + expected_message = 'Unable to retrieve user details with username {username} from SAPSuccessFactors ' \ + 'for company ID {company_id} with url "{odata_api_url}" and error message: ' \ + '500 Server Error: Internal Server Error for url: {odata_api_url}'.format( + username=self.USER_USERNAME, + company_id=odata_company_id, + odata_api_url=mocked_odata_ai_url, + ) + logging_messages = [log_msg.getMessage() for log_msg in log_capture.records] + self.assertTrue(expected_message in logging_messages) + @skip('Test not necessary for this subclass') def test_get_saml_idp_class_with_fake_identifier(self): pass