Feat/connect rtc #8

PhearZero · 2024-04-10T13:13:06Z

ℹ Overview

adds authenticated signaling service
adds shared core module to prevent encoding errors (to be replaced by algo-models in the future)
adds example demo for txn messages

📝 Related Issues

🛑 Blocked by

Feature: Rekeyed Account Support #2

✅ Acceptance:

Conventional Commits

# Conflicts: # package-lock.json # services/liquid-auth-api-js/package.json # services/liquid-auth-api-js/src/app.controller.ts

…ect-rtc # Conflicts: # package-lock.json # services/liquid-auth-api-js/src/connect/connect.controller.ts # services/liquid-auth-api-js/src/connect/connect.module.ts # sites/dapp-ui/src/pages/home/ConnectModal.tsx

- adds tests to the core library - adds ci for pull requests

…connect-rtc # Conflicts: # sites/dapp-ui/src/pages/home/ConnectModal.tsx

- removes awesome-algorand tweetnacl fork

ori-shem-tov · 2024-04-16T09:51:02Z

.decisions/3-Peer-to-Peer-Signaling.md

+
+- Limit dependency on WebSockets to signaling
+- Allow bidirectional communication between peers
+- Enforce locality of device?


do we enforce it?

We could if we remove the STUN/TURN servers but we would lose a decent percentage of devices behind symmetric NAT

is enforced locality even possible if apple / chrome sync passkeys across devices?

Not sure about chrome but apple really doesn't adhere to the standard well

https://www.slashid.dev/blog/passkeys-deepdive/#:~:text=Passkeys%20and%20traditional%20platform%20authenticator,event%20of%20a%20device%20compromise.

"impossible to verify the device type used to generate a key, and hence the trustworthiness of the key and its metadata. In fact, the lack of an attestation statement means that you can’t prove the key has been stored, or even generated, safely because you can’t infer properties/provenance of the authenticator."

@kylebeee Enforcing locality of the WebRTC peer channel, limiting discovery to only local addresses (no STUN/TURN)

if the decision is no enforcing locality we should remove this bullet IMO

ori-shem-tov · 2024-04-16T10:07:40Z

services/liquid-auth-api-js/src/main.ts

@@ -47,10 +56,12 @@ async function bootstrap() {
    ttl: 20000,
  });

+  app.enableCors();
+
  const sessionHandler = session({
    secret: 'my-secret',


Requires CORS for hosting as a third party API, this may be a requirement if projects can't integrate the service as a proxy/side-car. We can remove it for now but we should consider the following:

Hosting from a different domain than the rendered application

With a different domain, we require third-party cookies which are deprecated and will only have partial support by end of 2024 with eventual removal*

*Future techniques will involve device fingerprinting or assigning ID's from the service to the devices local storage. For now, it's a hard rule that the service must be the request layer and operate as a sidecar/proxy

sorry, I wasn't clear with my comment here. I was referring to secret: 'my-secret'. can we change it? reading from env maybe? something else?

We should definitely read this from the env, could be a new ticket

clients/liquid-auth-client-js/package.json

clients/liquid-auth-client-js/src/assertion.ts

clients/liquid-auth-core/src/encoding.ts

clients/liquid-auth-client-js/src/assertion.ts

clients/liquid-auth-client-js/tests/encoding.test.js

services/liquid-auth-api-js/src/app.service.ts

services/liquid-auth-api-js/src/signals/signals.interceptor.spec.ts

kylebeee

left a few comments, otherwise LGTM

kylebeee

Looks good

ori-shem-tov · 2024-04-22T07:36:54Z

clients/liquid-auth-client-js/src/assertion.ts

+  }
+  return await fetch(`/assertion/request/${credId}`, {
+    ...DEFAULT_FETCH_OPTIONS,
+  }).then((r) => {


any chance fetch throws an exception? I don't think we handle that if that's the case

Yes fetch can fail to get the URL, handling the error from the call site is good here since it's client/sdk code. We handle the invalid responses and let all other errors bubble to the call site.

ori-shem-tov · 2024-04-22T07:50:08Z

client-gen.sh

couldn't find any mention of this in the readme

This is mostly in the docs branch to limit merge conflicts with the unit tests

ori-shem-tov · 2024-04-22T09:13:01Z

clients/liquid-auth-client-js/src/connect.ts

+   * @param key
+   * @deprecated
+   */
+  sign(key: string | Account | Uint8Array | SignKeyPair): this {


why do we need signing capabilities in the client?

This interface is marked as deprecated. We will need to refactor it at some point for extension wallets but it's largely been replaced by the Signaling Client and the FIDO Liquid Extension

ori-shem-tov · 2024-04-22T09:27:45Z

clients/liquid-auth-client-js/src/signal.ts

+    super();
+    this.url = url;
+    this.socket = io(url, options);
+    globalThis.socket = this.socket;


why do we need to set the globalThis?

Convenience for using in projects, eventually it will be attached to the standard algorand namespace the other wallet providers use.

ori-shem-tov · 2024-04-22T09:30:32Z

clients/liquid-auth-client-js/src/signal.ts

+  }
+
+  static generateRequestId() {
+    //TODO: replace with toBase64URL(nacl.randomBytes(nacl.sign.seedLength)


shouldn't be hard to add

This needs some changes in the API and Android client, we could kick off a new ticket to harden the requestId

ori-shem-tov · 2024-04-22T09:44:34Z

clients/liquid-auth-client-js/src/signal.ts

+   * @param config
+   */
+  async peer(
+    requestId: any,


why not string?

It should be refactored to a encoded string when we harden the requestId. It's any to signal it's going to change/not stable API.

- fixes #11 - fixes #12

ori-shem-tov · 2024-04-24T14:58:50Z

LGTM

PhearZero and others added 5 commits February 15, 2024 16:54

docs: add webrtc decision

f0293a4

feat: session status and webvitals performance increase

0c7a935

feat: WebRTC with websocket signaling

77353cd

fix: session adapter

9683478

wip: adds basic message passing demo

6396c36

PhearZero added the enhancement New feature or request label Apr 10, 2024

PhearZero self-assigned this Apr 10, 2024

PhearZero added 6 commits April 10, 2024 15:58

Merge branch 'refs/heads/develop' into feat/connect-rtc

785b999

# Conflicts: # package-lock.json # services/liquid-auth-api-js/package.json # services/liquid-auth-api-js/src/app.controller.ts

fix: test stubs

da647ae

test: add signals unit tests

fb021a4

refactor: add core module and enforce style guides

1d88a24

chore: cleanup docker files

d1a61e0

PhearZero marked this pull request as ready for review April 12, 2024 13:03

PhearZero requested a review from ori-shem-tov April 12, 2024 13:04

chore: cleanup demo application

9203910

PhearZero force-pushed the feat/connect-rtc branch from 9799efb to 9203910 Compare April 12, 2024 13:07

PhearZero added 3 commits April 15, 2024 18:32

test: enforce linters and tests

102a55c

- adds tests to the core library - adds ci for pull requests

Merge remote-tracking branch 'refs/remotes/origin/develop' into feat/…

9ce32c4

…connect-rtc # Conflicts: # sites/dapp-ui/src/pages/home/ConnectModal.tsx

build: remove preinstall hooks

8d2f3f3

- removes awesome-algorand tweetnacl fork

ori-shem-tov requested review from kylebeee and HashMapsData2Value April 16, 2024 09:48

ori-shem-tov reviewed Apr 16, 2024

View reviewed changes

kylebeee reviewed Apr 16, 2024

View reviewed changes

services/liquid-auth-api-js/src/app.service.ts Show resolved Hide resolved

kylebeee reviewed Apr 16, 2024

View reviewed changes

services/liquid-auth-api-js/src/signals/signals.interceptor.spec.ts Show resolved Hide resolved

kylebeee reviewed Apr 16, 2024

View reviewed changes

services/liquid-auth-api-js/src/signals/signals.interceptor.spec.ts Show resolved Hide resolved

kylebeee reviewed Apr 16, 2024

View reviewed changes

PhearZero added 3 commits April 17, 2024 10:24

refactor: change call to offer

6c6a257

test: add signaling fixture for session

68e6cab

feat: js signaling client

af6b7e7

PhearZero force-pushed the feat/connect-rtc branch from 8a87888 to af6b7e7 Compare April 18, 2024 22:14

chore: lint fixes

e98cedd

PhearZero requested review from kylebeee and ori-shem-tov April 19, 2024 14:05

kylebeee approved these changes Apr 19, 2024

View reviewed changes

ori-shem-tov reviewed Apr 22, 2024

View reviewed changes

PhearZero and others added 5 commits April 22, 2024 09:14

docs: update contrib and top level compose

d59ff07

fix: patch service for docker

6cc3eef

build: add ngrok to docker

325edae

- fixes #11 - fixes #12

docs: cleanup config docs

3e34494

chore: cleanup files and update README

ce75747

PhearZero merged commit 0a37622 into develop Apr 24, 2024
2 checks passed

PhearZero deleted the feat/connect-rtc branch May 4, 2024 00:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/connect rtc #8

Feat/connect rtc #8

PhearZero commented Apr 10, 2024 •

edited

Loading

ori-shem-tov Apr 16, 2024

PhearZero Apr 16, 2024

kylebeee Apr 16, 2024 •

edited

Loading

PhearZero Apr 18, 2024

ori-shem-tov Apr 22, 2024

ori-shem-tov Apr 16, 2024

PhearZero Apr 16, 2024 •

edited

Loading

ori-shem-tov Apr 22, 2024

PhearZero Apr 22, 2024

kylebeee left a comment

kylebeee left a comment

ori-shem-tov Apr 22, 2024

PhearZero Apr 22, 2024

ori-shem-tov Apr 22, 2024

PhearZero Apr 22, 2024

ori-shem-tov Apr 22, 2024 •

edited

Loading

PhearZero Apr 22, 2024

ori-shem-tov Apr 22, 2024

PhearZero Apr 22, 2024

ori-shem-tov Apr 22, 2024

PhearZero Apr 22, 2024

ori-shem-tov Apr 22, 2024

PhearZero Apr 22, 2024

ori-shem-tov commented Apr 24, 2024

Feat/connect rtc #8

Feat/connect rtc #8

Conversation

PhearZero commented Apr 10, 2024 • edited Loading

ℹ Overview

📝 Related Issues

🛑 Blocked by

✅ Acceptance:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kylebeee Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhearZero Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kylebeee left a comment

Choose a reason for hiding this comment

kylebeee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ori-shem-tov Apr 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ori-shem-tov commented Apr 24, 2024

PhearZero commented Apr 10, 2024 •

edited

Loading

kylebeee Apr 16, 2024 •

edited

Loading

PhearZero Apr 16, 2024 •

edited

Loading

ori-shem-tov Apr 22, 2024 •

edited

Loading