Merge pull request #793 from MolSSI/jwt_fix

More small JWT improvements

qcfractal/qcfractal/components/auth/auth_socket.py

@@ -3,7 +3,7 @@
 import logging
 from typing import TYPE_CHECKING, Tuple, List, Dict, Any, Optional
 
-from qcportal.auth import UserInfo
+from qcportal.auth import UserInfo, RoleInfo
 from qcportal.exceptions import AuthorizationFailure
 from .policyuniverse import Policy
 
@@ -27,9 +27,9 @@ def __init__(self, root_socket: SQLAlchemySocket):
         self.unauth_read_permissions = self.root_socket.roles.get("read")["permissions"]
         self.protected_resources = {"users", "roles", "me"}
 
-    def authenticate(self, username: str, password: str, *, session: Optional[Session] = None) -> UserInfo:
+    def authenticate(self, username: str, password: str, *, session: Optional[Session] = None) -> Tuple[UserInfo, RoleInfo]:
         """
-        Authenticates a given username and password, returning the users permissions.
+        Authenticates a given username and password, returning info about the user and their role
 
         If the user is not found, or is disabled, or the password is incorrect, an exception is raised.
 
@@ -46,10 +46,40 @@ def authenticate(self, username: str, password: str, *, session: Optional[Sessio
         Returns
         --------
         :
-            All information about the user
+            All information about the user, and all information about the user's role
         """
 
-        return self.root_socket.users.verify(username=username, password=password, session=session)
+        with self.root_socket.optional_session(session, True) as session:
+            user_info = self.root_socket.users.authenticate(username=username, password=password, session=session)
+            role_info_dict = self.root_socket.roles.get(user_info.role, session=session)
+            return user_info, RoleInfo(**role_info_dict)
+
+    def verify(self, user_id: int, *, session: Optional[Session] = None) -> Tuple[UserInfo, RoleInfo]:
+        """
+        Verifies that a given user id exists and is enabled, returning info about the user and their role
+
+        This does not check the user's password.
+
+        If the user is not found, or is disabled, an exception is raised.
+
+        Parameters
+        ----------
+        user_id
+            The id of the user to check
+        session
+            An existing SQLAlchemy session to use. If None, one will be created. If an existing session
+            is used, it will be flushed (but not committed) before returning from this function.
+
+        Returns
+        --------
+        :
+            All information about the user, and all information about the user's role
+        """
+
+        with self.root_socket.optional_session(session, True) as session:
+            user_info = self.root_socket.users.verify(user_id=user_id, session=session)
+            role_info_dict = self.root_socket.roles.get(user_info.role, session=session)
+            return user_info, RoleInfo(**role_info_dict)
 
     def is_authorized(
         self, resource: Dict[str, Any], action: str, subject: Dict[str, Any], context: Dict[str, Any], policies: Any

qcfractal/qcfractal/flask_app/auth_v1/test_auth.py → qcfractal/qcfractal/components/auth/test_auth_permissions.py

@@ -1,5 +1,4 @@
 import pytest
-import requests
 
 from qcarchivetesting import test_users
 from qcarchivetesting.testing_classes import QCATestingSnowflake
@@ -503,38 +502,3 @@ def test_auth_default_role_compute(secure_snowflake):
     uinfo.fullname = "A new full name"
     client.modify_user(uinfo)
     client.change_user_password()
-
-
-@pytest.mark.parametrize("use_forms", [True, False])
-def test_auth_cookies(secure_snowflake, use_forms):
-    username = "admin_user"
-    password = test_users["admin_user"]["pw"]
-    uri = secure_snowflake.get_uri()
-
-    sess = requests.Session()  # will store cookies automatically
-
-    # First, not logged in = forbidden
-    r = sess.get(f"{uri}/api/v1/information")
-    assert r.status_code == 403  # forbidden
-
-    # Now go through the browser login. This should set a cookie
-    if use_forms:
-        r = sess.post(f"{uri}/auth/v1/browser_login", data={"username": username, "password": password})
-    else:
-        r = sess.post(f"{uri}/auth/v1/browser_login", json={"username": username, "password": password})
-
-    assert r.status_code == 200
-    assert "access_token_cookie" in sess.cookies
-
-    # Can get to protected endpoint
-    r = sess.get(f"{uri}/api/v1/information")
-    assert r.status_code == 200
-
-    # Now logout - cookie is removed
-    r = sess.post(f"{uri}/auth/v1/browser_logout")
-    assert r.status_code == 200
-    assert "access_token_cookie" not in sess.cookies
-
-    # Not logged in anymore
-    r = sess.get(f"{uri}/api/v1/information")
-    assert r.status_code == 403  # forbidden

qcfractal/qcfractal/components/auth/test_user_socket.py

@@ -180,7 +180,7 @@ def test_user_socket_use_unknown_user(storage_socket: SQLAlchemySocket):
         storage_socket.users.get("geoff")
 
     with pytest.raises(AuthenticationFailure, match=r"Incorrect username or password"):
-        storage_socket.users.verify("geoff", "a password")
+        storage_socket.users.authenticate("geoff", "a password")
 
     with pytest.raises(UserManagementError, match=r"User.*not found"):
         uinfo = UserInfo(id=1234, username="geoff", role="read", enabled=True)
@@ -207,11 +207,11 @@ def test_user_socket_verify_password(storage_socket: SQLAlchemySocket):
 
         add_pw = storage_socket.users.add(uinfo, password=password)
         assert add_pw == password
-        storage_socket.users.verify(username, add_pw)
+        storage_socket.users.authenticate(username, add_pw)
 
         for guess in ["Simple", "ABC%1234", "ÃØ©þꝎB"]:
             with pytest.raises(AuthenticationFailure):
-                storage_socket.users.verify(username, guess)
+                storage_socket.users.authenticate(username, guess)
 
 
 def test_user_socket_verify_user_disabled(storage_socket: SQLAlchemySocket):
@@ -223,13 +223,13 @@ def test_user_socket_verify_user_disabled(storage_socket: SQLAlchemySocket):
 
     gen_pw = storage_socket.users.add(uinfo)
 
-    uinfo2 = storage_socket.users.verify("george", gen_pw)
+    uinfo2 = storage_socket.users.authenticate("george", gen_pw)
 
     uinfo2.enabled = False
     storage_socket.users.modify(uinfo2, as_admin=True)
 
     with pytest.raises(AuthenticationFailure):
-        storage_socket.users.verify("george", gen_pw)
+        storage_socket.users.authenticate("george", gen_pw)
 
 
 def test_user_socket_change_password(storage_socket: SQLAlchemySocket):
@@ -242,16 +242,16 @@ def test_user_socket_change_password(storage_socket: SQLAlchemySocket):
     old_pw = storage_socket.users.add(uinfo, "oldpw123")
     assert old_pw == "oldpw123"
 
-    storage_socket.users.verify("george", "oldpw123")
+    storage_socket.users.authenticate("george", "oldpw123")
 
     # update password...
     storage_socket.users.change_password("george", password="newpw123")
 
     # Raises exception on failure
-    storage_socket.users.verify("george", "newpw123")
+    storage_socket.users.authenticate("george", "newpw123")
 
     with pytest.raises(AuthenticationFailure):
-        storage_socket.users.verify("george", "oldpw123")
+        storage_socket.users.authenticate("george", "oldpw123")
 
 
 def test_user_socket_password_generation(storage_socket: SQLAlchemySocket):
@@ -262,16 +262,16 @@ def test_user_socket_password_generation(storage_socket: SQLAlchemySocket):
     )
 
     gen_pw = storage_socket.users.add(uinfo)
-    storage_socket.users.verify("george", gen_pw)
+    storage_socket.users.authenticate("george", gen_pw)
     is_valid_password(gen_pw)
-    storage_socket.users.verify("george", gen_pw)
+    storage_socket.users.authenticate("george", gen_pw)
 
     gen_pw_2 = storage_socket.users.change_password("george", None)
-    storage_socket.users.verify("george", gen_pw_2)
+    storage_socket.users.authenticate("george", gen_pw_2)
     is_valid_password(gen_pw)
 
     with pytest.raises(AuthenticationFailure):
-        storage_socket.users.verify("george", gen_pw)
+        storage_socket.users.authenticate("george", gen_pw)
 
 
 @pytest.mark.parametrize("as_admin", [True, False])
@@ -351,7 +351,7 @@ def test_user_socket_use_invalid_username(storage_socket: SQLAlchemySocket):
             storage_socket.users.get(username)
 
         with pytest.raises(InvalidUsernameError):
-            storage_socket.users.verify(username, "a_password")
+            storage_socket.users.authenticate(username, "a_password")
 
         with pytest.raises(InvalidUsernameError):
             storage_socket.users.change_password(username, "a_password")
@@ -386,4 +386,4 @@ def test_user_socket_use_invalid_password(storage_socket: SQLAlchemySocket):
             storage_socket.users.change_password(uid, password)
 
         with pytest.raises(InvalidPasswordError):
-            storage_socket.users.verify(username, password)
+            storage_socket.users.authenticate(username, password)

qcfractal/qcfractal/components/auth/user_socket.py

@@ -185,9 +185,9 @@ def _verify_local_password(self, user: UserORM, password: str):
         if pwcheck is False:
             raise AuthenticationFailure("Incorrect username or password")
 
-    def verify(self, username: str, password: str, *, session: Optional[Session] = None) -> UserInfo:
+    def authenticate(self, username: str, password: str, *, session: Optional[Session] = None) -> UserInfo:
         """
-        Verifies a given username and password, returning all info about the user
+        Authenticates a given username and password, returning all info about the user
 
         If the user is not found, or is disabled, or the password is incorrect, an exception is raised.
 
@@ -228,6 +228,38 @@ def verify(self, username: str, password: str, *, session: Optional[Session] = N
 
             return user.to_model(UserInfo)
 
+    def verify(self, user_id: int, *, session: Optional[Session] = None) -> UserInfo:
+        """
+        Verifies that a given user id exists and is enabled, returning all info about the user
+
+        If the user id is not found, or is disabled, an exception is raised.
+
+        Parameters
+        ----------
+        user_id
+            The id of the user to check
+        session
+            An existing SQLAlchemy session to use. If None, one will be created. If an existing session
+            is used, it will be flushed (but not committed) before returning from this function.
+
+        Returns
+        --------
+        :
+            All information about the user
+        """
+
+        with self.root_socket.optional_session(session, True) as session:
+            try:
+                user = self._get_internal(session, user_id)
+            except UserManagementError as e:
+                # Turn missing user into an Authentication error
+                raise AuthenticationFailure("User does not exist")
+
+            if not user.enabled:
+                raise AuthenticationFailure(f"User {user_id} is disabled.")
+
+            return user.to_model(UserInfo)
+
     def modify(self, user_info: UserInfo, as_admin: bool, *, session: Optional[Session] = None) -> Dict[str, Any]:
         """
         Alters a user's information

qcfractal/qcfractal/flask_app/auth_v1/routes.py

@@ -11,7 +11,6 @@
 from qcfractal.flask_app import storage_socket
 from qcfractal.flask_app.auth_v1.blueprint import auth_v1
 from qcfractal.flask_app.helpers import get_all_endpoints, access_token_from_user, login_and_get_jwt
-from qcportal.auth import UserInfo, RoleInfo
 
 
 @auth_v1.route("/login", methods=["POST"])
@@ -41,11 +40,7 @@ def browser_logout():
 def refresh():
     user_id = get_jwt_identity()
 
-    user_dict = storage_socket.users.get(user_id)
-    role_dict = storage_socket.roles.get(user_dict["role"])
-    user_info = UserInfo(**user_dict)
-    role_info = RoleInfo(**role_dict)
-
+    user_info, role_info = storage_socket.auth.verify(user_id)
     access_token = access_token_from_user(user_info, role_info)
 
     # For logging purposes (in the after_request_func)