fix libre-graph status codes

owncloud · Nov 12, 2023 · 2bfe422 · 2bfe422
1 parent 920265f
commit 2bfe422
Show file tree

Hide file tree

Showing 11 changed files with 52 additions and 37 deletions.
diff --git a/changelog/unreleased/fix-graph-codes.md b/changelog/unreleased/fix-graph-codes.md
@@ -0,0 +1,15 @@
+Bugfix: Fix libre-graph status codes
+
+creating group: https://owncloud.dev/libre-graph-api/#/groups/CreateGroup
+changed: 200 -> 201
+
+creating users:
+https://owncloud.dev/libre-graph-api/#/users/CreateUser
+changed: 200 -> 201
+
+export GDPR:
+https://owncloud.dev/libre-graph-api/#/user/ExportPersonalData
+changed: 201 -> 202
+
+https://github.com/owncloud/ocis/issues/7678
+https://github.com/owncloud/ocis/pull/7705
diff --git a/services/graph/pkg/service/v0/groups.go b/services/graph/pkg/service/v0/groups.go
@@ -97,7 +97,7 @@ func (g Graph) PostGroup(w http.ResponseWriter, r *http.Request) {
 		}
 		g.publishEvent(r.Context(), e)
 	}
-	render.Status(r, http.StatusOK) // FIXME 201 should return 201 created
+	render.Status(r, http.StatusCreated)
 	render.JSON(w, r, grp)
 }
 

diff --git a/services/graph/pkg/service/v0/groups_test.go b/services/graph/pkg/service/v0/groups_test.go
@@ -259,7 +259,7 @@ var _ = Describe("Groups", func() {
 
 			svc.PostGroup(rr, r)
 
-			Expect(rr.Code).To(Equal(http.StatusOK))
+			Expect(rr.Code).To(Equal(http.StatusCreated))
 		})
 	})
 	Describe("PatchGroup", func() {

diff --git a/services/graph/pkg/service/v0/personaldata.go b/services/graph/pkg/service/v0/personaldata.go
@@ -87,7 +87,7 @@ func (g Graph) ExportPersonalData(w http.ResponseWriter, r *http.Request) {
 	// go start gathering
 	go g.GatherPersonalData(u, ref, r.Header.Get(revactx.TokenHeader), marsh)
 
-	w.WriteHeader(http.StatusCreated)
+	w.WriteHeader(http.StatusAccepted)
 }
 
 // GatherPersonalData will all gather all personal data of the user and save it to a file in the users personal space

diff --git a/services/graph/pkg/service/v0/users.go b/services/graph/pkg/service/v0/users.go
@@ -334,7 +334,7 @@ func (g Graph) PostUser(w http.ResponseWriter, r *http.Request) {
 	}
 	g.publishEvent(r.Context(), e)
 
-	render.Status(r, http.StatusOK) // FIXME 201 should return 201 created
+	render.Status(r, http.StatusCreated)
 	render.JSON(w, r, u)
 }
 

diff --git a/services/graph/pkg/service/v0/users_test.go b/services/graph/pkg/service/v0/users_test.go
@@ -617,7 +617,7 @@ var _ = Describe("Users", func() {
 			r = r.WithContext(revactx.ContextSetUser(ctx, currentUser))
 			svc.PostUser(rr, r)
 
-			Expect(rr.Code).To(Equal(http.StatusOK))
+			Expect(rr.Code).To(Equal(http.StatusCreated))
 			data, err := io.ReadAll(rr.Body)
 			Expect(err).ToNot(HaveOccurred())
 
@@ -642,7 +642,7 @@ var _ = Describe("Users", func() {
 			r = r.WithContext(revactx.ContextSetUser(ctx, currentUser))
 			svc.PostUser(rr, r)
 
-			Expect(rr.Code).To(Equal(http.StatusOK))
+			Expect(rr.Code).To(Equal(http.StatusCreated))
 			data, err := io.ReadAll(rr.Body)
 			Expect(err).ToNot(HaveOccurred())
 
@@ -667,7 +667,7 @@ var _ = Describe("Users", func() {
 			r = r.WithContext(revactx.ContextSetUser(ctx, currentUser))
 			svc.PostUser(rr, r)
 
-			Expect(rr.Code).To(Equal(http.StatusOK))
+			Expect(rr.Code).To(Equal(http.StatusCreated))
 			data, err := io.ReadAll(rr.Body)
 			Expect(err).ToNot(HaveOccurred())
 
@@ -728,7 +728,7 @@ var _ = Describe("Users", func() {
 				r = r.WithContext(revactx.ContextSetUser(ctx, currentUser))
 				newSvc("none").PostUser(rr, r)
 
-				Expect(rr.Code).To(Equal(http.StatusOK))
+				Expect(rr.Code).To(Equal(http.StatusCreated))
 			})
 		})
 

diff --git a/tests/acceptance/features/apiGraph/createGroup.feature b/tests/acceptance/features/apiGraph/createGroup.feature
@@ -10,7 +10,7 @@ Feature: create group
 
   Scenario Outline: admin user creates a group
     When user "Alice" creates a group "<groupname>" using the Graph API
-    Then the HTTP status code should be "200"
+    Then the HTTP status code should be "201"
     And group "<groupname>" should exist
     Examples:
     | groupname       |

diff --git a/tests/acceptance/features/apiGraph/createUser.feature b/tests/acceptance/features/apiGraph/createUser.feature
@@ -22,18 +22,18 @@ Feature: create user
     And user "<userName>" <shouldOrNot> exist
     Examples:
       | userName                     | displayName                             | email                   | password                     | code | enable | shouldOrNot |
-      | SameDisplayName              | Alice Hansen                            | new@example.org         | containsCharacters(*:!;_+-&) | 200  | true   | should      |
-      | withoutPassSameEmail         | without pass                            | alice@example.org       |                              | 200  | true   | should      |
-      | name                         | pass with space                         | example@example.org     | my pass                      | 200  | true   | should      |
-      | user1                        | user names must not start with a number | example@example.org     | my pass                      | 200  | true   | should      |
+      | SameDisplayName              | Alice Hansen                            | new@example.org         | containsCharacters(*:!;_+-&) | 201  | true   | should      |
+      | withoutPassSameEmail         | without pass                            | alice@example.org       |                              | 201  | true   | should      |
+      | name                         | pass with space                         | example@example.org     | my pass                      | 201  | true   | should      |
+      | user1                        | user names must not start with a number | example@example.org     | my pass                      | 201  | true   | should      |
       | nameWithCharacters(*:!;_+-&) | user                                    | new@example.org         | 123                          | 400  | true   | should not  |
       | name with space              | name with space                         | example@example.org     | 123                          | 400  | true   | should not  |
-      | createDisabledUser           | disabled user                           | example@example.org     | 123                          | 200  | false  | should      |
-      | nameWithNumbers0123456       | user                                    | name0123456@example.org | 123                          | 200  | true   | should      |
-      | name.with.dots               | user                                    | name.w.dots@example.org | 123                          | 200  | true   | should      |
+      | createDisabledUser           | disabled user                           | example@example.org     | 123                          | 201  | false  | should      |
+      | nameWithNumbers0123456       | user                                    | name0123456@example.org | 123                          | 201  | true   | should      |
+      | name.with.dots               | user                                    | name.w.dots@example.org | 123                          | 201  | true   | should      |
       | 123456789                    | user                                    | 123456789@example.org   | 123                          | 400  | true   | should not  |
       | 0.0                          | user                                    | float@example.org       | 123                          | 400  | true   | should not  |
-      | withoutEmail                 | without email                           |                         | 123                          | 200  | true   | should      |
+      | withoutEmail                 | without email                           |                         | 123                          | 201  | true   | should      |
       | Alice                        | same userName                           | new@example.org         | 123                          | 409  | true   | should      |
 
 
@@ -88,7 +88,7 @@ Feature: create user
       | email          | brian@example.com     |
       | password       | 123                   |
       | accountEnabled | true                  |
-    Then the HTTP status code should be "200"
+    Then the HTTP status code should be "201"
     And user "Brian" should exist
 
 
@@ -102,7 +102,7 @@ Feature: create user
       | email          | new@example.org |
       | password       | 123             |
       | accountEnabled | true            |
-    Then the HTTP status code should be "200"
+    Then the HTTP status code should be "201"
     And user "<userName>" should exist
     Examples:
       | userName          | description                                 |
@@ -119,7 +119,7 @@ Feature: create user
       | email          | new@example.org |
       | password       | 123             |
       | accountEnabled | true            |
-    Then the HTTP status code should be "200"
+    Then the HTTP status code should be "201"
     And user "sam" should exist
     When the administrator retrieves the assigned role of user "sam" using the Graph API
     Then the HTTP status code should be "200"
@@ -135,7 +135,7 @@ Feature: create user
       | email          | new@example.org |
       | password       | 123             |
       | accountEnabled | true            |
-    Then the HTTP status code should be "200"
+    Then the HTTP status code should be "201"
     And user "sam" should exist
     And user "sam" should have the role "User" assigned
 
@@ -147,6 +147,6 @@ Feature: create user
       | email          | new@example.org |
       | password       | 123             |
       | accountEnabled | true            |
-    Then the HTTP status code should be "200"
+    Then the HTTP status code should be "201"
     And user "sam" should exist
     And user "sam" should have the role "User" assigned
diff --git a/tests/acceptance/features/apiGraph/userGDPRExport.feature b/tests/acceptance/features/apiGraph/userGDPRExport.feature
@@ -11,7 +11,7 @@ Feature: user GDPR (General Data Protection Regulation) report
   Scenario: generate a GDPR report and check user data in the downloaded report
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain key 'user' and match
     """
     {
@@ -74,7 +74,7 @@ Feature: user GDPR (General Data Protection Regulation) report
   Scenario: generate a GDPR report and check events when a user is created
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain event type "events.UserCreated" in item 'events' and should match
     """
     {
@@ -183,7 +183,7 @@ Feature: user GDPR (General Data Protection Regulation) report
     Given user "Alice" has uploaded file with content "sample text" to "lorem.txt"
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain event type "events.BytesReceived" in item 'events' and should match
     """
     {
@@ -309,7 +309,7 @@ Feature: user GDPR (General Data Protection Regulation) report
     And user "Alice" has been added to group "tea-lover"
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain event type "events.GroupMemberAdded" in item 'events' and should match
     """
     {
@@ -413,7 +413,7 @@ Feature: user GDPR (General Data Protection Regulation) report
     Given user "Admin" has changed the quota of the personal space of "Alice Hansen" space to "10000"
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain event type "events.SpaceUpdated" in item 'events' and should match
     """
     {
@@ -523,7 +523,7 @@ Feature: user GDPR (General Data Protection Regulation) report
     Given user "Alice" has created folder "/folderMain"
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain event type "events.ContainerCreated" in item 'events' and should match
     """
     {
@@ -589,7 +589,7 @@ Feature: user GDPR (General Data Protection Regulation) report
     And user "Alice" has shared entry "/folderMain" with user "Brian"
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain event type "events.ShareCreated" in item 'events' and should match
     """
     {
@@ -717,7 +717,7 @@ Feature: user GDPR (General Data Protection Regulation) report
       | name | sharedlink  |
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain event type "events.LinkCreated" in item 'events' and should match
     """
     {
@@ -814,7 +814,7 @@ Feature: user GDPR (General Data Protection Regulation) report
     And user "Alice" has deleted folder "/folderMain"
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain event type "events.ItemTrashed" in item 'events' and should match
     """
     {
@@ -861,7 +861,7 @@ Feature: user GDPR (General Data Protection Regulation) report
       | role      | viewer |
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain event type "events.SpaceShared" in item 'events' and should match
     """
     {
@@ -961,7 +961,7 @@ Feature: user GDPR (General Data Protection Regulation) report
     And user "Alice" has created a space "GDPR Space" with the default quota using the Graph API
     When user "Alice" exports her GDPR report to "/.personal_data_export.json" using the Graph API
     And user "Alice" downloads the content of GDPR report ".personal_data_export.json"
-    Then the HTTP status code of responses on each endpoint should be "201, 200" respectively
+    Then the HTTP status code of responses on each endpoint should be "202, 200" respectively
     And the downloaded JSON content should contain event type "events.SpaceCreated" for "project" space and should match
     """
     {

diff --git a/tests/acceptance/features/bootstrap/GraphContext.php b/tests/acceptance/features/bootstrap/GraphContext.php
@@ -916,7 +916,7 @@ public function userCreatesGroupUsingTheGraphApi(string $group, ?string $user =
 		$this->featureContext->setResponse($response);
 		$this->featureContext->pushToLastHttpStatusCodesArray((string) $response->getStatusCode());
 
-		if ($response->getStatusCode() === 200) {
+		if ($response->getStatusCode() === 201) {
 			$groupId = $this->featureContext->getJsonDecodedResponse($response)["id"];
 			$this->featureContext->addGroupToCreatedGroupsList($group, true, true, $groupId);
 		}
@@ -936,7 +936,7 @@ public function userCreatesGroupUsingTheGraphApi(string $group, ?string $user =
 	public function userHasCreatedGroupUsingTheGraphApi(string $group, ?string $user = null): void {
 		$response = $this->createGroup($group, $user);
 
-		if ($response->getStatusCode() === 200) {
+		if ($response->getStatusCode() === 201) {
 			$groupId = $this->featureContext->getJsonDecodedResponse($response)["id"];
 			$this->featureContext->addGroupToCreatedGroupsList($group, true, true, $groupId);
 		} else {
@@ -955,7 +955,7 @@ public function userHasCreatedGroupUsingTheGraphApi(string $group, ?string $user
 	 */
 	public function adminHasCreatedGroupUsingTheGraphApi(string $group): array {
 		$result = $this->createGroup($group);
-		if ($result->getStatusCode() === 200) {
+		if ($result->getStatusCode() === 201) {
 			return $this->featureContext->getJsonDecodedResponse($result);
 		} else {
 			$this->throwHttpException($result, "Could not create group '$group'.");

diff --git a/tests/acceptance/features/bootstrap/Provisioning.php b/tests/acceptance/features/bootstrap/Provisioning.php
@@ -2647,7 +2647,7 @@ public function userHasBeenCreated(
 				$displayName,
 			);
 			Assert::assertEquals(
-				200,
+				201,
 				$response->getStatusCode(),
 				__METHOD__ . " cannot create user '$user' using Graph API.\nResponse:" .
 				json_encode($this->getJsonDecodedResponse($response))