Skip to content
Prev Previous commit
Next Next commit
Renamed "name" field to "label" in db and added unit tests
  • Loading branch information
siddardh committed May 10, 2023
commit 9547a736183d47e6ea7efd796e877e31db7e2413
14 changes: 6 additions & 8 deletions lib/pbench/server/api/resources/api_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def __init__(self, config: PbenchServerConfig):
ApiMethod.POST,
OperationCode.CREATE,
query_schema=Schema(
Parameter("name", ParamType.STRING, required=False),
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",
Expand Down Expand Up @@ -80,23 +80,21 @@ def _get(
key_id = params.uri.get("key")
if not key_id:
keys = APIKey.query(user=user)
response = [key.as_json() for key in keys]
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")
response = key[0].as_json()

return response
return key[0].as_json()

def _post(
self, params: ApiParams, request: Request, context: ApiContext
) -> Response:
"""
Post request for generating a new persistent API key.

POST /api/v1/key?name=name
POST /api/v1/key?label=label

Required headers include
Content-Type: application/json
Expand All @@ -110,7 +108,7 @@ def _post(
APIInternalError, reporting the failure message
"""
user = Auth.token_auth.current_user()
name = params.query.get("name")
label = params.query.get("label")

if not user:
raise APIAbort(
Expand All @@ -122,7 +120,7 @@ def _post(
except Exception as e:
raise APIInternalError(str(e)) from e
try:
key = APIKey(key=new_key, user=user, name=name)
key = APIKey(key=new_key, user=user, label=label)
key.add()
status = HTTPStatus.CREATED
except DuplicateApiKey:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@
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("name", sa.String(length=128), nullable=True))
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(None, "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", "name")
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"])
4 changes: 2 additions & 2 deletions lib/pbench/server/database/models/api_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class APIKey(Database.Base):
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)
name = Column(String(128), nullable=True)
label = Column(String(128), nullable=True)
# ID of the owning user
user_id = Column(String, ForeignKey("users.id"), nullable=False)

Expand Down Expand Up @@ -109,7 +109,7 @@ def as_json(self) -> JSONOBJECT:
"""
return {
"id": self.id,
"name": self.name,
"label": self.label,
"key": self.key,
"username": self.user.username,
"created": self.created.isoformat(),
Expand Down
17 changes: 10 additions & 7 deletions lib/pbench/test/unit/server/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ def pbench_drb_api_key(client, server_config, create_drb_user):
"""Valid api_key for the 'drb' user"""
return generate_api_key(
username="drb",
name="drb_key",
label="drb_key",
user=create_drb_user,
)

Expand All @@ -844,9 +844,7 @@ def pbench_drb_api_key(client, server_config, create_drb_user):
def pbench_drb_secondary_api_key(client, server_config, create_drb_user):
"""Create secondary api_key for the 'drb' user"""
return generate_api_key(
username="drb",
name="secondary_key",
user=create_drb_user,
username="drb", label="secondary_key", user=create_drb_user, offset=True
)


Expand Down Expand Up @@ -1006,8 +1004,9 @@ def tarball(tmp_path):

def generate_api_key(
username: str,
name: str,
label: str,
user: Optional[User] = None,
offset: bool = False,
) -> APIKey:
"""Generates an api_key which will be mimic of how POST v1/generate_key call works.

Expand All @@ -1020,6 +1019,8 @@ def generate_api_key(
name: name or description of the key.
user: user attributes will be extracted from the user object to include
in the token payload.
offset: If True, 10 min will be added to 'current_utc',to avoid duplicate error
while generating multiple api_key to a user

Returns:
APIKey Object
Expand All @@ -1031,13 +1032,15 @@ def generate_api_key(
user = User.query(username=username)
assert user

if offset:
current_utc = current_utc + datetime.timedelta(minutes=10)

payload = {
"iat": current_utc,
"user_id": user.id,
"username": user.username,
"name": name,
}
key = jwt.encode(payload, jwt_secret, algorithm="HS256")
api_key = APIKey(key=key, user=user, name=name)
api_key = APIKey(key=key, user=user, label=label)
api_key.add()
return api_key
24 changes: 18 additions & 6 deletions lib/pbench/test/unit/server/database/test_api_keys_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,31 +26,43 @@ def fake_db(self, monkeypatch, server_config):
def test_construct(self, db_session, create_drb_user, create_user):
"""Test api_key constructor"""

api_key = APIKey(key="generated_api_key", user=create_user, name="test_api_key")
api_key = APIKey(
key="generated_api_key", user=create_user, label="test_api_key"
)
api_key.add()

assert api_key.key == "generated_api_key"
assert api_key.user.id is create_user.id
assert api_key.name == "test_api_key"
assert api_key.label == "test_api_key"

def test_query(
self,
db_session,
pbench_drb_api_key,
pbench_drb_secondary_api_key,
create_drb_user,
create_user,
):
"""Test that we are able to query api_key by user"""

api_key = APIKey(
key="generated_api_key", user=create_user, label="test_api_key"
)
api_key.add()

assert api_key.key == "generated_api_key"
assert api_key.user.id is create_user.id
assert api_key.label == "test_api_key"

key_list = APIKey.query(user=create_drb_user)
assert len(key_list) == 2

assert key_list[0].user.id == pbench_drb_api_key.user.id
assert key_list[0].name == pbench_drb_api_key.name
assert key_list[0].user.id == create_drb_user.id
assert key_list[0].label == pbench_drb_api_key.label
assert key_list[0].id == pbench_drb_api_key.id
assert key_list[0].key == pbench_drb_api_key.key
assert key_list[1].user.id == pbench_drb_secondary_api_key.user.id
assert key_list[1].name == pbench_drb_secondary_api_key.name
assert key_list[1].user.id == create_drb_user.id
assert key_list[1].label == pbench_drb_secondary_api_key.label
assert key_list[1].id == pbench_drb_secondary_api_key.id
assert key_list[1].key == pbench_drb_secondary_api_key.key

Expand Down
41 changes: 28 additions & 13 deletions lib/pbench/test/unit/server/test_api_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ def query_post_as(self, client, server_config):
"""

def query_api(
user_token, expected_status: HTTPStatus, name: Optional[str] = "new_key"
user_token, expected_status: HTTPStatus, label: Optional[str] = "new_key"
) -> requests.Response:
headers = {"authorization": f"bearer {user_token}"}
payload = {"name": name} if name is not None else {}
payload = {"label": label} if label is not None else {}

response = client.post(
f"{server_config.rest_uri}/key",
Expand Down Expand Up @@ -100,16 +100,16 @@ def test_successful_api_key_generation_with_name(
assert audit[1].user_name == "drb"
assert audit[1].reason is None
assert audit[1].attributes["id"] == response.json["id"]
assert audit[1].attributes["name"] == response.json["name"]
assert audit[1].attributes["label"] == response.json["label"]

def test_successful_api_key_generation_without_name(
def test_successful_api_key_generation_without_label(
self, query_post_as, pbench_drb_token
):
response = query_post_as(pbench_drb_token, HTTPStatus.CREATED, name=None)
response = query_post_as(pbench_drb_token, HTTPStatus.CREATED, label=None)
assert response.json["key"]
audit = Audit.query()
assert audit[1].attributes["id"] == response.json["id"]
assert audit[1].attributes["name"] is None
assert audit[1].attributes["label"] is None


class TestAPIKeyGet:
Expand All @@ -125,7 +125,7 @@ def query_get_as(self, client, server_config):
"""

def query_api_get(
user_token, expected_status: HTTPStatus, key_id: Optional[int] = None
user_token, expected_status: HTTPStatus, key_id: Optional[str] = None
) -> requests.Response:
headers = {"authorization": f"bearer {user_token}"}
uri = f"{server_config.rest_uri}/key"
Expand All @@ -148,13 +148,13 @@ def test_successful_api_key_get(

assert response.json[0]["username"] == pbench_drb_api_key.user.username
assert response.json[0]["id"] == pbench_drb_api_key.id
assert response.json[0]["name"] == pbench_drb_api_key.name
assert response.json[0]["label"] == pbench_drb_api_key.label
assert response.json[0]["key"] == pbench_drb_api_key.key
assert (
response.json[1]["username"] == pbench_drb_secondary_api_key.user.username
)
assert response.json[1]["id"] == pbench_drb_secondary_api_key.id
assert response.json[1]["name"] == pbench_drb_secondary_api_key.name
assert response.json[1]["label"] == pbench_drb_secondary_api_key.label
assert response.json[1]["key"] == pbench_drb_secondary_api_key.key

def test_unauthorized_get(self, query_get_as, pbench_drb_token_invalid):
Expand Down Expand Up @@ -224,7 +224,11 @@ def query_api_delete(
return query_api_delete

def test_delete_api_key(
self, query_delete_as, pbench_drb_token, pbench_drb_api_key
self,
query_delete_as,
pbench_drb_token,
pbench_drb_api_key,
pbench_drb_secondary_api_key,
):

# we can find it
Expand Down Expand Up @@ -263,9 +267,10 @@ def test_delete_api_key(
assert audit[1].user_name == "drb"
assert audit[1].reason is None
assert audit[1].attributes["id"] == pbench_drb_api_key.id
assert audit[1].attributes["name"] == pbench_drb_api_key.name

assert audit[1].attributes["label"] == pbench_drb_api_key.label
assert APIKey.query(id=pbench_drb_api_key.id) == []
keys = APIKey.query(id=pbench_drb_secondary_api_key.id)
assert keys[0].key == pbench_drb_secondary_api_key.key

def test_unauthorized_delete(
self, query_delete_as, pbench_drb_token_invalid, pbench_drb_api_key
Expand All @@ -279,6 +284,8 @@ def test_unauthorized_delete(
assert response.json == {
"message": "User provided access_token is invalid or expired"
}
keys = APIKey.query(id=pbench_drb_api_key.id)
assert keys[0].id == pbench_drb_api_key.id

def test_delete_api_key_notfound(
self, query_delete_as, pbench_drb_token, pbench_invalid_api_key
Expand Down Expand Up @@ -312,9 +319,15 @@ def test_delete_api_key_fail(
get_token_func("test"), pbench_drb_api_key.id, HTTPStatus.NOT_FOUND
)
assert response.json == {"message": "Requested key not found"}
keys = APIKey.query(id=pbench_drb_api_key.id)
assert keys[0].id == pbench_drb_api_key.id

def test_delete_api_key_by_admin(
self, query_delete_as, pbench_admin_token, pbench_drb_api_key
self,
query_delete_as,
pbench_admin_token,
pbench_drb_api_key,
pbench_drb_secondary_api_key,
):

# we can find it
Expand All @@ -326,3 +339,5 @@ def test_delete_api_key_by_admin(
)
assert response.json == "deleted"
assert APIKey.query(id=pbench_drb_api_key.id) == []
keys = APIKey.query(id=pbench_drb_secondary_api_key.id)
assert keys[0].key == pbench_drb_secondary_api_key.key