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

Add authentication for system_restore_post #17

Merged
merged 1 commit into from
May 15, 2024

Conversation

robin-nitrokey
Copy link
Member

This patch fixes the API spec to add authentication to the POST /system/restore endpoint.

Fixes: #15

@robin-nitrokey
Copy link
Member Author

@dvzrv Can you please test if this fixes your issue?

@jans23
Copy link
Member

jans23 commented May 6, 2024

Shouldn't the OpenAPI spec be fixed in the original nethsm repo instead?

@robin-nitrokey
Copy link
Member Author

@jans23 Yes, but this PR allows us to test the changes before it is fixed upstream.

@robin-nitrokey
Copy link
Member Author

The corresponding NetHSM issue is:

@dvzrv
Copy link

dvzrv commented May 11, 2024

Without this change I get:

error: nethsm::Error - NetHSM API error: Restoring backup failed: Deseralization error (no message in body) (status code 401)

With this change I get:

error: nethsm::Error - NetHSM API error: Restoring backup failed: Body cannot be empty. (status code 400)

I am running restore against an "operational" container.

While the authentication error appears gone, I don't really know what to do about the 400, which is documented as

Bad request - restore did not apply.

@dvzrv
Copy link

dvzrv commented May 11, 2024

I am also getting a 400 when trying to restore an "unprovisioned" container from backup.

@dvzrv
Copy link

dvzrv commented May 11, 2024

Um, backup_file appears to be entirely unused in system_restore_post?!

@robin-nitrokey
Copy link
Member Author

It looks like we missed the code path for multi-part requests when replacing reqwest with ureq in #1. Let’s handle that in a different issue: #20.

This patch fixes the API spec to add authentication to the POST
/system/restore endpoint.

Fixes: #15
@robin-nitrokey robin-nitrokey merged commit a88b461 into main May 15, 2024
6 checks passed
@robin-nitrokey robin-nitrokey deleted the restore-basic-auth branch May 15, 2024 19:46
@ansiwen
Copy link
Collaborator

ansiwen commented May 17, 2024

FWIW: we implemented multipart after we changed to ureq, but because we only used the Rust SDK for the PKCS#11 module, which doesn't handle restore or updates, we didn't care. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use restore from backup due to missing authentication
4 participants