[GH-183] Add jaas support #219

raghavaggarwal2308 · 2022-07-11T13:08:13Z

Summary

Most of the code here is from #185 . The person who created this PR was not responding to the review comments for a while. So, we recreated the same PR with the review comments fixed.

The review comments fixed are-

Added i18n translation to texts in Jitsi plugin system console settings.
Updated UI of system console to clarify the type of Jitsi server user will be using
An error occurred when we were using the locally-generated private key to sign the JWT token for JaaS. That error is also fixed in this PR.

Screenshots

Updated UI
Updated description to clarify how to generate a private key

Ticket

Fixes #183 (JWT Configuration with JaaS)

…si into MI-1865

…i into MI-1865

1. Added translation to texts in jitsi custom settings

1. Upgraded UI of jitsi settings in system console.

1. Fixed linting errors. 2. Fixed error while using own generated private key.

1. Specified method to generate a new key pair in private key description.

1. Removed console statements. 2. Used ?? operator instead of ||. 3. Created constants for "RSA PRIVATE KEY" and "PRIVATE KEY".

1. Converted to class components. 2. Created common func for duplicate code. 3. Used mux router. 4. Removed use of "any".

* [MI-2589]:Fixed the bug and refactor the code * [MI-2589]:Refactoring the code * [MI-2589]:Fixed self review fixes * [MI-2589]:Fixed review fixes * [MI-2589]:Fixed review comments * [MI-2735]:Added check for loaded script

VincentSC · 2023-02-08T11:49:53Z

Small remark: README.md needs an update too. But preferably as a follow-up PR, since this PR has been delayed for years already...

VincentSC · 2023-02-13T22:13:16Z

For who wants to test, see #206 how to build. Use the gh pr checkout 219 to checkout this PR.

I'll be testing it out the coming weeks. I think it needs some more documentation, but that should not be obstructing for this PR

VincentSC · 2023-02-14T15:43:05Z

Problems we found till now (everybody in EU):

doesn't handle high DPI very well. In some moments some UI elements were off-screen
for some users the video would freeze
audio and video were slightly out-of-sync
video would turn blurry once in a while

I think all of these problems can be seen as different problem from this PR, except maybe the first. We have not tested with Jitsi's public server.

raghavaggarwal2308 · 2023-12-26T10:45:43Z

@mickmister @VincentSC This PR has been on hold for a while. Should we assign this for QA review?

doesn't handle high DPI very well. In some moments some UI elements were off-screen

I am not sure if this is something we can handle as we are just sending a payload to an external Jitsi API. Can we create a separate issue for this if this is not concerning? So, this PR can be merged as it has been delayed a lot already.

VincentSC · 2023-12-27T11:26:41Z

@mickmister @VincentSC This PR has been on hold for a while. Should we assign this for QA review?

As the plugin has many problems (it does not build, as the dependencies are very outdated), I have not managed to test it yet. Even my comment of #206 (comment) is now outdated...

Suggestion for order of fixes:

update dependencies (so we can close Manual Build part is broken #206)
8x8 support (this PR, rebased)
UI glitches (of later concern, needs new issue as suggested)

VincentSC · 2024-01-15T14:11:12Z

Can this be rebased against #237 ? Simply because that PR builds, and this one doesn't.

mickmister · 2024-01-16T23:02:26Z

@raghavaggarwal2308 Can you please update your branch with master to so it includes #237?

VincentSC · 2024-01-17T06:27:23Z

It has been merged into master, I just saw.

…i into add_jaas_support

raghavaggarwal2308 · 2024-01-17T11:34:09Z

@mickmister @VincentSC Updated

Snektron · 2024-01-17T14:36:14Z

webapp/package.json

    "react": "16.14.0",
+    "react-dom": "18.2.0",


This is causing build errors for me. I removed it again and it built fine. There are some test failures, though, but I'm not sure if they are caused by this.

@Snektron It's building fine for me with this dependency and falling if I remove it. Also, this dependency is being used in the code, so I don't think it can be removed. Can you please share some screenshots of the errors you are facing?

Note that if we are going to install react-dom, it should always have the same version number as react

@mickmister I have updated it in the last commit

server/api.go

Snektron · 2024-01-17T14:38:49Z

It seems to be working for me (after fixing above issues)

VincentSC · 2024-02-02T14:06:00Z

We tested for a few weeks. One big problem: every reconnect adds another MAU (Monthly Active User).
So it becomes more a pay-per-use or a pay-per-connection instead of a pay-per-user. :(

Got this as a reply from 8x8:

Regarding the MAU count, you can check our docs here, so in short we store a deviceId on the local storage of the browser when a user joins a meeting, if the user clears the cookies, switches browser or device it will join with a different deviceId thus it will be counted as an extra MAU.

mickmister · 2024-02-06T01:24:28Z

Hi @VincentSC, thanks for reporting this.

every reconnect adds another MAU (Monthly Active User).

Do you happen to have any suggestions on how we might solve this on our end? I'm not 100% sure what is meant by "reconnect" here

…i into add_jaas_support

mickmister

Did another passthrough with some comments

mickmister · 2024-03-14T06:04:36Z

.gitignore

@@ -11,6 +11,8 @@ server/manifest.go

 assets/i18n/translate.*.json

+public/jaas


Why is this gitignored?

make dist will automatically generate folder named Jaas inside the public directory

webapp/tsconfig.json

mickmister · 2024-03-14T06:15:00Z

webapp/src/jaas/components/conference/jaas_conference.tsx

+                const params = new URLSearchParams(window.location.search);
+                const jwt = params.get(JWT);
+                const meetingId = params.get(constants.MEETING_ID);
+
+                this.startJaaSMeetingWindow(jwt, meetingId);


IIUC, there is a JWT token in the user's location URL, while in Mattermost? That seems unnecessary if that's the case

Can you please elaborate more on this comment?

@ayusht2810 What is in window.location.search at the time of this code running? Is this in a Mattermost webapp context? Meaning is location.origin the Mattermost server at this time?

@mickmister Yes, the origin is the mattermost URL at this time, and the location.search contains the meeting ID and the JWT token.

The JWT token is being used to set the expiry of the meeting
https://developer.8x8.com/jaas/docs/api-keys-jwt

webapp/tsconfig.json

…lugin-jitsi into add_jaas_support

AayushChaudhary0001

@raghavaggarwal2308 I found a issue while testing this PR.
Issue: Authentication failed: Sorry you're not allowed to join this call.

Steps to reproduce:

Upload the Jitsi plugin to mattermost server
Switch the Jitsi server to Jaas
Setup the AppID and API key
Create a new Jitsi meeting and try to join it

Actual: Error toast message is shown - Authentication failed: Sorry you're not allowed to join this call and user is unable to join the call.
Expected: User should be able to join the jitsi call for jaas server.

SS for the reference:

…itsiJWT is true in setting

jasonblais and others added 30 commits July 13, 2020 07:30

Add contributing information to README

30a2ff8

Update README.md

74a6f0c

Create CONTRIBUTING.md

18b443c

Update CONTRIBUTING.md

0a41971

Apply suggestions from code review

e9e710d

Update CONTRIBUTING.md

79dc804

Add support for JaaS

a43d1ec

Address review comments

6b6c268

Fix compatibility mode

d4f14bb

Address review comments

0e3394c

Move plugin struct definitions to model.go

01e20f6

Address review comments

a476e04

Keep manifest version 2.0.0

4d07154

Remove generated file from commit

cd1edfc

Fix lint issue

f4bd5f8

Fix lint issue

6f78a25

Add ts manifest

cc5cf92

Merge remote-tracking branch 'origin/master' into theunafraid-master

97b6e79

pull from jasonblais-contribute

250d91d

Merge branch 'master' of github.com:theunafraid/mattermost-plugin-jit…

a1f9a18

…si into MI-1865

Merge branch 'master' of github.com:mattermost/mattermost-plugin-jits…

9546ad0

…i into MI-1865

[MI-1865] Fixed error in jaas/index.tsx file

2ad938a

[MI-1865]Added translation

ba58fb5

1. Added translation to texts in jitsi custom settings

[MI-1865]Upgraded UI

683ef17

1. Upgraded UI of jitsi settings in system console.

[MI-1865]Fixes

947dbaa

1. Fixed linting errors. 2. Fixed error while using own generated private key.

[MI-1865]Updatedd description of private key

06fb874

1. Specified method to generate a new key pair in private key description.

[MI-1865]Removed redundant jaas directory

4dae89d

removed extra console.log statements

fd53d20

[MI-1865]Review fixes

e5ca98e

1. Removed console statements. 2. Used ?? operator instead of ||. 3. Created constants for "RSA PRIVATE KEY" and "PRIVATE KEY".

[MI-1865]Review fixes

37f5b93

1. Converted to class components. 2. Created common func for duplicate code. 3. Used mux router. 4. Removed use of "any".

[MI-2735]:Added check for loaded script (#12)

0ddc924

* [MI-2589]:Fixed the bug and refactor the code * [MI-2589]:Refactoring the code * [MI-2589]:Fixed self review fixes * [MI-2589]:Fixed review fixes * [MI-2589]:Fixed review comments * [MI-2735]:Added check for loaded script

mickmister requested a review from AayushChaudhary0001 January 4, 2024 17:11

raghavaggarwal2308 added 3 commits January 17, 2024 16:17

Merge branch 'master' of github.com:mattermost/mattermost-plugin-jits…

2e76cd9

…i into add_jaas_support

Fix lint errors

4f2a796

Fix test cases

a7cf239

Snektron reviewed Jan 17, 2024

View reviewed changes

Fix build errors

6cc34c1

Merge branch 'master' of github.com:mattermost/mattermost-plugin-jits…

1d39e73

…i into add_jaas_support

mickmister reviewed Mar 14, 2024

View reviewed changes

mickmister requested a review from hanzei March 14, 2024 06:21

hanzei removed their request for review March 15, 2024 13:05

raghavaggarwal2308 commented Apr 30, 2024

View reviewed changes

webapp/tsconfig.json Outdated Show resolved Hide resolved

raghavaggarwal2308 added 4 commits April 30, 2024 15:38

Update webapp/tsconfig.json

915b5e3

Merge branch 'master' of github.com:mattermost-community/mattermost-p…

425a001

…lugin-jitsi into add_jaas_support

Fix lint error

89b261c

[MM-552] Fix build error

a7d92c6

AayushChaudhary0001 reviewed Jul 10, 2024

View reviewed changes

Fix issue: jaas meeting not getting created with appbar button when j…

97af4ac

…itsiJWT is true in setting

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GH-183] Add jaas support #219

[GH-183] Add jaas support #219

raghavaggarwal2308 commented Jul 11, 2022

VincentSC commented Feb 8, 2023

VincentSC commented Feb 13, 2023 •

edited

Loading

VincentSC commented Feb 14, 2023 •

edited

Loading

raghavaggarwal2308 commented Dec 26, 2023

VincentSC commented Dec 27, 2023

VincentSC commented Jan 15, 2024

mickmister commented Jan 16, 2024

VincentSC commented Jan 17, 2024

raghavaggarwal2308 commented Jan 17, 2024

Snektron Jan 17, 2024

raghavaggarwal2308 Jan 19, 2024

mickmister Jan 22, 2024 •

edited

Loading

raghavaggarwal2308 Jan 23, 2024

Snektron commented Jan 17, 2024

VincentSC commented Feb 2, 2024

mickmister commented Feb 6, 2024

mickmister left a comment

mickmister Mar 14, 2024

ayusht2810 Mar 28, 2024

mickmister Mar 14, 2024

ayusht2810 Mar 28, 2024

mickmister Apr 23, 2024

raghavaggarwal2308 Apr 26, 2024

raghavaggarwal2308 Apr 30, 2024 •

edited

Loading

AayushChaudhary0001 left a comment

		@@ -11,6 +11,8 @@ server/manifest.go

		assets/i18n/translate.*.json

		public/jaas

[GH-183] Add jaas support #219

Are you sure you want to change the base?

[GH-183] Add jaas support #219

Conversation

raghavaggarwal2308 commented Jul 11, 2022

Summary

Screenshots

Ticket

VincentSC commented Feb 8, 2023

VincentSC commented Feb 13, 2023 • edited Loading

VincentSC commented Feb 14, 2023 • edited Loading

raghavaggarwal2308 commented Dec 26, 2023

VincentSC commented Dec 27, 2023

VincentSC commented Jan 15, 2024

mickmister commented Jan 16, 2024

VincentSC commented Jan 17, 2024

raghavaggarwal2308 commented Jan 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mickmister Jan 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Snektron commented Jan 17, 2024

VincentSC commented Feb 2, 2024

mickmister commented Feb 6, 2024

mickmister left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raghavaggarwal2308 Apr 30, 2024 • edited Loading

Choose a reason for hiding this comment

AayushChaudhary0001 left a comment

Choose a reason for hiding this comment

VincentSC commented Feb 13, 2023 •

edited

Loading

VincentSC commented Feb 14, 2023 •

edited

Loading

mickmister Jan 22, 2024 •

edited

Loading

raghavaggarwal2308 Apr 30, 2024 •

edited

Loading