Skip to content

Commit

Permalink
fix: Access node owner by top level owner property
Browse files Browse the repository at this point in the history
Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
susnux committed Aug 16, 2024
1 parent c8e09d1 commit aeb6317
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 13 deletions.
1 change: 1 addition & 0 deletions apps/files/src/components/FileEntry/FileEntryActions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
:close-after-click="!isMenu(action.id)"
:data-cy-files-list-row-action="action.id"
:is-menu="isMenu(action.id)"
:aria-label="action.title?.([source], currentView)"
:title="action.title?.([source], currentView)"
@click="onActionClick(action)">
<template #icon>
Expand Down
2 changes: 1 addition & 1 deletion apps/files/src/newMenu/newFolder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const entry = {
source,
id: fileid,
mtime: new Date(),
owner: getCurrentUser()?.uid || null,
owner: context.owner,
permissions: Permission.ALL,
root: context?.root || '/files/' + getCurrentUser()?.uid,
// Include mount-type from parent folder as this is inherited
Expand Down
16 changes: 6 additions & 10 deletions apps/files_sharing/src/actions/sharingStatusAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@ export const action = new FileAction({
displayName(nodes: Node[]) {
const node = nodes[0]
const shareTypes = Object.values(node?.attributes?.['share-types'] || {}).flat() as number[]
const ownerId = node?.attributes?.['owner-id']

if (shareTypes.length > 0
|| (ownerId !== getCurrentUser()?.uid || isExternal(node))) {
|| (node.owner !== getCurrentUser()?.uid || isExternal(node))) {
return t('files_sharing', 'Shared')
}

Expand All @@ -38,15 +37,14 @@ export const action = new FileAction({

title(nodes: Node[]) {
const node = nodes[0]
const ownerId = node?.attributes?.['owner-id']
const ownerDisplayName = node?.attributes?.['owner-display-name']

// Mixed share types
if (Array.isArray(node.attributes?.['share-types']) && node.attributes?.['share-types'].length > 1) {
return t('files_sharing', 'Shared multiple times with different people')
}

if (ownerId && (ownerId !== getCurrentUser()?.uid || isExternal(node))) {
if (node.owner && (node.owner !== getCurrentUser()?.uid || isExternal(node))) {
return t('files_sharing', 'Shared by {ownerDisplayName}', { ownerDisplayName })
}

Expand All @@ -69,7 +67,7 @@ export const action = new FileAction({
}

// Group shares
if (shareTypes.includes(ShareType.Grup)
if (shareTypes.includes(ShareType.Group)
|| shareTypes.includes(ShareType.RemoteGroup)) {
return AccountGroupSvg
}
Expand All @@ -79,9 +77,8 @@ export const action = new FileAction({
return CircleSvg
}

const ownerId = node?.attributes?.['owner-id']
if (ownerId && (ownerId !== getCurrentUser()?.uid || isExternal(node))) {
return generateAvatarSvg(ownerId, isExternal(node))
if (node.owner && (node.owner !== getCurrentUser()?.uid || isExternal(node))) {
return generateAvatarSvg(node.owner, isExternal(node))
}

return AccountPlusSvg
Expand All @@ -93,7 +90,6 @@ export const action = new FileAction({
}

const node = nodes[0]
const ownerId = node?.attributes?.['owner-id']
const shareTypes = node.attributes?.['share-types']
const isMixed = Array.isArray(shareTypes) && shareTypes.length > 0

Expand All @@ -104,7 +100,7 @@ export const action = new FileAction({
}

// If the node is shared by someone else
if (ownerId && (ownerId !== getCurrentUser()?.uid || isExternal(node))) {
if (node.owner !== getCurrentUser()?.uid || isExternal(node)) {
return true
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ async function updateAvailableAccounts(path: string = '/') {
const { contents } = await currentView.value.getContents(path)
const available = new Map<string, IUserSelectData>()
for (const node of contents) {
const owner = node.owner ?? node.attributes['owner-id']
const owner = node.owner
if (owner && !available.has(owner)) {
available.set(owner, {
id: owner,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import NcUserBubble from '@nextcloud/vue/dist/Components/NcUserBubble.js'
const folder = ref<Folder>()
const note = computed<string>(() => folder.value?.attributes.note ?? '')
const user = computed(() => {
const id = folder.value?.attributes?.['owner-id']
const id = folder.value?.owner
const displayName = folder.value?.attributes?.['owner-display-name']
if (id !== getCurrentUser()?.uid) {
return {
Expand Down
105 changes: 105 additions & 0 deletions cypress/e2e/files_sharing/files-inline-action.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*!
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
import type { User } from '@nextcloud/cypress'
import { createShare } from './FilesSharingUtils.ts'
import { closeSidebar, getRowForFile } from '../files/FilesUtils.ts'

describe('files_sharing: Files inline status action', { testIsolation: true }, () => {
/**
* Regression test of https://github.com/nextcloud/server/issues/45723
*/
it('No "shared" tag when user ID is purely numerical', () => {
const user = {
language: 'en',
password: 'test1234',
userId: String(Math.floor(Math.random() * 1000)),

Check failure

Code scanning / CodeQL

Insecure randomness High

This uses a cryptographically insecure random number generated at
Math.random()
in a security context.
} as User
cy.createUser(user)
cy.mkdir(user, '/folder')
cy.login(user)

cy.visit('/apps/files')

getRowForFile('folder')
.should('be.visible')
.find('[data-cy-files-list-row-actions]')
.findByRole('button', { name: 'Shared' })
.should('not.exist')
})

describe('', () => {
let user: User
let sharee: User

beforeEach(() => {
cy.createRandomUser().then(($user) => {
user = $user
})
cy.createRandomUser().then(($user) => {
sharee = $user
})
})

it('Render quick option for sharing', () => {
cy.mkdir(user, '/folder')
cy.login(user)

cy.visit('/apps/files')
getRowForFile('folder')
.should('be.visible')

getRowForFile('folder')
.should('be.visible')
.find('[data-cy-files-list-row-actions]')
.findByRole('button', { name: /Show sharing options/ })
.should('be.visible')
.click()

// check the click opened the sidebar
cy.get('[data-cy-sidebar]')
.should('be.visible')
// and ensure the sharing tab is selected
.findByRole('tab', { name: 'Sharing', selected: true })
.should('exist')
})

it('Render inline status action for sharer', () => {
cy.mkdir(user, '/folder')
cy.login(user)

cy.visit('/apps/files')
getRowForFile('folder')
.should('be.visible')
createShare('folder', sharee.userId)
closeSidebar()

getRowForFile('folder')
.should('be.visible')
.find('[data-cy-files-list-row-actions]')
.findByRole('button', { name: 'Shared' })
.should('be.visible')
})

it('Render inline status action for sharee', () => {
cy.mkdir(user, '/folder')
cy.login(user)

cy.visit('/apps/files')
getRowForFile('folder')
.should('be.visible')
createShare('folder', sharee.userId)
closeSidebar()

cy.login(sharee)
cy.visit('/apps/files')

getRowForFile('folder')
.should('be.visible')
.find('[data-cy-files-list-row-actions]')
.findByRole('button', { name: `Shared by ${user.userId}` })
.should('be.visible')
})
})
})

0 comments on commit aeb6317

Please sign in to comment.