-
Notifications
You must be signed in to change notification settings - Fork 7
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
Feature/api keys #1084
Feature/api keys #1084
Conversation
1e0cd7c
to
9c42595
Compare
9c42595
to
f6ab0a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Works well when I tested and I just left a few minor comments.
Apart from that, I find that when trying to insert an already existing key, it takes more than 60 seconds to return the response. I guess this is caused by repeated retrying to insert the key? Would be great if the check of the existence a key can be done at an earlier stage so that the user don't need to wait 60 seconds to get the error message.
Great catch @nanjiangshu! I was wondering why the integrations tests were even slower than usual 😅 |
- Add the structure - Add the database functions for inserting the key and its description in the encryption_keys table - Add the endpoint and the function for calling the db functions
Add unit test for adding key hash in the encryptio_keys table function
- The write time out of the http server was increased for proper response in case of duplicate - Spit the errors to confict and server error - Add logging - Update function comments
cba4dff
to
97893e3
Compare
24f92b3
to
440b590
Compare
440b590
to
b488b73
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Related issue(s) and PR(s)
This PR closes #1036.
Description
With this PR an endpoint for adding key hash in added in the api service.
An extra step of adding an api role in the database was necessary in order to make it work.
base64 -w0 c4gh.pub.pem
How to test
From the root of the folder run
make build-all
and then for linux run:
PR_NUMBER=$(/usr/bin/date '+%F') docker compose -f .github/integration/sda-s3-integration.yml run integration_test
while for mac run:
PR_NUMBER=$(/bin/date '+%F') docker compose -f .github/integration/sda-s3-integration.yml run integration_test
Then set the token as envvar:
export ACCESS_TOKEN=$(curl -s -k http://localhost:8080/tokens | jq -r '.[0]')
and run:
Confirm that the key hash is in the database by running:
docker exec -it postgres psql -U postgres -d sda
and then the queryselect * from sda.encryption_keys;