ncstate-delta / moodle-mod_zoom

Moodle plugin for Zoom meeting
https://moodle.org/plugins/mod_zoom
61 stars 108 forks source link

Improvement: Better control who is allowed to see join links and dial-in informations within the "Phone/Dial-In info" row and the iCal download #193

Closed abias closed 3 years ago

abias commented 3 years ago

Hi Rex,

this has an overlap with #173, however I think it's still separate.

On the Zoom activity instance overview page (/mod/zoom/view.php?id=...), mod_zoom currently offers teachers and students a row labeled "Phone/Dial-In info" which contains a full invitation text including everything to join the meeting (including meeting link, dial-in information, SIP information and H.323 information). The same invitation text is contained in the iCal file which can be downloaded on the same page.

Both places get this text from the get_meeting_invitation() function (https://github.com/ucla/moodle-mod_zoom/blob/master/classes/webservice.php#L631) which fetches it from the https://marketplace.zoom.us/docs/api-reference/zoom-api/meetings/meetinginvitation Zoom API Endpoint.

The invitation text looked similar to this:

Eva Example is inviting you to a scheduled Zoom meeting.

Topic: Zoom Meeting
Time: Mar 15, 2021 06:08 AM London

Join Zoom Meeting https://us02web.zoom.us/j/123456789?pwd=ABCABCACBABCABCABC

Meeting ID: 123 456 789
Passcode: 064981

One tap mobile
+61861193900,,123456789#,,,,*064981# Australia
+61871501149,,123456789#,,,,*064981# Australia

Dial by your location
        +61 8 6119 3900 Australia
        +61 8 7150 1149 Australia
        +61 2 8015 6011 Australia
        +61 3 7018 2005 Australia
        +61 7 3185 3730 Australia
Meeting ID: 123 456 789
Passcode: 064981
Find your local number: https://us02web.zoom.us/u/kcOV0px514 

At our university, we are aware that the Moodle-Zoom integration with mod_zoom just works with redirecting the user to the Join URL and that this integration isn't bulletproof against leaking Join URLs to non-course participants. However, we still would like to have more control who is directly seeing the Join URLs and Dial-In information and who is not. Instead, we would like to see users primarily join into the meetings through the Moodle course or the Moodle App.

Against this background, I would like to propose this improvement:

Join through Moodle: https://yourmoodle.com/mod/zoom/view.php?id=12345

Join directly: https://us02web.zoom.us/j/123456789?pwd=ABCABCACBABCABCABC

Join from the Zoom client: Meeting ID: 123 456 789 Passcode: 064981

Joint through Dial-In: One tap mobile +61861193900,,123456789#,,,,064981# Australia +61871501149,,123456789#,,,,064981# Australia

Dial by your location +61 8 6119 3900 Australia +61 8 7150 1149 Australia +61 2 8015 6011 Australia +61 3 7018 2005 Australia +61 7 3185 3730 Australia Meeting ID: 123 456 789 Passcode: 064981 Find your local number: https://us02web.zoom.us/u/kcOV0px514

** If the user has the mod/zoom:viewdialin but not the mod/zoom:viewjoinurl capabilities:

Topic: Zoom Meeting Time: Mar 15, 2021 06:08 AM London

Join through Moodle: https://yourmoodle.com/mod/zoom/view.php?id=12345

Joint through Dial-In: One tap mobile +61861193900,,123456789#,,,,064981# Australia +61871501149,,123456789#,,,,064981# Australia

Dial by your location +61 8 6119 3900 Australia +61 8 7150 1149 Australia +61 2 8015 6011 Australia +61 3 7018 2005 Australia +61 7 3185 3730 Australia Meeting ID: 123 456 789 Passcode: 064981 Find your local number: https://us02web.zoom.us/u/kcOV0px514


* If shown based on the `showdownloadical` plugin setting, output this message in the downloadable iCal file:
** If the user has the mod/zoom:viewjoinurl and the mod/zoom:viewdialin capabilities:

Topic: Zoom Meeting Time: Mar 15, 2021 06:08 AM London

Join through Moodle: https://yourmoodle.com/mod/zoom/view.php?id=12345

Join directly: https://us02web.zoom.us/j/123456789?pwd=ABCABCACBABCABCABC

Join from the Zoom client: Meeting ID: 123 456 789 Passcode: 064981

Joint through Dial-In: One tap mobile +61861193900,,123456789#,,,,064981# Australia +61871501149,,123456789#,,,,064981# Australia

Dial by your location +61 8 6119 3900 Australia +61 8 7150 1149 Australia +61 2 8015 6011 Australia +61 3 7018 2005 Australia +61 7 3185 3730 Australia Meeting ID: 123 456 789 Passcode: 064981 Find your local number: https://us02web.zoom.us/u/kcOV0px514

** If the user has the mod/zoom:viewjoinurl but not the mod/zoom:viewdialin capabilities:

Topic: Zoom Meeting Time: Mar 15, 2021 06:08 AM London

Join through Moodle: https://yourmoodle.com/mod/zoom/view.php?id=12345

Join directly: https://us02web.zoom.us/j/123456789?pwd=ABCABCACBABCABCABC

Join from the Zoom client: Meeting ID: 123 456 789 Passcode: 064981

** If the user has the mod/zoom:viewdialin but not the mod/zoom:viewjoinurl capabilities:

Topic: Zoom Meeting Time: Mar 15, 2021 06:08 AM London

Join through Moodle: https://yourmoodle.com/mod/zoom/view.php?id=12345

Joint through Dial-In: One tap mobile +61861193900,,123456789#,,,,064981# Australia +61871501149,,123456789#,,,,064981# Australia

Dial by your location +61 8 6119 3900 Australia +61 8 7150 1149 Australia +61 2 8015 6011 Australia +61 3 7018 2005 Australia +61 7 3185 3730 Australia Meeting ID: 123 456 789 Passcode: 064981 Find your local number: https://us02web.zoom.us/u/kcOV0px514

** If the user has neither the mod/zoom:viewdialin nor the mod/zoom:viewjoinurl capabilities:

Topic: Zoom Meeting Time: Mar 15, 2021 06:08 AM London

Join through Moodle: https://yourmoodle.com/mod/zoom/view.php?id=12345



Cheers,
Alex
abias commented 3 years ago

I would like to announce that we, Ulm university, have contracted Moodle partner Catalyst IT some days ago to implement a solution for this issue based on the specification given above.

I will post updates about this work as they occur.

nstefanski commented 3 years ago

Not sure if this should be a separate issue, but I tested the latest commit to address this issue, and it caused an error in our system. We do not have SIP and H.323 enabled, so there is no text referencing them in the default message invite. This causes the following errors when viewing the meeting activity:

No match found in zoom invitation for element: "sip" with pattern: "/^join by sip.*\n.+$/mi".
* line 122 of /mod/zoom/classes/invitation.php: call to debugging()
* line 79 of /mod/zoom/classes/invitation.php: call to mod_zoom\invitation->remove_element()
* line 94 of /mod/zoom/view.php: call to mod_zoom\invitation->get_display_string()

No match found in zoom invitation for element: "h323" with pattern: "/^join by h\.323.*(\n.*)+?(\nmeeting id.+\npasscode.+)$/mi".
* line 122 of /mod/zoom/classes/invitation.php: call to debugging()
* line 79 of /mod/zoom/classes/invitation.php: call to mod_zoom\invitation->remove_element()
* line 94 of /mod/zoom/view.php: call to mod_zoom\invitation->get_display_string()

The invitation text is not displayed and the iCal download also seems to throw an error. Is there a way to account for if these options don't exist?

nstefanski commented 3 years ago

I should specify, this is after removing the mod/zoom:viewdialin permission from a user. If the permission is present, then the code shouldn't attempt to remove those items, as far as I know. However, this functionally limits our organization from restricting that permission if we want to display any other portion of the invitation or use the iCal button.

jrchamp commented 3 years ago

The debugging line should be set to only show when the debug level is set to DEBUG_DEVELOPER, so I'm surprised that you're seeing it on the production site.

You are correct that the code only attempts to remove those items when someone doesn't have the permission: https://github.com/ncstate-delta/moodle-mod_zoom/blob/09562dcceadcc28b533e36138c05262c783b392b/classes/invitation.php#L75-L79

nstefanski commented 3 years ago

This was on our dev site, not production. Oddly enough, even when turning off debugging, the invitation text would not display at all if the permission is missing. I see default behavior is to pass the full invitation text if the try{} failed, not sure why that wasn’t happening.

Even if the try loop were working properly, it would prevent us from hiding the info to specific users. Not sure what the best solution is here: maybe putting a try loop in the removal function itself?

nstefanski commented 3 years ago

I realized why the meeting invite is not displaying on the activity page. The invitation class is working correctly and removing all the elements, skipping SIP and H.323 when they're not found, and returning a shortened meeting invite. However, in view.php, there is an extra erroneous check for the mod/zoom:viewdialin permission, and when it is not found, the invitation is not displayed at all.

https://github.com/ncstate-delta/moodle-mod_zoom/blob/09562dcceadcc28b533e36138c05262c783b392b/view.php#L399

The issue with exportical.php seems to only occur when debugging is on, I forgot to retest that after I'd turned off debugging before.

jrchamp commented 3 years ago

Ah, right! The section was added by #170 specifically to provide a friendly set of dial-in information. So if the user is not supposed to see the dial-in information, it makes sense that they would not see the section whose sole purpose is to show dial-in information.

There is a fix in #245 that will skip the API call and meeting invitation parsing on that page when the meeting invitation information is not going to be displayed.

abias commented 3 years ago

Basically, this issue was solved by #235 on 15th april.

I am just skimming over the comments which were written on 29th and am wondering if we can proceed this way:

Would this be a good way forward?

nstefanski commented 3 years ago

@abias and @jrchamp I tested the latest build, unfortunately it doesn't address the issues above. There are actually two separate issues:

  1. Removing the "viewdialin" permission hides the entire invite. This wasn't caused by #235, but now that you've got regex to pull the dialin info out, it makes sense to let users without the permission see the rest of the invite. I'll make a separate issue and pull request, as I've already identified the one line of code that needs to change.
  2. If SIP or H.323 is not found in the Zoom invite AND developer debugging is turned on, then the iCal download does not work. I don't know if we want to consider this an issue, it only affects a minor function in a development environment. I imagine the "fix" would likely involve having separate settings to enable SIP and H.323 in the regex? But again, I'm not sure if this is worth fixing since it should not affect a production environment.
abias commented 3 years ago

Hi @nstefanski ,

thank you for these additional details.

  1. I just commented in #267. Let's keep the discussion there - if anything needs to be discussed at all anymore.
  2. I think that we should at least capture this issue in a separate issue and check the possible options there. Personally, I think that features should work regardless if debugging is on or off. Would you mind to create a new issue?

Cheers, Alex

nstefanski commented 3 years ago

@abias I added a new issue for the iCal error, #270.

@jrchamp the remaining errors have now been separated into their own issues, I agree that this issue should be marked as solved.

abias commented 3 years ago

Thank you, @nstefanski .

So I will close this issue.