Merge pull request #68 from adorton-adobe/feature/email-username

adorton-adobe · web-flow · commit 1c446d79643c · 2019-01-10T03:24:40.000Z
Update username/email/domain validation
diff --git a/tests/test_functional.py b/tests/test_functional.py
@@ -51,23 +51,6 @@ def test_user_adobeid_unicode():
                                 "useAdobeID": True}
 
 
-def test_user_adobeid_unicode_error_unicode_dot_above():
-    with pytest.raises(ValueError) as excinfo:
-        UserAction(email=u"lwałęsa@adobe.com")
-    assert excinfo.type == ArgumentError
-    if six.PY2:
-        assert excinfo.match(u"lwałęsa@adobe.com".encode('utf8'))
-        with pytest.raises(ValueError) as excinfo:
-            UserAction(email=u"lwałęsa@adobe.com".encode('utf8'))
-        assert excinfo.type == ArgumentError
-        assert excinfo.match(u"lwałęsa@adobe.com".encode('utf8'))
-
-
-def test_user_adobeid_unicode_error_trailing_dot():
-    with pytest.raises(ValueError):
-        UserAction(email=u"lwalesa.@adobe.com")
-
-
 def test_user_enterpriseid():
     user = UserAction(id_type=IdentityTypes.enterpriseID, email="dbrotsky@o.on-the-side.net")
     assert user.wire_dict() == {"do": [], "user": "dbrotsky@o.on-the-side.net"}
@@ -88,11 +71,6 @@ def test_user_federatedid_username():
     assert user.wire_dict() == {"do": [], "user": "dbrotsky", "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():
     user = UserAction(email="dbrotsky@adobe.com")
     user.create()
@@ -122,43 +100,55 @@ def test_create_user_enterpriseid():
 
 
 def test_create_user_federatedid():
-    user = UserAction(id_type=IdentityTypes.federatedID, email="dbrotsky@k.on-the-side.net")
-    user.create(first_name="Daniel", last_name="Brotsky", country="US")
-    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": "dbrotsky@k.on-the-side.net",
-                                                              "firstname": "Daniel", "lastname": "Brotsky",
+    """
+    Test federated ID action with email address and no username
+    """
+    user = UserAction(id_type=IdentityTypes.federatedID, email="user@example.com")
+    user.create(first_name="Example", last_name="User", country="US")
+    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": "user@example.com",
+                                                              "firstname": "Example", "lastname": "User",
                                                               "country": "US",
                                                               "option": "ignoreIfAlreadyExists"}}],
-                                "user": "dbrotsky@k.on-the-side.net"}
+                                "user": "user@example.com"}
 
 
-def test_create_user_federated_id_unicode():
-    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"lwalesa@k.on-the-side.net",
-                                                              "firstname": "Lech", "lastname": u"Wałęsa",
+def test_create_user_federatedid_unicode():
+    """
+    Test federated ID action with email address and no username and unicode in at least one attribute
+    """
+    user = UserAction(id_type=IdentityTypes.federatedID, email=u"user@example.com")
+    user.create(first_name=u"Exampłę", last_name="User", country="PL")
+    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": u"user@example.com",
+                                                              "firstname": u"Exampłę", "lastname": "User",
                                                               "country": "PL",
                                                               "option": "ignoreIfAlreadyExists"}}],
-                                "user": u"lwalesa@k.on-the-side.net"}
+                                "user": u"user@example.com"}
 
 
 def test_create_user_federatedid_username():
-    user = UserAction(id_type=IdentityTypes.federatedID, username="dbrotsky", domain="k.on-the-side.net")
-    user.create(first_name="Daniel", last_name="Brotsky", country="US", email="dbrotsky@k.on-the-side.net")
-    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": "dbrotsky@k.on-the-side.net",
-                                                              "firstname": "Daniel", "lastname": "Brotsky",
+    """
+    Test federated ID with a username (non-email format)
+    """
+    user = UserAction(id_type=IdentityTypes.federatedID, username="user", domain="example.com")
+    user.create(first_name="Example", last_name="User", country="US", email="user@example.com")
+    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": "user@example.com",
+                                                              "firstname": "Example", "lastname": "User",
                                                               "country": "US",
                                                               "option": "ignoreIfAlreadyExists"}}],
-                                "user": "dbrotsky", "domain": "k.on-the-side.net"}
+                                "user": "user", "domain": "example.com"}
 
 
 def test_create_user_federatedid_username_unicode():
-    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",
+    """
+    Test federated ID with a username and unicode attributes (non-email format)
+    """
+    user = UserAction(id_type=IdentityTypes.federatedID, username=u"user", domain="example.com")
+    user.create(first_name=u"Exampłę", last_name="User", country="PL", email=u"user@example.com")
+    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": u"user@example.com",
+                                                              "firstname": u"Exampłę", "lastname": "User",
                                                               "country": "PL",
                                                               "option": "ignoreIfAlreadyExists"}}],
-                                "user": u"lwalesa", "domain": "k.on-the-side.net"}
+                                "user": u"user", "domain": "example.com"}
 
 
 def test_create_user_federatedid_username_email():
@@ -172,11 +162,40 @@ def test_create_user_federatedid_username_email():
                                 "user": "dbrotsky", "domain": "k.on-the-side.net"}
 
 
-def test_create_user_federatedid_username_mismatch():
+def test_create_user_federatedid_email_format_username():
+    """
+    Create federated ID with username in email format (with email address)
+    :return:
+    """
     user = UserAction(id_type=IdentityTypes.federatedID, username="dbrotsky", domain="k.on-the-side.net",
-                      email="foo@bar.net")
+                      email="dbrotsky@k.on-the-side.net")
+    user.create(first_name="Daniel", last_name="Brotsky", country="US")
+    assert user.wire_dict() == {"do": [{"createFederatedID": {"email": "dbrotsky@k.on-the-side.net",
+                                                              "firstname": "Daniel", "lastname": "Brotsky",
+                                                              "country": "US",
+                                                              "option": "ignoreIfAlreadyExists"}}],
+                                "user": "dbrotsky", "domain": "k.on-the-side.net"}
+
+
+def test_create_user_federatedid_username_mismatch():
+    """
+    Mismatched email in UserAction constructor vs create()
+    """
+    user = UserAction(id_type=IdentityTypes.federatedID, username="user", domain="example.com",
+                      email="user@example.com")
     with pytest.raises(ValueError):
-        user.create(first_name="Daniel", last_name="Brotsky", country="US", email="foo@bar.com")
+        user.create(first_name="Example", last_name="User", country="US", email="user@example.net")
+
+
+def test_different_email_username():
+    """
+    Update a user record so the email address is different than email-type username (federated only)
+    """
+    user = UserAction(id_type=IdentityTypes.federatedID, email="user@example.com")
+    user.update(email="Example.User@example.com", username="user@example.com")
+    assert user.wire_dict() == {"do": [{"update": {"email": "Example.User@example.com",
+                                                   "username": "user@example.com"}}],
+                                "user": "user@example.com"}
 
 
 def test_update_user_adobeid():
diff --git a/umapi_client/functional.py b/umapi_client/functional.py
@@ -55,35 +55,6 @@ class UserAction(Action):
     A sequence of commands to perform on a single user.
     """
 
-    # regex patterns to be compiled once and used in _validate
-    # 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)
-    _username_regex = re.compile(r"^" + _local_pattern + r"$", re.UNICODE)
-    _domain_regex = re.compile(r"^" + _dns_pattern + r"$", re.UNICODE)
-
-    @classmethod
-    def _validate(cls, email=None, username=None, domain=None):
-        """
-        Validates the specified user attributes against their specifications.
-        Input values must be strings (standard or unicode).  Throws ArgumentError if any input is invalid
-        :param email: an email address
-        :param username: a username
-        :param domain: a domain
-        """
-        if email and not cls._email_regex.match(email):
-            raise ArgumentError("'%s': Illegal email format (must be ascii, unquoted, with no comment part)" % (email,))
-        if domain and not cls._domain_regex.match(domain):
-            raise ArgumentError("'%s': Illegal domain format" % (domain,))
-        if username and not cls._username_regex.match(username):
-            raise ArgumentError("'%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):
         """
         Create an Action for a user identified either by email or by username and domain.
@@ -112,27 +83,27 @@ def __init__(self, id_type=IdentityTypes.adobeID, email=None, username=None, dom
         self.id_type = id_type
         self.email = None
         self.domain = None
-        if username:
+        if username is not None:
             if email and username.lower() == email.lower():
                 # ignore the username if it's the same as the email (policy default)
                 username = None
             elif id_type is not IdentityTypes.federatedID:
                 raise ArgumentError("Username must match email except for Federated ID")
             else:
-                self._validate(username=username)
                 if domain:
-                    self._validate(domain=domain)
                     self.domain = domain
-        if email:
-            self._validate(email=email)
+        if email is not None:
+            if '@' not in email:
+                raise ArgumentError("Invalid email address: %s" % email)
             self.email = email
             if not self.domain:
                 atpos = email.index('@')
                 self.domain = email[atpos + 1:]
         elif not username:
-            raise ArgumentError("No user identity specified.")
+                raise ArgumentError("No user identity specified.")
         elif not domain:
             raise ArgumentError("Both username and domain must be specified")
+
         if username:
             Action.__init__(self, user=username, domain=self.domain, **kwargs)
         elif id_type == IdentityTypes.adobeID:
@@ -162,7 +133,6 @@ def create(self, first_name=None, last_name=None, country=None, email=None,
             if not self.email:
                 raise ArgumentError("You must specify email when creating a user")
         elif self.email is None:
-            self._validate(email=email)
             self.email = email
         elif self.email.lower() != email.lower():
             raise ArgumentError("Specified email (%s) doesn't match user's email (%s)" % (email, self.email))
@@ -195,15 +165,18 @@ def update(self, email=None, username=None, first_name=None, last_name=None, cou
         :param country: new country for this user
         :return: the User, so you can do User(...).update(...).add_to_groups(...)
         """
-        if email:
-            self._validate(email=email)
-        if username:
-            self._validate(username=username)
+        if username and self.id_type != IdentityTypes.federatedID:
+            raise ArgumentError("You cannot set username except for a federated ID")
+        if username and '@' in username and not email:
+            raise ArgumentError("Cannot update email-type username when email is not specified")
+        if email and username and email.lower() == username.lower():
+            raise ArgumentError("Specify just email to set both email and username for a federated ID")
         updates = {}
         for k, v in six.iteritems(dict(email=email, username=username,
                                        firstname=first_name, lastname=last_name,
                                        country=country)):
-            if v: updates[k] = v
+            if v:
+                updates[k] = v
         return self.append(update=updates)
 
     def add_to_groups(self, groups=None, all_groups=False, group_type=None):
diff --git a/umapi_client/version.py b/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.11"
+__version__ = "2.12"
