DAV endpoint #7

PVince81 · 2016-12-06T11:55:21Z

REQUIRES: Implement group backend #1 to be merged

This PR is based on top of #1.

This adds DAV endpoints for #3

CLAassistant · 2016-12-06T11:55:23Z

All committers have signed the CLA.

PVince81 · 2016-12-13T11:19:37Z

Rebased, added lots of tests.

A few calls and conditions are missing still.

PVince81 · 2016-12-13T16:54:51Z

Already implemented the main part of the spec #3 for managing groups and memberships.

Next step: REPORT

PVince81 · 2016-12-13T17:53:47Z

I've added the REPORT as well:

Here is how to get one.
Put this into report-customgroups.xml:

<?xml version="1.0" encoding="utf-8" ?>
<oc:filter-members xmlns:a="DAV:" xmlns:oc="http://owncloud.org/ns" >
	<oc:filter-rules>
		<oc:user-id>user1</oc:user-id>
		<!--	<oc:is-admin>1</oc:is-admin> -->
	</oc:filter-rules>
</oc:filter-members>

Then curl -u admin:admin -X REPORT -H "Content-Type: text/xml" --data-binary "@report-customgroups.xml" 'http://localhost/owncloud/remote.php/dav/customgroups/'

Gives:

<?xml version="1.0"?>
<d:multistatus xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns" xmlns:cal="urn:ietf:params:xml:ns:caldav" xmlns:cs="http://calendarserver.org/ns/" xmlns:card="urn:ietf:params:xml:ns:carddav" xmlns:oc="http://owncloud.org/ns">
 <d:response>
  <d:status>HTTP/1.1 200 OK</d:status>
  <d:href>/owncloud/remote.php/dav/customgroups/x/user1</d:href>
  <d:propstat>
   <d:prop>
    <oc:is-admin>0</oc:is-admin>
   </d:prop>
   <d:status>HTTP/1.1 200 OK</d:status>
  </d:propstat>
 </d:response>
 <d:response>
  <d:status>HTTP/1.1 200 OK</d:status>
  <d:href>/owncloud/remote.php/dav/customgroups/y/user1</d:href>
  <d:propstat>
   <d:prop>
    <oc:is-admin>0</oc:is-admin>
   </d:prop>
   <d:status>HTTP/1.1 200 OK</d:status>
  </d:propstat>
 </d:response>
</d:multistatus>

It basically returns the result for the CustomGroupMemberNode. I'm not too happy about this because it will not be useful for the UI. It misses the group display name and the group URI. Well, the group URI can be extracted from the result path.

I'd like to refine this separately once we start working on the UI.

PVince81 · 2016-12-13T18:00:22Z

make it possible for regular admins to do the same things like group admins

The sad thing about adding this is that now I need to inject a group manager everywhere...

DeepDiver1975 · 2016-12-14T18:22:31Z

lib/Dav/CustomGroupMemberNode.php

+/**
+ * @author Vincent Petry <pvince81@owncloud.com>
+ *
+ * @copyright Copyright (c) 2016, ownCloud GmbH.


DeepDiver1975 · 2016-12-14T18:24:07Z

tests/unit/Dav/CustomGroupNodeTest.php

+
+		$memberInfos = $this->node->getChildren();
+
+		$this->assertInstanceOf('\OCA\CustomGroups\Dav\CustomGroupMemberNode', $memberInfos[0]);


CustomGroupMemberNode::class

DeepDiver1975 · 2016-12-14T23:35:06Z

lib/Dav/CustomGroupsPlugin.php

+	 *
+	 * This method should set up the required event subscriptions.
+	 *
+	 * @param Server $server


Server is unknown

DeepDiver1975 · 2016-12-14T23:35:27Z

lib/Dav/CustomGroupsPlugin.php

+	 * @param [] $report
+	 * @param string $uri
+	 * @return bool
+	 * @throws NotFound


NotFound is unknown

DeepDiver1975 · 2016-12-14T23:35:37Z

lib/Dav/CustomGroupsPlugin.php

+	 * @param string $uri
+	 * @return bool
+	 * @throws NotFound
+	 * @throws ReportNotSupported


ReportNotSupported is unknown

DeepDiver1975 · 2016-12-14T23:42:35Z

lib/Dav/CustomGroupsPlugin.php

+	public function onReport($reportName, $report, $uri) {
+		$node = $this->server->tree->getNodeForPath($uri);
+		if(!$node instanceof RootCollection || $reportName !== self::REPORT_NAME) {
+			throw new ReportNotSupported();


ReportNotSupported is unknown

DeepDiver1975 · 2016-12-14T23:42:46Z

lib/Dav/RootCollection.php

+/**
+ * @author Vincent Petry <pvince81@owncloud.com>
+ *
+ * @copyright Copyright (c) 2016, ownCloud GmbH.


remove the .

DeepDiver1975 · 2016-12-14T23:43:11Z

lib/Dav/RootCollection.php

+	 */
+	private $userSession;
+
+	/**


PHPDoc missing

DeepDiver1975 · 2016-12-14T23:45:04Z

lib/Dav/CustomGroupMemberNode.php

+	/**
+	 * Update the admin flag
+	 *
+	 * @param int $isAdminPropValue 1 for true, 0 for false


return tag is missing

DeepDiver1975 · 2016-12-15T00:13:18Z

It basically returns the result for the CustomGroupMemberNode. I'm not too happy about this because it will not be useful for the UI. It misses the group display name and the group URI. Well, the group URI can be extracted from the result path.

please have a look at the carddav query request - there is a node where you select the elements which you expect to be returned ...

https://tools.ietf.org/html/rfc6352#section-8.6.3

     <D:prop>
       <D:getetag/>
       <CG:display/>
     </D:prop>

PVince81 · 2016-12-19T10:16:21Z

Are you kidding me...

Analysing /home/travis/build/owncloud/core/apps/customgroups/lib/composer/composer/ClassLoader.php

 1 errors

    line  317: == - is discouraged

App is not compliant

make: *** [test-codecheck] Error 101

Will need to exclude the composer stuff from the code checker

PVince81 · 2016-12-19T17:24:38Z

Squashed and removed the composer files I checked in by mistake from the other branch.

PVince81 · 2016-12-19T17:25:49Z

ownCloud admins can now also manage groups even when they aren't members.

Instead of passing around yet another object IGroupManaber, I created a helper class instead MembershipHelper which takes care of checking whether the current user can or cannot manage stuff based on their role and memberships.

PVince81 · 2016-12-19T17:34:20Z

@DeepDiver1975 please re-review.

My next step is to adjust the REPORT to implement pagination, but this could also be done separately if this PR is already mergable.

PVince81 · 2016-12-19T18:46:19Z

I'm feeling a bit lazy with the REPORT + pagination especially if I need to do it for all the different endpoints. So I came up with this generic paginated report idea, please check it out: owncloud/core#26840

PVince81 · 2016-12-20T18:14:13Z

REPORT will be done in a separate PR.

@DeepDiver1975 please review

DeepDiver1975 · 2017-01-19T08:24:52Z

lib/Dav/CustomGroupsPlugin.php

+
+		$this->server->xml->namespaceMap[self::NS_OWNCLOUD] = 'oc';
+		$ns = '{' . self::NS_OWNCLOUD . '}';
+        $this->server->resourceTypeMapping['OCA\\CustomGroups\\Dav\\MembershipNode'] = $ns . 'customgroups-membership';


and use ::class

DeepDiver1975 · 2017-01-30T22:32:06Z

lib/Dav/GroupMembershipCollection.php

+	public function createFile($userId, $data = null) {
+		$groupId = $this->groupInfo['group_id'];
+		if (!$this->helper->isUserAdmin($groupId)) {
+			throw new Forbidden('No permission to add members to group "' . $groupId . '"');


I'd use string interpolation for better readability

throw new Forbidden("No permission to add members to group '$groupId'");

DeepDiver1975 · 2017-01-31T09:42:23Z

listing users does not work /remote.php/dav/customgroups/users/

PVince81 · 2017-01-31T10:03:49Z

listing users does not work /remote.php/dav/customgroups/users/

This is by design, same like the "users" list principal.

You need to know the user name you want to query.

DeepDiver1975 · 2017-01-31T10:11:27Z

This is by design, same like the "users" list principal.

Okay - in debug mode we allow listing of users in other collections as well

PVince81 · 2017-01-31T16:20:31Z

Okay - in debug mode we allow listing of users in other collections as well

Yet another code path to test / unit test and maintain. I'd rather not.

Unless we make it more useful by enabling user listing for any admins
And even then, the admin should be able to get a listing from the other endpoint.

Let me know what you think.

PVince81 · 2017-02-02T18:59:50Z

Adjusted exception formatting.

Added missing case: ownCloud admin can now enter the "users/$userId" subcollection even if not a member.

@DeepDiver1975 I'd like to not add user listing here, it would require injecting the user manager, etc. I don't think it's useful enough, especially if only used in debug mode.

PVince81 added this to the 9.2 milestone Dec 6, 2016

This was referenced Dec 6, 2016

Implement group backend #1

Merged

Webdav API for groups management #3

Closed

PVince81 added the 2 - Developing label Dec 6, 2016

PVince81 force-pushed the dav-endpoint branch from de35e52 to fed5bef Compare December 13, 2016 11:19

PVince81 force-pushed the dav-endpoint branch from fed5bef to cb2f959 Compare December 13, 2016 16:54

PVince81 added 3 - To Review and removed 2 - Developing labels Dec 13, 2016

PVince81 changed the title ~~[WIP] DAV endpoint~~ DAV endpoint Dec 13, 2016

PVince81 requested a review from DeepDiver1975 December 13, 2016 17:54

PVince81 force-pushed the dav-endpoint branch from 0480c8c to 68df270 Compare December 13, 2016 17:56

DeepDiver1975 requested changes Dec 15, 2016

View reviewed changes

PVince81 force-pushed the dav-endpoint branch from 1e16954 to 8ff08b9 Compare December 16, 2016 20:43

PVince81 force-pushed the dav-endpoint branch from 8ff08b9 to 628cf27 Compare December 19, 2016 17:24

DeepDiver1975 requested changes Jan 31, 2017

View reviewed changes

Vincent Petry and others added 3 commits February 2, 2017 17:05

Adjust migrations to the new API

d3129dd

Add DAV endpoint

5c664e4

Also allow admin to enter the user's membership collection

1f2b0e3

PVince81 force-pushed the dav-endpoint branch from 628cf27 to 1f2b0e3 Compare February 2, 2017 19:00

DeepDiver1975 approved these changes Feb 3, 2017

View reviewed changes

DeepDiver1975 merged commit ad4d5ab into master Feb 3, 2017

DeepDiver1975 deleted the dav-endpoint branch February 3, 2017 12:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAV endpoint #7

DAV endpoint #7

PVince81 commented Dec 6, 2016 •

edited

Loading

CLAassistant commented Dec 6, 2016 •

edited

Loading

PVince81 commented Dec 13, 2016

PVince81 commented Dec 13, 2016

PVince81 commented Dec 13, 2016

PVince81 commented Dec 13, 2016 •

edited

Loading

DeepDiver1975 Dec 14, 2016

DeepDiver1975 Dec 14, 2016

DeepDiver1975 Dec 14, 2016

DeepDiver1975 Dec 14, 2016

DeepDiver1975 Dec 14, 2016

DeepDiver1975 Dec 14, 2016

DeepDiver1975 Dec 14, 2016

DeepDiver1975 Dec 14, 2016

DeepDiver1975 Dec 14, 2016

DeepDiver1975 commented Dec 15, 2016

PVince81 commented Dec 19, 2016

PVince81 commented Dec 19, 2016

PVince81 commented Dec 19, 2016

PVince81 commented Dec 19, 2016

PVince81 commented Dec 19, 2016

PVince81 commented Dec 20, 2016

DeepDiver1975 Jan 19, 2017

DeepDiver1975 Jan 30, 2017

DeepDiver1975 Jan 30, 2017

DeepDiver1975 commented Jan 31, 2017

PVince81 commented Jan 31, 2017

DeepDiver1975 commented Jan 31, 2017

PVince81 commented Jan 31, 2017

PVince81 commented Feb 2, 2017


		$memberInfos = $this->node->getChildren();

		$this->assertInstanceOf('\OCA\CustomGroups\Dav\CustomGroupMemberNode', $memberInfos[0]);

DAV endpoint #7

DAV endpoint #7

Conversation

PVince81 commented Dec 6, 2016 • edited Loading

CLAassistant commented Dec 6, 2016 • edited Loading

PVince81 commented Dec 13, 2016

PVince81 commented Dec 13, 2016

PVince81 commented Dec 13, 2016

PVince81 commented Dec 13, 2016 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DeepDiver1975 commented Dec 15, 2016

PVince81 commented Dec 19, 2016

PVince81 commented Dec 19, 2016

PVince81 commented Dec 19, 2016

PVince81 commented Dec 19, 2016

PVince81 commented Dec 19, 2016

PVince81 commented Dec 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DeepDiver1975 commented Jan 31, 2017

PVince81 commented Jan 31, 2017

DeepDiver1975 commented Jan 31, 2017

PVince81 commented Jan 31, 2017

PVince81 commented Feb 2, 2017

PVince81 commented Dec 6, 2016 •

edited

Loading

CLAassistant commented Dec 6, 2016 •

edited

Loading

PVince81 commented Dec 13, 2016 •

edited

Loading