Sync openid-connect branch with main

dashboard/src/actions/overviewActions.js

@@ -49,9 +49,7 @@ export const getDatasets = () => async (dispatch, getState) => {
       clearCachedSession(dispatch);
     } else {
       const msg = error.response?.data?.message;
-      dispatch(
-        showToast(DANGER, msg ? msg : `Error response: ERROR_MSG`)
-      );
+      dispatch(showToast(DANGER, msg ? msg : `Error response: ERROR_MSG`));
       dispatch({ type: TYPES.NETWORK_ERROR });
     }
   }

dashboard/src/utils/helper.js

@@ -4,3 +4,19 @@ export const uid = () => {
 
   return head + tail;
 };
+
+/**
+ * Expand a templated API URI like a Python `.format`
+ *
+ * @param {Object} endpoints - endpoint object from server
+ * @param {string} name - name of the API to expand
+ * @param {Object} args - value for each templated parameter
+ * @return {string} - formatted URI
+ */
+export const expandUriTemplate = (endpoints, name, args) => {
+  let uri = endpoints.uri[name].template;
+  for (const [key, value] of Object.entries(args)) {
+    uri = uri.replace(`{${key}}`, value);
+  }
+  return uri;
+};

lib/pbench/cli/server/user_management.py

@@ -32,12 +32,7 @@ def user_list(context: object) -> None:
         users = User.query_all()
 
         for user in users:
-            click.echo(
-                USER_LIST_ROW_FORMAT.format(
-                    user.username,
-                    user.id,
-                )
-            )
+            click.echo(USER_LIST_ROW_FORMAT.format(user.username, user.id))
 
         rv = 0
     except Exception as exc:

lib/pbench/client/__init__.py

@@ -321,7 +321,6 @@ def connect(self, headers: Optional[dict[str, str]] = None) -> None:
         self.endpoints = response.json()
         assert self.endpoints
 
-        # Create an OIDCAdmin object and confirm the connection was successful
         self.oidc_admin = OIDCAdmin(server_url=self.endpoints["openid"]["server"])
 
     def login(self, user: str, password: str):
@@ -439,16 +438,6 @@ 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/server/api/resources/__init__.py

@@ -1327,6 +1327,14 @@ def _check_authorization(self, mode: ApiAuthorization):
         role = mode.role
         authorized_user: User = Auth.token_auth.current_user()
 
+        username = "none"
+        if user_id:
+            user = User.query(id=user_id)
+            if user:
+                username = user.username
+            else:
+                current_app.logger.error("User ID {} not found", user_id)
+
         # The ADMIN authorization doesn't involve a target resource owner or
         # access, so take care of that first as a special case. If there is
         # an authenticated user, and that user holds ADMIN access rights, the
@@ -1348,9 +1356,10 @@ def _check_authorization(self, mode: ApiAuthorization):
         access = mode.access
 
         current_app.logger.debug(
-            "Authorizing {} access for {} to user (user id: {}) with access {} using {}",
+            "Authorizing {} access for {} to user {} ({}) with access {} using {}",
             role,
             authorized_user,
+            username,
             user_id,
             mode.access,
             mode.type,
@@ -1379,12 +1388,12 @@ def _check_authorization(self, mode: ApiAuthorization):
                 # An unauthenticated user is never allowed to access private
                 # data nor to perform an potential mutation of data: REJECT
                 current_app.logger.warning(
-                    "Attempt to {} user {} data without login", role, user_id
+                    "Attempt to {} user {} data without login", role, username
                 )
                 raise UnauthorizedAccess(
                     authorized_user,
                     role,
-                    user_id,
+                    username,
                     access,
                     HTTPStatus.UNAUTHORIZED,
                 )
@@ -1397,7 +1406,7 @@ def _check_authorization(self, mode: ApiAuthorization):
                     role,
                 )
                 raise UnauthorizedAccess(
-                    authorized_user, role, user_id, access, HTTPStatus.FORBIDDEN
+                    authorized_user, role, username, access, HTTPStatus.FORBIDDEN
                 )
             elif (
                 user_id
@@ -1411,10 +1420,10 @@ def _check_authorization(self, mode: ApiAuthorization):
                     "Unauthorized attempt by {} to {} user {} data",
                     authorized_user,
                     role,
-                    user_id,
+                    username,
                 )
                 raise UnauthorizedAccess(
-                    authorized_user, role, user_id, access, HTTPStatus.FORBIDDEN
+                    authorized_user, role, username, access, HTTPStatus.FORBIDDEN
                 )
             else:
                 # We have determined that there is an authenticated user with

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

@@ -458,7 +458,7 @@ def _get(
             if key == Metadata.USER:
                 if not auth_id:
                     continue
-                terms.append(table.user_id == auth_id)
+                terms.append(table.user_ref == auth_id)
             query = query.outerjoin(table, and_(*terms))
 
         if "start" in json and "end" in json:

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

@@ -23,7 +23,6 @@
     Metadata,
     MetadataBadValue,
     MetadataError,
-    User,
 )
 
 

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

@@ -178,9 +178,9 @@ def get(self):
         }
 
         try:
-            client = self.server_config.get("openid-connect", "client")
-            realm = self.server_config.get("openid-connect", "realm")
-            server = self.server_config.get("openid-connect", "server_url")
+            client = self.server_config.get("openid", "client")
+            realm = self.server_config.get("openid", "realm")
+            server = self.server_config.get("openid", "server_url")
         except (NoOptionError, NoSectionError):
             pass
         else:

lib/pbench/server/auth/__init__.py

@@ -262,9 +262,9 @@ def construct_oidc_client(cls, server_config: PbenchServerConfig) -> "OpenIDClie
             missing, or any of the required options are missing.
         """
         try:
-            server_url = server_config.get("openid-connect", "server_url")
-            client = server_config.get("openid-connect", "client")
-            realm = server_config.get("openid-connect", "realm")
+            server_url = server_config.get("openid", "server_url")
+            client = server_config.get("openid", "client")
+            realm = server_config.get("openid", "realm")
         except (NoOptionError, NoSectionError) as exc:
             raise OpenIDClient.NotConfigured() from exc
 

lib/pbench/server/database/alembic/env.py

@@ -60,7 +60,9 @@ def run_migrations_online() -> None:
 
         with connectable.connect() as connection:
             context.configure(
-                connection=connection, target_metadata=target_metadata, compare_type=True
+                connection=connection,
+                target_metadata=target_metadata,
+                compare_type=True,
             )
 
             with context.begin_transaction():

lib/pbench/server/database/alembic/versions/f628657bed56_user_table_update_oidc.py

@@ -11,7 +11,7 @@
 
 # revision identifiers, used by Alembic.
 revision = "f628657bed56"
-down_revision = "fa12f45a2a5a"
+down_revision = "9cc638cb865c"
 branch_labels = None
 depends_on = None
 
@@ -26,9 +26,6 @@ def upgrade() -> None:
         type_=sa.String(length=255),
         existing_nullable=False,
     )
-    op.alter_column(
-        "users", "username", existing_type=sa.VARCHAR(length=255), nullable=False
-    )
     op.create_primary_key("user_primary", "users", ["id"])
     op.drop_constraint("users_email_key", "users", type_="unique")
     op.drop_column("users", "role")
@@ -37,7 +34,7 @@ def upgrade() -> None:
     op.drop_column("users", "last_name")
     op.drop_column("users", "email")
     op.drop_column("users", "registered_on")
-    op.drop_column("active_tokens", "user_id")
+    op.drop_column("auth_tokens", "user_id")
     op.add_column("dataset_metadata", sa.Column("user_ref", sa.String(), nullable=True))
     op.create_foreign_key(None, "dataset_metadata", "users", ["user_ref"], ["id"])
     op.drop_column("dataset_metadata", "user_id")
@@ -83,9 +80,6 @@ def downgrade() -> None:
         ),
     )
     op.create_unique_constraint("users_email_key", "users", ["email"])
-    op.alter_column(
-        "users", "username", existing_type=sa.VARCHAR(length=255), nullable=False
-    )
     op.alter_column(
         "users",
         "id",
@@ -103,12 +97,12 @@ def downgrade() -> None:
     )
     op.drop_column("dataset_metadata", "user_ref")
     op.add_column(
-        "active_tokens",
+        "auth_tokens",
         sa.Column("user_id", sa.INTEGER(), autoincrement=False, nullable=False),
     )
     op.create_foreign_key(
-        "active_tokens_user_id_fkey",
-        "active_tokens",
+        "auth_tokens_user_id_fkey",
+        "auth_tokens",
         "users",
         ["user_id"],
         ["id"],

lib/pbench/server/database/models/auth_tokens.py

@@ -1,7 +1,6 @@
 from typing import Optional
 
-from sqlalchemy import Column, DateTime, ForeignKey, Integer, String
-from sqlalchemy.orm import relationship
+from sqlalchemy import Column, DateTime, Integer, String
 
 from pbench.server.database.database import Database
 
@@ -17,13 +16,6 @@ class AuthToken(Database.Base):
     id = Column(Integer, primary_key=True, autoincrement=True)
     token = Column(String(500), unique=True, nullable=False, index=True)
     expiration = Column(DateTime, nullable=False, index=True)
-    user_id = Column(
-        Integer,
-        ForeignKey("users.id", ondelete="CASCADE"),
-        nullable=False,
-        # no need to add index=True, all FKs have indexes
-    )
-    user = relationship("User", back_populates="auth_tokens")
 
     @staticmethod
     def query(auth_token: str) -> Optional["AuthToken"]:

lib/pbench/server/database/models/users.py

@@ -44,7 +44,7 @@ def __init__(self, user: "User", cause: str):
         self.cause = cause
 
     def __str__(self) -> str:
-        return f"Duplicate user setting in {self.user.get_json()}: {self.cause}"
+        return f"Duplicate user entry in {self.user.get_json()}: {self.cause}"
 
 
 class UserNullKey(UserError):
@@ -95,7 +95,9 @@ def get_json(self) -> JSONOBJECT:
         """
         return {"username": self.username, "id": self.id, "roles": self.roles}
 
-    def _decode(self, exception: IntegrityError) -> Exception:
+    def _decode(
+        self, exception: IntegrityError, operation: Optional[str] = ""
+    ) -> Exception:
         """Decode a SQLAlchemy IntegrityError to look for a recognizable UNIQUE
         or NOT NULL constraint violation.
 
@@ -114,7 +116,8 @@ def _decode(self, exception: IntegrityError) -> Exception:
             return UserDuplicate(self, cause)
         elif "NOT NULL constraint" in cause:
             return UserNullKey(self, cause)
-        return exception
+        else:
+            return UserSqlError(operation, self.get_json(), str(exception))
 
     @staticmethod
     def query(id: str = None, username: str = None) -> Optional["User"]:
@@ -146,9 +149,7 @@ def add(self):
             Database.db_session.commit()
         except Exception as e:
             Database.db_session.rollback()
-            if isinstance(e, IntegrityError):
-                raise self._decode(e) from e
-            raise UserSqlError("adding", self.get_json(), str(e)) from e
+            raise self._decode(e, "adding") from e
 
     def update(self, **kwargs):
         """Update the current user object with given keyword arguments."""
@@ -158,18 +159,7 @@ def update(self, **kwargs):
             Database.db_session.commit()
         except Exception as e:
             Database.db_session.rollback()
-            if isinstance(e, IntegrityError):
-                raise self._decode(e) from e
-            raise UserSqlError("Updating", self.get_json(), str(e)) from e
-
-    def delete(self):
-        """Delete the user with a given username, except admin."""
-        try:
-            Database.db_session.delete(self)
-            Database.db_session.commit()
-        except Exception as e:
-            Database.db_session.rollback()
-            raise UserSqlError("deleting", self.get_json(), str(e)) from e
+            raise self._decode(e, "updating") from e
 
     def is_admin(self) -> bool:
         """This method checks whether the given user has an admin role.

lib/pbench/test/functional/server/conftest.py

@@ -5,6 +5,7 @@
 
 from pbench.client import PbenchServerClient
 from pbench.client.oidc_admin import OIDCAdmin
+from pbench.server.auth import OpenIDClientError
 
 USERNAME: str = "tester"
 EMAIL: str = "[email protected]"
@@ -35,31 +36,36 @@ def server_client():
 def oidc_admin(server_client: PbenchServerClient):
     """
     Used by Pbench Server functional tests to get admin access
-    on OIDC server.
+    on the test OIDC server.
     """
     return OIDCAdmin(server_url=server_client.endpoints["openid"]["server"])
 
 
 @pytest.fixture(scope="session")
 def register_test_user(oidc_admin: OIDCAdmin):
     """Create a test user for functional tests."""
-    response = oidc_admin.create_new_user(
-        username=USERNAME,
-        email=EMAIL,
-        password=PASSWORD,
-        first_name=FIRST_NAME,
-        last_name=LAST_NAME,
-    )
-
-    # To allow testing outside our transient CI containers, allow the tester
-    # user to already exist.
-    assert (
-        response.ok or response.status_code == HTTPStatus.CONFLICT
-    ), f"Register failed with {response.json()}"
+    try:
+        response = oidc_admin.create_new_user(
+            username=USERNAME,
+            email=EMAIL,
+            password=PASSWORD,
+            first_name=FIRST_NAME,
+            last_name=LAST_NAME,
+        )
+    except OpenIDClientError as e:
+        # To allow testing outside our transient CI containers, allow the tester
+        # user to already exist.
+        if e.http_status != HTTPStatus.CONFLICT:
+            raise e
+        pass
+    else:
+        assert response.ok, f"Register failed with {response.json()}"
 
 
 @pytest.fixture
 def login_user(server_client: PbenchServerClient, register_test_user):
     """Log in the test user and return the authentication token"""
     server_client.login(USERNAME, PASSWORD)
     assert server_client.auth_token
+    yield
+    server_client.auth_token = None

lib/pbench/test/functional/server/test_connect.py

@@ -28,6 +28,5 @@ def test_connect(self, server_client: PbenchServerClient):
             assert e in endpoints["uri"].keys()
 
         # verify all the required openid-connect fields are present
-        if "openid" in endpoints:
-            expected = {"client", "realm", "server"}
-            assert set(endpoints["openid"]) >= expected
+        oid_ep = set(endpoints.get("openid", {}))
+        assert oid_ep >= {"client", "realm", "server"} or not oid_ep