-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add transcriber authentication to hidden domain #1663
Add transcriber authentication to hidden domain #1663
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.
Left some commentas, please take a look!
@@ -116,6 +116,10 @@ ETHERPAD_SKIN_VARIANTS="super-light-toolbar super-light-editor light-background | |||
# Enable guest access | |||
#ENABLE_GUESTS=1 | |||
|
|||
# Enable auth for Transcriber in Jigasi | |||
# This can only be enabled if the other auth options above are not used | |||
#JIGASI_TRANSCRIBER_AUTHENTICATION=1 |
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.
This is very confusing. The transcriber should be authenticated always, like a recorder. Can this be dropped?
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 guess that eventually, this can be dropped and the authentication of the transcriber can be made mandatory. I don't know whether this would be a "breaking change" though? Maybe it was never meant that the transcriber works without authentication, but I can't tell. Please see also my generic comment below.
@@ -51,3 +51,10 @@ if [[ $ENABLE_TRANSCRIPTIONS -eq 1 || $ENABLE_TRANSCRIPTIONS == "true" ]]; then | |||
}' \ | |||
> /config/key.json | |||
fi | |||
|
|||
# check that no conflicting authentications are configured | |||
if [[ $ENABLE_AUTH -eq 1 && $JIGASI_TRANSCRIBER_AUTHENTICATION -eq 1 ]]; then |
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.
Yeah nobody will understand this. Let's make it work authenticated always, like a recorder please.
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.
Agree, not actually intuitive.
@@ -104,6 +105,15 @@ if [[ ! -z $JIGASI_XMPP_PASSWORD ]]; then | |||
prosodyctl --config $PROSODY_CFG register $JIGASI_XMPP_USER $XMPP_AUTH_DOMAIN $JIGASI_XMPP_PASSWORD | |||
fi | |||
|
|||
if [[ ! -z $JIGASI_TRANSCRIBER_PASSWORD ]]; then | |||
OLD_JIGASI_TRANSCRIBER_PASSWORD=passw0rd |
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.
This never existed, so we don't need to check for it.
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.
Honestly I just copied the handling of other paswords. I don't know why this password would have to be handled specially. Or is this some sort of legacy stuff that is simply no longer necessary for newly introduced passwords?
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.
The latter.
Currently, the recorder is the only authenticated user that utilizes the "hidden domain" - and there is only one "hidden domain". If we want to add the transcriber, we might have to give up on the idea that the hidden domain is defined and used solely by the recorder, and managed by the "ENABLE_RECORDING" option. Is this reasonable? I might try and unbundle this, and have an explicit option for setting up the "hidden domain" which both the recorder and the transcriber need to authenticate to. |
Certainly!
Sounds like a good plan! |
Hi @saghul and @zobadaniel Can you please tell us when it will be available in production? Can we expect to get this feature in the next Docker-Jitsi-Meet stable release? Does this pull request (PR) solve both problems:
|
@surajprakashchoudhary This PR is intended to ease/allow the configuration of the transcriber as "hidden participant" (e.g. it will not be visible the same way as a "normal" participant without video). And if a moderator then enables subtitles, the subtitle menu will be visible to all participants (and be removed once the last person switches off subtitles again). During the discussion with saghul I learned that this PR shall be reworked to make the config better comprehensible. This means reworking a part of the meeting recorder config as well. I can't make any promises to when I will have a revised version ready for discussion. Nevertheless I wanted to post my first version, to get feedback on how to set up this feature properly. It took me a while to collect the info and consolidate into this PR. |
Thanks for your work @zobadaniel ! @surajprakashchoudhary With the holiday season approaching, we cannot make any promises. Everyone deserves a rest :-) |
@zobadaniel Do you intend to pick this back up? |
Yes, but it might take a while until I find the time to do so. I can't promise to have this done short term due to other stuff with unfortunately higher priority. |
No worries, just checking in! |
Closing this one as #1737 now covers a more generic attempt. |
It took me a while to understand what kind of configuration is required to have jigasi's transcriber join a conference as hidden participant. This allows normal participants to set up transcripts for themselves after the moderator has enabled transcripts (and thus invited transcriber to the meeting).
It seems that this configuration has been partly implemented already, e.g. some of the environment variables were already there. The commits just complete the config for jigasi and prosody, and add a variable for enabling/disabling.
It is confusing though that the authentication has to be for the same domain as the recorder, because there can be only one hidden domain in the frontend - and I did not want one to exclude the other. So now to use this feature, recording needs to be configured as well. I have no idea how to streamline this. Nevertheless I wanted to publish this, since the desired feature can be set up just by config.