💥(cli) add mandatory "agent" field to credentials

In link with openfun#288 , we need a mechanism to assign an "Agent" (in the sense of the xAPI specification) to a user (to later be able to infer the "Authority" field when writing a statement). It was proposed to add the "agent" representation as a field in the user credentials, which is what is implemented here. There is no contraint as to which of the 4 valid IFI's must be used. BREAKING: New users must now include an agent field It affects both Basic HTTP and OpenIdConnect authentication methods.
rprin · Aug 2, 2023 · 308c071 · 308c071
1 parent 1e7b393
commit 308c071
Show file tree

Hide file tree

Showing 19 changed files with 694 additions and 202 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,8 @@ and this project adheres to
 ### Changed
 
 - Helm chart: improve chart modularity
+- User credentials must now include an "agent" field which can be created 
+using the cli
 
 ## [3.9.0] - 2023-07-21
 

diff --git a/Makefile b/Makefile
@@ -33,6 +33,7 @@ RALPH_IMAGE_BUILD_TARGET ?= development
 RALPH_LRS_AUTH_USER_NAME  = ralph
 RALPH_LRS_AUTH_USER_PWD   = secret
 RALPH_LRS_AUTH_USER_SCOPE = ralph_scope
+RALPH_LRS_AUTH_USER_AGENT_MBOX = mailto:ralph@example.com
 
 # -- K3D
 K3D_CLUSTER_NAME              ?= ralph
@@ -65,6 +66,7 @@ bin/init-cluster:
 		-u $(RALPH_LRS_AUTH_USER_NAME) \
 		-p $(RALPH_LRS_AUTH_USER_PWD) \
 		-s $(RALPH_LRS_AUTH_USER_SCOPE) \
+		-M $(RALPH_LRS_AUTH_USER_AGENT_MBOX)
 		-w
 
 

diff --git a/docs/api.md b/docs/api.md
@@ -36,20 +36,31 @@ documentation for
 details](https://click.palletsprojects.com/en/8.1.x/api/#click.get_app_dir)).
 
 The expected format is a list of entries (JSON objects) each containing the
-username, the user's `bcrypt` hashed+salted password and scopes they can
-access:
+username, the user's `bcrypt` hashed+salted password, scopes they can
+access, and an `agent` object used to represent the user in the LRS. The 
+`agent` is constrained by [LRS specifications](https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#description-2), and must use one of four valid 
+[Inverse Functional Identifiers](https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-Data.md#inversefunctional).
 
 ```json
 [
   {
     "username": "john.doe@example.com",
     "hash": "$2b$12$yBXrzIuRIk6yaft5KUgVFOIPv0PskCCh9PXmF2t7pno.qUZ5LK0D2",
-    "scopes": ["example_scope"]
+    "scopes": ["example_scope"],
+    "agent": {
+      "mbox": "mailto:john.doe@example.com"
+    }
   },
   {
     "username": "simon.says@example.com",
     "hash": "$2b$12$yBXrzIuRIk6yaft5KUgVFOIPv0PskCCh9PXmF2t7pno.qUZ5LK0D2",
-    "scopes": ["second_scope", "third_scope"]
+    "scopes": ["second_scope", "third_scope"],
+    "agent": {
+      "account": {
+        "name": "simonsAccountName",
+        "homePage": "http://www.exampleHomePage.com"
+      }
+    }
   }
 ]
 ```
@@ -61,6 +72,11 @@ $ ralph auth \
     --username janedoe \
     --password supersecret \
     --scope janedoe_scope \
+    --agent-mbox mailto:janedoe@example.com \
+    # or --agent-mbox-sha1sum ebd31e95054c018b10727ccffd2ef2ec3a016ee9\
+    # or --agent-openid "http://jane.openid.example.org/" \
+    # or --agent-account-name exampleAccountname \
+    #    --agent-account-homePage http://www.exampleHomePage.com \
     -w
 ```
 
@@ -91,7 +107,7 @@ Send your username and password to the API server through HTTP Basic Auth:
 ```bash
 $ curl --user john.doe@example.com:PASSWORD http://localhost:8100/whoami
 < HTTP/1.1 200 OK
-< {"username":"john.doe@example.com","scopes":["authenticated","example_scope"]}
+< {"scopes":["example_scope"], "agent": {"mbox": "mailto:john.doe@example.com"}}
 ```
 
 ### OpenID Connect authentication

diff --git a/src/ralph/api/__init__.py b/src/ralph/api/__init__.py
@@ -49,4 +49,4 @@ async def whoami(
     user: AuthenticatedUser = Depends(get_authenticated_user),
 ):
     """Return the current user's username along with their scopes."""
-    return {"username": user.username, "scopes": user.scopes}
+    return {"agent": user.agent, "scopes": user.scopes}
diff --git a/src/ralph/api/auth/basic.py b/src/ralph/api/auth/basic.py
@@ -32,12 +32,12 @@ class UserCredentials(AuthenticatedUser):
     """Pydantic model for user credentials as stored in the credentials file.
 
     Attributes:
-        username (str): Consists of the username for a declared user.
         hash (str): Consists of the hashed password for a declared user.
-        scopes (List): Consists of the scopes a declared has access to.
+        username (str): Consists of the username for a declared user.
     """
 
     hash: str
+    username: str
 
 
 class ServerUsersCredentials(BaseModel):
@@ -182,4 +182,4 @@ def get_authenticated_user(
             headers={"WWW-Authenticate": "Basic"},
         )
 
-    return AuthenticatedUser(username=credentials.username, scopes=user.scopes)
+    return AuthenticatedUser(scopes=user.scopes, agent=user.agent)
diff --git a/src/ralph/api/auth/oidc.py b/src/ralph/api/auth/oidc.py
@@ -13,6 +13,7 @@
 
 from ralph.api.auth.user import AuthenticatedUser
 from ralph.conf import settings
+from ralph.models.xapi.base.ifi import BaseXapiAccount
 
 OPENID_CONFIGURATION_PATH = "/.well-known/openid-configuration"
 oauth2_scheme = OpenIdConnect(
@@ -29,7 +30,7 @@ class IDToken(BaseModel):
     """Pydantic model representing the core of an OpenID Connect ID Token.
 
     ID Tokens are polymorphic and may have many attributes not defined in the
-    spec thus this model accepts all addition fields.
+    specification. This model ignores all additional fields.
 
     Attributes:
         iss (str): Issuer Identifier for the Issuer of the response.
@@ -38,16 +39,18 @@ class IDToken(BaseModel):
         exp (int): Expiration time on or after which the ID Token MUST NOT be
                    accepted for processing.
         iat (int): Time at which the JWT was issued.
+        scope (str): Scope(s) for resource authorization.
     """
 
     iss: str
     sub: str
     aud: Optional[str]
     exp: int
     iat: int
+    scope: Optional[str]
 
     class Config:  # pylint: disable=missing-class-docstring # noqa: D106
-        extra = Extra.allow
+        extra = Extra.ignore
 
 
 @lru_cache()
@@ -141,7 +144,20 @@ def get_authenticated_user(
 
     id_token = IDToken.parse_obj(decoded_token)
 
+    try:
+        agent = BaseXapiAccount(
+            homePage=id_token.iss,
+            name=id_token.sub,
+        )
+    except (ValueError, TypeError) as exc:
+        logger.error("Claims cannot be mapped to an agent: %s", exc)
+        raise HTTPException(
+            status_code=status.HTTP_401_UNAUTHORIZED,
+            detail="Could not validate credentials",
+            headers={"WWW-Authenticate": "Bearer"},
+        ) from exc
+
     return AuthenticatedUser(
-        username=f"{id_token.iss}/{id_token.sub}",
-        scopes=id_token.roles if hasattr(id_token, "roles") else None,
+        agent=agent,
+        scopes=id_token.scope.split(" ") if id_token.scope else [],
     )
diff --git a/src/ralph/api/auth/user.py b/src/ralph/api/auth/user.py
@@ -1,6 +1,6 @@
 """Authenticated user for the Ralph API."""
 
-from typing import List, Optional
+from typing import Dict, List
 
 from pydantic import BaseModel
 
@@ -9,9 +9,9 @@ class AuthenticatedUser(BaseModel):
     """Pydantic model for user authentication.
 
     Attributes:
-        username (str): Consists of the username of the current user.
-        scopes (list): Consists of the scopes the user has access to.
+        agent (dict): The agent representing the current user.
+        scopes (list): The scopes the user has access to.
     """
 
-    username: str
-    scopes: Optional[List[str]]
+    agent: Dict
+    scopes: List[str]
diff --git a/src/ralph/api/routers/statements.py b/src/ralph/api/routers/statements.py
@@ -21,7 +21,11 @@
 
 from ralph.api.auth import get_authenticated_user
 from ralph.api.forwarding import forward_xapi_statements, get_active_xapi_forwardings
-from ralph.backends.database.base import BaseDatabase, StatementParameters
+from ralph.backends.database.base import (
+    AgentParameters,
+    BaseDatabase,
+    StatementParameters,
+)
 from ralph.conf import settings
 from ralph.exceptions import BackendException, BadFormatException
 from ralph.models.xapi.base.agents import (
@@ -240,21 +244,24 @@ async def get(
 
     # Parse the agent parameter (JSON) into multiple string parameters
     query_params = dict(request.query_params)
+
     if query_params.get("agent") is not None:
         # Transform agent to `dict` as FastAPI cannot parse JSON (seen as string)
-        agent = parse_raw_as(BaseXapiAgent, query_params["agent"])
 
-        query_params.pop("agent")
+        agent = parse_raw_as(BaseXapiAgent, query_params["agent"])
 
+        agent_query_params = {}
         if isinstance(agent, BaseXapiAgentWithMbox):
-            query_params["agent__mbox"] = agent.mbox
+            agent_query_params["mbox"] = agent.mbox
         elif isinstance(agent, BaseXapiAgentWithMboxSha1Sum):
-            query_params["agent__mbox_sha1sum"] = agent.mbox_sha1sum
+            agent_query_params["mbox_sha1sum"] = agent.mbox_sha1sum
         elif isinstance(agent, BaseXapiAgentWithOpenId):
-            query_params["agent__openid"] = agent.openid
+            agent_query_params["openid"] = agent.openid
         elif isinstance(agent, BaseXapiAgentWithAccount):
-            query_params["agent__account__name"] = agent.account.name
-            query_params["agent__account__home_page"] = agent.account.homePage
+            agent_query_params["account__name"] = agent.account.name
+            agent_query_params["account__home_page"] = agent.account.homePage
+
+        query_params["agent"] = AgentParameters(**agent_query_params)
 
     # Query Database
     try:

diff --git a/src/ralph/backends/database/base.py b/src/ralph/backends/database/base.py
@@ -43,6 +43,20 @@ class DatabaseStatus(Enum):
     ERROR = "error"
 
 
+@dataclass
+class AgentParameters:
+    """Dictionary of possible LRS query parameters for query on type Agent.
+
+    NB: Agent refers to the data structure, NOT to the LRS query parameter.
+    """
+
+    mbox: Optional[str] = None
+    mbox_sha1sum: Optional[str] = None
+    openid: Optional[str] = None
+    account__name: Optional[str] = None
+    account__home_page: Optional[str] = None
+
+
 @dataclass
 class StatementParameters:
     """Represents a dictionary of possible LRS query parameters."""
@@ -51,11 +65,7 @@ class StatementParameters:
 
     statementId: Optional[str] = None  # pylint: disable=invalid-name
     voidedStatementId: Optional[str] = None  # pylint: disable=invalid-name
-    agent__mbox: Optional[str] = None
-    agent__mbox_sha1sum: Optional[str] = None
-    agent__openid: Optional[str] = None
-    agent__account__name: Optional[str] = None
-    agent__account__home_page: Optional[str] = None
+    agent: Optional[AgentParameters] = None
     verb: Optional[str] = None
     activity: Optional[str] = None
     registration: Optional[UUID] = None
@@ -69,33 +79,41 @@ class StatementParameters:
     ascending: Optional[bool] = False
     search_after: Optional[str] = None
     pit_id: Optional[str] = None
+    authority: Optional[AgentParameters] = None
 
     def __post_init__(self):
         """Perform additional conformity verifications on parameters."""
-        # Check that both `homePage` and `name` are provided if `account` is being used
-        if (self.agent__account__name is not None) != (
-            self.agent__account__home_page is not None
-        ):
-            raise BackendParameterException(
-                "Invalid agent parameters: home_page and name are both required"
-            )
-
-        # Check that no more than one Inverse Functional Identifier is provided
-        if (
-            sum(
-                x is not None
-                for x in [
-                    self.agent__mbox,
-                    self.agent__mbox_sha1sum,
-                    self.agent__openid,
-                    self.agent__account__name,
-                ]
-            )
-            > 1
-        ):
-            raise BackendParameterException(
-                "Invalid agent parameters: Only one identifier can be used"
-            )
+        # Initiate agent parameters for queries "agent" and "authority"
+        for query_param in ["agent", "authority"]:
+            # Transform to object if None (cannot be done with defaults)
+            if self.__dict__[query_param] is None:
+                self.__dict__[query_param] = AgentParameters()
+
+            # Check that both `homePage` and `name` are provided if any are
+            if (self.__dict__[query_param].account__name is not None) != (
+                self.__dict__[query_param].account__home_page is not None
+            ):
+                raise BackendParameterException(
+                    f"Invalid {query_param} parameters: homePage and name are "
+                    "both required"
+                )
+
+            # Check that one or less Inverse Functional Identifier is provided
+            if (
+                sum(
+                    x is not None
+                    for x in [
+                        self.__dict__[query_param].mbox,
+                        self.__dict__[query_param].mbox_sha1sum,
+                        self.__dict__[query_param].openid,
+                        self.__dict__[query_param].account__name,
+                    ]
+                )
+                > 1
+            ):
+                raise BackendParameterException(
+                    f"Invalid {query_param} parameters: Only one identifier can be used"
+                )
 
 
 def enforce_query_checks(method):