Rework the User table

- Major refactoring in the User table
- Removed Register/Login/Logout functionality
- Users_api only handle get and put/update
- Extract user information from oidc token and import in the User table

PBENCH-1080

lib/pbench/cli/server/user_management.py

@@ -1,3 +1,5 @@
+import datetime
+
 import click
 
 from pbench import BadConfig
@@ -6,10 +8,8 @@
 from pbench.cli.server.options import common_options
 from pbench.server.database.models.users import Roles, User
 
-USER_LIST_ROW_FORMAT = "{0:15}\t{1:15}\t{2:15}\t{3:15}\t{4:20}"
-USER_LIST_HEADER_ROW = USER_LIST_ROW_FORMAT.format(
-    "Username", "First Name", "Last Name", "Registered On", "Email"
-)
+USER_LIST_ROW_FORMAT = "{0:15}\t{1:15}"
+USER_LIST_HEADER_ROW = USER_LIST_ROW_FORMAT.format("Username", "oidc id")
 
 
 # User create CLI
@@ -31,11 +31,10 @@ def user_command_cli(context):
     help="pbench server account username (will prompt if unspecified)",
 )
 @click.option(
-    "--password",
+    "--oidc-id",
     prompt=True,
-    hide_input=True,
     required=True,
-    help="pbench server account password (will prompt if unspecified)",
+    help="OIDC server user id (will prompt if unspecified)",
 )
 @click.option(
     "--email",
@@ -63,7 +62,7 @@ def user_command_cli(context):
 def user_create(
     context: object,
     username: str,
-    password: str,
+    oidc_id: str,
     email: str,
     first_name: str,
     last_name: str,
@@ -73,11 +72,20 @@ def user_create(
         config_setup(context)
         user = User(
             username=username,
-            password=password,
-            first_name=first_name,
-            last_name=last_name,
-            email=email,
-            role=role if role else "",
+            oidc_id=oidc_id,
+            profile={
+                "user": {
+                    "first_name": first_name,
+                    "last_name": last_name,
+                    "email": email,
+                },
+                "server": {
+                    "roles": [role] if role else [],
+                    "registered_on": datetime.datetime.now().strftime(
+                        "%m/%d/%Y, %H:%M:%S"
+                    ),
+                },
+            },
         )
         user.add()
         if user.is_admin():
@@ -100,9 +108,14 @@ def user_create(
 def user_delete(context: object, username: str) -> None:
     try:
         # Delete the the user with specified username
+        user = User.query(username=username)
         config_setup(context)
-        User.delete(username=username)
-        rv = 0
+        if not user:
+            click.echo(f"User {username} does not exist", err=True)
+            rv = 1
+        else:
+            user.delete()
+            rv = 0
     except Exception as exc:
         click.echo(exc, err=True)
         rv = 2 if isinstance(exc, BadConfig) else 1
@@ -126,10 +139,7 @@ def user_list(context: object) -> None:
             click.echo(
                 USER_LIST_ROW_FORMAT.format(
                     user.username,
-                    user.first_name,
-                    user.last_name,
-                    user.registered_on.strftime("%Y-%m-%d"),
-                    user.email,
+                    user.oidc_id,
                 )
             )
 
@@ -145,11 +155,6 @@ def user_list(context: object) -> None:
 @user_command_cli.command()
 @common_options
 @click.argument("updateuser")
-@click.option(
-    "--username",
-    required=False,
-    help="Specify the new username",
-)
 @click.option(
     "--email",
     required=False,
@@ -175,7 +180,6 @@ def user_list(context: object) -> None:
 def user_update(
     context: object,
     updateuser: str,
-    username: str,
     first_name: str,
     last_name: str,
     email: str,
@@ -191,23 +195,27 @@ def user_update(
             rv = 1
         else:
             dict_to_update = {}
-            if username:
-                dict_to_update["username"] = username
-
+            user_fields = {}
+            server_fields = {}
             if first_name:
-                dict_to_update["first_name"] = first_name
+                user_fields["first_name"] = first_name
 
             if last_name:
-                dict_to_update["last_name"] = last_name
+                user_fields["last_name"] = last_name
 
             if email:
-                dict_to_update["email"] = email
+                user_fields["email"] = email
 
             if role:
-                dict_to_update["role"] = role
+                server_fields["role"] = [role]
+
+            if user_fields:
+                dict_to_update["user"] = user_fields
+            if server_fields:
+                dict_to_update["server"] = server_fields
 
             # Update the user
-            user.update(**dict_to_update)
+            user.update(new_profile=dict_to_update)
 
             click.echo(f"User {updateuser} updated")
             rv = 0

lib/pbench/client/__init__.py

@@ -2,7 +2,7 @@
 from enum import Enum
 from pathlib import Path
 import tarfile
-from typing import Iterator, Optional
+from typing import Any, Iterator, Optional
 from urllib import parse
 
 import requests
@@ -52,9 +52,6 @@ class API(Enum):
     DATASETS_SEARCH = "datasets_search"
     DATASETS_VALUES = "datasets_values"
     ENDPOINTS = "endpoints"
-    LOGIN = "login"
-    LOGOUT = "logout"
-    REGISTER = "register"
     SERVER_AUDIT = "server_audit"
     SERVER_CONFIGURATION = "server_configuration"
     UPLOAD = "upload"
@@ -208,7 +205,7 @@ def put(
         api: API,
         uri_params: Optional[JSONOBJECT] = None,
         *,
-        json: Optional[dict[str, str]] = None,
+        json: Optional[dict[str, Any]] = None,
         headers: Optional[dict[str, str]] = None,
         raise_error: bool = True,
         **kwargs,
@@ -243,7 +240,7 @@ def post(
         api: API,
         uri_params: Optional[JSONOBJECT] = None,
         *,
-        json: Optional[dict[str, str]] = None,
+        json: Optional[dict[str, Any]] = None,
         headers: Optional[dict[str, str]] = None,
         raise_error: bool = True,
         **kwargs,
@@ -442,6 +439,16 @@ def get_list(self, **kwargs) -> Iterator[Dataset]:
                 break
             json = self.get(uri=next_url).json()
 
+    def get_user(self, username: str, add_auth_header: bool = True) -> JSONOBJECT:
+        """ """
+        if add_auth_header:
+            return self.get(
+                api=API.USER, uri_params={"target_username": username}
+            ).json()
+        response = self.session.get(self._uri(API.USER, {"target_username": username}))
+        response.raise_for_status()
+        return response.json()
+
     def get_metadata(self, dataset_id: str, metadata: list[str]) -> JSONOBJECT:
         """Return requested metadata for a specified dataset.
 

lib/pbench/client/oidc_admin.py

@@ -102,17 +102,16 @@ def user_login(self, client_id: str, username: str, password: str) -> dict:
         data = {
             "client_id": client_id,
             "grant_type": "password",
-            "scope": "profile email",
+            "scope": "openid profile email",
             "username": username,
             "password": password,
         }
         return self.post(path=url_path, data=data).json()
 
-    def get_user(self, username: str, token: str) -> dict:
+    def get_user(self, token: str) -> dict:
         """Get the OIDC user representation dict.
 
         Args:
-            username: username to query
             token: access_token string to validate
 
         Returns:
@@ -132,10 +131,9 @@ def get_user(self, username: str, token: str) -> dict:
             }
         """
         response = self.get(
-            f"admin/realms/{self.OIDC_REALM}/users",
+            f"/realms/{self.OIDC_REALM}/protocol/openid-connect/userinfo",
             headers={"Authorization": f"Bearer {token}"},
-            username=username,
         )
         if response.status_code == HTTPStatus.OK:
-            return response.json()[0]
+            return response.json()
         return {}

lib/pbench/server/api/__init__.py

@@ -37,7 +37,7 @@
 from pbench.server.api.resources.server_audit import ServerAudit
 from pbench.server.api.resources.server_configuration import ServerConfiguration
 from pbench.server.api.resources.upload_api import Upload
-from pbench.server.api.resources.users_api import Login, Logout, RegisterUser, UserAPI
+from pbench.server.api.resources.users_api import UserAPI
 import pbench.server.auth.auth as Auth
 from pbench.server.database import init_db
 from pbench.server.database.database import Database
@@ -138,24 +138,6 @@ def register_endpoints(api: Api, app: Flask, config: PbenchServerConfig):
         endpoint="endpoints",
         resource_class_args=(config,),
     )
-    api.add_resource(
-        Login,
-        f"{base_uri}/login",
-        endpoint="login",
-        resource_class_args=(config,),
-    )
-    api.add_resource(
-        Logout,
-        f"{base_uri}/logout",
-        endpoint="logout",
-        resource_class_args=(config,),
-    )
-    api.add_resource(
-        RegisterUser,
-        f"{base_uri}/register",
-        endpoint="register",
-        resource_class_args=(config,),
-    )
     api.add_resource(
         ServerAudit,
         f"{base_uri}/server/audit",
@@ -174,6 +156,7 @@ def register_endpoints(api: Api, app: Flask, config: PbenchServerConfig):
         UserAPI,
         f"{base_uri}/user/<string:target_username>",
         endpoint="user",
+        resource_class_args=(config,),
     )
     api.add_resource(
         Upload,

lib/pbench/server/api/resources/__init__.py

@@ -391,6 +391,8 @@ def convert_username(value: Union[str, None], _) -> Union[str, None]:
         ) from e
 
     if not user:
+        # TODO: Should we change the status to FORBIDDEN as we dont want other
+        # users to know about the usernames in our db
         raise ConversionError(value, "username", http_status=HTTPStatus.NOT_FOUND)
 
     return str(user.id)
@@ -1484,13 +1486,11 @@ def _get_dataset_metadata(
         for i in requested_items:
             if Metadata.is_key_path(i, Metadata.METADATA_KEYS):
                 native_key = Metadata.get_native_key(i)
-                user_id = None
+                user: Optional[User] = None
                 if native_key == Metadata.USER:
-                    user_id = Auth.get_current_user_id()
+                    user = Auth.token_auth.current_user()
                 try:
-                    metadata[i] = Metadata.getvalue(
-                        dataset=dataset, key=i, user_id=user_id
-                    )
+                    metadata[i] = Metadata.getvalue(dataset=dataset, key=i, user=user)
                 except MetadataError:
                     metadata[i] = None
             else:

lib/pbench/server/api/resources/datasets_metadata.py

@@ -1,5 +1,5 @@
 from http import HTTPStatus
-from typing import Any
+from typing import Any, Optional
 
 from flask.json import jsonify
 from flask.wrappers import Request, Response
@@ -24,6 +24,7 @@
     Metadata,
     MetadataBadValue,
     MetadataError,
+    User,
 )
 
 
@@ -188,11 +189,11 @@ def _put(
         fail: dict[str, Any] = {}
         for k, v in metadata.items():
             native_key = Metadata.get_native_key(k)
-            user_id = None
+            user: Optional[User] = None
             if native_key == Metadata.USER:
-                user_id = Auth.get_current_user_id()
+                user = Auth.token_auth.current_user()
             try:
-                Metadata.setvalue(key=k, value=v, dataset=dataset, user_id=user_id)
+                Metadata.setvalue(key=k, value=v, dataset=dataset, user=user)
             except MetadataError as e:
                 fail[k] = str(e)