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

[stable31] fix(files_external): request strict password auth on credentials enter action #50932

Open
wants to merge 10 commits into
base: stable31
Choose a base branch
from
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 …')

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