Page MenuHomePhabricator

[protos] Replace challengeResponse with nonce and signature pair
ClosedPublic

Authored by bartek on Thu, Apr 4, 6:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 10, 12:15 PM
Unknown Object (File)
Wed, Apr 10, 11:29 AM
Unknown Object (File)
Wed, Apr 10, 6:15 AM
Unknown Object (File)
Mon, Apr 8, 9:35 PM
Unknown Object (File)
Mon, Apr 8, 6:38 AM
Unknown Object (File)
Thu, Apr 4, 7:55 AM
Unknown Object (File)
Thu, Apr 4, 7:53 AM
Unknown Object (File)
Thu, Apr 4, 7:40 AM
Subscribers

Details

Summary

Addresses option 3 from ENG-7641.
Replaced challenge_response with separate nonce and nonce_signature fields.
Updated all usages

Depends on D11549

Test Plan
  • Everything compiles
  • Updated unit and integration tests
  • Tested QR Auth on native and web

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Thu, Apr 4, 7:16 AM
ashoat added inline comments.
native/qr-code/qr-code-screen.react.js
58 ↗(On Diff #38771)

Interesting that we're changing the approach here and signing the nonce directly instead of signing JSON.stringify({ nonce })

This revision is now accepted and ready to land.Fri, Apr 5, 6:40 AM
native/qr-code/qr-code-screen.react.js
58 ↗(On Diff #38771)

We agreed on changing this approach in ENG-7641, didn't we?

native/qr-code/qr-code-screen.react.js
58 ↗(On Diff #38771)

I interpreted that task to be about the proto format. Not sure where we decided about the string that gets signed (nonce vs. JSON.stringify({ nonce }))

It's probably not very important. My first thought was that the JSON.stringify({ nonce }) is more flexible if we want to change the format. But then I considered that changing the format wouldn't be that hard anyways, since this nonce is only used in an ephemeral way, to validate the session.