Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Knocking support #2281

Merged
merged 22 commits into from
Apr 23, 2024
Merged

Knocking support #2281

merged 22 commits into from
Apr 23, 2024

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented Apr 2, 2024

This PR does three things seperated in three commits (review can be done per commit)

  • Add joining with knocking.
  • Refactor Encryption information using one hook.
  • Change recent list to matrixRTC (it was dependant on the m.call event before)

Overall this makes EC fully compatible with join links created with EW in rooms that use the join rule Knock (not public)
It also enables the EW user to kick or ban users from the call as desired.

To test this EW needs to have:

  • The guest_spa_url set. (element_call:guest_spa_url)
  • The following feature flags activated
    • "feature_element_call_video_rooms": true,
    • "feature_video_rooms": true,
    • "feature_new_room_decoration_ui": true,
    • "feature_group_calls": true,
    • "feature_ask_to_join": true

(full config

    "features": {
       "feature_element_call_video_rooms": true,
       "feature_video_rooms": true,
       "feature_new_room_decoration_ui": true,
       "feature_group_calls": true,
       "feature_ask_to_join": true
   },

   "element_call": {
       "url": "https://localhost:3000",
       "guest_spa_url": "https://localhost:3000"
   }

)

Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for such a large review, there's just a lot to look through here. Here goes!

src/settings/submit-rageshake.ts Show resolved Hide resolved
src/e2ee/sharedKeyManagement.ts Outdated Show resolved Hide resolved
src/e2ee/sharedKeyManagement.ts Outdated Show resolved Hide resolved
src/home/useGroupCallRooms.ts Show resolved Hide resolved
src/home/useGroupCallRooms.ts Outdated Show resolved Hide resolved
src/room/GroupCallLoader.tsx Outdated Show resolved Hide resolved
src/room/GroupCallView.tsx Outdated Show resolved Hide resolved
Comment on lines 291 to 295
if (roomEncrypted && !perParticipantE2EE && !e2eeSharedKey) {
if (e2eeSystem.kind === E2eeType.NONE && !widget) {
// the url wants encryption, but we don't have a encryption system. (e.g. when joining a call without password)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we know this isn't supposed to be an unencrypted call? Does this change break unencrypted calls?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there were not unencrypted call in the SPA we always joieded that it can only do encrypted calls. Since this change introduces per sender keys in the SPA it only allows non shared secret calls in the SPA with shared secret. With the !widget it does not break unenecrypted calls in embededded mode.

I was debating if we would like to allow unencrypted calls in the SPA in general. but it would make us end up in a world where users could join a call where they used the wrong link so they can participate without warning but only see encrypted streams.

I am now wondering if this is a smaller issue then not beeing able to share a call link to unencrypted rooms...?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I get everything you've said, but… all I'm worried about is whether we're breaking existing call links. So if a link like https://call.element.dev/room/#/robin-unencrypted?roomId=!FkGOYsKvcjIQJsNpiu:call-unstable.ems.host, which (currently) leads to a unencrypted SPA call, will continue to work, then we're on the same page

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from the initial error message i figured that unenecrypted calls are never supported...
So yea the link you shared would be broken.
A shared secret room is also an unencrypted room.
So i though if the room is not encrypted and we dont have a key from the url or local storage this already implies we reach a failiour mode (since all calls have to be encrypted)
Otherwise the new hook basically covers all cases
encrypted room -> per sender keys
unenecrypted room

  • with password in url -> shared secret
  • with password in local storage -> shared secret
  • no password -> unenecrypted call

So I think we can entirely skip the error screen.
This makes it also possible to open a guest link to an unencrypted room in the spa.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated it to allow it intentional unencrypted calls.

src/room/GroupCallView.tsx Outdated Show resolved Hide resolved
public/locales/en-GB/app.json Outdated Show resolved Hide resolved
src/e2ee/sharedKeyManagement.ts Outdated Show resolved Hide resolved
src/e2ee/sharedKeyManagement.ts Outdated Show resolved Hide resolved
Signed-off-by: Timo K <toger5@hotmail.de>
src/Header.tsx Outdated Show resolved Hide resolved
Signed-off-by: Timo K <toger5@hotmail.de>
package.json Outdated Show resolved Hide resolved
Signed-off-by: Timo K <toger5@hotmail.de>
@AndrewFerr AndrewFerr merged commit 5284479 into livekit Apr 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants