Merge pull request #4048 from nextcloud/fix/guest-name-view-only

fix(sharing): Don't ask for guest name on view-only share

Author: elzody

cypress/e2e/share-link.js

@@ -8,8 +8,7 @@ import { randHash } from '../utils/index.js'
 const shareOwner = new User(randHash(), randHash())
 const otherUser = new User(randHash(), randHash())
 
-describe('Public sharing of office documents', function() {
-
+describe('Public sharing of office documents', () => {
 	before(function() {
 		cy.createUser(shareOwner)
 		cy.createUser(otherUser)
@@ -20,109 +19,102 @@ describe('Public sharing of office documents', function() {
 	})
 
 	const userMatrix = [shareOwner, otherUser]
-	const guestName = randHash()
 
-	describe('Open a shared file', function() {
-		for (const index in userMatrix) {
-			const viewingUser = userMatrix[index]
+	describe('Share with users', () => {
+		describe('Readonly file', () => {
+			for (const user of userMatrix) {
+				it('Loads readonly file as user: ' + user.userId, () => {
+					cy.shareLink(shareOwner, '/document.odt').then((token) => {
+						cy.login(user)
 
-			it('Loads file as user: ' + viewingUser?.userId, () => {
-				cy.shareLink(shareOwner, '/document.odt').then((token) => {
-					cy.login(viewingUser)
-
-					cy.visit(`/s/${token}`, {
-						onBeforeLoad(win) {
-							cy.spy(win, 'postMessage').as('postMessage')
-						},
-					})
+						cy.visit(`/s/${token}`, {
+							onBeforeLoad(win) {
+								cy.spy(win, 'postMessage').as('postMessage')
+							},
+						})
 
-					cy.waitForCollabora()
-					cy.waitForPostMessage('App_LoadingStatus', { Status: 'Document_Loaded' })
+						// Assert that we do not ask for guest name if logged in
+						cy.get('[data-cy="guestNameModal"]').should('not.exist')
 
-					cy.get('@loleafletframe').within(() => {
-						cy.get('#closebutton').click()
+						waitForCollabora()
 					})
-
-					cy.get('#viewer', { timeout: 5000 }).should('not.exist')
 				})
-			})
-		}
+			}
+		})
 
-		it('Loads file as guest: ' + guestName, () => {
-			cy.shareLink(shareOwner, '/document.odt').then((token) => {
-				cy.logout()
+		describe('Editable file', () => {
+			for (const user of userMatrix) {
+				it('Loads editable file as user: ' + user.userId, () => {
+					cy.shareLink(shareOwner, '/document.odt', { permissions: 19 }).then((token) => {
+						cy.login(user)
 
-				cy.visit(`/s/${token}`, {
-					onBeforeLoad(win) {
-						cy.spy(win, 'postMessage').as('postMessage')
-					},
-				})
+						cy.visit(`/s/${token}`, {
+							onBeforeLoad(win) {
+								cy.spy(win, 'postMessage').as('postMessage')
+							},
+						})
 
-				cy.inputCollaboraGuestName(guestName)
-				cy.waitForCollabora()
-				cy.waitForPostMessage('App_LoadingStatus', { Status: 'Document_Loaded' })
+						// Assert that we do not ask for guest name if logged in
+						cy.get('[data-cy="guestNameModal"]').should('not.exist')
 
-				cy.get('@loleafletframe').within(() => {
-					cy.get('#closebutton').click()
+						waitForCollabora()
+					})
 				})
-
-				cy.get('#viewer', { timeout: 5000 }).should('not.exist')
-			})
+			}
 		})
 	})
 
-	describe('Open a file in a shared folder', function() {
-		for (const index in userMatrix) {
-			const viewingUser = userMatrix[index]
-			it('Loads file in shared folder as user: ' + viewingUser?.userId, () => {
-				cy.login(viewingUser)
+	const guestName = randHash()
+	describe('Share with guests', () => {
+		describe('Readonly file', () => {
+			it('Loads readonly file as guest: ' + guestName, () => {
+				cy.shareLink(shareOwner, '/document.odt').then((token) => {
+					cy.logout()
 
-				cy.shareLink(shareOwner, '/my-share').then((token) => {
 					cy.visit(`/s/${token}`, {
 						onBeforeLoad(win) {
 							cy.spy(win, 'postMessage').as('postMessage')
 						},
 					})
 
-					cy.get('button[data-cy-files-list-row-name-link=""]')
-						.click()
-
-					cy.waitForViewer()
-					cy.waitForCollabora()
-					cy.waitForPostMessage('App_LoadingStatus', { Status: 'Document_Loaded' })
+					// Assert that we do not ask for guest name if we can't edit as a guest
+					cy.get('[data-cy="guestNameModal"]').should('not.exist')
 
-					cy.get('@loleafletframe').within(() => {
-						cy.get('#closebutton').click()
-					})
-
-					cy.get('#viewer', { timeout: 5000 }).should('not.exist')
+					waitForCollabora()
 				})
 			})
-		}
-
-		it('Loads file in shared folder as guest: ' + guestName, () => {
-			cy.shareLink(shareOwner, '/my-share').then((token) => {
-				cy.logout()
+		})
 
-				cy.visit(`/s/${token}`, {
-					onBeforeLoad(win) {
-						cy.spy(win, 'postMessage').as('postMessage')
-					},
-				})
+		describe('Editable file', () => {
+			it('Loads editable file as guest: ' + guestName, () => {
+				cy.shareLink(shareOwner, '/document.odt', { permissions: 19 }).then((token) => {
+					cy.logout()
 
-				cy.get('button[data-cy-files-list-row-name-link=""]').click()
+					cy.visit(`/s/${token}`, {
+						onBeforeLoad(win) {
+							cy.spy(win, 'postMessage').as('postMessage')
+						},
+					})
 
-				cy.inputCollaboraGuestName(guestName)
-				cy.waitForViewer()
-				cy.waitForCollabora()
-				cy.waitForPostMessage('App_LoadingStatus', { Status: 'Document_Loaded' })
+					// Assert that we do ask for guest name if we can edit as a guest
+					cy.get('[data-cy="guestNameModal"]').should('be.visible')
 
-				cy.get('@loleafletframe').within(() => {
-					cy.get('#closebutton').click()
+					cy.inputCollaboraGuestName(guestName)
+					waitForCollabora()
 				})
-
-				cy.get('#viewer', { timeout: 5000 }).should('not.exist')
 			})
 		})
 	})
 })
+
+function waitForCollabora() {
+	cy.waitForViewer()
+	cy.waitForCollabora()
+	cy.waitForPostMessage('App_LoadingStatus', { Status: 'Document_Loaded' })
+
+	cy.get('@loleafletframe').within(() => {
+		cy.get('#closebutton').click()
+	})
+
+	cy.get('#viewer', { timeout: 5000 }).should('not.exist')
+}

src/helpers/guestName.js

@@ -21,5 +21,9 @@ export function shouldAskForGuestName(mimetype, canWrite) {
 	const noCurrentUser = !getCurrentUser() || getCurrentUser()?.uid === ''
 	const isReadOnlyPDF = mimetype === 'application/pdf' && !canWrite
 
+	if (!canWrite) {
+		return false
+	}
+
 	return noLoggedInUser && noGuest && noCurrentUser && !isReadOnlyPDF
 }