circles icon indicating copy to clipboard operation
circles copied to clipboard

missing $currentAccess handling in ShareByCircleProvider::getAccessList() leads to errors in nextcloud server

Open Messj1 opened this issue 7 months ago • 0 comments

Situation

There are PHP errors accessing an string as array which is related to the circle app.

    "Exception": "TypeError",
    "Message": "Cannot access offset of type string on string",
    "Code": 0,
    "Trace": [
      {
        "file": "/var/www/html/lib/private/Share20/ShareHelper.php",
        "line": 53,
        "function": "getPathsForUsers",
        "class": "OC\\Share20\\ShareHelper",
        "type": "->",

The array (access list) is produced in ShareHelper::getPathsForAccessList() on L51

		$accessList = $this->shareManager->getAccessList($node, true, true);
		if (!empty($accessList['users'])) {
			$result['users'] = $this->getPathsForUsers($node, $accessList['users']);
		}

The getAccessList() function in Manager.php

	public function getAccessList(\OCP\Files\Node $path, $recursive = true, $currentAccess = false) {

As one can see, the Manager::getAccessList() is called with currentAccess set as true.

implementation contract

ShareByCircleProvider implements IShareProvider

https://github.com/nextcloud/circles/blob/8fb0ccf68d3e5f7746c2e2eac338a81fc5d6f34b/lib/ShareByCircleProvider.php#L73

having a look at IShareProvider::getAccessList() phpDoc

	/**
	 * Get the access list to the array of provided nodes.
	 *
	 * @see IManager::getAccessList() for sample docs
	 *
	 * @param Node[] $nodes The list of nodes to get access for
	 * @param bool $currentAccess If current access is required (like for removed shares that might get revived later)
	 * @return array
	 * @since 12
	 */
	public function getAccessList($nodes, $currentAccess);true, $currentAccess = false) {

looking into IManager::getAccessList() description

		/**
	 * Get access list to a path. This means
	 * all the users that can access a given path.
	 *
	 * Consider:
	 * -root
	 * |-folder1 (23)
	 *  |-folder2 (32)
	 *   |-fileA (42)
	 *
	 * fileA is shared with user1 and user1@server1 and email1@maildomain1
	 * folder2 is shared with group2 (user4 is a member of group2)
	 * folder1 is shared with user2 (renamed to "folder (1)") and user2@server2
	 *                        and email2@maildomain2
	 *
	 * Then the access list to '/folder1/folder2/fileA' with $currentAccess is:
	 * [
	 *  users  => [
	 *      'user1' => ['node_id' => 42, 'node_path' => '/fileA'],
	 *      'user4' => ['node_id' => 32, 'node_path' => '/folder2'],
	 *      'user2' => ['node_id' => 23, 'node_path' => '/folder (1)'],
	 *  ],
	 *  remote => [
	 *      'user1@server1' => ['node_id' => 42, 'token' => 'SeCr3t'],
	 *      'user2@server2' => ['node_id' => 23, 'token' => 'FooBaR'],
	 *  ],
	 *  public => bool
	 *  mail => [
	 *      'email1@maildomain1' => ['node_id' => 42, 'token' => 'aBcDeFg'],
	 *      'email2@maildomain2' => ['node_id' => 23, 'token' => 'hIjKlMn'],
	 *  ]
	 *
	 * The access list to '/folder1/folder2/fileA' **without** $currentAccess is:
	 * [
	 *  users  => ['user1', 'user2', 'user4'],
	 *  remote => bool,
	 *  public => bool
	 *  mail => ['email1@maildomain1', 'email2@maildomain2']
	 * ]
	 *
	 * This is required for encryption/activity
	 *
	 * @param \OCP\Files\Node $path
	 * @param bool $recursive Should we check all parent folders as well
	 * @param bool $currentAccess Should the user have currently access to the file
	 * @return array
	 * @since 12
	 */
	public function getAccessList(\OCP\Files\Node $path, $recursive = true, $currentAccess = false);

Conclusion

If $currentAccess is false: then the access list types (users, remote, public and mail) are a flat list

users  => ['user1', 'user2', 'user4'],

If $currentAccess is true: then the access list types are a list of objects with type specific path to the node

users  => [
    'user1' => ['node_id' => 42, 'node_path' => '/fileA'],
    'user4' => ['node_id' => 32, 'node_path' => '/folder2'],
    'user2' => ['node_id' => 23, 'node_path' => '/folder (1)'],
],

real circle implementation

In the ShareByCircleProvider implementation there is no interpretation of the parameter $currentAccess and the list is always flat, which throws me a TypeError: "Cannot access offset of type string on string"

https://github.com/nextcloud/circles/blob/8fb0ccf68d3e5f7746c2e2eac338a81fc5d6f34b/lib/ShareByCircleProvider.php#L649-L651

My temporary solution is to return an empty array since it is better than trowing errors to the User since it was only used for notification purpose - I guess.

Any other ideas?

Messj1 avatar Jul 17 '24 00:07 Messj1