Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(files_external): request strict password auth on credentials enter action #50910

Merged
merged 6 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions apps/files_external/src/actions/enterCredentialsAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { AxiosResponse } from '@nextcloud/axios'
import type { Node } from '@nextcloud/files'
import type { StorageConfig } from '../services/externalStorage'

import { addPasswordConfirmationInterceptors, PwdConfirmationMode } from '@nextcloud/password-confirmation'
import { generateUrl } from '@nextcloud/router'
import { showError, showSuccess, spawnDialog } from '@nextcloud/dialogs'
import { translate as t } from '@nextcloud/l10n'
Expand All @@ -18,6 +19,10 @@ import { FileAction, DefaultType } from '@nextcloud/files'
import { STORAGE_STATUS, isMissingAuthConfig } from '../utils/credentialsUtils'
import { isNodeExternalStorage } from '../utils/externalStorageUtils'

// Add password confirmation interceptors as
// the backend requires the user to confirm their password
addPasswordConfirmationInterceptors(axios)

type CredentialResponse = {
login?: string,
password?: string,
Expand All @@ -31,8 +36,13 @@ type CredentialResponse = {
* @param password The password
*/
async function setCredentials(node: Node, login: string, password: string): Promise<null|true> {
const configResponse = await axios.put(generateUrl('apps/files_external/userglobalstorages/{id}', { id: node.attributes.id }), {
backendOptions: { user: login, password },
const configResponse = await axios.request({
method: 'PUT',
url: generateUrl('apps/files_external/userglobalstorages/{id}', { id: node.attributes.id }),
confirmPassword: PwdConfirmationMode.Strict,
data: {
backendOptions: { user: login, password },
},
}) as AxiosResponse<StorageConfig>

const config = configResponse.data
Expand All @@ -49,8 +59,10 @@ async function setCredentials(node: Node, login: string, password: string): Prom
return true
}

export const ACTION_CREDENTIALS_EXTERNAL_STORAGE = 'credentials-external-storage'

export const action = new FileAction({
id: 'credentials-external-storage',
id: ACTION_CREDENTIALS_EXTERNAL_STORAGE,
displayName: () => t('files', 'Enter missing credentials'),
iconSvgInline: () => LoginSvg,

Expand Down Expand Up @@ -83,7 +95,14 @@ export const action = new FileAction({
))

if (login && password) {
return await setCredentials(node, login, password)
try {
await setCredentials(node, login, password)
showSuccess(t('files_external', 'Credentials successfully set'))
} catch (error) {
showError(t('files_external', 'Error while setting credentials: {error}', {
error: (error as Error).message,
}))
}
}

return null
Expand Down
72 changes: 39 additions & 33 deletions apps/files_external/src/actions/inlineStorageCheckAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import type { AxiosError } from '@nextcloud/axios'
import type { Node } from '@nextcloud/files'

import { FileAction } from '@nextcloud/files'
import { showWarning } from '@nextcloud/dialogs'
import { translate as t } from '@nextcloud/l10n'
import AlertSvg from '@mdi/svg/svg/alert-circle.svg?raw'
Expand All @@ -15,7 +16,6 @@ import '../css/fileEntryStatus.scss'
import { getStatus, type StorageConfig } from '../services/externalStorage'
import { isMissingAuthConfig, STORAGE_STATUS } from '../utils/credentialsUtils'
import { isNodeExternalStorage } from '../utils/externalStorageUtils'
import { FileAction } from '@nextcloud/files'

export const action = new FileAction({
id: 'check-external-storage',
Expand All @@ -34,45 +34,51 @@ export const action = new FileAction({
* @param node The node to render inline
*/
async renderInline(node: Node) {
const span = document.createElement('span')
span.className = 'files-list__row-status'
span.innerHTML = t('files_external', 'Checking storage...')
Copy link
Member

@nickvergessen nickvergessen Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Red CI is related, oh strange CI is only red on PRs based on this, but not here...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


let config = null as unknown as StorageConfig
try {
const response = await getStatus(node.attributes.id, node.attributes.scope === 'system')
config = response.data
Vue.set(node.attributes, 'config', config)
getStatus(node.attributes.id, node.attributes.scope === 'system')
.then(response => {

config = response.data
Vue.set(node.attributes, 'config', config)

if (config.status !== STORAGE_STATUS.SUCCESS) {
throw new Error(config?.statusMessage || t('files_external', 'There was an error with this external storage.'))
}

if (config.status !== STORAGE_STATUS.SUCCESS) {
throw new Error(config?.statusMessage || t('files_external', 'There was an error with this external storage.'))
}
span.remove()
})
.catch(error => {
// If axios failed or if something else prevented
// us from getting the config
if ((error as AxiosError).response && !config) {
showWarning(t('files_external', 'We were unable to check the external storage {basename}', {
basename: node.basename,
}))
}

return null
} catch (error) {
// If axios failed or if something else prevented
// us from getting the config
if ((error as AxiosError).response && !config) {
showWarning(t('files_external', 'We were unable to check the external storage {basename}', {
basename: node.basename,
}))
return null
}
// Reset inline status
span.innerHTML = ''

// Checking if we really have an error
const isWarning = isMissingAuthConfig(config)
const overlay = document.createElement('span')
overlay.classList.add(`files-list__row-status--${isWarning ? 'warning' : 'error'}`)
// Checking if we really have an error
const isWarning = !config ? false : isMissingAuthConfig(config)
const overlay = document.createElement('span')
overlay.classList.add(`files-list__row-status--${isWarning ? 'warning' : 'error'}`)

const span = document.createElement('span')
span.className = 'files-list__row-status'
// Only show an icon for errors, warning like missing credentials
// have a dedicated inline action button
if (!isWarning) {
span.innerHTML = AlertSvg
span.title = (error as Error).message
}

// Only show an icon for errors, warning like missing credentials
// have a dedicated inline action button
if (!isWarning) {
span.innerHTML = AlertSvg
span.title = (error as Error).message
}
span.prepend(overlay)
})

span.prepend(overlay)
return span
}
return span
},

order: 10,
Expand Down
3 changes: 2 additions & 1 deletion apps/files_external/src/actions/openInFilesAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { translate as t } from '@nextcloud/l10n'

import { FileAction, DefaultType } from '@nextcloud/files'
import { STORAGE_STATUS } from '../utils/credentialsUtils'
import { getCurrentUser } from '@nextcloud/auth'

export const action = new FileAction({
id: 'open-in-files-external-storage',
Expand All @@ -32,7 +33,7 @@ export const action = new FileAction({
t('files_external', 'External mount error'),
(redirect) => {
if (redirect === true) {
const scope = node.attributes.scope === 'personal' ? 'user' : 'admin'
const scope = getCurrentUser()?.isAdmin ? 'admin' : 'user'
window.location.href = generateUrl(`/settings/${scope}/externalstorages`)
}
},
Expand Down
5 changes: 4 additions & 1 deletion apps/files_external/src/css/fileEntryStatus.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
*/
.files-list__row-status {
display: flex;
width: 44px;
min-width: 44px;
justify-content: center;
align-items: center;
height: 100%;
text-overflow: ellipsis;
white-space: nowrap;
overflow: hidden;

svg {
width: 24px;
Expand Down
2 changes: 1 addition & 1 deletion apps/files_external/src/views/CredentialsDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export default defineComponent({
computed: {
dialogButtons() {
return [{
label: t('files_external', 'Submit'),
label: t('files_external', 'Confirm'),
type: 'primary',
nativeType: 'submit',
}]
Expand Down
4 changes: 4 additions & 0 deletions cypress/dockerNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ export const startNextcloud = async function(branch: string = getCurrentGitBranc
reject(err)
}
}))

const digest = await (await docker.getImage(SERVER_IMAGE).inspect()).RepoDigests.at(0)
const sha = digest?.split('@').at(1)
console.log('├─ Using image ' + sha)
console.log('└─ Done')
} catch (e) {
console.log('└─ Failed to pull images')
Expand Down
24 changes: 20 additions & 4 deletions cypress/e2e/files/FilesUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,27 @@ export const getActionsForFile = (filename: string) => getRowForFile(filename).f
export const getActionButtonForFileId = (fileid: number) => getActionsForFileId(fileid).findByRole('button', { name: 'Actions' })
export const getActionButtonForFile = (filename: string) => getActionsForFile(filename).findByRole('button', { name: 'Actions' })

export const getActionEntryForFileId = (fileid: number, actionId: string) => {
return cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"]`)
const searchForActionInRow = (row: JQuery<HTMLElement>, actionId: string): Cypress.Chainable<JQuery<HTMLElement>> => {
const action = row.find(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"]`)
if (action.length > 0) {
cy.log('Found action in row')
return cy.wrap(action)
}

// Else look in the action menu
const menuButtonId = row.find('button[aria-controls]').attr('aria-controls')
return cy.get(`#${menuButtonId} [data-cy-files-list-row-action="${CSS.escape(actionId)}"]`)
}

export const getActionEntryForFileId = (fileid: number, actionId: string): Cypress.Chainable<JQuery<HTMLElement>> => {
// If we cannot find the action in the row, it might be in the action menu
return getRowForFileId(fileid).should('be.visible')
.then(row => searchForActionInRow(row, actionId))
}
export const getActionEntryForFile = (filename: string, actionId: string) => {
return cy.get(`[data-cy-files-list-row-action="${CSS.escape(actionId)}"]`)
export const getActionEntryForFile = (filename: string, actionId: string): Cypress.Chainable<JQuery<HTMLElement>> => {
// If we cannot find the action in the row, it might be in the action menu
return getRowForFile(filename).should('be.visible')
.then(row => searchForActionInRow(row, actionId))
}

export const triggerActionForFileId = (fileid: number, actionId: string) => {
Expand Down
4 changes: 3 additions & 1 deletion cypress/e2e/files/files-actions.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import type { User } from '@nextcloud/cypress'
import { FileAction } from '@nextcloud/files'

import { getActionButtonForFileId, getActionEntryForFileId, getRowForFile, getSelectionActionButton, getSelectionActionEntry, selectRowForFile, triggerActionForFile, triggerActionForFileId } from './FilesUtils'
import { getActionButtonForFileId, getActionEntryForFileId, getRowForFile, getSelectionActionButton, getSelectionActionEntry, selectRowForFile } from './FilesUtils'
import { ACTION_COPY_MOVE } from '../../../apps/files/src/actions/moveOrCopyAction'
import { ACTION_DELETE } from '../../../apps/files/src/actions/deleteAction'
import { ACTION_DETAILS } from '../../../apps/files/src/actions/sidebarAction'
Expand Down Expand Up @@ -53,6 +53,8 @@ describe('Files: Actions', { testIsolation: true }, () => {
getActionButtonForFileId(fileId).click({ force: true })
// Check the action is visible
getActionEntryForFileId(fileId, actionId).should('be.visible')
// Close the menu
cy.get('body').click({ force: true})
})
})

Expand Down
38 changes: 38 additions & 0 deletions cypress/e2e/files_external/StorageUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

import type { User } from "@nextcloud/cypress"

export type StorageConfig = {
[key: string]: string
}

export enum StorageBackend {
DAV = 'dav',
SMB = 'smb',
SFTP = 'sftp',
}

export enum AuthBackend {
GlobalAuth = 'password::global',
LoginCredentials = 'password::logincredentials',
Password = 'password::password',
SessionCredentials = 'password::sessioncredentials',
UserGlobalAuth = 'password::global::user',
UserProvided = 'password::userprovided',
}

/**
* Create a storage via occ
*/
export function createStorageWithConfig(mountPoint: string, storageBackend: StorageBackend, authBackend: AuthBackend, configs: StorageConfig, user?: User): Cypress.Chainable {
const configsFlag = Object.keys(configs).map(key => `--config "${key}=${configs[key]}"`).join(' ')
const userFlag = user ? `--user ${user.userId}` : ''

const command = `files_external:create "${mountPoint}" "${storageBackend}" "${authBackend}" ${configsFlag} ${userFlag}`

cy.log(`Creating storage with command: ${command}`)
return cy.runOccCommand(command)
}
Loading
Loading