Add support for JaaS

[theunafraid]

Hi everyone,

Summary

This pull request adds support for Jitsi as a Service.

The added code will generate a JaaS JWT for meetings. Mattermost users are moderators while non users are handled as guests. To add mutually exclusive Jitsi/JaaS option had to create a custom setting for the admin console which is compatible with current version. The previous settings are moved to JitsiSettings key and a few more added for JaaS. The jitsiembedded setting is used by both the Jitsi and JaaS version. JitsiSettings component was added for future improvements and mutual exclusivity selection Jitsi/JaaS. When the user upgrades the Jitsi settings should be imported from the previous version. Jitsi server and client code was reused for JaaS except in cases where the client runs outside of Mattermost UI(open in new tab).

If user enables JaaS for this version has to setup Branding, AppID, ApiKey and RSA keys as shown in the JaaS Api Key tutorial and for more info JaaS Start Guide. After following the Api Key tutorial (no code), branding can be done by going to JaaS Branding where the invite URL can be customised for example: https://mymattermostdomain.com/plugins/jitsi/api/v1/meetings, must include /plugins/jitsi/api/v1/meetings. The AppID, ApiKey and RSA private key can be copied in the admin console https://mymattermostdomain.com/admin_console/plugins/plugin_jitsi if JaaS is enabled.

Ticket Link

Fixes https://github.com/mattermost/mattermost-plugin-jitsi/issues/183

[codecov-io]

Codecov Report

Merging #185 (25e30b6) into master (e5e3252) will decrease coverage by 11.83%. The diff coverage is 4.01%.

:exclamation: Current head 25e30b6 differs from pull request most recent head b903bb9. Consider uploading reports for the commit b903bb9 to get more accurate results

@@             Coverage Diff             @@
##           master     #185       +/-   ##
===========================================
- Coverage   44.95%   33.12%   -11.84%     
===========================================
  Files           7        7               
  Lines         803     1105      +302     
===========================================
+ Hits          361      366        +5     
- Misses        417      710      +293     
- Partials       25       29        +4     

Impacted Files	Coverage Δ
server/api.go	0.00% <0.00%> (ø)
server/command.go	60.08% <ø> (ø)
server/manifest.go	100.00% <ø> (ø)
server/configuration.go	7.46% <2.94%> (-6.83%)	:arrow_down:
server/plugin.go	43.03% <7.10%> (-18.39%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e5e3252...b903bb9. Read the comment docs.

[jespino]

@theunafraid one question, is there any difference between the external api script from the jitsi installation and the Jaas one?

[theunafraid]

@theunafraid one question, is there any difference between the external api script from the jitsi installation and the Jaas one?

From what i know, the version JaaS version may contain updates that may not be on meet.jit.si but maybe i'm wrong, have to check with someone else.

[saghul]

unafraid one question, is there any difference between the external api script from the jitsi installation and the Jaas one?

No difference, they are exactly the same. Usually using a bundled version works, but sometimes an update is necessary. Right now I'm pretty sure copying the meeting link (with the button) is broken in Mattermost because we need to pass a new allow directive to the iframe. This is requires an updated bundle.

Regarding the external JS file. I understand the concern. I think there are 2 ways to go about it:

• Treat 8x8.vc as a trusted source, JaaS customers are already trusting it with their meetings after all.
• Honor the setting also on JaaS, but recommend it be disabled with a note in the settings panel.

Given the simple use of the API the plugin uses, I'd say we could start either way and adapt as needed once it starts to see some use.

[jespino]

@saghul @theunafraid ok, then my proposal is to update the file that is bundled with mattermost, and allow to configure the "Compatibility mode" (it does exactly that, instead of using the copy, we proxy the jitsi server file).

Our policy is "secure by default" so the default configuration should be to use our copy of the file.

The main problem in security is. If somebody enter in your system and is able to modify that file, automatically can load arbitrary javascript in our application under our same domain, giving access to session information. That means, enabling this, the surface of attack is the union of both products, and one if them is completely out of our control.

[saghul]

Sounds good @jespino. @theunafraid can you please make those changes?

[theunafraid]

Sounds good @jespino. @theunafraid can you please make those changes?

Sure, i'll take care of it.

[saghul]

@jespino Compatibility mode should be restored now. Can you please take another look?

[codecov-commenter]

Codecov Report

Merging #185 (91b667e) into master (e5e3252) will decrease coverage by 11.89%. The diff coverage is 4.01%.

@@             Coverage Diff             @@
##           master     #185       +/-   ##
===========================================
- Coverage   44.95%   33.06%   -11.90%     
===========================================
  Files           7        6        -1     
  Lines         803     1104      +301     
===========================================
+ Hits          361      365        +4     
- Misses        417      710      +293     
- Partials       25       29        +4     

Impacted Files	Coverage Δ
server/api.go	0.00% <0.00%> (ø)
server/configuration.go	7.46% <2.94%> (-6.83%)	:arrow_down:
server/plugin.go	43.03% <7.10%> (-18.39%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e5e3252...91b667e. Read the comment docs.

[saghul]

👋 @jespino Any chance we can get this moved forward?

[jespino]

@DHaussermann would be great to test it using regular meet.jit.si and the JaaS (you can get a free JaaS subscription in the https://jaas.8x8.vc web page.)

[DHaussermann]

/update-branch

[larkox]

@cristifg Would you mind solving the conflicts? Thanks!

[theunafraid]

@larkox @DHaussermann Yes, sorry will do in the next hours, currently afk.

[theunafraid]

@larkox @DHaussermann Conflicts should now be solved.

[larkox]

@theunafraid there seems to be some linting issues. Could you check it?

[theunafraid]

@theunafraid there seems to be some linting issues. Could you check it?

I checked, but it is not from my changes, the lint issue is from the "manifest" used in configuration.go which is part of the changes that were added by someone else. If i run make dist and skip the check it builds ok. Should i include the automatically generated manifest.go in the PR? Shouldn't lint skip issues like this, since the new code depends on a file that was not yet generated?

[theunafraid]

@larkox @DHaussermann I've added manifest.go, it seems to be ok.

[larkox]

@theunafraid I think that running make apply will generate both the go and ts manifests which were removed for some reason in this PR (maybe due to the merge). Could you try?

[theunafraid]

@theunafraid I think that running make apply will generate both the go and ts manifests which were removed for some reason in this PR (maybe due to the merge). Could you try?

Yes, that was my assumption, i ran make apply but i did not add the ts manifest, will add it now. I thought those files should not be added.

[appcoders]

When will this be merged? Thanks :-)

[larkox]

@DHaussermann Do you know when will you be able to test this one?

[DHaussermann]

@dipak-demansol I have been struggling to find time for this PR. Let's follow up on this change and see if it's something we can have you review instead.

[hanzei]

@theunafraid Could you please merge master into your PR?

[hanzei]

@dipak-demansol Maybe you can help with testing this PR? Not a high priority.

[DHaussermann]

Hi @theunafraid, @dipak-demansol attempted to test this but it was not functional. Perhaps you could help with a couple questions?

1. Does generating the key have to occur directly on the MM server directly? I believe Dipak may have generated the key locally and then uploaded using the UI provided

2. When attempting to use the plgin the web shows a 500 error and the logs show

{"timestamp":"2022-04-04 13:18:45.350 Z","level":"error","msg":"","caller":"web/context.go:105","path":"/api/v4/commands/execute","request_id":"7gdxef5isbdgtx3ougpnnz3cxo","ip_addr":"1x.xxxx.xxx8","user_id":"6enjudxerp8j5eqqe9xw5x569r","method":"POST","err_where":"","http_code":500,"err_details":"startMeeting() threw error: <nil>"}

Can you provide any insight into what may have occurred here or some isolation steps?

3. @aaronrothschild we may need UX input here. The UI may be a counter intuitive. We might be able to add a field label or make it morew clear that the top radio will switch all the fields.
   

[aaronrothschild]

Yes, there should be a setting Label asking "Which type of Jitsi server will you be using?" then show those 2 options.

[DHaussermann]

Does generating the key have to occur directly on the MM server directly?

As per @dipak-demansol the same issue still occurs when creating the token directly on the MM server.

[hanzei]

Closing in favor of https://github.com/mattermost/mattermost-plugin-jitsi/pull/219