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

[FIX] this simply makes a curve key conform to the KeyPair interface by rejecting uses that are not applicable #38

Closed
wants to merge 3 commits into from

Conversation

aricart
Copy link
Member

@aricart aricart commented Dec 9, 2022

This change makes the interface for nkeys/curvekeys the same to allow tooling downstream to just deal with seeds/public keys properly.

I think this is a first cut at this issue. Internally it would be better if the old encode/decode/create were augmented to understand the curve key and delegate their initialization to them as necessary.

@aricart aricart requested review from derekcollison and philpennock and removed request for derekcollison December 9, 2022 15:11
@coveralls
Copy link

Pull Request Test Coverage Report for Build 165

  • 0 of 6 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.0%) to 77.98%

Changes Missing Coverage Covered Lines Changed/Added Lines %
xkeys.go 0 6 0.0%
Totals Coverage Status
Change from base Build 163: -1.0%
Covered Lines: 386
Relevant Lines: 495

💛 - Coveralls

@coveralls
Copy link

coveralls commented Dec 9, 2022

Pull Request Test Coverage Report for Build 184

  • 24 of 35 (68.57%) changed or added relevant lines in 5 files are covered.
  • 7 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.9%) to 78.035%

Changes Missing Coverage Covered Lines Changed/Added Lines %
nkeys.go 2 4 50.0%
public.go 0 9 0.0%
Files with Coverage Reduction New Missed Lines %
creds_utils.go 7 73.58%
Totals Coverage Status
Change from base Build 176: -0.9%
Covered Lines: 405
Relevant Lines: 519

💛 - Coveralls

Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

I prefer the compile time checking here for this one, and like when the code will point out when you are using a signing key vs encryption key.

errors.go Outdated Show resolved Hide resolved
errors.go Outdated Show resolved Hide resolved
@derekcollison
Copy link
Member

Maybe open up new PR on top pf fresh main.. Should have rebased this one vs merge.

gcolliso and others added 3 commits December 14, 2022 17:27
Signed-off-by: ainsley <[email protected]>
Since the JWT is read from a file that may be produced programmatically
(in one form or another), it is possible for there to be one or more
additional whitespace surrounding the JWT in the decorated file. This
simply ensure the whitespace is trimmed as is performed when parsing the
seed value.

Signed-off-by: Byron Ruth <[email protected]>
…by rejecting uses of Sign/Verify

[FIX] error messages as per PR

[FIX] unified the nkey and curve key interfaces
[FIX] made CreatePairWithRand delegate to CreateCurveKeysWithRand for curve prefixes
[FIX] made FromSeed delegate to FromCurveSeed for curve prefixes
@aricart aricart closed this Dec 14, 2022
@aricart aricart deleted the normalize-curve branch December 14, 2022 21:36
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.

5 participants