GET and DELETE method for managing api_key (#3410)

Add GET and DELETE methods to support API Key management

---------

Co-authored-by: siddardh <sira@redhat27!>

lib/pbench/client/__init__.py

@@ -346,8 +346,8 @@ def create_api_key(self):
         Creating an API key will cause the new key to be used instead of a
         normal login auth_token until the API key is removed.
         """
-        response = self.post(api=API.KEY)
-        self.api_key = response.json()["api_key"]
+        response = self.post(api=API.KEY, uri_params={"key": ""})
+        self.api_key = response.json()["key"]
         assert self.api_key, f"API key creation failed, {response.json()}"
 
     def remove_api_key(self):

lib/pbench/server/api/__init__.py

@@ -127,6 +127,8 @@ def register_endpoints(api: Api, app: Flask, config: PbenchServerConfig):
     api.add_resource(
         APIKeyManage,
         f"{base_uri}/key",
+        f"{base_uri}/key/",
+        f"{base_uri}/key/<string:key>",
         endpoint="key",
         resource_class_args=(config,),
     )

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

@@ -13,6 +13,9 @@
     ApiMethod,
     ApiParams,
     ApiSchema,
+    Parameter,
+    ParamType,
+    Schema,
 )
 import pbench.server.auth.auth as Auth
 from pbench.server.database.models.api_keys import APIKey, DuplicateApiKey
@@ -26,20 +29,75 @@ def __init__(self, config: PbenchServerConfig):
             ApiSchema(
                 ApiMethod.POST,
                 OperationCode.CREATE,
+                query_schema=Schema(
+                    Parameter("label", ParamType.STRING, required=False),
+                ),
+                audit_type=AuditType.API_KEY,
+                audit_name="apikey",
+                authorization=ApiAuthorizationType.NONE,
+            ),
+            ApiSchema(
+                ApiMethod.GET,
+                OperationCode.READ,
+                uri_schema=Schema(
+                    Parameter("key", ParamType.STRING, required=False),
+                ),
+                authorization=ApiAuthorizationType.NONE,
+            ),
+            ApiSchema(
+                ApiMethod.DELETE,
+                OperationCode.DELETE,
+                uri_schema=Schema(
+                    Parameter("key", ParamType.STRING, required=True),
+                ),
                 audit_type=AuditType.API_KEY,
                 audit_name="apikey",
                 authorization=ApiAuthorizationType.NONE,
             ),
         )
 
+    def _get(
+        self, params: ApiParams, request: Request, context: ApiContext
+    ) -> Response:
+        """Get a list of API keys associated with the user.
+
+        GET /api/v1/key
+
+        Returns:
+            Success: 200 with response containing the requested api_key
+                or list of api_key
+
+        Raises:
+            APIAbort, reporting "UNAUTHORIZED" or "NOT_FOUND"
+        """
+        user = Auth.token_auth.current_user()
+
+        if not user:
+            raise APIAbort(
+                HTTPStatus.UNAUTHORIZED,
+                "User provided access_token is invalid or expired",
+            )
+
+        key_id = params.uri.get("key")
+        if not key_id:
+            keys = APIKey.query(user=user)
+            return [key.as_json() for key in keys]
+
+        else:
+            key = APIKey.query(id=key_id, user=user)
+            if not key:
+                raise APIAbort(HTTPStatus.NOT_FOUND, "Requested key not found")
+            return key[0].as_json()
+
     def _post(
         self, params: ApiParams, request: Request, context: ApiContext
     ) -> Response:
         """
         Post request for generating a new persistent API key.
 
-        Required headers include
+        POST /api/v1/key?label=label
 
+        Required headers include
             Content-Type:   application/json
             Accept:         application/json
 
@@ -51,6 +109,12 @@ def _post(
             APIInternalError, reporting the failure message
         """
         user = Auth.token_auth.current_user()
+        label = params.query.get("label")
+
+        if context["raw_params"].uri:
+            raise APIAbort(
+                HTTPStatus.BAD_REQUEST, "Key cannot be specified by the user"
+            )
 
         if not user:
             raise APIAbort(
@@ -61,17 +125,51 @@ def _post(
             new_key = APIKey.generate_api_key(user)
         except Exception as e:
             raise APIInternalError(str(e)) from e
-
         try:
-            key = APIKey(api_key=new_key, user=user)
+            key = APIKey(key=new_key, user=user, label=label)
             key.add()
             status = HTTPStatus.CREATED
         except DuplicateApiKey:
             status = HTTPStatus.OK
         except Exception as e:
             raise APIInternalError(str(e)) from e
-
-        context["auditing"]["attributes"] = {"key": new_key}
-        response = jsonify({"api_key": new_key})
+        context["auditing"]["attributes"] = key.as_json()
+        response = jsonify(key.as_json())
         response.status_code = status
         return response
+
+    def _delete(
+        self, params: ApiParams, request: Request, context: ApiContext
+    ) -> Response:
+        """Delete the requested key.
+
+        DELETE /api/v1/key/{key}
+
+        Returns:
+            Success: 200
+
+        Raises:
+            APIAbort, reporting "UNAUTHORIZED" or "NOT_FOUND"
+            APIInternalError, reporting the failure message
+        """
+        key_id = params.uri["key"]
+        user = Auth.token_auth.current_user()
+
+        if not user:
+            raise APIAbort(
+                HTTPStatus.UNAUTHORIZED,
+                "User provided access_token is invalid or expired",
+            )
+        term = {"id": key_id}
+        if not user.is_admin():
+            term["user"] = user
+        keys = APIKey.query(**term)
+        if not keys:
+            raise APIAbort(HTTPStatus.NOT_FOUND, "Requested key not found")
+        key = keys[0]
+        try:
+            context["auditing"]["attributes"] = key.as_json()
+            key.delete()
+            return "deleted", HTTPStatus.OK
+        except Exception as e:
+            raise APIInternalError(str(e)) from e

lib/pbench/server/auth/auth.py

@@ -119,8 +119,8 @@ def verify_auth_api_key(api_key: str) -> Optional[User]:
         None if the api_key is not valid, a `User` object when the api_key is valid.
 
     """
-    key = APIKey.query(api_key)
-    return key.user if key else None
+    key = APIKey.query(key=api_key)
+    return key[0].user if key and len(key) == 1 else None
 
 
 def verify_auth_oidc(auth_token: str) -> Optional[User]:

lib/pbench/server/database/alembic/versions/1a91bc68d6de_update_api_key.py

@@ -0,0 +1,33 @@
+"""Update api_key table with "id" as primary_key
+
+
+Revision ID: 1a91bc68d6de
+Revises: 5679217a62bb
+Create Date: 2023-05-03 09:50:29.609672
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+# revision identifiers, used by Alembic.
+revision = "1a91bc68d6de"
+down_revision = "5679217a62bb"
+branch_labels = None
+depends_on = None
+
+
+def upgrade() -> None:
+    op.drop_constraint("api_keys_pkey", "api_keys", type_="primary")
+    op.execute("ALTER TABLE api_keys ADD COLUMN id SERIAL PRIMARY KEY")
+    op.add_column("api_keys", sa.Column("label", sa.String(length=128), nullable=True))
+    op.add_column("api_keys", sa.Column("key", sa.String(length=500), nullable=False))
+    op.create_unique_constraint("api_keys_key_unique", "api_keys", ["key"])
+    op.drop_column("api_keys", "api_key")
+
+
+def downgrade() -> None:
+    op.drop_constraint("api_keys_pkey", "api_keys", type_="primary")
+    op.drop_column("api_keys", "label")
+    op.drop_column("api_keys", "id")
+    op.drop_column("api_keys", "key")
+    op.create_primary_key("api_keys_pkey", "api_keys", ["api_key"])

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

@@ -2,9 +2,10 @@
 
 from flask import current_app
 import jwt
-from sqlalchemy import Column, ForeignKey, String
+from sqlalchemy import Column, ForeignKey, Integer, String
 from sqlalchemy.orm import relationship
 
+from pbench.server import JSONOBJECT
 from pbench.server.database.database import Database
 from pbench.server.database.models import decode_integrity_error, TZDateTime
 from pbench.server.database.models.users import User
@@ -47,16 +48,18 @@ class APIKey(Database.Base):
     """Model for storing the API key associated with a user."""
 
     __tablename__ = "api_keys"
-    api_key = Column(String(500), primary_key=True)
+    id = Column(Integer, primary_key=True, autoincrement=True)
+    key = Column(String(500), unique=True, nullable=False)
     created = Column(TZDateTime, nullable=False, default=TZDateTime.current_time)
+    label = Column(String(128), nullable=True)
     # ID of the owning user
     user_id = Column(String, ForeignKey("users.id"), nullable=False)
 
     # Indirect reference to the owning User record
     user = relationship("User")
 
     def __str__(self):
-        return f"API key {self.api_key}"
+        return f"API key {self.key}"
 
     def add(self):
         """Add an api_key object to the database."""
@@ -69,35 +72,49 @@ def add(self):
             decode_exc = decode_integrity_error(
                 e, on_duplicate=DuplicateApiKey, on_null=NullKey
             )
-            if decode_exc == e:
+            if decode_exc is e:
                 raise APIKeyError(str(e)) from e
             else:
                 raise decode_exc from e
 
     @staticmethod
-    def query(key: str) -> Optional["APIKey"]:
+    def query(**kwargs) -> Optional["APIKey"]:
         """Find the given api_key in the database.
 
         Returns:
-            An APIKey object if found, otherwise None
+            List of APIKey object if found, otherwise []
         """
-        return Database.db_session.query(APIKey).filter_by(api_key=key).first()
 
-    @staticmethod
-    def delete(api_key: str):
-        """Delete the given api_key.
+        return (
+            Database.db_session.query(APIKey)
+            .filter_by(**kwargs)
+            .order_by(APIKey.id)
+            .all()
+        )
 
-        Args:
-            api_key : the api_key to delete
-        """
-        dbs = Database.db_session
+    def delete(self):
+        """Remove the api_key instance from the database."""
         try:
-            dbs.query(APIKey).filter_by(api_key=api_key).delete()
-            dbs.commit()
+            Database.db_session.delete(self)
+            Database.db_session.commit()
         except Exception as e:
-            dbs.rollback()
+            Database.db_session.rollback()
             raise APIKeyError(f"Error deleting api_key from db : {e}") from e
 
+    def as_json(self) -> JSONOBJECT:
+        """Return a JSON object for this APIkey object.
+
+        Returns:
+            A JSONOBJECT with all the object fields mapped to appropriate names.
+        """
+        return {
+            "id": self.id,
+            "label": self.label,
+            "key": self.key,
+            "username": self.user.username,
+            "created": self.created.isoformat(),
+        }
+
     @staticmethod
     def generate_api_key(user: User):
         """Creates an `api_key` for the requested user

lib/pbench/test/unit/server/auth/test_auth.py

@@ -700,11 +700,11 @@ def tio_exc(token: str) -> JSON:
         with app.app_context():
             monkeypatch.setattr(oidc_client, "token_introspect", tio_exc)
             current_app.secret_key = jwt_secret
-            user = Auth.verify_auth(pbench_drb_api_key)
+            user = Auth.verify_auth(pbench_drb_api_key.key)
         assert user.id == DRB_USER_ID
 
     def test_verify_auth_api_key_invalid(
-        self, monkeypatch, rsa_keys, make_logger, pbench_drb_api_key_invalid
+        self, monkeypatch, rsa_keys, make_logger, pbench_invalid_api_key
     ):
         """Verify api_key verification via Auth.verify_auth() fails
         gracefully with an invalid token
@@ -723,5 +723,5 @@ def tio_exc(token: str) -> JSON:
         with app.app_context():
             monkeypatch.setattr(oidc_client, "token_introspect", tio_exc)
             current_app.secret_key = jwt_secret
-            user = Auth.verify_auth(pbench_drb_api_key_invalid)
+            user = Auth.verify_auth(pbench_invalid_api_key)
         assert user is None