#135: Make client objects bound to an endpoint, not a backend

- This will prevent the same client object being used across different endpoints, which could be a security concern. This means that for each request, the client object is built with the session ID assigned each time, so each request checks that the user has a valid, active session ID

common/python_icat_backend.py

@@ -5,108 +5,170 @@
 
 from common.backend import Backend
 from common.helpers import queries_records
-from common.python_icat_helpers import requires_session_id, get_session_details_helper, logout_icat_client, refresh_client_session
+from common.python_icat_helpers import (
+    requires_session_id,
+    get_session_details_helper,
+    logout_icat_client,
+    refresh_client_session,
+    create_client,
+)
 from common.config import config
 from common.exceptions import AuthenticationError
 from common.models.db_models import SESSION
 
 log = logging.getLogger()
 
+
 class PythonICATBackend(Backend):
     """
     Class that contains functions to access and modify data in an ICAT database directly
     """
-    
+
     def __init__(self):
-        # Client object is created here as well as in login() to avoid uncaught exceptions 
-        # where the object is None. This could happen where a user tries to use an endpoint before
-        # logging in. Also helps to give a bit of certainty to what's stored here
-        self.client = icat.client.Client(config.get_icat_url(), checkCert=config.get_icat_check_cert())
+        pass
 
     def login(self, credentials):
-        # Client object is re-created here so session IDs aren't overwritten in the database
-        self.client = icat.client.Client(config.get_icat_url(), checkCert=config.get_icat_check_cert())
+        client = create_client()
 
         # Syntax for Python ICAT
-        login_details = {'username': credentials['username'], 'password': credentials['password']}
+        login_details = {
+            "username": credentials["username"],
+            "password": credentials["password"],
+        }
         try:
-            session_id = self.client.login(credentials["mechanism"], login_details)
+            session_id = client.login(credentials["mechanism"], login_details)
             return session_id
         except ICATSessionError:
             raise AuthenticationError("User credentials are incorrect")
 
     @requires_session_id
-    def get_session_details(self, session_id):
-        return get_session_details_helper(self.client)
+    def get_session_details(self, session_id, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
+        return get_session_details_helper(client)
 
     @requires_session_id
-    def refresh(self, session_id):
-        return refresh_client_session(self.client)
+    def refresh(self, session_id, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
+        return refresh_client_session(client)
 
     @requires_session_id
     @queries_records
-    def logout(self, session_id):
-        return logout_icat_client(self.client)
+    def logout(self, session_id, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
+        return logout_icat_client(client)
 
     @requires_session_id
     @queries_records
-    def get_with_filters(self, session_id, table, filters):
-        pass
+    def get_with_filters(self, session_id, table, filters, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
 
     @requires_session_id
     @queries_records
-    def create(self, session_id, table, data):
-        pass
+    def create(self, session_id, table, data, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
 
     @requires_session_id
     @queries_records
-    def update(self, session_id, table, data):
-        pass
+    def update(self, session_id, table, data, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
 
     @requires_session_id
     @queries_records
-    def get_one_with_filters(self, session_id, table, filters):
-        pass
+    def get_one_with_filters(self, session_id, table, filters, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
 
     @requires_session_id
     @queries_records
-    def count_with_filters(self, session_id, table, filters):
-        pass
+    def count_with_filters(self, session_id, table, filters, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
 
     @requires_session_id
     @queries_records
-    def get_with_id(self, session_id, table, id):
-        pass
+    def get_with_id(self, session_id, table, id, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
 
     @requires_session_id
     @queries_records
-    def delete_with_id(self, session_id, table, id):
-        pass
+    def delete_with_id(self, session_id, table, id, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
 
     @requires_session_id
     @queries_records
-    def update_with_id(self, session_id, table, id, data):
-        pass
+    def update_with_id(self, session_id, table, id, data, **kwargs):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
 
     @requires_session_id
     @queries_records
-    def get_instrument_facilitycycles_with_filters(self, session_id, instrument_id, filters):
-        pass
+    def get_instrument_facilitycycles_with_filters(
+        self, session_id, instrument_id, filters, **kwargs
+    ):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
 
     @requires_session_id
     @queries_records
-    def count_instrument_facilitycycles_with_filters(self, session_id, instrument_id, filters):
-        pass
-        #return get_facility_cycles_for_instrument_count(instrument_id, filters)
+    def count_instrument_facilitycycles_with_filters(
+        self, session_id, instrument_id, filters, **kwargs
+    ):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
+        # return get_facility_cycles_for_instrument_count(instrument_id, filters)
 
     @requires_session_id
     @queries_records
-    def get_instrument_facilitycycle_investigations_with_filters(self, session_id, instrument_id, facilitycycle_id, filters):
-        pass
-        #return get_investigations_for_instrument_in_facility_cycle(instrument_id, facilitycycle_id, filters)
+    def get_instrument_facilitycycle_investigations_with_filters(
+        self, session_id, instrument_id, facilitycycle_id, filters, **kwargs
+    ):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
+        # return get_investigations_for_instrument_in_facility_cycle(instrument_id, facilitycycle_id, filters)
 
     @requires_session_id
     @queries_records
-    def count_instrument_facilitycycles_investigations_with_filters(self, session_id, instrument_id, facilitycycle_id, filters):
-        pass
-        #return get_investigations_for_instrument_in_facility_cycle_count(instrument_id, facilitycycle_id, filters)
+    def count_instrument_facilitycycles_investigations_with_filters(
+        self, session_id, instrument_id, facilitycycle_id, filters, **kwargs
+    ):
+        if kwargs["client"]:
+            client = kwargs["client"]
+        else:
+            client = create_client()
+        # return get_investigations_for_instrument_in_facility_cycle_count(instrument_id, facilitycycle_id, filters)

common/python_icat_helpers.py

@@ -3,11 +3,15 @@
 from datetime import datetime, timedelta
 
 
+import icat.client
 from icat.exception import ICATSessionError
+
 from common.exceptions import AuthenticationError
+from common.config import config
 
 log = logging.getLogger()
 
+
 def requires_session_id(method):
     """
     Decorator for Python ICAT backend methods that looks out for session errors when using the API.
@@ -29,8 +33,10 @@ def requires_session_id(method):
     @wraps(method)
     def wrapper_requires_session(*args, **kwargs):
         try:
-            client = args[0].client
+            client = create_client()
             client.sessionId = args[1]
+            # Client object put into kwargs so it can be accessed by backend functions
+            kwargs["client"] = client
 
             # Find out if session has expired
             session_time = client.getRemainingMinutes()
@@ -58,14 +64,25 @@ def wrapper_gets_records(*args, **kwargs):
     return wrapper_gets_records
 
 
+def create_client():
+    client = icat.client.Client(
+        config.get_icat_url(), checkCert=config.get_icat_check_cert()
+    )
+    return client
+
+
 def get_session_details_helper(client):
-    # Remove rounding 
+    # Remove rounding
     session_time_remaining = client.getRemainingMinutes()
     session_expiry_time = datetime.now() + timedelta(minutes=session_time_remaining)
 
     username = client.getUserName()
 
-    return {"ID": client.sessionId, "EXPIREDATETIME": str(session_expiry_time), "USERNAME": username}
+    return {
+        "ID": client.sessionId,
+        "EXPIREDATETIME": str(session_expiry_time),
+        "USERNAME": username,
+    }
 
 
 def logout_icat_client(client):