Add support for sending the password for a share by Nextcloud Talk #1049

danxuliu · 2018-07-13T16:01:40Z

When a file is shared by email the file can be accessed by anyone as long as the token of the share is known, so to limit that access the share can be protected with a password. When a password was set it was also sent by email to the sharee. This pull request (and its server counterpart) adds another option: sending the password with Nextcloud Talk.

Now when the sharer protects the share with a password no email is sent. Instead, when the receiver tries to open the share the public share authentication page is shown, which now contains a Request password button. When that button is clicked an embedded Nextcloud UI is shown and a conversation is started between the sharer and the user trying to open the file, which allows the sharer to verify the identity of the user before telling her the password and thus granting her access to the file.

Note that it is the sharer who decides whether the receiver of an email share can call her or not; you will not be bothered with password requests if you do not choose to ;-)

This pull request provides the handling of rooms and the Talk UI that is embedded in the public share authentication page; the template publicshareauth of Talk is appended to the one in the server when needed (that is, when sending the password by Talk is enabled for the share), and that template provides some JavaScript code that, when run on the browser, adjusts the page generated by the server to inject the Talk UI as needed.

Currently the embedded Talk UI just provides chat; this will be improved in a different pull request with call support. Due to how the UI is injected this can be done without requiring any changes in the server, only in the Talk app.

Also note that there are other shortcomings, like no information about whether the sharer has joined the room or not, or not being able to open and close the sidebar at will, which causes them to always overlap with the password input field during the conversation on narrow screens. For the time being it is just a basic UI... but at least it works :-)

Rooms can now be associated with arbitrary objects using an object type and an object id. This makes possible to provide custom behaviour for rooms based on that type using hooks to, for example, destroy the room as soon as one of the participants leave it or prevent extra users from joining the room.

The "share:password" rooms are a bit special, as they are one-time rooms created by one user, the receiver of the share, in behalf of another user, the sharer, who then becomes the owner of the room. In standard rooms no notification is sent to the owner when a room is created, so a special notification was added for this case. Depending on which one gets merged first, this pull request or #1029 will need to be updated.

@pygi @Ivansss The notification works without problems in the WebUI, but I do not know if you may need to change something in the mobile apps to handle it. It would be good too to improve the message to include the name of the file or something like that, but that is something for another pull request too ;-)

nickvergessen · 2018-07-13T16:21:10Z

Why did you make it so complicated and coded talk into the Ui?
Just do a new window with a special public room. Free video support 🙈

nickvergessen · 2018-07-18T07:46:11Z

js/publicshareauth.js

+		setupRequestPasswordButton: function() {
+			// "submit-wrapper" is used to mimic the login button and thus get
+			// automatic colouring of the confirm icon by the Theming app
+			$('main').append('<div id="submit-wrapper" class="request-password-wrapper">' +


missing the . before main ?

No, it refers to an element, not to a class.

MorrisJobke

See nextcloud/server#10238 (comment)

MorrisJobke · 2018-07-24T15:06:36Z

@nickvergessen Feel free to merge - I will merge the server PR now.

nickvergessen · 2018-07-25T13:22:15Z

lib/Migration/Version4000Date20180710162342.php

+				'default' => '',
+			]);
+			$table->addColumn('object_id', Type::STRING, [
+				'notnull' => true,


nickvergessen · 2018-07-25T13:22:18Z

lib/Migration/Version4000Date20180710162342.php

+
+		if (!$table->hasColumn('object_type')) {
+			$table->addColumn('object_type', Type::STRING, [
+				'notnull' => true,


nickvergessen · 2018-07-25T15:34:14Z

@danxuliu I added some more commits, please have a look.
As requested by frank we are opening a new tab for now where you can do a video call to verify the identity before giving the password. This also has the advantage that the call survives your try to enter the password and you can tell the owner that the password worked.

I'm fine with merging it like that for now. If you manage to get calling in the "side"bar working we can simply revert the last commit and are back to the JS mode.

nickvergessen · 2018-07-31T12:25:39Z

Rebased to solve conlicts

Please review @danxuliu @Ivansss

danxuliu · 2018-07-31T21:07:08Z

I have cleaned up a bit the commit history, fixed some issues with the layout due to the changes in the server, and made the Request password link look like a button.

danxuliu · 2018-07-31T21:11:33Z

lib/Migration/Version3003Date20180720162342.php

+
+		if (!$table->hasColumn('object_type')) {
+			$table->addColumn('object_type', Type::STRING, [
+				'notnull' => false,


Why this change? Were not empty strings allowed already (in fact the default value is an empty string)?

nextcloud/activity@34279ae

All hail to oracle, storing empty string as null which then fails.

Urg... Thanks for the explanation, I added it to the commit message for future reference.

Note that in Oracle string fields have to be nullable to be able to set empty strings on them. As the table "comments" also has columns named "object_type" and "object_id" they must be namespaced when the last comment is included in the room information; otherwise the created comment object would have the object type and id from the room instead of from the comment. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com> Signed-off-by: Joas Schilling <coding@schilljs.com>

Associating a room to another object is optional; by default a room is not associated to any object. The association is just a "flag" on the room; any special behaviour for an association must be implemented as needed outside the room (for example, by responding to events emitted by the room). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Due to their nature there is no point in keeping a room to request the password of a share once any of the participants has left, so they are automatically destroyed in that case. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

The rooms to request the password for a share are public rooms, so anyone could join them provided she knows its token. Thus, now it is enforced that only a single participant besides the owner can join the room. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Before the public share auth page is rendered an event is dispatched that can be used by apps to load additional scripts. This event is now used to load the scripts that, when run on the browser, will inject the Talk UI as needed in the page generated by the server. The scripts will be loaded only when the share has the "send password by Talk" option enabled; they add a button to the page that, when pressed, creates a new public share auth room with the sharer and shows, in a sidebar, the Talk UI for that room. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Currently the embedded UI is a big hack that just shows the desired part of the main UI, but the whole UI and its callbacks are being run nevertheless. This will be cleaned at a later time, but for the time being the embedded UI just overrides the conflicting functions from the main UI to prevent them from being executed and mess with the embedded UI. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

When the main UI is used, during a conversation the browser shows the URL for the conversation, and when the user leaves the room, either explicitly or implicitly due to a conversation ending, the URL is set to the main Talk URL. However, when the embedded UI is used, during the conversation the browser shows the URL for the public share authentication page, as the conversation is just an extra element of that page. Thus, when the conversation ends, the URL should not be modified. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

When the sharer leaves the conversation the conversation ends, and the Talk sidebar just shows "This conversation has ended". In that case the Talk sidebar is no longer useful, so it is automatically closed after a small delay. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Signed-off-by: Joas Schilling <coding@schilljs.com>

danxuliu added 3. to review enhancement labels Jul 13, 2018

danxuliu added this to the Next Major milestone Jul 13, 2018

danxuliu requested review from nickvergessen, MorrisJobke and schiessle July 13, 2018 16:01

danxuliu mentioned this pull request Jul 13, 2018

Add support for sending the password for a share by Nextcloud Talk nextcloud/server#10238

Merged

nickvergessen reviewed Jul 18, 2018

View reviewed changes

danxuliu force-pushed the add-support-for-sending-the-password-for-a-share-by-nextcloud-talk branch 5 times, most recently from 68710ea to 1c16e86 Compare July 24, 2018 13:19

MorrisJobke approved these changes Jul 24, 2018

View reviewed changes

MorrisJobke mentioned this pull request Jul 24, 2018

14.0.0 Beta 1 nextcloud/server#10370

Merged

21 tasks

nickvergessen force-pushed the add-support-for-sending-the-password-for-a-share-by-nextcloud-talk branch 2 times, most recently from 5f8c7b6 to 989aac2 Compare July 25, 2018 13:18

nickvergessen reviewed Jul 25, 2018

View reviewed changes

nickvergessen approved these changes Jul 25, 2018

View reviewed changes

nickvergessen force-pushed the add-support-for-sending-the-password-for-a-share-by-nextcloud-talk branch from f232170 to 08fea27 Compare July 31, 2018 12:24

danxuliu force-pushed the add-support-for-sending-the-password-for-a-share-by-nextcloud-talk branch from 08fea27 to 2e66db4 Compare July 31, 2018 21:02

danxuliu commented Jul 31, 2018

View reviewed changes

danxuliu force-pushed the add-support-for-sending-the-password-for-a-share-by-nextcloud-talk branch 2 times, most recently from bac2e68 to 815b059 Compare August 6, 2018 02:29

danxuliu and others added 15 commits August 8, 2018 10:20

Add end point to create rooms for requesting the password of a share

80705ad

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Send notification to the sharer that a password has been requested

efc9d95

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

Rename the Listener and register it like we register new listeners

e667836

Signed-off-by: Joas Schilling <coding@schilljs.com>

Use short class names

041f524

Signed-off-by: Joas Schilling <coding@schilljs.com>

Use the language of the owner for the room name

55a983f

Signed-off-by: Joas Schilling <coding@schilljs.com>

Do not reuse the room

f81f5e0

Signed-off-by: Joas Schilling <coding@schilljs.com>

Open the call in a new tab for now

112c73b

Signed-off-by: Joas Schilling <coding@schilljs.com>

danxuliu force-pushed the add-support-for-sending-the-password-for-a-share-by-nextcloud-talk branch from 815b059 to 112c73b Compare August 8, 2018 08:28

danxuliu mentioned this pull request Aug 8, 2018

Add support for video calls when requesting the password for a share #1123

Merged

nickvergessen merged commit fbe0f56 into master Aug 8, 2018

nickvergessen deleted the add-support-for-sending-the-password-for-a-share-by-nextcloud-talk branch August 8, 2018 12:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for sending the password for a share by Nextcloud Talk #1049

Add support for sending the password for a share by Nextcloud Talk #1049

danxuliu commented Jul 13, 2018

nickvergessen commented Jul 13, 2018

nickvergessen Jul 18, 2018

danxuliu Jul 18, 2018

MorrisJobke left a comment

MorrisJobke commented Jul 24, 2018

nickvergessen Jul 25, 2018

nickvergessen Jul 25, 2018

nickvergessen commented Jul 25, 2018

nickvergessen commented Jul 31, 2018

danxuliu commented Jul 31, 2018

danxuliu Jul 31, 2018

nickvergessen Aug 1, 2018

danxuliu Aug 1, 2018

Add support for sending the password for a share by Nextcloud Talk #1049

Add support for sending the password for a share by Nextcloud Talk #1049

Conversation

danxuliu commented Jul 13, 2018

nickvergessen commented Jul 13, 2018

nickvergessen Jul 18, 2018

Choose a reason for hiding this comment

danxuliu Jul 18, 2018

Choose a reason for hiding this comment

MorrisJobke left a comment

Choose a reason for hiding this comment

MorrisJobke commented Jul 24, 2018

nickvergessen Jul 25, 2018

Choose a reason for hiding this comment

nickvergessen Jul 25, 2018

Choose a reason for hiding this comment

nickvergessen commented Jul 25, 2018

nickvergessen commented Jul 31, 2018

danxuliu commented Jul 31, 2018

danxuliu Jul 31, 2018

Choose a reason for hiding this comment

nickvergessen Aug 1, 2018

Choose a reason for hiding this comment

danxuliu Aug 1, 2018

Choose a reason for hiding this comment