From a18d4d8a9a43db1516626a2462a479691e057b13 Mon Sep 17 00:00:00 2001 From: Jonathan Green Date: Tue, 17 Sep 2024 15:05:18 -0300 Subject: [PATCH] Replace parametrize parameters with pytest.param (#2066) --- tests/manager/api/lcp/test_hash.py | 20 +- .../api/saml/configuration/test_model.py | 12 +- .../api/saml/configuration/test_validator.py | 35 ++- .../metadata/federations/test_validator.py | 75 ++++--- .../manager/api/saml/metadata/test_filter.py | 48 ++-- tests/manager/api/saml/metadata/test_model.py | 63 +++--- .../manager/api/saml/metadata/test_parser.py | 135 ++++++------ .../python_expression_dsl/test_evaluator.py | 207 ++++++++++++------ .../saml/python_expression_dsl/test_parser.py | 18 +- tests/manager/api/saml/test_auth.py | 38 ++-- tests/manager/api/saml/test_controller.py | 58 +++-- tests/manager/api/saml/test_provider.py | 93 ++++---- .../sqlalchemy/model/test_identifier.py | 56 +++-- .../sqlalchemy/model/test_licensing.py | 9 +- tests/manager/util/test_opds_writer.py | 24 +- 15 files changed, 491 insertions(+), 400 deletions(-) diff --git a/tests/manager/api/lcp/test_hash.py b/tests/manager/api/lcp/test_hash.py index 1185b25937..4ae0cf1e02 100644 --- a/tests/manager/api/lcp/test_hash.py +++ b/tests/manager/api/lcp/test_hash.py @@ -5,35 +5,35 @@ class TestHasherFactory: @pytest.mark.parametrize( - "_,hashing_algorithm,value,expected_value", + "hashing_algorithm,value,expected_value", [ - ( - "sha256", + pytest.param( HashingAlgorithm.SHA256, "12345", "5994471abb01112afcc18159f6cc74b4f511b99806da59b3caf5a9c173cacfc5", + id="sha256", ), - ( - "sha256_value", + pytest.param( HashingAlgorithm.SHA256.value, "12345", "5994471abb01112afcc18159f6cc74b4f511b99806da59b3caf5a9c173cacfc5", + id="sha256_value", ), - ( - "sha512", + pytest.param( HashingAlgorithm.SHA512, "12345", "3627909a29c31381a071ec27f7c9ca97726182aed29a7ddd2e54353322cfb30abb9e3a6df2ac2c20fe23436311d678564d0c8d305930575f60e2d3d048184d79", + id="sha512", ), - ( - "sha512_value", + pytest.param( HashingAlgorithm.SHA512.value, "12345", "3627909a29c31381a071ec27f7c9ca97726182aed29a7ddd2e54353322cfb30abb9e3a6df2ac2c20fe23436311d678564d0c8d305930575f60e2d3d048184d79", + id="sha512_value", ), ], ) - def test_create(self, _, hashing_algorithm, value, expected_value): + def test_create(self, hashing_algorithm, value, expected_value): hasher_factory = HasherFactory() hasher = hasher_factory.create(hashing_algorithm) diff --git a/tests/manager/api/saml/configuration/test_model.py b/tests/manager/api/saml/configuration/test_model.py index f18a81f9b1..e1de71d41a 100644 --- a/tests/manager/api/saml/configuration/test_model.py +++ b/tests/manager/api/saml/configuration/test_model.py @@ -324,10 +324,9 @@ def test_get_identity_provider_settings_returns_correct_result(self): onelogin_configuration.get_identity_providers.assert_called_once_with(db) @pytest.mark.parametrize( - "_,service_provider,expected_result", + "service_provider,expected_result", [ - ( - "service_provider_without_certificates", + pytest.param( SERVICE_PROVIDER_WITHOUT_CERTIFICATE, { "sp": { @@ -344,9 +343,9 @@ def test_get_identity_provider_settings_returns_correct_result(self): "authnRequestsSigned": SERVICE_PROVIDER_WITH_CERTIFICATE.authn_requests_signed }, }, + id="service_provider_without_certificates", ), - ( - "service_provider_with_certificate", + pytest.param( SERVICE_PROVIDER_WITH_CERTIFICATE, { "sp": { @@ -365,11 +364,12 @@ def test_get_identity_provider_settings_returns_correct_result(self): "authnRequestsSigned": SERVICE_PROVIDER_WITH_CERTIFICATE.authn_requests_signed }, }, + id="service_provider_with_certificate", ), ], ) def test_get_service_provider_settings_returns_correct_result( - self, _, service_provider, expected_result + self, service_provider, expected_result ): # Arrange configuration = create_autospec(spec=SAMLWebSSOAuthSettings) diff --git a/tests/manager/api/saml/configuration/test_validator.py b/tests/manager/api/saml/configuration/test_validator.py index 774a9120e1..23e56f63c1 100644 --- a/tests/manager/api/saml/configuration/test_validator.py +++ b/tests/manager/api/saml/configuration/test_validator.py @@ -15,69 +15,68 @@ class TestSAMLSettingsValidator: @pytest.mark.parametrize( - "_,sp_xml_metadata,idp_xml_metadata,patron_id_regular_expression,expected_validation_result", + "sp_xml_metadata,idp_xml_metadata,patron_id_regular_expression,expected_validation_result", [ - ( - "missing_sp_metadata_and_missing_idp_metadata", + pytest.param( None, None, None, INCOMPLETE_CONFIGURATION, + id="missing_sp_metadata_and_missing_idp_metadata", ), - ( - "empty_sp_metadata_and_empty_idp_metadata", + pytest.param( saml_strings.INCORRECT_XML, saml_strings.INCORRECT_XML, None, INCOMPLETE_CONFIGURATION, + id="empty_sp_metadata_and_empty_idp_metadata", ), - ( - "incorrect_sp_metadata_and_incorrect_idp_metadata", + pytest.param( saml_strings.INCORRECT_XML_WITH_ONE_SP_METADATA_WITHOUT_ACS_SERVICE, saml_strings.INCORRECT_XML_WITH_ONE_IDP_METADATA_WITHOUT_SSO_SERVICE, None, SAML_INCORRECT_METADATA, + id="incorrect_sp_metadata_and_incorrect_idp_metadata", ), - ( - "correct_sp_metadata_and_incorrect_idp_metadata", + pytest.param( saml_strings.CORRECT_XML_WITH_ONE_SP, saml_strings.INCORRECT_XML_WITH_ONE_IDP_METADATA_WITHOUT_SSO_SERVICE, None, SAML_INCORRECT_METADATA, + id="correct_sp_metadata_and_incorrect_idp_metadata", ), - ( - "correct_sp_and_idp_metadata", + pytest.param( saml_strings.CORRECT_XML_WITH_ONE_SP, saml_strings.CORRECT_XML_WITH_IDP_1, None, None, + id="correct_sp_and_idp_metadata", ), - ( - "correct_patron_id_regular_expression", + pytest.param( saml_strings.CORRECT_XML_WITH_ONE_SP, saml_strings.CORRECT_XML_WITH_IDP_1, r"(?P.+)@university\.org", None, + id="correct_patron_id_regular_expression", ), - ( - "correct_patron_id_regular_expression_without_patron_id_named_group", + pytest.param( saml_strings.CORRECT_XML_WITH_ONE_SP, saml_strings.CORRECT_XML_WITH_IDP_1, r"(?P.+)@university\.org", SAML_INCORRECT_PATRON_ID_REGULAR_EXPRESSION, + id="correct_patron_id_regular_expression_without_patron_id_named_group", ), - ( - "incorrect_patron_id_regular_expression", + pytest.param( saml_strings.CORRECT_XML_WITH_ONE_SP, saml_strings.CORRECT_XML_WITH_IDP_1, r"[", INVALID_CONFIGURATION_OPTION, + id="incorrect_patron_id_regular_expression", ), ], ) def test_validate( self, - _, sp_xml_metadata, idp_xml_metadata, patron_id_regular_expression, diff --git a/tests/manager/api/saml/metadata/federations/test_validator.py b/tests/manager/api/saml/metadata/federations/test_validator.py index 15e67bb2ab..e20a985a29 100644 --- a/tests/manager/api/saml/metadata/federations/test_validator.py +++ b/tests/manager/api/saml/metadata/federations/test_validator.py @@ -19,111 +19,110 @@ class TestSAMLFederatedMetadataExpirationValidator: @pytest.mark.parametrize( - "_,current_time,metadata,expected_exception", + "current_time,metadata,expected_exception", [ - ( - "incorrect_xml_str_type", + pytest.param( utc_now(), fixtures.INCORRECT_XML, SAMLFederatedMetadataValidationError, + id="incorrect_xml_str_type", ), - ( - "incorrect_xml_bytes_type", + pytest.param( utc_now(), fixtures.INCORRECT_XML.encode(), SAMLFederatedMetadataValidationError, + id="incorrect_xml_bytes_type", ), - ( - "without_valid_until_attribute_metadata_str_type", + pytest.param( utc_now(), fixtures.FEDERATED_METADATA_WITHOUT_VALID_UNTIL_ATTRIBUTE, SAMLFederatedMetadataValidationError, + id="without_valid_until_attribute_metadata_str_type", ), - ( - "without_valid_until_attribute_metadata_bytes_type", + pytest.param( utc_now(), fixtures.FEDERATED_METADATA_WITHOUT_VALID_UNTIL_ATTRIBUTE.encode(), SAMLFederatedMetadataValidationError, + id="without_valid_until_attribute_metadata_bytes_type", ), - ( - "with_expired_valid_until_attribute_metadata_str_type", + pytest.param( fixtures.FEDERATED_METADATA_VALID_UNTIL + SAMLFederatedMetadataExpirationValidator.MAX_CLOCK_SKEW + datetime.timedelta(minutes=1), fixtures.FEDERATED_METADATA_WITH_VALID_UNTIL_ATTRIBUTE, SAMLFederatedMetadataValidationError, + id="with_expired_valid_until_attribute_metadata_str_type", ), - ( - "with_expired_valid_until_attribute_metadata_bytes_type", + pytest.param( fixtures.FEDERATED_METADATA_VALID_UNTIL + SAMLFederatedMetadataExpirationValidator.MAX_CLOCK_SKEW + datetime.timedelta(minutes=1), fixtures.FEDERATED_METADATA_WITH_VALID_UNTIL_ATTRIBUTE.encode(), SAMLFederatedMetadataValidationError, + id="with_expired_valid_until_attribute_metadata_bytes_type", ), - ( - "with_valid_until_attribute_too_far_in_the_future_metadata_str_type", + pytest.param( fixtures.FEDERATED_METADATA_VALID_UNTIL - SAMLFederatedMetadataExpirationValidator.MAX_VALID_TIME - datetime.timedelta(minutes=1), fixtures.FEDERATED_METADATA_WITH_VALID_UNTIL_ATTRIBUTE, SAMLFederatedMetadataValidationError, + id="with_valid_until_attribute_too_far_in_the_future_metadata_str_type", ), - ( - "with_valid_until_attribute_too_far_in_the_future_metadata_bytes_type", + pytest.param( fixtures.FEDERATED_METADATA_VALID_UNTIL - SAMLFederatedMetadataExpirationValidator.MAX_VALID_TIME - datetime.timedelta(minutes=1), fixtures.FEDERATED_METADATA_WITH_VALID_UNTIL_ATTRIBUTE.encode(), SAMLFederatedMetadataValidationError, + id="with_valid_until_attribute_too_far_in_the_future_metadata_bytes_type", ), - ( - "with_valid_until_attribute_less_than_current_time_and_less_than_max_clock_skew_metadata_str_type", + pytest.param( fixtures.FEDERATED_METADATA_VALID_UNTIL + SAMLFederatedMetadataExpirationValidator.MAX_CLOCK_SKEW, fixtures.FEDERATED_METADATA_WITH_VALID_UNTIL_ATTRIBUTE, None, + id="with_valid_until_attribute_less_than_current_time_and_less_than_max_clock_skew_metadata_str_type", ), - ( - "with_valid_until_attribute_less_than_current_time_and_less_than_max_clock_skew_metadata_bytes_type", + pytest.param( fixtures.FEDERATED_METADATA_VALID_UNTIL + SAMLFederatedMetadataExpirationValidator.MAX_CLOCK_SKEW, fixtures.FEDERATED_METADATA_WITH_VALID_UNTIL_ATTRIBUTE.encode(), None, + id="with_valid_until_attribute_less_than_current_time_and_less_than_max_clock_skew_metadata_bytes_type", ), - ( - "with_valid_until_attribute_greater_than_current_time_and_less_than_max_valid_time_metadata_str_type", + pytest.param( fixtures.FEDERATED_METADATA_VALID_UNTIL - SAMLFederatedMetadataExpirationValidator.MAX_VALID_TIME + datetime.timedelta(minutes=1), fixtures.FEDERATED_METADATA_WITH_VALID_UNTIL_ATTRIBUTE, None, + id="with_valid_until_attribute_greater_than_current_time_and_less_than_max_valid_time_metadata_str_type", ), - ( - "with_valid_until_attribute_greater_than_current_time_and_less_than_max_valid_time_metadata_bytes_type", + pytest.param( fixtures.FEDERATED_METADATA_VALID_UNTIL - SAMLFederatedMetadataExpirationValidator.MAX_VALID_TIME + datetime.timedelta(minutes=1), fixtures.FEDERATED_METADATA_WITH_VALID_UNTIL_ATTRIBUTE.encode(), None, + id="with_valid_until_attribute_greater_than_current_time_and_less_than_max_valid_time_metadata_bytes_type", ), - ( - "with_real_incommon_metadata_str_type", + pytest.param( datetime_utc(2020, 11, 26, 14, 32, 42), SamlFilesFixture.sample_text("incommon-metadata-idp-only.xml"), None, + id="with_real_incommon_metadata_str_type", ), - ( - "with_real_incommon_metadata_bytes_type", + pytest.param( datetime_utc(2020, 11, 26, 14, 32, 42), SamlFilesFixture.sample_data("incommon-metadata-idp-only.xml"), None, + id="with_real_incommon_metadata_bytes_type", ), ], ) def test_validate( self, - _, current_time: datetime.datetime, metadata: str | bytes, expected_exception: type[Exception] | None, @@ -145,29 +144,29 @@ def test_validate( class TestSAMLMetadataSignatureValidator: @pytest.mark.parametrize( - "_,certificate,metadata,expected_exception", + "certificate,metadata,expected_exception", [ - ( - "without_signature", + pytest.param( fixtures.FEDERATED_METADATA_CERTIFICATE, fixtures.FEDERATED_METADATA_WITH_VALID_UNTIL_ATTRIBUTE, SAMLFederatedMetadataValidationError, + id="without_signature", ), - ( - "with_invalid_signature", + pytest.param( fixtures.FEDERATED_METADATA_CERTIFICATE.strip(), fixtures.FEDERATED_METADATA_WITH_INVALID_SIGNATURE, SAMLFederatedMetadataValidationError, + id="with_invalid_signature", ), - ( - "with_valid_signature", + pytest.param( fixtures.FEDERATED_METADATA_CERTIFICATE.strip(), SamlFilesFixture.sample_text("incommon-metadata-idp-only.xml"), None, + id="with_valid_signature", ), ], ) - def test_validate(self, _, certificate, metadata, expected_exception): + def test_validate(self, certificate, metadata, expected_exception): # Arrange validator = SAMLMetadataSignatureValidator() federation = SAMLFederation( diff --git a/tests/manager/api/saml/metadata/test_filter.py b/tests/manager/api/saml/metadata/test_filter.py index 91dcb89f42..fd58b46957 100644 --- a/tests/manager/api/saml/metadata/test_filter.py +++ b/tests/manager/api/saml/metadata/test_filter.py @@ -19,10 +19,9 @@ class TestSAMLSubjectFilter: @pytest.mark.parametrize( - "_,expression,subject,expected_result,expected_exception", + "expression,subject,expected_result,expected_exception", [ - ( - "fails_in_the_case_of_syntax_error", + pytest.param( 'subject.attribute_statement.attributes["eduPersonEntitlement"].values[0 == "urn:mace:nyu.edu:entl:lib:eresources"', SAMLSubject( "http://idp.example.com", @@ -38,9 +37,9 @@ class TestSAMLSubjectFilter: ), None, SAMLSubjectFilterError, + id="fails_in_the_case_of_syntax_error", ), - ( - "fails_in_the_case_of_unknown_attribute", + pytest.param( 'subject.attribute_statement.attributes["mail"].values[0] == "urn:mace:nyu.edu:entl:lib:eresources"', SAMLSubject( "http://idp.example.com", @@ -56,9 +55,9 @@ class TestSAMLSubjectFilter: ), None, SAMLSubjectFilterError, + id="fails_in_the_case_of_unknown_attribute", ), - ( - "fails_when_subject_is_not_used", + pytest.param( 'attributes["eduPersonEntitlement"].values[0] == "urn:mace:nyu.edu:entl:lib:eresources"', SAMLSubject( "http://idp.example.com", @@ -74,9 +73,9 @@ class TestSAMLSubjectFilter: ), None, SAMLSubjectFilterError, + id="fails_when_subject_is_not_used", ), - ( - "can_filter_when_attribute_has_one_value", + pytest.param( '"urn:mace:nyu.edu:entl:lib:eresources" == subject.attribute_statement.attributes["eduPersonEntitlement"].values[0]', SAMLSubject( "http://idp.example.com", @@ -92,9 +91,9 @@ class TestSAMLSubjectFilter: ), True, None, + id="can_filter_when_attribute_has_one_value", ), - ( - "can_filter_when_attribute_has_multiple_values", + pytest.param( '"urn:mace:nyu.edu:entl:lib:eresources" in subject.attribute_statement.attributes["eduPersonEntitlement"].values', SAMLSubject( "http://idp.example.com", @@ -113,10 +112,11 @@ class TestSAMLSubjectFilter: ), True, None, + id="can_filter_when_attribute_has_multiple_values", ), ], ) - def test_execute(self, _, expression, subject, expected_result, expected_exception): + def test_execute(self, expression, subject, expected_result, expected_exception): # Arrange parser = DSLParser() visitor = DSLEvaluationVisitor() @@ -134,36 +134,36 @@ def test_execute(self, _, expression, subject, expected_result, expected_excepti assert expected_result == result @pytest.mark.parametrize( - "_,expression,expected_exception", + "expression,expected_exception", [ - ( - "fails_in_the_case_of_syntax_error", + pytest.param( 'subject.attribute_statement.attributes["eduPersonEntitlement"].values[0 == "urn:mace:nyu.edu:entl:lib:eresources"', SAMLSubjectFilterError, + id="fails_in_the_case_of_syntax_error", ), - ( - "fails_when_subject_is_not_used", + pytest.param( 'attributes["eduPersonEntitlement"].values[0] == "urn:mace:nyu.edu:entl:lib:eresources"', SAMLSubjectFilterError, + id="fails_when_subject_is_not_used", ), - ( - "can_filter_by_attribute_oid", + pytest.param( 'subject.attribute_statement.attributes["urn:oid:1.3.6.1.4.1.5923.1.8"].values[0] == "urn:mace:nyu.edu:entl:lib:eresources"', None, + id="can_filter_by_attribute_oid", ), - ( - "can_filter_when_attribute_has_one_value", + pytest.param( '"urn:mace:nyu.edu:entl:lib:eresources" == subject.attribute_statement.attributes["eduPersonEntitlement"].values[0]', None, + id="can_filter_when_attribute_has_one_value", ), - ( - "can_filter_when_attribute_has_multiple_values", + pytest.param( '"urn:mace:nyu.edu:entl:lib:eresources" in subject.attribute_statement.attributes["eduPersonEntitlement"].values', None, + id="can_filter_when_attribute_has_multiple_values", ), ], ) - def test_validate(self, _, expression, expected_exception): + def test_validate(self, expression, expected_exception): # Arrange parser = DSLParser() visitor = DSLEvaluationVisitor() diff --git a/tests/manager/api/saml/metadata/test_model.py b/tests/manager/api/saml/metadata/test_model.py index f16c60a4d4..79567b0b3f 100644 --- a/tests/manager/api/saml/metadata/test_model.py +++ b/tests/manager/api/saml/metadata/test_model.py @@ -43,18 +43,17 @@ def test_init_accepts_list_of_attributes(self): class TestSAMLSubjectPatronIDExtractor: @pytest.mark.parametrize( - "_,subject,expected_patron_id,use_name_id,patron_id_attributes,patron_id_regular_expression", + "subject,expected_patron_id,use_name_id,patron_id_attributes,patron_id_regular_expression", [ - ( - "subject_without_patron_id", + pytest.param( SAMLSubject("http://idp.example.com", None, None), None, True, None, None, + id="subject_without_patron_id", ), - ( - "subject_with_eduPersonTargetedID_attribute", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -78,9 +77,9 @@ class TestSAMLSubjectPatronIDExtractor: True, None, None, + id="subject_with_eduPersonTargetedID_attribute", ), - ( - "subject_with_eduPersonUniqueId_attribute", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -100,9 +99,9 @@ class TestSAMLSubjectPatronIDExtractor: True, None, None, + id="subject_with_eduPersonUniqueId_attribute", ), - ( - "subject_with_uid_attribute", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -114,9 +113,9 @@ class TestSAMLSubjectPatronIDExtractor: True, None, None, + id="subject_with_uid_attribute", ), - ( - "subject_with_name_id", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -133,9 +132,9 @@ class TestSAMLSubjectPatronIDExtractor: True, None, None, + id="subject_with_name_id", ), - ( - "subject_with_switched_off_use_of_name_id", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -152,9 +151,9 @@ class TestSAMLSubjectPatronIDExtractor: False, None, None, + id="subject_with_switched_off_use_of_name_id", ), - ( - "patron_id_attributes_matching_attributes_in_subject", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -178,9 +177,9 @@ class TestSAMLSubjectPatronIDExtractor: False, [SAMLAttributeType.uid.name], None, + id="patron_id_attributes_matching_attributes_in_subject", ), - ( - "patron_id_attributes_matching_second_saml_attribute", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -207,9 +206,9 @@ class TestSAMLSubjectPatronIDExtractor: SAMLAttributeType.uid.name, ], None, + id="patron_id_attributes_matching_second_saml_attribute", ), - ( - "patron_id_attributes_not_matching_attributes_in_subject", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -233,9 +232,9 @@ class TestSAMLSubjectPatronIDExtractor: False, [SAMLAttributeType.givenName.name], None, + id="patron_id_attributes_not_matching_attributes_in_subject", ), - ( - "patron_id_attributes_not_matching_attributes_in_subject_and_using_name_id_instead", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -259,9 +258,9 @@ class TestSAMLSubjectPatronIDExtractor: True, [SAMLAttributeType.givenName.name], None, + id="patron_id_attributes_not_matching_attributes_in_subject_and_using_name_id_instead", ), - ( - "patron_id_regular_expression_matching_saml_subject", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -288,9 +287,9 @@ class TestSAMLSubjectPatronIDExtractor: SAMLAttributeType.mail.name, ], saml_strings.PATRON_ID_REGULAR_EXPRESSION_ORG, + id="patron_id_regular_expression_matching_saml_subject", ), - ( - "patron_id_regular_expression_matching_second_saml_attribute", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -318,9 +317,9 @@ class TestSAMLSubjectPatronIDExtractor: SAMLAttributeType.mail.name, ], saml_strings.PATRON_ID_REGULAR_EXPRESSION_ORG, + id="patron_id_regular_expression_matching_second_saml_attribute", ), - ( - "unicode_patron_id_regular_expression_matching_saml_subject", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -347,9 +346,9 @@ class TestSAMLSubjectPatronIDExtractor: SAMLAttributeType.mail.name, ], saml_strings.PATRON_ID_REGULAR_EXPRESSION_ORG, + id="unicode_patron_id_regular_expression_matching_saml_subject", ), - ( - "patron_id_regular_expression_not_matching_saml_subject", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "1"), @@ -376,9 +375,9 @@ class TestSAMLSubjectPatronIDExtractor: SAMLAttributeType.mail.name, ], saml_strings.PATRON_ID_REGULAR_EXPRESSION_COM, + id="patron_id_regular_expression_not_matching_saml_subject", ), - ( - "patron_id_regular_expression_not_matching_saml_attributes_but_matching_name_id", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID( @@ -410,12 +409,12 @@ class TestSAMLSubjectPatronIDExtractor: SAMLAttributeType.mail.name, ], saml_strings.PATRON_ID_REGULAR_EXPRESSION_COM, + id="patron_id_regular_expression_not_matching_saml_attributes_but_matching_name_id", ), ], ) def test( self, - _, subject, expected_patron_id, use_name_id, diff --git a/tests/manager/api/saml/metadata/test_parser.py b/tests/manager/api/saml/metadata/test_parser.py index fa5ff5ee71..d0554db711 100644 --- a/tests/manager/api/saml/metadata/test_parser.py +++ b/tests/manager/api/saml/metadata/test_parser.py @@ -30,14 +30,16 @@ class TestSAMLMetadataParser: @pytest.mark.parametrize( - "_,incorrect_xml", + "incorrect_xml", [ - ("incorrect_xml_str_type", saml_strings.INCORRECT_XML), - ("incorrect_xml_bytes_type", saml_strings.INCORRECT_XML.encode()), + pytest.param(saml_strings.INCORRECT_XML, id="incorrect_xml_str_type"), + pytest.param( + saml_strings.INCORRECT_XML.encode(), id="incorrect_xml_bytes_type" + ), ], ) def test_parse_raises_exception_when_xml_metadata_has_incorrect_format( - self, _, incorrect_xml: str | bytes + self, incorrect_xml: str | bytes ): # Arrange metadata_parser = SAMLMetadataParser() @@ -47,21 +49,20 @@ def test_parse_raises_exception_when_xml_metadata_has_incorrect_format( metadata_parser.parse(incorrect_xml) @pytest.mark.parametrize( - "_,incorrect_xml_with_one_idp_metadata_without_sso_service", + "incorrect_xml_with_one_idp_metadata_without_sso_service", [ - ( - "incorrect_xml_with_one_idp_metadata_without_sso_service_str_type", + pytest.param( saml_strings.INCORRECT_XML_WITH_ONE_IDP_METADATA_WITHOUT_SSO_SERVICE, + id="incorrect_xml_with_one_idp_metadata_without_sso_service_str_type", ), - ( - "incorrect_xml_with_one_idp_metadata_without_sso_service_bytes_type", + pytest.param( saml_strings.INCORRECT_XML_WITH_ONE_IDP_METADATA_WITHOUT_SSO_SERVICE.encode(), + id="incorrect_xml_with_one_idp_metadata_without_sso_service_bytes_type", ), ], ) def test_parse_raises_exception_when_idp_metadata_does_not_contain_sso_service( self, - _, incorrect_xml_with_one_idp_metadata_without_sso_service: str | bytes, ): # Arrange @@ -74,21 +75,20 @@ def test_parse_raises_exception_when_idp_metadata_does_not_contain_sso_service( ) @pytest.mark.parametrize( - "_,incorrect_xml_with_one_idp_metadata_with_sso_service_with_wrong_binding", + "incorrect_xml_with_one_idp_metadata_with_sso_service_with_wrong_binding", [ - ( - "incorrect_xml_with_one_idp_metadata_with_sso_service_with_wrong_binding_str_type", + pytest.param( saml_strings.INCORRECT_XML_WITH_ONE_IDP_METADATA_WITH_SSO_SERVICE_WITH_WRONG_BINDING, + id="incorrect_xml_with_one_idp_metadata_with_sso_service_with_wrong_binding_str_type", ), - ( - "incorrect_xml_with_one_idp_metadata_with_sso_service_with_wrong_binding_bytes_type", + pytest.param( saml_strings.INCORRECT_XML_WITH_ONE_IDP_METADATA_WITH_SSO_SERVICE_WITH_WRONG_BINDING.encode(), + id="incorrect_xml_with_one_idp_metadata_with_sso_service_with_wrong_binding_bytes_type", ), ], ) def test_parse_raises_exception_when_idp_metadata_contains_sso_service_with_wrong_binding( self, - _, incorrect_xml_with_one_idp_metadata_with_sso_service_with_wrong_binding: ( str | bytes ), @@ -103,21 +103,20 @@ def test_parse_raises_exception_when_idp_metadata_contains_sso_service_with_wron ) @pytest.mark.parametrize( - "_,correct_xml_with_one_idp_metadata_without_display_names", + "correct_xml_with_one_idp_metadata_without_display_names", [ - ( - "correct_xml_with_one_idp_metadata_without_display_names_str_type", + pytest.param( saml_strings.CORRECT_XML_WITH_ONE_IDP_METADATA_WITHOUT_DISPLAY_NAMES, + id="correct_xml_with_one_idp_metadata_without_display_names_str_type", ), - ( - "correct_xml_with_one_idp_metadata_without_display_names_bytes_type", + pytest.param( saml_strings.CORRECT_XML_WITH_ONE_IDP_METADATA_WITHOUT_DISPLAY_NAMES.encode(), + id="correct_xml_with_one_idp_metadata_without_display_names_bytes_type", ), ], ) def test_parse_does_not_raise_exception_when_xml_metadata_does_not_have_display_names( self, - _, correct_xml_with_one_idp_metadata_without_display_names: str | bytes, ): # Arrange @@ -156,17 +155,20 @@ def test_parse_does_not_raise_exception_when_xml_metadata_does_not_have_display_ ) @pytest.mark.parametrize( - "_,correct_xml_with_idp_1", + "correct_xml_with_idp_1", [ - ("correct_xml_with_idp_1_str_type", saml_strings.CORRECT_XML_WITH_IDP_1), - ( - "correct_xml_with_idp_1_bytes_type", + pytest.param( + saml_strings.CORRECT_XML_WITH_IDP_1, + id="correct_xml_with_idp_1_str_type", + ), + pytest.param( saml_strings.CORRECT_XML_WITH_IDP_1.encode(), + id="correct_xml_with_idp_1_bytes_type", ), ], ) def test_parse_correctly_parses_one_idp_metadata( - self, _, correct_xml_with_idp_1: str | bytes + self, correct_xml_with_idp_1: str | bytes ): # Arrange metadata_parser = SAMLMetadataParser() @@ -258,17 +260,20 @@ def test_parse_correctly_parses_one_idp_metadata( ) @pytest.mark.parametrize( - "_,correct_xml_with_idp_1", + "correct_xml_with_idp_1", [ - ("correct_xml_with_idp_1_str_type", saml_strings.CORRECT_XML_WITH_IDP_1), - ( - "correct_xml_with_idp_1_bytes_type", + pytest.param( + saml_strings.CORRECT_XML_WITH_IDP_1, + id="correct_xml_with_idp_1_str_type", + ), + pytest.param( saml_strings.CORRECT_XML_WITH_IDP_1.encode(), + id="correct_xml_with_idp_1_bytes_type", ), ], ) def test_parse_correctly_parses_idp_metadata_without_name_id_format( - self, _, correct_xml_with_idp_1: str | bytes + self, correct_xml_with_idp_1: str | bytes ): # Arrange metadata_parser = SAMLMetadataParser() @@ -360,21 +365,20 @@ def test_parse_correctly_parses_idp_metadata_without_name_id_format( ) @pytest.mark.parametrize( - "_,correct_xml_with_one_idp_metadata_with_one_certificate", + "correct_xml_with_one_idp_metadata_with_one_certificate", [ - ( - "correct_xml_with_one_idp_metadata_with_one_certificate_str_type", + pytest.param( saml_strings.CORRECT_XML_WITH_ONE_IDP_METADATA_WITH_ONE_CERTIFICATE, + id="correct_xml_with_one_idp_metadata_with_one_certificate_str_type", ), - ( - "correct_xml_with_one_idp_metadata_with_one_certificate_bytes_type", + pytest.param( saml_strings.CORRECT_XML_WITH_ONE_IDP_METADATA_WITH_ONE_CERTIFICATE.encode(), + id="correct_xml_with_one_idp_metadata_with_one_certificate_bytes_type", ), ], ) def test_parse_correctly_parses_idp_metadata_with_one_certificate( self, - _, correct_xml_with_one_idp_metadata_with_one_certificate: str | bytes, ): # Arrange @@ -469,20 +473,20 @@ def test_parse_correctly_parses_idp_metadata_with_one_certificate( ) @pytest.mark.parametrize( - "_,correct_xml_with_multiple_idps", + "correct_xml_with_multiple_idps", [ - ( - "correct_xml_with_multiple_idps_str_type", + pytest.param( saml_strings.CORRECT_XML_WITH_MULTIPLE_IDPS, + id="correct_xml_with_multiple_idps_str_type", ), - ( - "correct_xml_with_multiple_idps_bytes_type", + pytest.param( saml_strings.CORRECT_XML_WITH_MULTIPLE_IDPS.encode(), + id="correct_xml_with_multiple_idps_bytes_type", ), ], ) def test_parse_correctly_parses_metadata_with_multiple_descriptors( - self, _, correct_xml_with_multiple_idps: str | bytes + self, correct_xml_with_multiple_idps: str | bytes ): # Arrange metadata_parser = SAMLMetadataParser() @@ -615,21 +619,20 @@ def test_parse_correctly_parses_metadata_with_multiple_descriptors( ) @pytest.mark.parametrize( - "_,incorrect_xml_with_one_sp_metadata_without_acs_service", + "incorrect_xml_with_one_sp_metadata_without_acs_service", [ - ( - "incorrect_xml_with_one_sp_metadata_without_acs_service_str_type", + pytest.param( saml_strings.INCORRECT_XML_WITH_ONE_SP_METADATA_WITHOUT_ACS_SERVICE, + id="incorrect_xml_with_one_sp_metadata_without_acs_service_str_type", ), - ( - "incorrect_xml_with_one_sp_metadata_without_acs_service_bytes_type", + pytest.param( saml_strings.INCORRECT_XML_WITH_ONE_SP_METADATA_WITHOUT_ACS_SERVICE.encode(), + id="incorrect_xml_with_one_sp_metadata_without_acs_service_bytes_type", ), ], ) def test_parse_raises_exception_when_sp_metadata_does_not_contain_acs_service( self, - _, incorrect_xml_with_one_sp_metadata_without_acs_service: str | bytes, ): # Arrange @@ -642,17 +645,20 @@ def test_parse_raises_exception_when_sp_metadata_does_not_contain_acs_service( ) @pytest.mark.parametrize( - "_,correct_xml_with_one_sp", + "correct_xml_with_one_sp", [ - ("correct_xml_with_one_sp_str_type", saml_strings.CORRECT_XML_WITH_ONE_SP), - ( - "correct_xml_with_one_sp_bytes_type", + pytest.param( + saml_strings.CORRECT_XML_WITH_ONE_SP, + id="correct_xml_with_one_sp_str_type", + ), + pytest.param( saml_strings.CORRECT_XML_WITH_ONE_SP.encode(), + id="correct_xml_with_one_sp_bytes_type", ), ], ) def test_parse_correctly_parses_one_sp_metadata( - self, _, correct_xml_with_one_sp: str | bytes + self, correct_xml_with_one_sp: str | bytes ): # Arrange metadata_parser = SAMLMetadataParser() @@ -741,10 +747,9 @@ def test_parse_correctly_parses_one_sp_metadata( class TestSAMLSubjectParser: @pytest.mark.parametrize( - "_,idp,name_id_format,name_id_nq,name_id_spnq,name_id,attributes,expected_result", + "idp,name_id_format,name_id_nq,name_id_spnq,name_id,attributes,expected_result", [ - ( - "name_id_and_attributes", + pytest.param( "http://idp.example.com", SAMLNameIDFormat.TRANSIENT.value, saml_strings.IDP_1_ENTITY_ID, @@ -767,9 +772,9 @@ class TestSAMLSubjectParser: ] ), ), + id="name_id_and_attributes", ), - ( - "edu_person_targeted_id_as_name_id", + pytest.param( "http://idp.example.com", None, None, @@ -802,9 +807,9 @@ class TestSAMLSubjectParser: ] ), ), + id="edu_person_targeted_id_as_name_id", ), - ( - "edu_person_targeted_id_as_name_id_and_other_attributes", + pytest.param( "http://idp.example.com", None, None, @@ -841,9 +846,9 @@ class TestSAMLSubjectParser: ] ), ), + id="edu_person_targeted_id_as_name_id_and_other_attributes", ), - ( - "edu_person_principal_name_as_name_id", + pytest.param( "http://idp.example.com", None, None, @@ -876,12 +881,12 @@ class TestSAMLSubjectParser: ] ), ), + id="edu_person_principal_name_as_name_id", ), ], ) def test_parse( self, - _, idp: str, name_id_format: str, name_id_nq: str, diff --git a/tests/manager/api/saml/python_expression_dsl/test_evaluator.py b/tests/manager/api/saml/python_expression_dsl/test_evaluator.py index 89f24813b8..29c6628b7e 100644 --- a/tests/manager/api/saml/python_expression_dsl/test_evaluator.py +++ b/tests/manager/api/saml/python_expression_dsl/test_evaluator.py @@ -45,214 +45,279 @@ def get_attribute_value(self, index): class TestDSLEvaluator: @pytest.mark.parametrize( - "_,expression,expected_result,context,safe_classes,expected_exception", + "expression,expected_result,context,safe_classes,expected_exception", [ - ("incorrect_expression", "?", None, None, None, DSLParseError), - ("numeric_literal", "9", 9, None, None, None), - ("numeric_float_literal", "9.5", 9.5, None, None, None), - ("unknown_identifier", "foo", None, None, None, DSLEvaluationError), - ("known_identifier", "foo", 9, {"foo": 9}, None, None), - ( - "unknown_nested_identifier", + pytest.param( + "?", None, None, None, DSLParseError, id="incorrect_expression" + ), + pytest.param("9", 9, None, None, None, id="numeric_literal"), + pytest.param("9.5", 9.5, None, None, None, id="numeric_float_literal"), + pytest.param( + "foo", None, None, None, DSLEvaluationError, id="unknown_identifier" + ), + pytest.param("foo", 9, {"foo": 9}, None, None, id="known_identifier"), + pytest.param( "foo.bar", None, {"foo": 9}, None, DSLEvaluationError, + id="unknown_nested_identifier", + ), + pytest.param( + "foo.bar", + 9, + {"foo": {"bar": 9}}, + None, + None, + id="known_nested_identifier", ), - ("known_nested_identifier", "foo.bar", 9, {"foo": {"bar": 9}}, None, None), - ( - "known_nested_identifier", + pytest.param( "foo.bar.baz", 9, {"foo": {"bar": {"baz": 9}}}, None, None, + id="known_nested_identifier", ), - ( - "known_nested_identifier", + pytest.param( "foo.bar[0].baz", 9, {"foo": {"bar": [{"baz": 9}]}}, None, None, + id="known_nested_identifier", ), - ( - "identifier_pointing_to_the_object", + pytest.param( "'eresources' in subject.attributes", True, {"subject": Subject(["eresources"])}, None, None, + id="identifier_pointing_to_the_object", ), - ("simple_negation", "-9", -9, None, None, None), - ( - "simple_parenthesized_expression_negation", + pytest.param("-9", -9, None, None, None, id="simple_negation"), + pytest.param( "-(9)", -(9), None, None, None, + id="simple_parenthesized_expression_negation", ), - ( - "parenthesized_expression_negation", + pytest.param( "-(9 + 3)", -(9 + 3), None, None, None, + id="parenthesized_expression_negation", ), - ( - "slice_expression_negation", + pytest.param( "-(arr[1])", -12, {"arr": [1, 12, 3]}, None, None, + id="slice_expression_negation", + ), + pytest.param( + "9 + 3", 9 + 3, None, None, None, id="addition_with_two_operands" ), - ("addition_with_two_operands", "9 + 3", 9 + 3, None, None, None), - ("addition_with_three_operands", "9 + 3 + 3", 9 + 3 + 3, None, None, None), - ( - "addition_with_four_operands", + pytest.param( + "9 + 3 + 3", + 9 + 3 + 3, + None, + None, + None, + id="addition_with_three_operands", + ), + pytest.param( "9 + 3 + 3 + 3", 9 + 3 + 3 + 3, None, None, None, + id="addition_with_four_operands", + ), + pytest.param( + "9 - 3", 9 - 3, None, None, None, id="subtraction_with_two_operands" + ), + pytest.param( + "9 * 3", 9 * 3, None, None, None, id="multiplication_with_two_operands" + ), + pytest.param( + "9 / 3", 9 / 3, None, None, None, id="division_with_two_operands" ), - ("subtraction_with_two_operands", "9 - 3", 9 - 3, None, None, None), - ("multiplication_with_two_operands", "9 * 3", 9 * 3, None, None, None), - ("division_with_two_operands", "9 / 3", 9 / 3, None, None, None), - ( - "division_with_two_operands_and_remainder", + pytest.param( "9 / 4", 9.0 / 4.0, None, None, None, + id="division_with_two_operands_and_remainder", + ), + pytest.param( + "9 ** 3", + 9**3, + None, + None, + None, + id="exponentiation_with_two_operands", ), - ("exponentiation_with_two_operands", "9 ** 3", 9**3, None, None, None), - ( - "exponentiation_with_three_operands", + pytest.param( "2 ** 3 ** 3", 2**3**3, None, None, None, + id="exponentiation_with_three_operands", ), - ( - "associative_law_for_addition", + pytest.param( "(a + b) + c == a + (b + c)", True, {"a": 9, "b": 3, "c": 3}, None, None, + id="associative_law_for_addition", ), - ( - "associative_law_for_multiplication", + pytest.param( "(a * b) * c == a * (b * c)", True, {"a": 9, "b": 3, "c": 3}, None, None, + id="associative_law_for_multiplication", ), - ( - "commutative_law_for_addition", + pytest.param( "a + b == b + a", True, {"a": 9, "b": 3}, None, None, + id="commutative_law_for_addition", ), - ( - "commutative_law_for_multiplication", + pytest.param( "a * b == b * a", True, {"a": 9, "b": 3}, None, None, + id="commutative_law_for_multiplication", ), - ( - "distributive_law", + pytest.param( "a * (b + c) == a * b + a * c", True, {"a": 9, "b": 3, "c": 3}, None, None, + id="distributive_law", ), - ("less_comparison", "9 < 3", 9 < 3, None, None, None), - ("less_or_equal_comparison", "3 <= 3", 3 <= 3, None, None, None), - ("greater_comparison", "9 > 3", 9 > 3, None, None, None), - ("greater_or_equal_comparison", "3 >= 2", 3 >= 2, None, None, None), - ("in_operator", "3 in list", True, {"list": [1, 2, 3]}, None, None), - ("inversion", "not 9 < 3", not 9 < 3, None, None, None), - ("double_inversion", "not not 9 < 3", not not 9 < 3, None, None, None), - ( - "triple_inversion", + pytest.param("9 < 3", 9 < 3, None, None, None, id="less_comparison"), + pytest.param( + "3 <= 3", 3 <= 3, None, None, None, id="less_or_equal_comparison" + ), + pytest.param("9 > 3", 9 > 3, None, None, None, id="greater_comparison"), + pytest.param( + "3 >= 2", 3 >= 2, None, None, None, id="greater_or_equal_comparison" + ), + pytest.param( + "3 in list", True, {"list": [1, 2, 3]}, None, None, id="in_operator" + ), + pytest.param("not 9 < 3", not 9 < 3, None, None, None, id="inversion"), + pytest.param( + "not not 9 < 3", not not 9 < 3, None, None, None, id="double_inversion" + ), + pytest.param( "not not not 9 < 3", not not not 9 < 3, None, None, None, + id="triple_inversion", + ), + pytest.param( + "9 == 9 and 3 == 3", + 9 == 9 and 3 == 3, + None, + None, + None, + id="conjunction", ), - ("conjunction", "9 == 9 and 3 == 3", 9 == 9 and 3 == 3, None, None, None), - ("disjunction", "9 == 3 or 3 == 3", 9 == 3 or 3 == 3, None, None, None), - ("simple_parenthesized_expression", "(9 + 3)", (9 + 3), None, None, None), - ( - "arithmetic_parenthesized_expression", + pytest.param( + "9 == 3 or 3 == 3", 9 == 3 or 3 == 3, None, None, None, id="disjunction" + ), + pytest.param( + "(9 + 3)", + (9 + 3), + None, + None, + None, + id="simple_parenthesized_expression", + ), + pytest.param( "2 * (9 + 3) * 2", 2 * (9 + 3) * 2, None, None, None, + id="arithmetic_parenthesized_expression", ), - ("slice_expression", "arr[1] == 12", True, {"arr": [1, 12, 3]}, None, None), - ( - "complex_slice_expression", + pytest.param( + "arr[1] == 12", + True, + {"arr": [1, 12, 3]}, + None, + None, + id="slice_expression", + ), + pytest.param( "arr[1] + arr[2]", 15, {"arr": [1, 12, 3]}, None, None, + id="complex_slice_expression", ), - ( - "method_call", + pytest.param( "string.upper()", "HELLO WORLD", {"string": "Hello World"}, None, None, + id="method_call", + ), + pytest.param( + "min(1, 2)", min(1, 2), None, None, None, id="builtin_function_call" ), - ("builtin_function_call", "min(1, 2)", min(1, 2), None, None, None), - ( - "unsafe_class_method_call", + pytest.param( "subject.get_attribute_value(0)", "eresources", {"subject": Subject(["eresources"])}, None, DSLEvaluationError, + id="unsafe_class_method_call", ), - ( - "safe_class_method_call", + pytest.param( "subject.get_attribute_value(0)", "eresources", {"subject": Subject(["eresources"])}, [Subject], None, + id="safe_class_method_call", ), - ( - "safe_class_method_call_with_direct_context", + pytest.param( "get_attribute_value(0)", "eresources", Subject(["eresources"]), [Subject], None, + id="safe_class_method_call_with_direct_context", ), ], ) def test( self, - _, expression, expected_result, context, diff --git a/tests/manager/api/saml/python_expression_dsl/test_parser.py b/tests/manager/api/saml/python_expression_dsl/test_parser.py index c3edd9faa1..2dcdc9c867 100644 --- a/tests/manager/api/saml/python_expression_dsl/test_parser.py +++ b/tests/manager/api/saml/python_expression_dsl/test_parser.py @@ -8,15 +8,23 @@ class TestDSLParser: @pytest.mark.parametrize( - "_,expression,expected_error_message", + "expression,expected_error_message", [ - ("incorrect_expression", "?", "Unexpected symbol '?' at position 0"), - ("incorrect_expression_2", "(+", "Unexpected symbol '+' at position 1"), - ("incorrect_expression_2", "(1 +", "Unexpected symbol '+' at position 3"), + pytest.param( + "?", "Unexpected symbol '?' at position 0", id="incorrect_expression" + ), + pytest.param( + "(+", "Unexpected symbol '+' at position 1", id="incorrect_expression_2" + ), + pytest.param( + "(1 +", + "Unexpected symbol '+' at position 3", + id="incorrect_expression_3", + ), ], ) def test_parse_generates_correct_error_message( - self, _, expression, expected_error_message + self, expression, expected_error_message ): # Arrange parser = DSLParser() diff --git a/tests/manager/api/saml/test_auth.py b/tests/manager/api/saml/test_auth.py index a71ec325fd..5fa5f7d6e0 100644 --- a/tests/manager/api/saml/test_auth.py +++ b/tests/manager/api/saml/test_auth.py @@ -83,17 +83,17 @@ class TestSAMLAuthenticationManager: @pytest.mark.parametrize( - "_, service_provider, identity_providers", + "service_provider, identity_providers", [ - ( - "with_unsigned_authentication_request", + pytest.param( SERVICE_PROVIDER_WITH_UNSIGNED_REQUESTS, IDENTITY_PROVIDERS, + id="with_unsigned_authentication_request", ), - ( - "with_signed_authentication_request", + pytest.param( SERVICE_PROVIDER_WITH_SIGNED_REQUESTS, IDENTITY_PROVIDERS, + id="with_signed_authentication_request", ), ], ) @@ -101,7 +101,6 @@ def test_start_authentication( self, controller_fixture: ControllerFixture, create_mock_onelogin_configuration: Callable[..., SAMLOneLoginConfiguration], - _, service_provider, identity_providers, ): @@ -162,10 +161,9 @@ def test_start_authentication( ) @pytest.mark.parametrize( - "_, saml_response, current_time, filter_expression, expected_value, mock_validation", + "saml_response, current_time, filter_expression, expected_value, mock_validation", [ - ( - "with_name_id_and_attributes", + pytest.param( saml_strings.SAML_RESPONSE, datetime_utc(2020, 6, 7, 23, 43, 0), None, @@ -189,17 +187,17 @@ def test_start_authentication( ), ), False, + id="with_name_id_and_attributes", ), - ( - "with_name_id_attributes_and_filter_expression_returning_false", + pytest.param( saml_strings.SAML_RESPONSE, datetime_utc(2020, 6, 7, 23, 43, 0), "subject.attribute_statement.attributes['uid'].values[0] != 'student1'", SAML_NO_ACCESS_ERROR, False, + id="with_name_id_attributes_and_filter_expression_returning_false", ), - ( - "with_name_id_attributes_and_filter_expression_returning_true", + pytest.param( saml_strings.SAML_RESPONSE, datetime_utc(2020, 6, 7, 23, 43, 0), "subject.attribute_statement.attributes['uid'].values[0] == 'student1'", @@ -223,9 +221,9 @@ def test_start_authentication( ), ), False, + id="with_name_id_attributes_and_filter_expression_returning_true", ), - ( - "with_name_id_inside_edu_person_targeted_id_attribute", + pytest.param( saml_strings.SAML_COLUMBIA_RESPONSE, datetime_utc(2020, 6, 7, 23, 43, 0), None, @@ -254,17 +252,17 @@ def test_start_authentication( ), ), True, + id="with_name_id_inside_edu_person_targeted_id_attribute", ), - ( - "with_name_id_inside_edu_person_targeted_id_attribute_and_filter_expression_returning_false", + pytest.param( saml_strings.SAML_COLUMBIA_RESPONSE, datetime_utc(2020, 6, 7, 23, 43, 0), "subject.attribute_statement.attributes['eduPersonScopedAffiliation'].values[0] != 'alum@columbia.edu'", SAML_NO_ACCESS_ERROR, True, + id="with_name_id_inside_edu_person_targeted_id_attribute_and_filter_expression_returning_false", ), - ( - "with_name_id_inside_edu_person_targeted_id_attribute_and_filter_expression_returning_true", + pytest.param( saml_strings.SAML_COLUMBIA_RESPONSE, datetime_utc(2020, 6, 7, 23, 43, 0), "subject.attribute_statement.attributes['eduPersonScopedAffiliation'].values[0] == 'alum@columbia.edu'", @@ -293,6 +291,7 @@ def test_start_authentication( ), ), True, + id="with_name_id_inside_edu_person_targeted_id_attribute_and_filter_expression_returning_true", ), ], ) @@ -301,7 +300,6 @@ def test_finish_authentication( controller_fixture: ControllerFixture, create_saml_configuration, create_mock_onelogin_configuration: Callable[..., SAMLOneLoginConfiguration], - _, saml_response, current_time, filter_expression, diff --git a/tests/manager/api/saml/test_controller.py b/tests/manager/api/saml/test_controller.py index 37c4b2b79b..8e9bd22f02 100644 --- a/tests/manager/api/saml/test_controller.py +++ b/tests/manager/api/saml/test_controller.py @@ -71,10 +71,9 @@ def create_patron_data_mock(): class TestSAMLController: @pytest.mark.parametrize( - "_, provider_name, idp_entity_id, redirect_uri, expected_problem, expected_relay_state", + "provider_name, idp_entity_id, redirect_uri, expected_problem, expected_relay_state", [ - ( - "with_missing_provider_name", + pytest.param( None, None, None, @@ -84,9 +83,9 @@ class TestSAMLController: ) ), None, + id="with_missing_provider_name", ), - ( - "with_missing_idp_entity_id", + pytest.param( SAMLWebSSOAuthenticationProvider.label(), None, None, @@ -96,9 +95,9 @@ class TestSAMLController: ) ), None, + id="with_missing_idp_entity_id", ), - ( - "with_missing_redirect_uri", + pytest.param( SAMLWebSSOAuthenticationProvider.label(), IDENTITY_PROVIDERS[0].entity_id, None, @@ -115,9 +114,9 @@ class TestSAMLController: SAMLController.IDP_ENTITY_ID: IDENTITY_PROVIDERS[0].entity_id, } ), + id="with_missing_redirect_uri", ), - ( - "with_all_parameters_set", + pytest.param( SAMLWebSSOAuthenticationProvider.label(), IDENTITY_PROVIDERS[0].entity_id, "http://localhost", @@ -130,9 +129,9 @@ class TestSAMLController: SAMLController.IDP_ENTITY_ID: IDENTITY_PROVIDERS[0].entity_id, } ), + id="with_all_parameters_set", ), - ( - "with_all_parameters_set_and_fragment", + pytest.param( SAMLWebSSOAuthenticationProvider.label(), IDENTITY_PROVIDERS[0].entity_id, "http://localhost#fragment", @@ -146,9 +145,9 @@ class TestSAMLController: } ) + "#fragment", + id="with_all_parameters_set_and_fragment", ), - ( - "with_all_parameters_set_and_redirect_uri_containing_other_parameters", + pytest.param( SAMLWebSSOAuthenticationProvider.label(), IDENTITY_PROVIDERS[0].entity_id, "http://localhost?patron_info=%7B%7D&access_token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9", @@ -163,13 +162,13 @@ class TestSAMLController: "access_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9", } ), + id="with_all_parameters_set_and_redirect_uri_containing_other_parameters", ), ], ) def test_saml_authentication_redirect( self, controller_fixture: ControllerFixture, - _, provider_name, idp_entity_id, redirect_uri, @@ -253,10 +252,9 @@ def test_saml_authentication_redirect( ) @pytest.mark.parametrize( - "_, data, finish_authentication_result, saml_callback_result, bearer_token, expected_authentication_redirect_uri, expected_problem,", + "data, finish_authentication_result, saml_callback_result, bearer_token, expected_authentication_redirect_uri, expected_problem,", [ - ( - "with_missing_relay_state", + pytest.param( None, None, None, @@ -267,9 +265,9 @@ def test_saml_authentication_redirect( SAMLController.RELAY_STATE ) ), + id="with_missing_relay_state", ), - ( - "with_incorrect_relay_state", + pytest.param( {SAMLController.RELAY_STATE: "<>"}, None, None, @@ -280,9 +278,9 @@ def test_saml_authentication_redirect( SAMLController.LIBRARY_SHORT_NAME ) ), + id="with_incorrect_relay_state", ), - ( - "with_missing_provider_name", + pytest.param( { SAMLController.RELAY_STATE: "http://localhost?" + urlencode({SAMLController.LIBRARY_SHORT_NAME: "default"}) @@ -296,9 +294,9 @@ def test_saml_authentication_redirect( SAMLController.PROVIDER_NAME ) ), + id="with_missing_provider_name", ), - ( - "with_missing_idp_entity_id", + pytest.param( { SAMLController.RELAY_STATE: "http://localhost?" + urlencode( @@ -317,9 +315,9 @@ def test_saml_authentication_redirect( SAMLController.IDP_ENTITY_ID ) ), + id="with_missing_idp_entity_id", ), - ( - "when_finish_authentication_fails", + pytest.param( { SAMLController.RELAY_STATE: "http://localhost?" + urlencode( @@ -337,9 +335,9 @@ def test_saml_authentication_redirect( None, None, None, + id="when_finish_authentication_fails", ), - ( - "when_saml_callback_fails", + pytest.param( { SAMLController.RELAY_STATE: "http://localhost?" + urlencode( @@ -357,9 +355,9 @@ def test_saml_authentication_redirect( None, None, None, + id="when_saml_callback_fails", ), - ( - "when_saml_callback_returns_correct_patron", + pytest.param( { SAMLController.RELAY_STATE: "http://localhost?" + urlencode( @@ -377,13 +375,13 @@ def test_saml_authentication_redirect( "ABCDEFG", "http://localhost?access_token=ABCDEFG&patron_info=%22%22", None, + id="when_saml_callback_returns_correct_patron", ), ], ) def test_saml_authentication_callback( self, controller_fixture: ControllerFixture, - _, data, finish_authentication_result, saml_callback_result, diff --git a/tests/manager/api/saml/test_provider.py b/tests/manager/api/saml/test_provider.py index 18b743ef89..ad5322c430 100644 --- a/tests/manager/api/saml/test_provider.py +++ b/tests/manager/api/saml/test_provider.py @@ -116,10 +116,9 @@ class TestSAMLWebSSOAuthenticationProvider: @pytest.mark.parametrize( - "_, identity_providers, expected_result", + "identity_providers, expected_result", [ - ( - "identity_provider_with_display_name", + pytest.param( [IDENTITY_PROVIDER_WITH_DISPLAY_NAME], { "type": "http://librarysimplified.org/authtype/SAML-2.0", @@ -181,9 +180,9 @@ class TestSAMLWebSSOAuthenticationProvider: } ], }, + id="identity_provider_with_display_name", ), - ( - "identity_provider_with_organization_display_name", + pytest.param( [IDENTITY_PROVIDER_WITH_ORGANIZATION_DISPLAY_NAME], { "type": "http://librarysimplified.org/authtype/SAML-2.0", @@ -209,9 +208,9 @@ class TestSAMLWebSSOAuthenticationProvider: } ], }, + id="identity_provider_with_organization_display_name", ), - ( - "identity_provider_without_display_names_and_default_template", + pytest.param( [ IDENTITY_PROVIDER_WITHOUT_DISPLAY_NAMES, IDENTITY_PROVIDER_WITHOUT_DISPLAY_NAMES, @@ -250,6 +249,7 @@ class TestSAMLWebSSOAuthenticationProvider: }, ], }, + id="identity_provider_without_display_names_and_default_template", ), ], ) @@ -259,7 +259,6 @@ def test_authentication_document( create_saml_configuration: Callable[..., SAMLWebSSOAuthSettings], create_saml_provider: Callable[..., SAMLWebSSOAuthenticationProvider], create_mock_onelogin_configuration: Callable[..., SAMLOneLoginConfiguration], - _, identity_providers, expected_result, ): @@ -311,34 +310,33 @@ def test_authentication_document( assert expected_result == result @pytest.mark.parametrize( - "_, subject, expected_result, patron_id_use_name_id, patron_id_attributes, patron_id_regular_expression", + "subject, expected_result, patron_id_use_name_id, patron_id_attributes, patron_id_regular_expression", [ - ( - "empty_subject", + pytest.param( None, SAML_INVALID_SUBJECT.detailed("Subject is empty"), None, None, None, + id="empty_subject", ), - ( - "subject_is_patron_data", + pytest.param( PatronData(permanent_id=12345), PatronData(permanent_id=12345), None, None, None, + id="subject_is_patron_data", ), - ( - "subject_does_not_have_unique_id", + pytest.param( SAMLSubject("http://idp.example.com", None, None), SAML_INVALID_SUBJECT.detailed("Subject does not have a unique ID"), None, None, None, + id="subject_does_not_have_unique_id", ), - ( - "subject_has_unique_id", + pytest.param( SAMLSubject( "http://idp.example.com", None, @@ -360,9 +358,9 @@ def test_authentication_document( None, None, None, + id="subject_has_unique_id", ), - ( - "subject_has_unique_name_id_but_use_of_name_id_is_switched_off_using_string_literal", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "12345"), @@ -372,9 +370,9 @@ def test_authentication_document( "false", None, None, + id="subject_has_unique_name_id_but_use_of_name_id_is_switched_off_using_string_literal", ), - ( - "subject_has_unique_name_id_and_use_of_name_id_is_switched_on_using_string_literal_true", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID(SAMLNameIDFormat.UNSPECIFIED.value, "", "", "12345"), @@ -389,9 +387,9 @@ def test_authentication_document( "true", None, None, + id="subject_has_unique_name_id_and_use_of_name_id_is_switched_on_using_string_literal_true", ), - ( - "subject_has_unique_id_matching_the_regular_expression", + pytest.param( SAMLSubject( "http://idp.example.com", None, @@ -413,9 +411,9 @@ def test_authentication_document( False, [SAMLAttributeType.eduPersonPrincipalName.name], saml_strings.PATRON_ID_REGULAR_EXPRESSION_ORG, + id="subject_has_unique_id_matching_the_regular_expression", ), - ( - "subject_has_unique_id_not_matching_the_regular_expression", + pytest.param( SAMLSubject( "http://idp.example.com", None, @@ -432,6 +430,7 @@ def test_authentication_document( False, [SAMLAttributeType.eduPersonPrincipalName.name], saml_strings.PATRON_ID_REGULAR_EXPRESSION_ORG, + id="subject_has_unique_id_not_matching_the_regular_expression", ), ], ) @@ -439,7 +438,6 @@ def test_remote_patron_lookup( self, create_saml_configuration: Callable[..., SAMLWebSSOAuthSettings], create_saml_provider: Callable[..., SAMLWebSSOAuthenticationProvider], - _, subject, expected_result, patron_id_use_name_id, @@ -467,26 +465,25 @@ def test_remote_patron_lookup( assert result == expected_result @pytest.mark.parametrize( - "_, subject, expected_patron_data, expected_credential, expected_expiration_time, cm_session_lifetime", + "subject, expected_patron_data, expected_credential, expected_expiration_time, cm_session_lifetime", [ - ( - "empty_subject", + pytest.param( None, SAML_INVALID_SUBJECT.detailed("Subject is empty"), None, None, None, + id="empty_subject", ), - ( - "subject_does_not_have_unique_id", + pytest.param( SAMLSubject("http://idp.example.com", None, None), SAML_INVALID_SUBJECT.detailed("Subject does not have a unique ID"), None, None, None, + id="subject_does_not_have_unique_id", ), - ( - "subject_has_unique_id", + pytest.param( SAMLSubject( "http://idp.example.com", None, @@ -508,9 +505,9 @@ def test_remote_patron_lookup( None, None, None, + id="subject_has_unique_id", ), - ( - "subject_has_unique_id_and_persistent_name_id", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID( @@ -537,9 +534,9 @@ def test_remote_patron_lookup( None, None, None, + id="subject_has_unique_id_and_persistent_name_id", ), - ( - "subject_has_unique_id_and_transient_name_id", + pytest.param( SAMLSubject( "http://idp.example.com", SAMLNameID( @@ -566,9 +563,9 @@ def test_remote_patron_lookup( '{"idp": "http://idp.example.com", "attributes": {"eduPersonUniqueId": ["12345"]}}', None, None, + id="subject_has_unique_id_and_transient_name_id", ), - ( - "subject_has_unique_id_and_custom_session_lifetime", + pytest.param( SAMLSubject( "http://idp.example.com", None, @@ -590,9 +587,9 @@ def test_remote_patron_lookup( None, datetime_utc(2020, 1, 1) + datetime.timedelta(days=42), 42, + id="subject_has_unique_id_and_custom_session_lifetime", ), - ( - "subject_has_unique_id_and_empty_session_lifetime", + pytest.param( SAMLSubject( "http://idp.example.com", None, @@ -614,9 +611,9 @@ def test_remote_patron_lookup( None, None, "", + id="subject_has_unique_id_and_empty_session_lifetime", ), - ( - "subject_has_unique_id_and_non_default_expiration_timeout", + pytest.param( SAMLSubject( "http://idp.example.com", None, @@ -639,9 +636,9 @@ def test_remote_patron_lookup( None, None, None, + id="subject_has_unique_id_and_non_default_expiration_timeout", ), - ( - "subject_has_unique_id_non_default_expiration_timeout_and_custom_session_lifetime", + pytest.param( SAMLSubject( "http://idp.example.com", None, @@ -664,9 +661,9 @@ def test_remote_patron_lookup( None, datetime_utc(2020, 1, 1) + datetime.timedelta(days=42), 42, + id="subject_has_unique_id_non_default_expiration_timeout_and_custom_session_lifetime", ), - ( - "subject_has_unique_id_non_default_expiration_timeout_and_empty_session_lifetime", + pytest.param( SAMLSubject( "http://idp.example.com", None, @@ -689,6 +686,7 @@ def test_remote_patron_lookup( None, None, "", + id="subject_has_unique_id_non_default_expiration_timeout_and_empty_session_lifetime", ), ], ) @@ -698,7 +696,6 @@ def test_saml_callback( controller_fixture: ControllerFixture, create_saml_configuration: Callable[..., SAMLWebSSOAuthSettings], create_saml_provider: Callable[..., SAMLWebSSOAuthenticationProvider], - _, subject, expected_patron_data, expected_credential, diff --git a/tests/manager/sqlalchemy/model/test_identifier.py b/tests/manager/sqlalchemy/model/test_identifier.py index 3d1b6c4767..c0eb9ed13e 100644 --- a/tests/manager/sqlalchemy/model/test_identifier.py +++ b/tests/manager/sqlalchemy/model/test_identifier.py @@ -632,23 +632,39 @@ def test_missing_coverage_from_with_cutoff_date( ).all() @pytest.mark.parametrize( - "_,identifier_type,identifier,title", + "identifier_type,identifier,title", [ - ("ascii_type_ascii_identifier_no_title", "a", "a", None), - ("ascii_type_non_ascii_identifier_no_title", "a", "ą", None), - ("non_ascii_type_ascii_identifier_no_title", "ą", "a", None), - ("non_ascii_type_non_ascii_identifier_no_title", "ą", "ą", None), - ("ascii_type_ascii_identifier_ascii_title", "a", "a", "a"), - ("ascii_type_non_ascii_identifier_ascii_title", "a", "ą", "a"), - ("non_ascii_type_ascii_identifier_ascii_title", "ą", "a", "a"), - ("non_ascii_type_non_ascii_identifier_ascii_title", "ą", "ą", "a"), - ("ascii_type_ascii_identifier_non_ascii_title", "a", "a", "ą"), - ("ascii_type_non_ascii_identifier_non_ascii_title", "a", "ą", "ą"), - ("non_ascii_type_ascii_identifier_non_ascii_title", "ą", "a", "ą"), - ("non_ascii_type_non_ascii_identifier_non_ascii_title", "ą", "ą", "ą"), + pytest.param("a", "a", None, id="ascii_type_ascii_identifier_no_title"), + pytest.param("a", "ą", None, id="ascii_type_non_ascii_identifier_no_title"), + pytest.param("ą", "a", None, id="non_ascii_type_ascii_identifier_no_title"), + pytest.param( + "ą", "ą", None, id="non_ascii_type_non_ascii_identifier_no_title" + ), + pytest.param("a", "a", "a", id="ascii_type_ascii_identifier_ascii_title"), + pytest.param( + "a", "ą", "a", id="ascii_type_non_ascii_identifier_ascii_title" + ), + pytest.param( + "ą", "a", "a", id="non_ascii_type_ascii_identifier_ascii_title" + ), + pytest.param( + "ą", "ą", "a", id="non_ascii_type_non_ascii_identifier_ascii_title" + ), + pytest.param( + "a", "a", "ą", id="ascii_type_ascii_identifier_non_ascii_title" + ), + pytest.param( + "a", "ą", "ą", id="ascii_type_non_ascii_identifier_non_ascii_title" + ), + pytest.param( + "ą", "a", "ą", id="non_ascii_type_ascii_identifier_non_ascii_title" + ), + pytest.param( + "ą", "ą", "ą", id="non_ascii_type_non_ascii_identifier_non_ascii_title" + ), ], ) - def test_repr(self, _, identifier_type, identifier, title): + def test_repr(self, identifier_type, identifier, title): """Test that Identifier.__repr__ correctly works with both ASCII and non-ASCII symbols. :param _: Name of the test case @@ -752,21 +768,21 @@ def test_identifier_delete_cascade_parent( class TestProQuestIdentifierParser: @pytest.mark.parametrize( - "_,identifier_string,expected_result", + "identifier_string,expected_result", [ - ( - "incorrect_identifier", + pytest.param( "urn:librarysimplified.org/terms/id/Overdrive%20ID/adfcc11a-cc5b-4c82-8048-e005e4a90222", None, + id="incorrect_identifier", ), - ( - "correct_identifier", + pytest.param( "urn:proquest.com/document-id/12345", (Identifier.PROQUEST_ID, "12345"), + id="correct_identifier", ), ], ) - def test_parse(self, _, identifier_string, expected_result): + def test_parse(self, identifier_string, expected_result): parser = ProQuestIdentifierParser() result = parser.parse(identifier_string) assert expected_result == result diff --git a/tests/manager/sqlalchemy/model/test_licensing.py b/tests/manager/sqlalchemy/model/test_licensing.py index 05f149ad65..50fb2c07f7 100644 --- a/tests/manager/sqlalchemy/model/test_licensing.py +++ b/tests/manager/sqlalchemy/model/test_licensing.py @@ -1457,10 +1457,13 @@ def compatible_with(cls, other, open_access): assert (mech2.delivery_mechanism, True) == Mock.called_with @pytest.mark.parametrize( - "_,data_source,identifier,delivery_mechanism", - [("ascii_sy", "a", "a", "a"), ("", "ą", "ą", "ą")], + "data_source,identifier,delivery_mechanism", + [ + pytest.param("a", "a", "a", id="ascii_sy"), + pytest.param("ą", "ą", "ą", id=""), + ], ) - def test_repr(self, _, data_source, identifier, delivery_mechanism): + def test_repr(self, data_source, identifier, delivery_mechanism): """Test that LicensePoolDeliveryMechanism.__repr__ correctly works for both ASCII and non-ASCII symbols. :param _: Name of the test case diff --git a/tests/manager/util/test_opds_writer.py b/tests/manager/util/test_opds_writer.py index 4c11af8a2a..300f117d33 100644 --- a/tests/manager/util/test_opds_writer.py +++ b/tests/manager/util/test_opds_writer.py @@ -54,32 +54,36 @@ def test_add_link_to_entry(self): ) in etree.tostring(entry, method="c14n2") @pytest.mark.parametrize( - "_,obj,formatted", + "obj,formatted", [ - ("date", datetime.date(2020, 1, 2), "2020-01-02T00:00:00Z"), - ("naive", datetime.datetime(2020, 1, 2, 3, 4, 5), "2020-01-02T03:04:05Z"), - ( - "explicit_utc", + pytest.param(datetime.date(2020, 1, 2), "2020-01-02T00:00:00Z", id="date"), + pytest.param( + datetime.datetime(2020, 1, 2, 3, 4, 5), + "2020-01-02T03:04:05Z", + id="naive", + ), + pytest.param( datetime.datetime(2020, 1, 2, 3, 4, 5, tzinfo=pytz.UTC), "2020-01-02T03:04:05+00:00", + id="explicit_utc", ), - ( - "eastern", + pytest.param( pytz.timezone("US/Eastern").localize( datetime.datetime(2020, 1, 2, 3, 4, 5) ), "2020-01-02T08:04:05+00:00", + id="eastern", ), - ( - "central", + pytest.param( pytz.timezone("US/Central").localize( datetime.datetime(2020, 1, 2, 3, 4, 5) ), "2020-01-02T09:04:05+00:00", + id="central", ), ], ) - def test__strftime(self, _, obj, formatted): + def test__strftime(self, obj, formatted): # Verify that dates and datetimes are formatted according to # the rules laid down in the Atom spec. assert AtomFeed._strftime(obj) == formatted