Paths

Table of Contentst

Differential D11551

[protos] Replace challengeResponse with nonce and signature pair
ClosedPublic
Actions

Authored by bartek on Apr 4 2024, 6:59 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Sun, Dec 22, 4:32 AM

	Unknown Object (File)
	Sun, Dec 22, 4:32 AM

	Unknown Object (File)
	Sun, Dec 22, 4:32 AM

	Unknown Object (File)
	Sun, Dec 22, 4:32 AM

	Unknown Object (File)
	Fri, Dec 20, 5:09 AM

	Unknown Object (File)
	Tue, Dec 17, 2:29 PM

	Unknown Object (File)
	Nov 18 2024, 5:53 AM

	Unknown Object (File)
	Nov 3 2024, 11:41 AM

View All 43 Files

Subscribers

Details

Reviewers

ashoat
varun

Commits

rCOMMecd0073aef88: [protos] Replace challengeResponse with nonce and signature pair

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Apr 4 2024, 6:59 AM

bartek held this revision as a draft.

Herald added a subscriber: tomek. · View Herald TranscriptApr 4 2024, 6:59 AM

Harbormaster completed remote builds in B27974: Diff 38771.Apr 4 2024, 7:16 AM

bartek published this revision for review.Apr 4 2024, 7:16 AM

varun accepted this revision.Apr 4 2024, 11:38 AM

ashoat accepted this revision.Apr 5 2024, 6:40 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.Apr 5 2024, 6:40 AM

bartek mentioned this in D11570: [protos][identity] Add RPC for fetching multiple device lists.Apr 8 2024, 2:27 AM

bartek added a child revision: D11570: [protos][identity] Add RPC for fetching multiple device lists.Apr 8 2024, 2:28 AM

bartek added inline comments.Apr 12 2024, 3:25 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?

ashoat added inline comments.Apr 12 2024, 8:42 AM

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.

Closed by commit rCOMMecd0073aef88: [protos] Replace challengeResponse with nonce and signature pair (authored by bartek). · Explain WhyApr 15 2024, 1:44 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMMecd0073aef88: [protos] Replace challengeResponse with nonce and signature pair.

Revision Contents
Changeset List

Path

Size

lib/

types/

identity-service-types.js

10 lines

native/

cpp/

CommonCpp/

NativeModules/

CommRustModule.h

6 lines

CommRustModule.cpp

21 lines

_generated/

rustJSI-generated.cpp

identity-service/

identity-service-context-provider.react.js

9 lines

native_rust_library/

src/

identity/

31 lines

6 lines

qr-code/

qr-code-screen.react.js

15 lines

schema/

CommRustModuleSchema.js

6 lines

services/

commtest/

src/

identity/

17 lines

tests/

identity_access_tokens_tests.rs

5 lines

identity/

src/

client_service.rs

13 lines

62 lines

shared/

protos/

identity_unauth.proto

10 lines

web/

account/

qr-code-login.react.js

15 lines

grpc/

identity-service-client-wrapper.js

9 lines

protobufs/

identity-unauth-structs.cjs

identity-unauth-structs.cjs.flow

19 lines

Diff 39106

lib/types/identity-service-types.js

Loading...

native/cpp/CommonCpp/NativeModules/CommRustModule.h

Loading...

native/cpp/CommonCpp/NativeModules/CommRustModule.cpp

Loading...

native/cpp/CommonCpp/_generated/rustJSI-generated.cpp

Loading...

native/cpp/CommonCpp/_generated/rustJSI.h

Loading...

native/identity-service/identity-service-context-provider.react.js

Loading...

native/native_rust_library/src/identity/login.rs

Loading...

native/native_rust_library/src/lib.rs

Loading...

native/qr-code/qr-code-screen.react.js

Loading...

native/schema/CommRustModuleSchema.js

Loading...

services/commtest/src/identity/mod.rs

Loading...

services/commtest/tests/identity_access_tokens_tests.rs

Loading...

services/identity/src/client_service.rs

Loading...

services/identity/src/grpc_utils.rs

Loading...

shared/protos/identity_unauth.proto

Loading...

web/account/qr-code-login.react.js

Loading...

web/grpc/identity-service-client-wrapper.js

Loading...

web/protobufs/identity-unauth-structs.cjs

Loading...

web/protobufs/identity-unauth-structs.cjs.flow

Loading...