File refactored_auth_module.diff of Package cobbler

Overview Repositories Revisions Requests Users Attributes Meta

File refactored_auth_module.diff of Package cobbler

diff --git a/cobbler/modules/authentication/spacewalk.py b/cobbler/modules/authentication/spacewalk.py
index c553c8e6f..2bd1c918b 100644
--- a/cobbler/modules/authentication/spacewalk.py
+++ b/cobbler/modules/authentication/spacewalk.py
@@ -21,10 +21,14 @@
 02110-1301  USA
 """
 
-
 from future import standard_library
+
 standard_library.install_aliases()
-import xmlrpc.client
+
+from cobbler.cexceptions import CX
+from cobbler.utils import log_exc
+from cobbler import clogger
+from xmlrpc.client import ServerProxy, Error
 
 
 def register():
@@ -40,32 +44,66 @@ def __looks_like_a_token(password):
     /likely/ a token, and we should try to treat it as a token first, if not, we should treat it as a password.  All of
     this code is there to avoid extra XMLRPC calls, which are slow.
 
+    A password gets detected as a token if it is lowercase and under 45 characters.
+
     :param password: The password which is possibly a token.
     :return: True if it is possibly a token or False otherwise.
     :rtype: bool
     """
 
-    if password.lower() != password:
-        # Tokens are always lowercase, this isn't a token.
+    return password.lower() == password and len(password) > 45
+
+
+def __check_auth_token(xmlrpc_client, api_handle, username, password):
+    """
+    This checks if the auth token is valid.
+    :param xmlrpc_client: The xmlrpc client to check access for.
+    :param api_handle: The api instance to retrieve settings of.
+    :param username: The username to try.
+    :param password: The password to try.
+    :return: In any error case this will return 0. Otherwise the return value of the API which should be 1.
+    """
+    # If the token is not a token this will raise an exception rather than return an integer.
+    try:
+        return xmlrpc_client.auth.checkAuthToken(username, password)
+    except Error:
+        logger = api_handle.logger
+        logger.error("Error while checking authentication token.")
+        log_exc(logger)
         return False
 
-    # We can't use binascii.unhexlify here as it's an "odd length string".
-    # try:
-    #    #data = binascii.unhexlify(password)
-    #    return True # looks like a token, but we can't be sure
-    # except:
-    #    return False # definitely not a token
 
-    return (len(password) > 45)
+def __check_user_login(xmlrpc_client, api_handle, user_enabled, username, password):
+    """
+    This actually performs the login to spacewalk.
+
+    :param xmlrpc_client: The xmlrpc client bound to the target spacewalk instance.
+    :param api_handle: The api instance to retrieve settings of.
+    :param user_enabled: Weather we allow Spacewalk users to log in or not.
+    :type user_enabled: bool
+    :param username: The username to log in.
+    :param password: The password to log in.
+    :return: True if users are allowed to log in and he is of the role ``config_admin`` or ``org_admin``.
+    """
+    try:
+        session = xmlrpc_client.auth.login(username, password)
+        # login success by username, role must also match and user_enabled needs to be true.
+        roles = xmlrpc_client.user.listRoles(session, username)
+        if user_enabled and ("config_admin" in roles or "org_admin" in roles):
+            return True
+    except Error:
+        logger = api_handle.logger
+        logger.error("Error while checking user authentication data.")
+        log_exc(logger)
+    return False
 
 
 def authenticate(api_handle, username, password):
     """
-    Validate a username/password combo, returning True/False
-
-    This will pass the username and password back to Spacewalk to see if this authentication request is valid.
+    Validate a username/password combo. This will pass the username and password back to Spacewalk to see if this
+    authentication request is valid.
 
-    See also: https://github.com/uyuni-project/uyuni/blob/bbbbbf537a1928c1922015c70322034a89b1cb9a/java/code/src/com/redhat/rhn/frontend/xmlrpc/auth/AuthHandler.java#L133
+    See also: https://github.com/uyuni-project/uyuni/blob/master/java/code/src/com/redhat/rhn/frontend/xmlrpc/auth/AuthHandler.java#L133
 
     :param api_handle: The api instance to retrieve settings of.
     :param username: The username to authenticate agains spacewalk/uyuni/SUSE Manager
@@ -74,78 +112,25 @@ def authenticate(api_handle, username, password):
     :rtype: bool
     """
 
-    if api_handle is not None:
-        server = api_handle.settings().redhat_management_server
-        user_enabled = api_handle.settings().redhat_management_permissive
-    else:
-        server = "columbia.devel.redhat.com"
-        user_enabled = True
-
-    if server == "xmlrpc.rhn.redhat.com":
-        # Emergency fail, don't bother RHN!
-        return False
+    if api_handle is None:
+        raise CX("api_handle required. Please don't call this without it.")
+    server = api_handle.settings().redhat_management_server
+    user_enabled = api_handle.settings().redhat_management_permissive
 
     spacewalk_url = "https://%s/rpc/api" % server
-    client = xmlrpc.client.Server(spacewalk_url, verbose=0)
-
-    if __looks_like_a_token(password) or username == 'taskomatic_user':
-        # The tokens are lowercase hex, but a password can also be lowercase hex, so we have to try it as both a token
-        # and then a password if we are unsure. We do it this way to be faster but also to avoid any login failed stuff
-        # in the logs that we don't need to send.
-
-        try:
-            valid = client.auth.checkAuthToken(username, password)
-        except:
-            # If the token is not a token this will raise an exception rather than return an integer.
-            valid = 0
-
-        # Problem at this point, 0xdeadbeef is valid as a token but if that fails, it's also a valid password, so we
-        # must try auth system #2
-
-        if valid != 1:
-            # First API code returns 1 on success the second uses exceptions for login failed.
-            # So... token check failed, but maybe the username/password is just a simple username/pass!
-
-            if user_enabled == 0:
-                # this feature must be explicitly enabled.
-                return False
-
-            session = ""
-            try:
-                session = client.auth.login(username, password)
-            except:
-                # FIXME: Should log exceptions that are not excepted as we could detect spacewalk java errors here that
-                #        are not login related.
-                return False
-
-            # login success by username, role must also match
-            roles = client.user.listRoles(session, username)
-            if not ("config_admin" in roles or "org_admin" in roles):
-                return False
-
-        return True
-
-    else:
-        # It's an older version of spacewalk, so just try the username/pass.
-        # OR: We know for sure it's not a token because it's not lowercase hex.
-
-        if user_enabled == 0:
-            # this feature must be explicitly enabled.
-            return False
-
-        session = ""
-        try:
-            session = client.auth.login(username, password)
-        except:
-            return False
-
-        # login success by username, role must also match
-        roles = client.user.listRoles(session, username)
-        if not ("config_admin" in roles or "org_admin" in roles):
-            return False
-
-        return True
-
-
-if __name__ == "__main__":
-    print((authenticate(None, "admin", "redhat")))
+    with ServerProxy(spacewalk_url, verbose=True) as client:
+        if username == 'taskomatic_user' or __looks_like_a_token(password):
+            # The tokens are lowercase hex, but a password can also be lowercase hex, so we have to try it as both a
+            # token and then a password if we are unsure. We do it this way to be faster but also to avoid any login
+            # failed stuff in the logs that we don't need to send.
+
+            # Problem at this point, 0xdeadbeef is valid as a token but if that fails, it's also a valid password, so we
+            # must try auth system #2
+
+            if __check_auth_token(client, api_handle, username, password) != 1:
+                return __check_user_login(client, api_handle, user_enabled, username, password)
+            return True
+        else:
+            # It's an older version of spacewalk, so just try the username/pass.
+            # OR: We know for sure it's not a token because it's not lowercase hex.
+            return __check_user_login(client, api_handle, user_enabled, username, password)

Places

File refactored_auth_module.diff of Package cobbler

Places