feat(webauthn): session messagepack support #427
Conversation
This adds MessagePack support to the SessionData struct.
WalkthroughThe changes refactor the definitions and serialization logic for Changes
Sequence Diagram(s)sequenceDiagram
participant RelyingParty
participant WebAuthnLib
participant msgp
RelyingParty->>WebAuthnLib: Initiate authentication ceremony
WebAuthnLib->>WebAuthnLib: Create SessionData (with CredentialParameter)
WebAuthnLib->>msgp: Marshal SessionData to MessagePack
msgp-->>WebAuthnLib: Serialized data
WebAuthnLib-->>RelyingParty: Store/return session data
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
webauthn/session_gen.go (3)
53-60: Potential slice-retention causing unnecessary heap growth
AllowedCredentialIDsis re-sliced in-place when the incoming array is smaller than the previous allocation:if cap(z.AllowedCredentialIDs) >= int(zb0002) { z.AllowedCredentialIDs = (z.AllowedCredentialIDs)[:zb0002] }If a very large slice is decoded first and subsequent messages are much smaller, the backing array (and all referenced credential IDs) stay on the heap for the lifetime of
SessionData, increasing RSS and GC pressure.Because this file is generated, a direct edit isn’t ideal, but consider adding the
-noresizeflag (or similar) in yourmsgpgeneration pipeline, or manually post-processing to:- z.AllowedCredentialIDs = (z.AllowedCredentialIDs)[:zb0002] + if int(float64(cap(z.AllowedCredentialIDs))*0.5) > int(zb0002) { + // allocate a fresh slice if we’re dramatically overshooting + z.AllowedCredentialIDs = make([][]byte, zb0002) + } else { + z.AllowedCredentialIDs = (z.AllowedCredentialIDs)[:zb0002] + }This prevents pathological memory retention while still favouring reuse for similar-sized payloads.
88-94: Map clearing retains large backing map
Extensionsis cleared withdeletein a loop. For messages whereExtensionsonce held many keys but now holds few (or none), the backing map still occupies the old bucket array.Again, since the file is generated, one workaround is to set
z.Extensions = make(map[string]interface{}, zb0004)unconditionally when the decoded size is less than, say, half oflen(z.Extensions). That keeps memory proportional to actual usage.Not critical, but worth noting if
SessionDataobjects are long-lived.
440-449: Minor size-estimate micro-optimisation
Msgsizeconverts the enum to string forlen(string(z.UserVerification)). Repeated allocations could be avoided by caching the conversion once:- s += 4 + msgp.TimeSize + 3 + msgp.StringPrefixSize + len(string(z.UserVerification)) + uv := string(z.UserVerification) + s += 4 + msgp.TimeSize + 3 + msgp.StringPrefixSize + len(uv)Negligible for most cases, so feel free to ignore.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (8)
protocol/options.go(0 hunks)protocol/options_credparam.go(1 hunks)protocol/options_credparam_gen.go(1 hunks)protocol/options_credparam_gen_test.go(1 hunks)webauthn/session.go(1 hunks)webauthn/session_gen.go(1 hunks)webauthn/session_gen_test.go(1 hunks)webauthn/types.go(0 hunks)
💤 Files with no reviewable changes (2)
- protocol/options.go
- webauthn/types.go
🧰 Additional context used
🧬 Code Graph Analysis (6)
protocol/options_credparam.go (2)
protocol/options.go (1)
CredentialType(83-83)protocol/webauthncose/webauthncose.go (1)
COSEAlgorithmIdentifier(281-281)
webauthn/session_gen_test.go (1)
webauthn/session.go (1)
SessionData(15-25)
protocol/options_credparam_gen_test.go (1)
protocol/options_credparam.go (1)
CredentialParameter(11-14)
webauthn/session.go (3)
protocol/authenticator.go (1)
UserVerificationRequirement(191-191)protocol/options.go (2)
Extensions(255-255)AuthenticationExtensions(94-94)protocol/options_credparam.go (1)
CredentialParameter(11-14)
protocol/options_credparam_gen.go (3)
protocol/options_credparam.go (1)
CredentialParameter(11-14)protocol/options.go (1)
CredentialType(83-83)protocol/webauthncose/webauthncose.go (1)
COSEAlgorithmIdentifier(281-281)
webauthn/session_gen.go (4)
webauthn/session.go (1)
SessionData(15-25)protocol/authenticator.go (1)
UserVerificationRequirement(191-191)protocol/options.go (1)
Extensions(255-255)protocol/options_credparam.go (1)
CredentialParameter(11-14)
🔇 Additional comments (9)
protocol/options_credparam.go (3)
3-4: Verify import path correctness.
Ensure that the pathimport "github.com/go-webauthn/webauthn/protocol/webauthncose"resolves to the intended package in your module.
5-7: Confirm code generation directives.
The directives//go:generate msgp //msgp:replace CredentialType with:string //msgp:replace webauthncose.COSEAlgorithmIdentifier with:intcorrectly instruct
msgpto treatCredentialTypeasstringand the COSE algorithm identifier asint.
12-13: Validate struct tags.
The tags`json:"type" msg:"type"` `json:"alg" msg:"alg"`match the JSON and MessagePack keys used throughout the codebase. Confirm they remain consistent with any external consumers of this data.
webauthn/session_gen_test.go (1)
1-124: Skip review of generated test file.
This file is autogenerated bymsgpand should not be manually edited.webauthn/session.go (2)
9-11: Ensure serialization directives correctly match types.
The directives//msgp:replace protocol.UserVerificationRequirement with:string //msgp:replace protocol.AuthenticationExtensions with:map[string]anymust align with the underlying definitions in
protocol(UserVerificationRequirementis a string alias, andAuthenticationExtensionsis a map).
15-25: Review struct field tags and ordering for compatibility.
All fields inSessionDatacarry both JSON andmsgtags:Challenge string `json:"challenge" msg:"challenge"` ... CredParams []protocol.CredentialParameter `json:"credParams,omitempty" msg:"params"`Verify that key names (
"challenge","rpid","uid","allowed","exp","uv","ext","params") and the use ofomitemptymaintain backward compatibility with existing session storage and APIs.protocol/options_credparam_gen_test.go (1)
1-124: Skip review of generated test file.
This file is autogenerated bymsgpand should not be manually edited.protocol/options_credparam_gen.go (1)
1-153: Skip review of generated serialization code.
This file is autogenerated bymsgpand should not be manually edited.webauthn/session_gen.go (1)
3-8: Autogenerated file – generally LGTMThe file is clearly generated by
msgpand adheres to the expected encoding/decoding contract forSessionData. No functional issues observed in the generated code itself.
This adds MessagePack support to the SessionData struct. MessagePack allows for serialized storage of data structures similar to JSON but it's faster and more compact. This makes it ideal for storage of session data which is expected to be very fast. It's made to be even faster by the use of the code generator, avoiding the need for runtime reflection in most cases.