From 18d5ea61a41e9a42b942064752943f6f22b90f72 Mon Sep 17 00:00:00 2001 From: Keith James Date: Mon, 17 Jun 2024 11:49:17 +0100 Subject: [PATCH 1/2] Add a Token model and return this instead of a bare string This makes the token creation endpoint better aligned with the others in behaviour (all now return JSON with correct Content-Type for both success and HTTP errors). The token JSON includes the relevant pipeline name and token description, an improvment over anonymous strings. --- src/npg_porch/db/data_access.py | 15 +++++------ src/npg_porch/db/models/token.py | 4 +-- src/npg_porch/endpoints/pipelines.py | 9 ++++--- src/npg_porch/models/token.py | 38 ++++++++++++++++++++++++++++ tests/pipeline_route_test.py | 8 +++--- 5 files changed, 57 insertions(+), 17 deletions(-) create mode 100644 src/npg_porch/models/token.py diff --git a/src/npg_porch/db/data_access.py b/src/npg_porch/db/data_access.py index 0bb2931..a55cf45 100644 --- a/src/npg_porch/db/data_access.py +++ b/src/npg_porch/db/data_access.py @@ -24,10 +24,9 @@ from sqlalchemy.orm import contains_eager, joinedload from sqlalchemy.orm.exc import NoResultFound -from npg_porch.db.models import Pipeline as DbPipeline, Task as DbTask, Event +from npg_porch.db.models import Pipeline as DbPipeline, Task as DbTask, Token as DbToken, Event from npg_porch.models import Task, Pipeline, TaskStateEnum - -from npg_porch.db.models import Token +from npg_porch.models.token import Token class AsyncDbAccessor: @@ -94,15 +93,15 @@ async def create_pipeline(self, pipeline: Pipeline) -> Pipeline: await session.commit() return pipe.convert_to_model() - - async def create_pipeline_token(self, name: str, desc: str) -> str: + async def create_pipeline_token(self, name: str, desc: str) -> Token: session = self.session db_pipeline = await self._get_pipeline_db_object(name) - token = Token(pipeline=db_pipeline, description=desc) - session.add(token) + db_token = DbToken(pipeline=db_pipeline, description=desc) + session.add(db_token) await session.commit() - return token.token + + return Token(name=db_pipeline.name, token=db_token.token, description=desc) async def create_task(self, token_id: int, task: Task) -> Task: diff --git a/src/npg_porch/db/models/token.py b/src/npg_porch/db/models/token.py index db9bdd2..aa3d725 100644 --- a/src/npg_porch/db/models/token.py +++ b/src/npg_porch/db/models/token.py @@ -29,11 +29,11 @@ class Token(Base): ''' - A string token ussued to client applications for the purpose of + A string token issued to client applications for the purpose of authorizing them to perform certain actions. ''' - def random_token(): + def random_token(self): ''' Returns a 32 characters long random string. The chance of a collision is small. diff --git a/src/npg_porch/endpoints/pipelines.py b/src/npg_porch/endpoints/pipelines.py index efe4d2f..9a9588f 100644 --- a/src/npg_porch/endpoints/pipelines.py +++ b/src/npg_porch/endpoints/pipelines.py @@ -29,7 +29,7 @@ from npg_porch.models.permission import RolesEnum from npg_porch.db.connection import get_DbAccessor from npg_porch.auth.token import validate - +from npg_porch.models.token import Token router = APIRouter( prefix="/pipelines", @@ -86,7 +86,8 @@ async def get_pipeline( @router.post( "/{pipeline_name}/token/{token_desc}", - response_model=str, + response_model=Token, + status_code=status.HTTP_201_CREATED, responses={ status.HTTP_201_CREATED: {"description": "Token was created"}, status.HTTP_404_NOT_FOUND: {"description": "Pipeline not found"}, @@ -101,13 +102,13 @@ async def create_pipeline_token( token_desc: str, db_accessor=Depends(get_DbAccessor), permissions=Depends(validate) -) -> Response: +) -> Token: try: token = await db_accessor.create_pipeline_token(name=pipeline_name, desc=token_desc) except NoResultFound: raise HTTPException(status_code=404, detail=f"Pipeline '{pipeline_name}' not found") - return Response(status_code=status.HTTP_201_CREATED, content=token, media_type="text/plain") + return token @router.post( diff --git a/src/npg_porch/models/token.py b/src/npg_porch/models/token.py new file mode 100644 index 0000000..c84e0dd --- /dev/null +++ b/src/npg_porch/models/token.py @@ -0,0 +1,38 @@ +# Copyright (C) 2024 Genome Research Ltd. +# +# This file is part of npg_porch +# +# npg_porch is free software: you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the Free +# Software Foundation; either version 3 of the License, or (at your option) any +# later version. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program. If not, see . + +from pydantic import BaseModel, ConfigDict, Field + + +class Token(BaseModel): + model_config = ConfigDict(extra='forbid') + + name: str = Field( + default=None, + title='Pipeline Name', + description='A user-controlled name of an existing pipeline' + ) + description: str | None = Field( + default=None, + title='Description', + description='A user-controlled description of the token' + ) + token: str | None = Field( + default=None, + title='Token', + description='An authorisation token' + ) diff --git a/tests/pipeline_route_test.py b/tests/pipeline_route_test.py index 606eb2f..19feab4 100644 --- a/tests/pipeline_route_test.py +++ b/tests/pipeline_route_test.py @@ -173,8 +173,8 @@ def test_create_pipeline_token(async_minimum, fastapi_testclient): # Create a pipeline desired_pipeline = Pipeline( name=pipeline_name, - uri='http://test.com', - version='1' + uri="http://test.com", + version="1" ) http_create_pipeline(fastapi_testclient, desired_pipeline) @@ -189,7 +189,9 @@ def test_create_pipeline_token(async_minimum, fastapi_testclient): ) assert response.status_code == status.HTTP_201_CREATED - assert len(response.content.decode(encoding="utf-8")) == 32 + assert response.json()["name"] == pipeline_name + assert response.json()["description"] == token_desc + assert len(response.json()["token"]) == 32 # Create a new token for a non-existent pipeline response = fastapi_testclient.post( From 15e2adca8458024a13f9ec88704a1e49b1f18432 Mon Sep 17 00:00:00 2001 From: Keith James Date: Mon, 17 Jun 2024 14:34:11 +0100 Subject: [PATCH 2/2] Update user guide for tokens --- docs/user_guide.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/user_guide.md b/docs/user_guide.md index 4d261a8..169eb55 100644 --- a/docs/user_guide.md +++ b/docs/user_guide.md @@ -32,6 +32,16 @@ Authorisation tokens are specific to a pipeline and more than one token can be i `curl -L -X POST -H "Authorization: Bearer $ADMIN_TOKEN" https://$SERVER:$PORT/pipelines/$PIPELINE_NAME/token/$TOKEN_DESCRIPTION` +The server will respond with a JSON document containing the new bearer token which you may use for subsequent pipeline-specific communication: + +```javascript +{ + "name": "$PIPELINE_NAME", + "description": "$TOKEN_DESCRIPTION", + "token": "$TOKEN" +} +``` + ### Step 1 - register your pipeline with npg_porch *Schema: npg_porch.model.pipeline*