Skip to content

IdCred enhancements#325

Merged
geonnave merged 4 commits intolake-rs:mainfrom
chrysn-pull-requests:id-cred-docs
Nov 25, 2024
Merged

IdCred enhancements#325
geonnave merged 4 commits intolake-rs:mainfrom
chrysn-pull-requests:id-cred-docs

Conversation

@chrysn
Copy link
Copy Markdown
Member

@chrysn chrysn commented Nov 25, 2024

Some small fixes I found when updating Ariel OS to use the latest lakers:

  • Some documentation on how IdCred is used would have helped, so I added an example that doubles as a doctest.
  • Implementing that I found a typo (KCSS instead of KCCS); the fix is API compatible by deprecating the old version.
  • The from_encoded_value constructor accepted some values that look like KIDs but are really longer. It may be a good idea to accept non-1-byte- KIDs, but this PR is designed for "all no-brainers so let's just merge it". I'm happy to provide longer KID support in there, but I don't know whether there's a limit on KID lengths in other places (won't hurt for this function to support more, though).
    [edit] Note that the way it is used internally (eg. decode_plaintext_3), the length is pre-checked to be correct due to decoder.any_as_encoded(), but that also means we're doing double work – still, a public function should do proper error handling.

@geonnave
Copy link
Copy Markdown
Collaborator

Thanks, just went through (including latest commit for larger KIDs) and is looking good. Happy to merge once CI is green.

@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented Nov 25, 2024

That latest commit was pushed by accident and is now in #326.

CI gets upset because apparently the doc comments get translated to C (cbindgen?), and there the compiler is not super happy about comments-in-comments (no way!); I'll look briefly into whether cbindgen can be made to play ball, but chances are I'll just remove those comments.

@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented Nov 25, 2024

The bindgen issue seems to be previously unnoticed; I'm adding a workaround.

@chrysn
Copy link
Copy Markdown
Member Author

chrysn commented Nov 25, 2024

I think this is ready now; fixing things on cbindgen would have been excessive, so the comments in the doctest are in a less-than-ideal position as a workaround.

@geonnave
Copy link
Copy Markdown
Collaborator

Looks good!

@geonnave geonnave merged commit eb32a07 into lake-rs:main Nov 25, 2024
@chrysn chrysn deleted the id-cred-docs branch November 25, 2024 19:14
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.

2 participants