Skip to content

Commit

Permalink
fix(settings): Ask for authentication before action on sessions
Browse files Browse the repository at this point in the history
- Ask authentication on update/delete/wipe
- Adapt dialog message to action

Signed-off-by: Florent Hernandez <flo@flhz.is>
  • Loading branch information
lithrel committed Sep 18, 2024
1 parent 13fce9f commit ad42dcb
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 15 deletions.
2 changes: 2 additions & 0 deletions apps/settings/lib/Controller/AuthSettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ private function checkAppToken(): bool {
* @return array|JSONResponse
*/
#[NoAdminRequired]
#[PasswordConfirmationRequired]
public function destroy($id) {
if ($this->checkAppToken()) {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
Expand Down Expand Up @@ -205,6 +206,7 @@ public function destroy($id) {
* @return array|JSONResponse
*/
#[NoAdminRequired]
#[PasswordConfirmationRequired]
public function update($id, array $scope, string $name) {
if ($this->checkAppToken()) {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
Expand Down
43 changes: 31 additions & 12 deletions apps/settings/src/store/authtoken.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,23 @@ import '@nextcloud/password-confirmation/dist/style.css'

const BASE_URL = generateUrl('/settings/personal/authtokens')

const confirm = () => {
const confirm = (message: string, title: string) => {
return new Promise(resolve => {
window.OC.dialogs.confirm(
t('settings', 'Do you really want to wipe your data from this device?'),
t('settings', 'Confirm wipe'),
resolve,
true,
)
window.OC.dialogs.confirm(message, title, resolve, true)
})
}
const confirmUpdate = () => confirm(
t('settings', 'Do you really want to update this session?'),
t('settings', 'Confirm update'),
)
const confirmDelete = () => confirm(
t('settings', 'Do you really want to delete this session?'),
t('settings', 'Confirm delete'),
)
const confirmWipe = () => confirm(
t('settings', 'Do you really want to wipe this session?'),
t('settings', 'Confirm wipe'),
)

export enum TokenType {
TEMPORARY_TOKEN = 0,
Expand Down Expand Up @@ -74,6 +81,13 @@ export const useAuthTokenStore = defineStore('auth-token', {
* @param token Token to update
*/
async updateToken(token: IToken) {
await confirmPassword()

if (!(await confirmUpdate())) {
logger.debug('Update aborted by user')
return
}

const { data } = await axios.put(`${BASE_URL}/${token.id}`, token)
return data
},
Expand Down Expand Up @@ -104,17 +118,22 @@ export const useAuthTokenStore = defineStore('auth-token', {
async deleteToken(token: IToken) {
logger.debug('Deleting app token', { token })

this.tokens = this.tokens.filter(({ id }) => id !== token.id)

try {
await confirmPassword()

if (!(await confirmDelete())) {
logger.debug('Delete aborted by user')
return
}

await axios.delete(`${BASE_URL}/${token.id}`)
logger.debug('App token deleted')

this.tokens = this.tokens.filter(({ id }) => id !== token.id)
return true
} catch (error) {
logger.error('Could not delete app token', { error })
showError(t('settings', 'Could not delete the app token'))
// Restore
this.tokens.push(token)
}
return false
},
Expand All @@ -129,7 +148,7 @@ export const useAuthTokenStore = defineStore('auth-token', {
try {
await confirmPassword()

if (!(await confirm())) {
if (!(await confirmWipe())) {
logger.debug('Wipe aborted by user')
return
}
Expand Down
101 changes: 101 additions & 0 deletions cypress/e2e/settings/user_security.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import { User } from '@nextcloud/cypress'
const admin = new User('admin', 'admin')

const secDevice = 'Secondary test device'
const secDeviceEdited = 'Edited test device'

describe('Settings: Edit and delete user sessions', function() {
beforeEach(function() {
cy.logout()
cy.visit('/login')

// Create a secondary session on same user with different User-Agent
cy.request('/csrftoken').then(({ body }) => {
const requestToken = body.token
cy.request({
method: 'POST',
url: '/login',
body: {
user: admin.userId,
password: admin.password,
requesttoken: requestToken,
},
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
'User-Agent': secDevice,
},
followRedirect: false,
}).then(() => {
// Forget secondary session to leave it open
cy.clearAllCookies()
})
})
})

it('Can revoke a session', function() {
cy.login(admin)
cy.visit('/settings/user/security')

// ensure secondary device is present
cy.get('tr.auth-token')
.contains(secDevice)
.should('exist')
// open actions menu on device
cy.contains('tr.auth-token', secDevice)
.find('.auth-token__actions')
.click()
// revoke
cy.get('button').contains('Revoke').click()
cy.get('.modal-container button').contains('Yes').click({ force: true })
cy.get('tr.auth-token')
.contains(secDevice)
.should('not.exist')
})

it('Can edit a device name', function() {
cy.login(admin)
cy.visit('/settings/user/security')

// ensure secondary device is present
cy.get('tr.auth-token')
.contains(secDevice)
.should('exist')
// open actions menu on device
cy.contains('tr.auth-token', secDevice)
.find('.auth-token__actions')
.click()
// rename
cy.get('button').contains('Rename').click()
cy.get('.auth-token__name-form input[type="text"]').type(secDeviceEdited)
cy.get('button[aria-label="Save new name"]').click()
cy.get('.modal-container button').contains('Yes').click({ force: true })
// make sure rename worked
cy.get('tr.auth-token')
.contains(secDeviceEdited)
.should('exist')
cy.get('tr.auth-token')
.contains(secDevice)
.should('not.exist')
})

it('Can wipe a device', function() {
cy.login(admin)
cy.visit('/settings/user/security')

// ensure secondary device is present
cy.get('tr.auth-token')
.contains(secDevice)
.should('exist')
// open actions menu on device
cy.contains('tr.auth-token', secDevice)
.find('.auth-token__actions')
.click()
// mark for wipe
cy.get('button').contains('Wipe device').click()
cy.get('.modal-container button').contains('Yes').click({ force: true })
// make sure wipe was selected
cy.contains('tr.auth-token', secDevice)
.contains('(Marked for remote wipe)')
.should('exist')
})
})
4 changes: 2 additions & 2 deletions dist/settings-vue-settings-personal-security.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/settings-vue-settings-personal-security.js.map

Large diffs are not rendered by default.

0 comments on commit ad42dcb

Please sign in to comment.