Skip to content

Expose AccountCredentials::private_key()#198

Merged
djc merged 2 commits intomainfrom
credentials-key-api
Oct 4, 2025
Merged

Expose AccountCredentials::private_key()#198
djc merged 2 commits intomainfrom
credentials-key-api

Conversation

@djc
Copy link
Owner

@djc djc commented Oct 4, 2025

Fixes #197. @cpu what do you think, is this misuse-resistant enough? I like how long we've been able to get buy with just AccountCredentials serialization, but I guess it is a bit of "security" by obscurity. Given that this is clearly labeled as private_key() (unless you've got better name suggestions) and yields the PrivateKeyDer type (and only a reference at that), seems like this might be okay?

@djc djc requested a review from cpu October 4, 2025 07:33
@notABot101010
Copy link

notABot101010 commented Oct 4, 2025

One minor inconsistency (which probably warrants its own issue) is that this method exposes PrivateKeyDer while AccountBuild::from_parts accepts a PrivatePkcs8KeyDer which is counterintuitive as we may expect the "export side" to give a more specific type (PrivatePkcs8KeyDer), and the "import side" to accept, maybe, a more generic type (PrivateKeyDer).

It might be nice to use the same type everywhere and maybe this PR could benefit from changing the type of AccountCredential.key_pkcs8 (and thus also of this getter) to the more specific PrivatePkcs8KeyDer.

Otherwise +1 for me.

Regarding the misuse-resistance, I understand the idea behind, but opaque types unfortunately directly clash with interoperability (with other programming languages for example).

@djc djc force-pushed the credentials-key-api branch from e5251f5 to 3d8157f Compare October 4, 2025 09:24
@djc
Copy link
Owner Author

djc commented Oct 4, 2025

One minor inconsistency (which probably warrants its own issue) is that this method exposes PrivateKeyDer while AccountBuild::from_parts accepts a PrivatePkcs8KeyDer which is counterintuitive as we may expect the "export side" to give a more specific type (PrivatePkcs8KeyDer), and the "import side" to accept, maybe, a more generic type (PrivateKeyDer).

That's fair -- added a commit to make the API more consistent.

@djc djc force-pushed the credentials-key-api branch from 3d8157f to 6ffbbb4 Compare October 4, 2025 09:26
Copy link
Collaborator

@cpu cpu left a comment

Choose a reason for hiding this comment

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

I think it's a reasonable addition and the sensitivity is obvious enough from the name. 👍

@djc djc merged commit f0b6f55 into main Oct 4, 2025
12 checks passed
@djc djc deleted the credentials-key-api branch October 4, 2025 13:49
@djc
Copy link
Owner Author

djc commented Oct 6, 2025

Released as 0.8.3.

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.

Add getter for the AccountCredentials key

3 participants