Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GET and DELETE method for managing api_key #3410

Merged
merged 11 commits into from
May 10, 2023
4 changes: 2 additions & 2 deletions lib/pbench/client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": ""})
webbnh marked this conversation as resolved.
Show resolved Hide resolved
self.api_key = response.json()["key"]
assert self.api_key, f"API key creation failed, {response.json()}"

def remove_api_key(self):
Expand Down
2 changes: 2 additions & 0 deletions lib/pbench/server/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,),
)
Expand Down
110 changes: 104 additions & 6 deletions lib/pbench/server/api/resources/api_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -26,20 +29,75 @@ def __init__(self, config: PbenchServerConfig):
ApiSchema(
ApiMethod.POST,
OperationCode.CREATE,
query_schema=Schema(
Parameter("label", ParamType.STRING, required=False),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default required value is False but I guess it's fine to be explicit.

),
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
webbnh marked this conversation as resolved.
Show resolved Hide resolved

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()
webbnh marked this conversation as resolved.
Show resolved Hide resolved

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

Expand All @@ -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(
Expand All @@ -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()
dbutenhof marked this conversation as resolved.
Show resolved Hide resolved
key.delete()
return "deleted", HTTPStatus.OK
except Exception as e:
raise APIInternalError(str(e)) from e
4 changes: 2 additions & 2 deletions lib/pbench/server/auth/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down
Original file line number Diff line number Diff line change
@@ -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")
webbnh marked this conversation as resolved.
Show resolved Hide resolved


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"])
51 changes: 34 additions & 17 deletions lib/pbench/server/database/models/api_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
webbnh marked this conversation as resolved.
Show resolved Hide resolved
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."""
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions lib/pbench/test/unit/server/auth/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -641,11 +641,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
Expand All @@ -664,5 +664,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
Loading