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

Create new endpoint in Testflinger Server to authenticate clients and distribute tokens for submitting jobs with priority #335

Conversation

val500
Copy link
Contributor

@val500 val500 commented Aug 19, 2024

Description

In order to support authorized clients sending jobs to Testflinger with priority, we need some way to identify clients and their permissions. Every authorized client will have client_id and client-key(stored as a hash in the database) with a list of permissions.
This PR adds a new endpoint which takes in a client_id and client-key and returns an encoded list of their permissions in the form of a JWT:

{
    "exp": <datetime> # time that the token expires
    "iat": <datetime> # time that the token was generated
    "sub": <str> # description of token("access_token")
    "max_priority": {
        "queue_name": <int> # queue name to priority map
        ...
    }
}

The JWT is then signed with a secret key, provided by the environment variable JWT_SIGNING_KEY, which is set in the charm config. The client can then use this token to submit privileged jobs.

Resolved issues

Resolves https://warthogs.atlassian.net/browse/CERTTF-370

Documentation

Documentation for the new endpoint was added in the server README and documentation regarding the new environment variable was added to the server configuration docs.

Web service API changes

GET /v1/authenticate/token/<client_id>

  • Authorization
    • client_id must be provided in the url
    • client-key must be provided in a header with title "client-key"
    • Invalid client_id and client-key tested in test_v1.py
  • Returns status code 200 with correct credentials
  • Returns status code 401 with invalid credentials

Tests

Unit Tests were added to test_v1.py. These tests test JWT generation and the new endpoint, testing both valid and invalid credentials. A new test fixture was added to conftest.py to set up the mongo_app with permissions pre-populated in the database. In support of this, the "mongo_app" fixture was renamed to "mongo_app_fixture" to fix a linting issue where the "mongo_app" name was shadowed.

@val500 val500 marked this pull request as draft August 19, 2024 22:50
@val500 val500 requested review from plars and boukeas August 19, 2024 22:51
@val500 val500 force-pushed the CERTTF-370-Create-new-endpoint-in-Testflinger-Server-to-authenticate-clients-and-distribute-tokens-for-submitting-jobs-with-priority branch from fcd6312 to 93266de Compare August 19, 2024 22:54
server/src/api/v1.py Outdated Show resolved Hide resolved
@boukeas
Copy link
Contributor

boukeas commented Aug 21, 2024

General naming question: the use of the word "client" suggests (to me at least) that we are authenticating machines rather than users (I am using the term "user" here broadly, it could refer to teams as well). It's not too important and I might be wrong, I am just querying what the intention is, i.e. what we are trying to convey with the term.

server/src/api/v1.py Outdated Show resolved Hide resolved
server/src/api/v1.py Outdated Show resolved Hide resolved
server/src/api/v1.py Show resolved Hide resolved
server/src/api/v1.py Outdated Show resolved Hide resolved
@boukeas
Copy link
Contributor

boukeas commented Aug 23, 2024

General naming question: the use of the word "client" suggests (to me at least) that we are authenticating machines rather than users (I am using the term "user" here broadly, it could refer to teams as well). It's not too important and I might be wrong, I am just querying what the intention is, i.e. what we are trying to convey with the term.

Alright, after reading around a little bit I retract that comment. We could be authenticating users, machines or applications and the general term used for all these is indeed "client". And, in our case, we could really be authenticating users (submitting jobs) or agents (requesting jobs) so a general term is what we need.

@val500 val500 force-pushed the CERTTF-370-Create-new-endpoint-in-Testflinger-Server-to-authenticate-clients-and-distribute-tokens-for-submitting-jobs-with-priority branch 2 times, most recently from 6aee42d to 7e76408 Compare September 16, 2024 21:12
@val500 val500 force-pushed the CERTTF-370-Create-new-endpoint-in-Testflinger-Server-to-authenticate-clients-and-distribute-tokens-for-submitting-jobs-with-priority branch from b2c6537 to 98ea5c0 Compare September 17, 2024 21:24
@val500 val500 marked this pull request as ready for review September 17, 2024 21:26
Copy link
Collaborator

@plars plars left a comment

Choose a reason for hiding this comment

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

Looking good so far, I'll keep testing but I spotted a few things I think we should go ahead and change.

server/src/api/v1.py Outdated Show resolved Hide resolved
server/README.rst Outdated Show resolved Hide resolved
@val500 val500 force-pushed the CERTTF-370-Create-new-endpoint-in-Testflinger-Server-to-authenticate-clients-and-distribute-tokens-for-submitting-jobs-with-priority branch 2 times, most recently from d1714cc to 7f0275a Compare September 26, 2024 22:16
@val500 val500 force-pushed the CERTTF-370-Create-new-endpoint-in-Testflinger-Server-to-authenticate-clients-and-distribute-tokens-for-submitting-jobs-with-priority branch from 7f0275a to 39addbe Compare September 30, 2024 20:31
Copy link
Collaborator

@plars plars left a comment

Choose a reason for hiding this comment

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

Thanks for fixing it - github seemed to be choking on the need for a merge. I was able to successfully run it and get a jwt back. I just have a few more minor things here which should be real quick I think, and one thing you missed from the previous review. I'm looking forward to testing it out with the bits that make use of it!

server/src/api/v1.py Outdated Show resolved Hide resolved
server/src/api/v1.py Outdated Show resolved Hide resolved
docs/reference/testflinger-server-conf.rst Show resolved Hide resolved
server/README.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@plars plars left a comment

Choose a reason for hiding this comment

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

+1 thanks!

@val500 val500 merged commit 7e2e334 into main Oct 2, 2024
5 checks passed
@val500 val500 deleted the CERTTF-370-Create-new-endpoint-in-Testflinger-Server-to-authenticate-clients-and-distribute-tokens-for-submitting-jobs-with-priority branch October 2, 2024 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants