Details

Reviewers

varun
bartek

Commits

rCOMM68bbaa297a06: [Identity] Handle key payload

Summary

Allow for the key payload to be serialized and deserialized.

This is needed later for verifying that the payload is correctly
structured and a device is also identified by the identity public key.

Test Plan

cd services/identity
cargo test

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Clean up accidental additions

• jon requested review of this revision.Apr 4 2023, 3:25 PM

varun requested changes to this revision.Apr 5 2023, 6:22 PM

varun added inline comments.

services/identity/src/database.rs
40–46 ↗	(On Diff #24599)
49 ↗	(On Diff #24599)	DDB complains if we don't escape the payload?
51 ↗	(On Diff #24599)	How about impl FromStr for KeyPayload { type Err = serde_json::Error; fn from_str(s: &str) -> Result<Self, Self::Err> { serde_json::from_str(&s.replace(r#"\""#, r#"""#)) } }

This revision now requires changes to proceed.Apr 5 2023, 6:22 PM

Address feedback

services/identity/src/database.rs
49 ↗	(On Diff #24599)	no, that's just what is currently in DDB. I'm assuming how we create the payload on the JS side is creating an escaped payload
51 ↗	(On Diff #24599)	sure

varun added inline comments.

services/identity/src/database.rs
52 ↗	(On Diff #24725)
57–59 ↗	(On Diff #24725)	we're intentionally leaving this commented? we probably won't ever need this method since the payload is created on the client, right?

This revision is now accepted and ready to land.Apr 6 2023, 7:32 PM

• jon marked 2 inline comments as done.Apr 7 2023, 8:31 PM

• jon added inline comments.

services/identity/src/database.rs
57–59 ↗	(On Diff #24725)	no, looks like I forgot about this. we probably won't ever need this method since the payload is created on the client, right? Did it because of it felt "right" for serialize/deserialize, but yea, this should be pretty much immutable, so there's no need to "serialize" it.