-
Notifications
You must be signed in to change notification settings - Fork 95
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
Knocking support #2281
Conversation
There was a problem hiding this 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/room/GroupCallView.tsx
Outdated
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) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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...?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Signed-off-by: Timo K <toger5@hotmail.de>
Signed-off-by: Timo K <toger5@hotmail.de>
Signed-off-by: Timo K <toger5@hotmail.de>
This PR does three things seperated in three commits (review can be done per commit)
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:
guest_spa_url
set. (element_call:guest_spa_url
)(full config
)