-
-
Notifications
You must be signed in to change notification settings - Fork 22
Implement PASERK serialization #44
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
base: main
Are you sure you want to change the base?
Conversation
|
Hey! I just wanted to say thanks for the PR. I'd been meaning to pick this up myself, so really appreciated 🙂 Have had a pretty busy couple of weeks so apologies for not getting to this yet, I'll give this a proper review as soon as I can. |
|
Hey, just a gentle reminder in case this went under |
|
@aidantwoods do you not had any time for this? :( |
|
This can be rebased without conflicts onto main/v2, but needs paths fixed, patch: diff --git a/paserk/keys.go b/paserk/keys.go
index ae0debd..04f1522 100644
--- a/paserk/keys.go
+++ b/paserk/keys.go
@@ -1,7 +1,7 @@
package paserk
import (
- "aidanwoods.dev/go-paseto"
+ "aidanwoods.dev/go-paseto/v2"
)
type Key interface {
diff --git a/paserk/paserk.go b/paserk/paserk.go
index 7e02929..114db7d 100644
--- a/paserk/paserk.go
+++ b/paserk/paserk.go
@@ -10,7 +10,7 @@ import (
"strconv"
"strings"
- "aidanwoods.dev/go-paseto"
+ "aidanwoods.dev/go-paseto/v2"
)
// type of PASERK token
diff --git a/paserk/paserk_test.go b/paserk/paserk_test.go
index d7f8ca4..c01aed2 100644
--- a/paserk/paserk_test.go
+++ b/paserk/paserk_test.go
@@ -1,7 +1,7 @@
package paserk
import (
- "aidanwoods.dev/go-paseto"
+ "aidanwoods.dev/go-paseto/v2"
"github.com/stretchr/testify/require"
"testing"
) |
|
Thanks, rebased |
|
Cleaned up the wording a bit ( |
|
Thanks for the hard work @minus7 and @aidantwoods! Happy new year! Just to chime in it would be amazing if you did get a chance to review this @aidantwoods |
| @@ -0,0 +1,45 @@ | |||
| package paserk | |||
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.
My only comment is it might be better to run the official test suite here.
You could use go generate to embed all the test vectors into the repo and run the tests.
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.
I could copy the JSON file into the repo like it's done for the PASETO test vectors and run those additionally
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.
Yep that would make a lot of sense. I only suggest go generate so that it's easy to update as the test vectors change for future versions, though that's a nice-to-have.
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.
Added them now.
Some of the test cases make no sense imo, like k2.secret-fail-2 which doesn't test PASERK itself. Instead, it seems to be meant to assert that a V4SymmetricKey can't be serialized as e.g. k1.local. Since the target type is inferred from the key itself, and it is tested whether a key serializes to the correct PASERK type by the other tests, I think it's fine that the test just does nothing (it hits the expected fail branch since the PASERK in the test data is empty). Same applies for the other expected-fail tests that are meant to test if a cut short key will serialize.
|
It would also be nice if PASERK-wrapped keys were JSON-(un)marshalable. |
Implement serialization and deserialization of v2/3/4 local/secret/public keys as well as key identifiers (lid/sid/pid). Co-authored-by: Daniel SUTTO <[email protected]>
|
This PR is complete and from what I can tell works flawlessly. Would be cool if this can land in master eventually o/ Since this PR targets v2 I have a v1 version at https://github.com/authenticvision/paseto-go/tree/paserk-v1 to experiment with. Changes to move this over to v1 are trivial though (just adjust the module path). (edit: confused v1 and v2) |
|
@minus7 |
Implement serialization and deserialization of v2/3/4 local/secret/public keys as well as key identifiers (lid/sid/pid).
Wrapped/encrypted keys are not supported.
I based this on @suttod's work (#3) and integrated the feedback you left there, @aidantwoods.