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

COSE Signed API #194

Open
wants to merge 13 commits into
base: development
Choose a base branch
from
Open

COSE Signed API #194

wants to merge 13 commits into from

Conversation

nodh
Copy link
Collaborator

@nodh nodh commented Nov 13, 2024

Adds a type parameter to CoseSigned. While not as thorough as for JwsSigned, it's still worth it to provide some form of guidance to clients as which payload to expect.

See also a-sit-plus/vck#165

@nodh nodh self-assigned this Nov 13, 2024
Copy link
Collaborator

@JesusMcCloud JesusMcCloud left a comment

Choose a reason for hiding this comment

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

Why does this behave differently from JwsSigned, that has the typed payload as property? should not the seriializer handle conversion from/to bytestringwrapper?

fun serialize() = coseCompliantSerializer.encodeToByteArray(this)
fun serialize(): ByteArray = coseCompliantSerializer.encodeToByteArray(CoseSignedSerializer(), this)

fun getTypedPayload(deserializationStrategy: DeserializationStrategy<P>): KmmResult<P?> = catching {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be lazy and a test that is not already based on a bytearray would be nice

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can't be lazy property since it needs a parameter.

@JesusMcCloud
Copy link
Collaborator

Why does this behave differently from JwsSigned, that has the typed payload as property? should not the seriializer handle conversion from/to bytestringwrapper?

In VCK, there's bytestringwrapper all over the place, i think we can do away with it and add this wrapping logic into the serializer, while we're at it

@JesusMcCloud
Copy link
Collaborator

@n0900 can this be made more convenient: as in: directly pass the typed payload to the constructor and have the serializer handle the byte string wrapping? I know it's different from your serialization foo, because it can by any type, but still…

@nodh
Copy link
Collaborator Author

nodh commented Nov 15, 2024

Best I can do with my knowledge of kotlinx serialization ... CoseSignedSerializer can't handle the payload directly, because it doesn't know about the type of the payload, since it's referenced by annotation on CoseSigned. In contrast, JwsSigned itself is not annotated with @Serializable, so it is easier there. I'm happy for any suggestions on how to solve the shortcomings.

@n0900 n0900 self-assigned this Nov 21, 2024
@n0900 n0900 removed their request for review November 21, 2024 09:08
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.

3 participants