Merge pull request #43 from adobe-apiplatform/v2

Fix #41 (again!).  Prepare for v2.4.1 bug fix release.

Author: adobeDan

HISTORY.md

@@ -70,3 +70,11 @@ Bug fix release:
 * [Issue 41](https://github.com/adobe-apiplatform/umapi-client.py/issues/41)
     * accept unicode strings from Python 2.7 clients
     * do unicode-compliant validation of usernames and email addresses
+
+### Version 2.4.1
+
+Bug fix release:
+
+* [Issue 41](https://github.com/adobe-apiplatform/umapi-client.py/issues/41)
+    * original fix had an overly accepting email/username validator
+    * there were formatting errors in the failed validation reports

tests/test_functional.py

@@ -39,32 +39,27 @@ def test_user_adobeid():
 
 
 def test_user_adobeid_unicode():
-    user = UserAction(email=u"lwałęsa@adobe.com")
+    user = UserAction(email=u"lwalesa@adobe.com")
     assert user.wire_dict() == {"do": [],
-                                "user": u"lwałęsa@adobe.com",
+                                "user": u"lwalesa@adobe.com",
                                 "useAdobeID": True}
 
 
-def test_user_adobeid_unicode_error_trailing_dot():
+def test_user_adobeid_unicode_error_unicode_dot_above():
     with pytest.raises(ValueError):
-        UserAction(email=u"lwałęsa.@adobe.com")
+        UserAction(email=u"lwałę[email protected]")
 
 
-def test_user_adobeid_unicode_error_unicode_dot_above():
+def test_user_adobeid_unicode_error_trailing_dot():
     with pytest.raises(ValueError):
-        UserAction(email=u"l˙wałęsa@adobe.com")
+        UserAction(email=u"lwalesa.@adobe.com")
 
 
 def test_user_enterpriseid():
     user = UserAction(id_type=IdentityTypes.enterpriseID, email="[email protected]")
     assert user.wire_dict() == {"do": [], "user": "[email protected]"}
 
 
-def test_user_enterpriseid_unicode():
-    user = UserAction(id_type=IdentityTypes.enterpriseID, email=u"lwałę[email protected]")
-    assert user.wire_dict() == {"do": [], "user": u"lwałę[email protected]"}
-
-
 def test_user_enterpriseid_username():
     with pytest.raises(ValueError):
         UserAction(id_type=IdentityTypes.enterpriseID, username="dbrotsky", domain="o.on-the-side.net")
@@ -75,19 +70,14 @@ def test_user_federatedid():
     assert user.wire_dict() == {"do": [], "user": "[email protected]"}
 
 
-def test_user_federatedid_unicode():
-    user = UserAction(id_type=IdentityTypes.federatedID, email=u"lwałę[email protected]")
-    assert user.wire_dict() == {"do": [], "user": u"lwałę[email protected]"}
-
-
 def test_user_federatedid_username():
     user = UserAction(id_type=IdentityTypes.federatedID, username="dbrotsky", domain="k.on-the-side.net")
     assert user.wire_dict() == {"do": [], "user": "dbrotsky", "domain": "k.on-the-side.net"}
 
 
-def test_user_federatedid_username_unicode():
-    user = UserAction(id_type=IdentityTypes.federatedID, username=u"lwałęsa", domain="k.on-the-side.net")
-    assert user.wire_dict() == {"do": [], "user": u"lwałęsa", "domain": "k.on-the-side.net"}
+def test_user_federatedid_username_unicode_error():
+    with pytest.raises(ValueError):
+        UserAction(id_type=IdentityTypes.federatedID, username=u"lwałęsa", domain="k.on-the-side.net")
 
 
 def test_create_user_adobeid():
@@ -126,13 +116,13 @@ def test_create_user_federatedid():
 
 
 def test_create_user_federated_id_unicode():
-    user = UserAction(id_type=IdentityTypes.federatedID, email=u"lwałęsa@k.on-the-side.net")
+    user = UserAction(id_type=IdentityTypes.federatedID, email=u"lwalesa@k.on-the-side.net")
     user.create(first_name="Lech", last_name=u"Wałęsa", country="PL")
-    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": u"lwałęsa@k.on-the-side.net",
+    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": u"lwalesa@k.on-the-side.net",
                                                               "firstname": "Lech", "lastname": u"Wałęsa",
                                                               "country": "PL",
                                                               "option": "ignoreIfAlreadyExists"}}],
-                                "user": u"lwałęsa@k.on-the-side.net"}
+                                "user": u"lwalesa@k.on-the-side.net"}
 
 
 def test_create_user_federatedid_username():
@@ -146,13 +136,13 @@ def test_create_user_federatedid_username():
 
 
 def test_create_user_federatedid_username_unicode():
-    user = UserAction(id_type=IdentityTypes.federatedID, username=u"lwałęsa", domain="k.on-the-side.net")
-    user.create(first_name="Lech", last_name=u"Wałęsa", country="PL", email=u"lwałęsa@k.on-the-side.net")
-    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": u"lwałęsa@k.on-the-side.net",
+    user = UserAction(id_type=IdentityTypes.federatedID, username=u"lwalesa", domain="k.on-the-side.net")
+    user.create(first_name="Lech", last_name=u"Wałęsa", country="PL", email=u"lwalesa@k.on-the-side.net")
+    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": u"lwalesa@k.on-the-side.net",
                                                               "firstname": "Lech", "lastname": u"Wałęsa",
                                                               "country": "PL",
                                                               "option": "ignoreIfAlreadyExists"}}],
-                                "user": u"lwałęsa", "domain": "k.on-the-side.net"}
+                                "user": u"lwalesa", "domain": "k.on-the-side.net"}
 
 
 def test_create_user_federatedid_username_email():
@@ -310,14 +300,6 @@ def test_remove_from_organization_adobeid():
                                 "useAdobeID": True}
 
 
-def test_remove_from_organization_adobeid_unicode():
-    user = UserAction(id_type='adobeID', email=u"lwałę[email protected]")
-    user.remove_from_organization()
-    assert user.wire_dict() == {"do": [{"removeFromOrg": {"deleteAccount": False}}],
-                                "user": u"lwałę[email protected]",
-                                "useAdobeID": True}
-
-
 def test_remove_from_organization_delete_federatedid():
     user = UserAction(id_type=IdentityTypes.federatedID, email="[email protected]")
     user.remove_from_organization(delete_account=True)

umapi_client/functional.py

@@ -55,7 +55,12 @@ class UserAction(Action):
     """
 
     # regex patterns to be compiled once and used in _validate
-    _atext_pattern = r"(\w|[!#$%&'*+/=?^_`{|}~;-])+"
+    # The RFC6531 extended syntax for email addresses allows Unicode alphanumerics in the local part,
+    # but the Adobe identity system doesn't support that extended syntax for user account emails.
+    # This is the RFC-allowed pattern:
+    # _atext_pattern = r"(\w|[!#$%&'*+/=?^_`{|}~;-])+"
+    # This is the one allowed by Adobe:
+    _atext_pattern = r"[a-zA-Z0-9!#$%&'*+/=?^_`{|}~-]+"
     _local_pattern = _atext_pattern + r"([.]" + _atext_pattern + ")*"
     _dns_pattern = r"[a-zA-Z0-9-]+([.][a-zA-Z0-9-]+)+"
     _email_regex = re.compile(r"\A" + _local_pattern + r"@" + _dns_pattern + r"\Z", re.UNICODE)
@@ -72,10 +77,13 @@ def _validate(cls, email=None, username=None, domain=None):
         :param domain: a domain
         '''
         if email and not cls._email_regex.match(email):
-            raise ValueError("'%s': Illegal email format (must not be quoted or contain comments)" % (email,))
+            email = email.encode('utf-8')
+            raise ValueError("'%s': Illegal email format (must be ascii, unquoted, with no comment part)" % (email,))
         if domain and not cls._domain_regex.match(domain):
+            domain = domain.encode('utf-8')
             raise ValueError("'%s': Illegal domain format" % (domain,))
         if username and not cls._username_regex.match(username):
+            username = username.encode('utf-8')
             raise ValueError("'%s': Illegal username format (must be unquoted email local part)" % (username,))
 
     def __init__(self, id_type=IdentityTypes.adobeID, email=None, username=None, domain=None, **kwargs):

umapi_client/version.py

@@ -18,4 +18,4 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-__version__ = "2.4"
+__version__ = "2.4.1"