Merge pull request #13702 from nextcloud/feature/6717/rename-app-passwords

Make it possible to rename app passwords

build/integration/features/bootstrap/Auth.php

@@ -132,6 +132,7 @@ trait Auth {
 				'requesttoken' => $this->requestToken,
 			],
 			'json' => [
+				'name' => md5(microtime()),
 				'scope' => [
 					'filesystem' => false,
 				],

lib/composer/composer/autoload_classmap.php

@@ -499,6 +499,7 @@ return array(
     'OC\\Authentication\\Token\\DefaultTokenCleanupJob' => $baseDir . '/lib/private/Authentication/Token/DefaultTokenCleanupJob.php',
     'OC\\Authentication\\Token\\DefaultTokenMapper' => $baseDir . '/lib/private/Authentication/Token/DefaultTokenMapper.php',
     'OC\\Authentication\\Token\\DefaultTokenProvider' => $baseDir . '/lib/private/Authentication/Token/DefaultTokenProvider.php',
+    'OC\\Authentication\\Token\\INamedToken' => $baseDir . '/lib/private/Authentication/Token/INamedToken.php',
     'OC\\Authentication\\Token\\IProvider' => $baseDir . '/lib/private/Authentication/Token/IProvider.php',
     'OC\\Authentication\\Token\\IToken' => $baseDir . '/lib/private/Authentication/Token/IToken.php',
     'OC\\Authentication\\Token\\Manager' => $baseDir . '/lib/private/Authentication/Token/Manager.php',

lib/composer/composer/autoload_static.php

@@ -529,6 +529,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Authentication\\Token\\DefaultTokenCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/DefaultTokenCleanupJob.php',
         'OC\\Authentication\\Token\\DefaultTokenMapper' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/DefaultTokenMapper.php',
         'OC\\Authentication\\Token\\DefaultTokenProvider' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/DefaultTokenProvider.php',
+        'OC\\Authentication\\Token\\INamedToken' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/INamedToken.php',
         'OC\\Authentication\\Token\\IProvider' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/IProvider.php',
         'OC\\Authentication\\Token\\IToken' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/IToken.php',
         'OC\\Authentication\\Token\\Manager' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/Manager.php',

lib/private/Authentication/Token/DefaultToken.php

@@ -30,7 +30,6 @@ use OCP\AppFramework\Db\Entity;
  * @method void setId(int $id)
  * @method void setUid(string $uid);
  * @method void setLoginName(string $loginname)
- * @method void setName(string $name)
  * @method string getToken()
  * @method void setType(int $type)
  * @method int getType()
@@ -39,7 +38,7 @@ use OCP\AppFramework\Db\Entity;
  * @method int getLastActivity()
  * @method void setVersion(int $version)
  */
-class DefaultToken extends Entity implements IToken {
+class DefaultToken extends Entity implements INamedToken {
 
 	const VERSION = 1;
 
@@ -179,6 +178,10 @@ class DefaultToken extends Entity implements IToken {
 		return parent::getName();
 	}
 
+	public function setName(string $name): void {
+		parent::setName($name);
+	}
+
 	public function getRemember(): int {
 		return parent::getRemember();
 	}

lib/private/Authentication/Token/INamedToken.php

@@ -0,0 +1,34 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019, Daniel Kesselberg (mail@danielkesselberg.de)
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\Authentication\Token;
+
+
+interface INamedToken extends IToken {
+	/**
+	 * Set token name
+	 *
+	 * @param string $name
+	 * @return void
+	 */
+	public function setName(string $name): void;
+}

lib/private/Authentication/Token/PublicKeyToken.php

@@ -31,7 +31,6 @@ use OCP\AppFramework\Db\Entity;
  * @method void setId(int $id)
  * @method void setUid(string $uid);
  * @method void setLoginName(string $loginname)
- * @method void setName(string $name)
  * @method string getToken()
  * @method void setType(int $type)
  * @method int getType()
@@ -45,7 +44,7 @@ use OCP\AppFramework\Db\Entity;
  * @method void setVersion(int $version)
  * @method bool getPasswordInvalid()
  */
-class PublicKeyToken extends Entity implements IToken {
+class PublicKeyToken extends Entity implements INamedToken {
 
 	const VERSION = 2;
 
@@ -197,6 +196,10 @@ class PublicKeyToken extends Entity implements IToken {
 		return parent::getName();
 	}
 
+	public function setName(string $name): void {
+		parent::setName($name);
+	}
+
 	public function getRemember(): int {
 		return parent::getRemember();
 	}

settings/Controller/AuthSettingsController.php

@@ -31,10 +31,12 @@ use BadMethodCallException;
 use OC\AppFramework\Http;
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
+use OC\Authentication\Token\INamedToken;
 use OC\Authentication\Token\IProvider;
 use OC\Authentication\Token\IToken;
 use OC\Settings\Activity\Provider;
 use OCP\Activity\IManager;
+use OC\Authentication\Token\PublicKeyToken;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\ILogger;
@@ -112,11 +114,12 @@ class AuthSettingsController extends Controller {
 
 		return array_map(function (IToken $token) use ($sessionToken) {
 			$data = $token->jsonSerialize();
+			$data['canDelete'] = true;
+			$data['canRename'] = $token instanceof INamedToken;
 			if ($sessionToken->getId() === $token->getId()) {
 				$data['canDelete'] = false;
+				$data['canRename'] = false;
 				$data['current'] = true;
-			} else {
-				$data['canDelete'] = true;
 			}
 			return $data;
 		}, $tokens);
@@ -153,6 +156,7 @@ class AuthSettingsController extends Controller {
 		$deviceToken = $this->tokenProvider->generateToken($token, $this->uid, $loginName, $password, $name, IToken::PERMANENT_TOKEN);
 		$tokenData = $deviceToken->jsonSerialize();
 		$tokenData['canDelete'] = true;
+		$tokenData['canRename'] = true;
 
 		$this->publishActivity(Provider::APP_TOKEN_CREATED, $deviceToken->getId(), $deviceToken->getName());
 
@@ -212,9 +216,10 @@ class AuthSettingsController extends Controller {
 	 *
 	 * @param int $id
 	 * @param array $scope
+	 * @param string $name
 	 * @return array|JSONResponse
 	 */
-	public function update($id, array $scope) {
+	public function update($id, array $scope, string $name) {
 		try {
 			$token = $this->findTokenByIdAndUser($id);
 		} catch (InvalidTokenException $e) {
@@ -225,6 +230,11 @@ class AuthSettingsController extends Controller {
 			'filesystem' => $scope['filesystem']
 		]);
 
+
+		if ($token instanceof INamedToken) {
+			$token->setName($name);
+		}
+
 		$this->tokenProvider->updateToken($token);
 		$this->publishActivity(Provider::APP_TOKEN_UPDATED, $token->getId(), $token->getName());
 		return [];

settings/css/settings.scss

@@ -440,7 +440,14 @@ table.nostyle {
 		}
 
 		&.token-name {
-			padding: 10px 0;
+			padding: 10px 6px;
+			&.token-rename {
+				padding: 0;
+			}
+			input {
+				width: 100%;
+				margin: 0;
+			}
 		}
 
 		&.more {
@@ -456,6 +463,9 @@ table.nostyle {
 			}
 		}
 	}
+	tr > *:nth-child(2) {
+		padding-left: 6px;
+	}
 	tr > *:nth-child(3) {
 		text-align: right;
 	}

settings/js/authtoken_view.js

@@ -35,6 +35,7 @@
 			data.revokeText = t('settings', 'Revoke');
 			data.settingsTitle = t('settings', 'Device settings');
 			data.allowFSAccess = t('settings', 'Allow filesystem access');
+			data.renameText = t('settings', 'Rename');
 			return OC.Settings.Templates['authtoken'](data);
 		},
 
@@ -106,7 +107,7 @@
 				iosTalkClient: /^Mozilla\/5\.0 \(iOS\) Nextcloud\-Talk.*$/,
 				androidTalkClient:/^Mozilla\/5\.0 \(Android\) Nextcloud\-Talk.*$/,
 				// DAVdroid/1.2 (2016/07/03; dav4android; okhttp3) Android/6.0.1
-				davDroid: /DAVdroid\/([0-9.]+)/,
+				davDroid: /DAV(droid|x5)\/([0-9.]+)/,
 				// Mozilla/5.0 (U; Linux; Maemo; Jolla; Sailfish; like Android 4.3) AppleWebKit/538.1 (KHTML, like Gecko) WebPirate/2.0 like Mobile Safari/538.1 (compatible)
 				webPirate: /(Sailfish).*WebPirate\/(\d+)/,
 				// Mozilla/5.0 (Maemo; Linux; U; Jolla; Sailfish; Mobile; rv:31.0) Gecko/31.0 Firefox/31.0 SailfishBrowser/1.0
@@ -218,6 +219,9 @@
 			$el.on('click', 'a.icon-delete', _.bind(this._onDeleteToken, this));
 			$el.on('click', '.icon-more', _.bind(this._onConfigureToken, this));
 			$el.on('change', 'input.filesystem', _.bind(this._onSetTokenScope, this));
+			$el.on('click', '.icon-rename', _.bind(this._onRenameToken, this));
+			$el.on('dblclick', '.token-name > span', _.bind(this._onRenameToken, this));
+			$el.on('keyup', '.token-name > input', _.bind(this._onEnterTokenName, this));
 
 			this._form = $('#app-password-form');
 			this._tokenName = $('#app-password-name');
@@ -241,7 +245,7 @@
 			this._result.find('.clipboardButton').tooltip({placement: 'bottom', title: t('core', 'Copy'), trigger: 'hover'});
 
 			// Clipboard!
-			var clipboard = new Clipboard('.clipboardButton');
+			var clipboard = new ClipboardJS('.clipboardButton');
 			clipboard.on('success', function(e) {
 				var $input = $(e.trigger);
 				$input.tooltip('hide')
@@ -414,6 +418,65 @@
 			token.save();
 		},
 
+		_onRenameToken: function (event) {
+			var $target = $(event.target);
+			var $row = $target.closest('tr');
+
+			var tokenId = $row.data('id');
+			var token = this.collection.get(tokenId);
+
+			if (_.isUndefined(token) || token.get('current') === true) {
+				// Ignore event
+				return;
+			}
+
+			var $tokenName = $row.find('.token-name');
+			var showTokenNameInput = !$tokenName.hasClass('token-rename'); // if class token-rename present input is already visible.
+
+			this._hideTokenNameInput();
+
+			if (showTokenNameInput) {
+				$tokenName.addClass('token-rename');
+				$tokenName.find('span').addClass('hidden');
+				$tokenName.find('input').removeClass('hidden').val(token.get('name')).focus();
+			}
+
+			this._hideConfigureToken();
+		},
+
+		_onEnterTokenName: function(event) {
+			var $target = $(event.target);
+			var $row = $target.closest('tr');
+
+			var tokenId = $row.data('id');
+			var token = this.collection.get(tokenId);
+
+			if (_.isUndefined(token) || token.get('current') === true) {
+				// Ignore event
+				return;
+			}
+
+			if (event.key === 'Enter') {
+				token.set('name', $target.context.value);
+
+				var _this = this;
+				$.when(token.save()).always(function () {
+					_this.render();
+				});
+			}
+
+			if (event.key === 'Escape') {
+				this._hideTokenNameInput();
+			}
+		},
+
+		_hideTokenNameInput: function () {
+			var $tokenList = $('.token-list td.token-name');
+			$tokenList.removeClass('token-rename');
+			$tokenList.find('span').removeClass('hidden');
+			$tokenList.find('input').addClass('hidden');
+		},
+
 		_toggleFormResult: function (showForm) {
 			if (showForm) {
 				this._result.slideUp();

settings/js/templates.js

@@ -3,24 +3,36 @@
 templates['authtoken'] = template({"1":function(container,depth0,helpers,partials,data) {
     var helper;
 
+  return "            <input class=\"hidden\" type=\"text\" value=\""
+    + container.escapeExpression(((helper = (helper = helpers.name || (depth0 != null ? depth0.name : depth0)) != null ? helper : helpers.helperMissing),(typeof helper === "function" ? helper.call(depth0 != null ? depth0 : (container.nullContext || {}),{"name":"name","hash":{},"data":data}) : helper)))
+    + "\" />\n";
+},"3":function(container,depth0,helpers,partials,data) {
+    var helper;
+
   return "<a class=\"icon icon-more has-tooltip\" tabindex=\"0\" title=\""
     + container.escapeExpression(((helper = (helper = helpers.settingsTitle || (depth0 != null ? depth0.settingsTitle : depth0)) != null ? helper : helpers.helperMissing),(typeof helper === "function" ? helper.call(depth0 != null ? depth0 : (container.nullContext || {}),{"name":"settingsTitle","hash":{},"data":data}) : helper)))
     + "\"/>";
-},"3":function(container,depth0,helpers,partials,data) {
+},"5":function(container,depth0,helpers,partials,data) {
     var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=helpers.helperMissing, alias3="function", alias4=container.escapeExpression;
 
   return "			<li><span class=\"menuitem\">\n				<input class=\"filesystem checkbox\" type=\"checkbox\" id=\""
     + alias4(((helper = (helper = helpers.id || (depth0 != null ? depth0.id : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"id","hash":{},"data":data}) : helper)))
     + "_filesystem\" "
-    + ((stack1 = helpers["if"].call(alias1,((stack1 = (depth0 != null ? depth0.scope : depth0)) != null ? stack1.filesystem : stack1),{"name":"if","hash":{},"fn":container.program(4, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
+    + ((stack1 = helpers["if"].call(alias1,((stack1 = (depth0 != null ? depth0.scope : depth0)) != null ? stack1.filesystem : stack1),{"name":"if","hash":{},"fn":container.program(6, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
     + " tabindex=\"0\" />\n				<label for=\""
     + alias4(((helper = (helper = helpers.id || (depth0 != null ? depth0.id : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"id","hash":{},"data":data}) : helper)))
     + "_filesystem\">"
     + alias4(((helper = (helper = helpers.allowFSAccess || (depth0 != null ? depth0.allowFSAccess : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"allowFSAccess","hash":{},"data":data}) : helper)))
     + "</label><br/>\n			</span></li>\n";
-},"4":function(container,depth0,helpers,partials,data) {
-    return "checked";
 },"6":function(container,depth0,helpers,partials,data) {
+    return "checked";
+},"8":function(container,depth0,helpers,partials,data) {
+    var helper;
+
+  return "            <li>\n                <a class=\"icon icon-rename\" tabindex=\"0\">"
+    + container.escapeExpression(((helper = (helper = helpers.renameText || (depth0 != null ? depth0.renameText : depth0)) != null ? helper : helpers.helperMissing),(typeof helper === "function" ? helper.call(depth0 != null ? depth0 : (container.nullContext || {}),{"name":"renameText","hash":{},"data":data}) : helper)))
+    + "</a>\n            </li>\n";
+},"10":function(container,depth0,helpers,partials,data) {
     var helper;
 
   return "			<li>\n				<a class=\"icon icon-delete\" tabindex=\"0\">"
@@ -33,17 +45,20 @@ templates['authtoken'] = template({"1":function(container,depth0,helpers,partial
     + alias4(((helper = (helper = helpers.id || (depth0 != null ? depth0.id : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"id","hash":{},"data":data}) : helper)))
     + "\">\n	<td class=\"client\">\n	    <div class=\""
     + alias4(((helper = (helper = helpers.icon || (depth0 != null ? depth0.icon : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"icon","hash":{},"data":data}) : helper)))
-    + "\" />\n	</td>\n	<td class=\"token-name\">\n		"
+    + "\" />\n	</td>\n	<td class=\"token-name\">\n		<span>"
     + alias4(((helper = (helper = helpers.name || (depth0 != null ? depth0.name : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"name","hash":{},"data":data}) : helper)))
-    + "\n	</td>\n	<td>\n		<span class=\"last-activity has-tooltip\" title=\""
+    + "</span>\n"
+    + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canRename : depth0),{"name":"if","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
+    + "	</td>\n	<td>\n		<span class=\"last-activity has-tooltip\" title=\""
     + alias4(((helper = (helper = helpers.lastActivityTime || (depth0 != null ? depth0.lastActivityTime : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"lastActivityTime","hash":{},"data":data}) : helper)))
     + "\">"
     + alias4(((helper = (helper = helpers.lastActivity || (depth0 != null ? depth0.lastActivity : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"lastActivity","hash":{},"data":data}) : helper)))
     + "</span></td>\n	<td class=\"more\">\n		"
-    + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showMore : depth0),{"name":"if","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
+    + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showMore : depth0),{"name":"if","hash":{},"fn":container.program(3, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
     + "\n		<div class=\"popovermenu menu\">\n"
-    + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canScope : depth0),{"name":"if","hash":{},"fn":container.program(3, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
-    + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canDelete : depth0),{"name":"if","hash":{},"fn":container.program(6, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
+    + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canScope : depth0),{"name":"if","hash":{},"fn":container.program(5, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
+    + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canRename : depth0),{"name":"if","hash":{},"fn":container.program(8, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
+    + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canDelete : depth0),{"name":"if","hash":{},"fn":container.program(10, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "")
     + "		</div>\n	</td>\n</tr>\n";
 },"useData":true});
 templates['federationscopemenu'] = template({"1":function(container,depth0,helpers,partials,data) {

settings/js/templates/authtoken.handlebars

@@ -3,7 +3,10 @@
 	    <div class="{{icon}}" />
 	</td>
 	<td class="token-name">
-		{{name}}
+		<span>{{name}}</span>
+        {{#if canRename}}
+            <input class="hidden" type="text" value="{{name}}" />
+        {{/if}}
 	</td>
 	<td>
 		<span class="last-activity has-tooltip" title="{{lastActivityTime}}">{{lastActivity}}</span></td>
@@ -16,6 +19,11 @@
 				<label for="{{id}}_filesystem">{{allowFSAccess}}</label><br/>
 			</span></li>
 		{{/if}}
+        {{#if canRename}}
+            <li>
+                <a class="icon icon-rename" tabindex="0">{{renameText}}</a>
+            </li>
+        {{/if}}
 		{{#if canDelete}}
 			<li>
 				<a class="icon icon-delete" tabindex="0">{{revokeText}}</a>

tests/Settings/Controller/AuthSettingsControllerTest.php

@@ -1,5 +1,4 @@
 <?php
-
 /**
  * @author Christoph Wurst <christoph@owncloud.com>
  *
@@ -109,7 +108,8 @@ class AuthSettingsControllerTest extends TestCase {
 				'type' => 0,
 				'canDelete' => false,
 				'current' => true,
-				'scope' => ['filesystem' => true]
+				'scope' => ['filesystem' => true],
+				'canRename' => false,
 			],
 			[
 				'id' => 200,
@@ -117,7 +117,8 @@ class AuthSettingsControllerTest extends TestCase {
 				'lastActivity' => 0,
 				'type' => 0,
 				'canDelete' => true,
-				'scope' => ['filesystem' => true]
+				'scope' => ['filesystem' => true],
+				'canRename' => true,
 			]
 		], $this->controller->index());
 	}
@@ -162,7 +163,7 @@ class AuthSettingsControllerTest extends TestCase {
 
 		$expected = [
 			'token' => $newToken,
-			'deviceToken' => ['dummy' => 'dummy', 'canDelete' => true],
+			'deviceToken' => ['dummy' => 'dummy', 'canDelete' => true, 'canRename' => true],
 			'loginName' => 'User13',
 		];
 
@@ -260,7 +261,7 @@ class AuthSettingsControllerTest extends TestCase {
 			->method('updateToken')
 			->with($this->equalTo($token));
 
-		$this->assertSame([], $this->controller->update($tokenId, ['filesystem' => true]));
+		$this->assertSame([], $this->controller->update($tokenId, ['filesystem' => true], 'App password'));
 	}
 
 	public function testUpdateTokenWrongUser() {
@@ -278,7 +279,7 @@ class AuthSettingsControllerTest extends TestCase {
 		$this->tokenProvider->expects($this->never())
 			->method('updateToken');
 
-		$response = $this->controller->update($tokenId, ['filesystem' => true]);
+		$response = $this->controller->update($tokenId, ['filesystem' => true], 'App password');
 		$this->assertSame([], $response->getData());
 		$this->assertSame(\OCP\AppFramework\Http::STATUS_NOT_FOUND, $response->getStatus());
 	}
@@ -292,7 +293,7 @@ class AuthSettingsControllerTest extends TestCase {
 		$this->tokenProvider->expects($this->never())
 			->method('updateToken');
 
-		$response = $this->controller->update(42, ['filesystem' => true]);
+		$response = $this->controller->update(42, ['filesystem' => true], 'App password');
 		$this->assertSame([], $response->getData());
 		$this->assertSame(\OCP\AppFramework\Http::STATUS_NOT_FOUND, $response->getStatus());
 	}