internal links shouldn't have a password (#8668)

* internal links shouldn't have a password

* commented out incorrect cases

changelog/unreleased/fix-graph-internal-link.md

@@ -0,0 +1,6 @@
+Bugfix: Internal links shouldn't have a password
+
+Internal links shouldn't have a password when create/update
+
+https://github.com/owncloud/ocis/pull/8668
+https://github.com/owncloud/ocis/issues/8619

services/graph/pkg/service/v0/driveitems_test.go

@@ -814,6 +814,75 @@ var _ = Describe("Driveitems", func() {
 			linkType := res.Link.GetType()
 			Expect(string(linkType)).To(Equal("edit"))
 		})
+		It("updates the public share to internal link", func() {
+			getShareMockResponse.Share = nil
+			getShareMockResponse.Status = status.NewNotFound(ctx, "not found")
+
+			getPublicShareMock := gatewayClient.On("GetPublicShare",
+				mock.Anything,
+				mock.MatchedBy(func(req *link.GetPublicShareRequest) bool {
+					return req.GetRef().GetId().GetOpaqueId() == "permissionid"
+				}),
+			)
+			getPublicShareMockResponse = &link.GetPublicShareResponse{
+				Status: status.NewOK(ctx),
+				Share: &link.PublicShare{
+					Id: &link.PublicShareId{
+						OpaqueId: "permissionid",
+					},
+					ResourceId: &provider.ResourceId{
+						StorageId: "1",
+						SpaceId:   "2",
+						OpaqueId:  "3",
+					},
+					PasswordProtected: true,
+					Permissions: &link.PublicSharePermissions{
+						Permissions: linktype.NewFileEditLinkPermissionSet().GetPermissions(),
+					},
+					Token: "token",
+				},
+			}
+			getPublicShareMock.Return(getPublicShareMockResponse, nil)
+
+			newLink := libregraph.NewSharingLink()
+			newLinkType, err := libregraph.NewSharingLinkTypeFromValue("internal")
+			Expect(err).ToNot(HaveOccurred())
+			newLink.SetType(*newLinkType)
+
+			updatePublicShareMock := gatewayClient.On("UpdatePublicShare",
+				mock.Anything,
+				mock.MatchedBy(func(req *link.UpdatePublicShareRequest) bool {
+					return req.GetRef().GetId().GetOpaqueId() == "permissionid"
+				}),
+			)
+
+			updatePublicShareMockResponse.Share.Permissions = &link.PublicSharePermissions{
+				Permissions: linktype.NewInternalLinkPermissionSet().Permissions,
+			}
+			updatePublicShareMock.Return(updatePublicShareMockResponse, nil)
+
+			driveItemPermission.SetLink(*newLink)
+			body, err := driveItemPermission.MarshalJSON()
+			Expect(err).To(BeNil())
+			svc.UpdatePermission(
+				rr,
+				httptest.NewRequest(http.MethodPatch, "/", strings.NewReader(string(body))).
+					WithContext(ctx),
+			)
+			Expect(rr.Code).To(Equal(http.StatusOK))
+			data, err := io.ReadAll(rr.Body)
+			Expect(err).ToNot(HaveOccurred())
+
+			res := libregraph.Permission{}
+
+			err = json.Unmarshal(data, &res)
+			Expect(err).ToNot(HaveOccurred())
+			linkType := res.Link.GetType()
+			Expect(string(linkType)).To(Equal("internal"))
+			pp, hasPP := res.GetHasPasswordOk()
+			Expect(hasPP).To(Equal(true))
+			Expect(*pp).To(Equal(false))
+		})
 		It("updates the password on a public share", func() {
 			getShareMockResponse.Share = nil
 			getShareMockResponse.Status = status.NewNotFound(ctx, "not found")

services/graph/pkg/service/v0/links.go

@@ -133,6 +133,9 @@ func (g Graph) createLink(ctx context.Context, driveItemID *providerv1beta1.Reso
 		g.logger.Debug().Interface("createLink", createLink).Msg(err.Error())
 		return nil, errorcode.New(errorcode.InvalidRequest, "invalid link type")
 	}
+	if createLink.GetType() == libregraph.INTERNAL && len(createLink.GetPassword()) > 0 {
+		return nil, errorcode.New(errorcode.InvalidRequest, "password is redundant for the internal link")
+	}
 	req := link.CreatePublicShareRequest{
 		ResourceInfo: statResp.GetInfo(),
 		Grant: &link.Grant{
@@ -312,6 +315,13 @@ func (g Graph) updatePublicLinkPermission(ctx context.Context, permissionID stri
 		if err != nil {
 			return nil, err
 		}
+		// reset the password for the internal link
+		if changedLink == libregraph.INTERNAL {
+			perm, err = g.updatePublicLinkPassword(ctx, permissionID, "")
+			if err != nil {
+				return nil, err
+			}
+		}
 	}
 
 	return perm, err

tests/acceptance/features/apiSharingNg/linkShare.feature

@@ -72,7 +72,7 @@ Feature: Create a share link for a resource
       | permissionsRole |
       | view            |
       | edit            |
-      | internal        |
+#      | internal        |
       | upload          |
       | createOnly      |
       | blocksDownload  |
@@ -143,7 +143,7 @@ Feature: Create a share link for a resource
       | permissionsRole |
       | view            |
       | edit            |
-      | internal        |
+#      | internal        |
       | blocksDownload  |
 
   @issue-7879
@@ -219,7 +219,7 @@ Feature: Create a share link for a resource
       | permissionsRole |
       | view            |
       | edit            |
-      | internal        |
+#      | internal        |
       | upload          |
       | createOnly      |
       | blocksDownload  |
@@ -297,7 +297,7 @@ Feature: Create a share link for a resource
       | permissionsRole |
       | view            |
       | edit            |
-      | internal        |
+#      | internal        |
       | blocksDownload  |
 
   @env-config @issue-7879
@@ -440,14 +440,14 @@ Feature: Create a share link for a resource
     Examples:
       | previousPermissionsRole | newPermissionsRole |
       | view                    | edit               |
-      | view                    | internal           |
+#      | view                    | internal           |
       | view                    | blocksDownload     |
       | edit                    | view               |
       | edit                    | blocksDownload     |
-      | view                    | internal           |
+#      | view                    | internal           |
       | blocksDownload          | edit               |
       | blocksDownload          | blocksDownload     |
-      | view                    | internal           |
+#      | view                    | internal           |
 
 
   Scenario: update expiration date of a file's link share