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

Add transcriber authentication to hidden domain #1663

Conversation

zobadaniel
Copy link

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.

Copy link
Member

@saghul saghul left a 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
Copy link
Member

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?

Copy link
Author

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
Copy link
Member

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.

Copy link
Author

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
Copy link
Member

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.

Copy link
Author

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?

Copy link
Member

Choose a reason for hiding this comment

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

The latter.

@zobadaniel
Copy link
Author

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.

@saghul
Copy link
Member

saghul commented Dec 18, 2023

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.

Certainly!

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.

Sounds like a good plan!

@surajprakashchoudhary
Copy link

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:

  • Hide transcriber frames for all participants.
  • Enable or show the translation to both the moderator and other participants.

@zobadaniel
Copy link
Author

@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.

@saghul
Copy link
Member

saghul commented Dec 19, 2023

Thanks for your work @zobadaniel !

@surajprakashchoudhary With the holiday season approaching, we cannot make any promises. Everyone deserves a rest :-)

@saghul
Copy link
Member

saghul commented Jan 11, 2024

@zobadaniel Do you intend to pick this back up?

@zobadaniel
Copy link
Author

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.

@saghul
Copy link
Member

saghul commented Jan 11, 2024

No worries, just checking in!

@zobadaniel
Copy link
Author

Closing this one as #1737 now covers a more generic attempt.

@zobadaniel zobadaniel closed this Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants