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

Ask for authentication before removing 'Device & Sessions' #39390

Open
javielico opened this issue Jul 14, 2023 · 2 comments · May be fixed by #48168
Open

Ask for authentication before removing 'Device & Sessions' #39390

javielico opened this issue Jul 14, 2023 · 2 comments · May be fixed by #48168
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: authentication feature: settings good first issue Small tasks with clear documentation about how and in which place you need to fix things in.

Comments

@javielico
Copy link

Describe the solution you'd like
As user
I want to make sure that when I remove a device or session from my NextCloud account I'm prompted to authenticate with my password
So that if my account is compromised no one can remove access without the original password

Additional context
I think this feature could help prevent users that leave their account open in a shared device from other users de-authenticating them from their devices and therefore loosing access.

@javielico javielico added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Jul 14, 2023
@joshtrichards joshtrichards changed the title [Feature]: Admin - Ask for authentication before removing 'Device & Sessions' Ask for authentication before removing 'Device & Sessions' Jul 19, 2024
@joshtrichards
Copy link
Member

To implement this I believe changes are required in two spots:

Add a confirmPassword() call here:

async deleteToken(token: IToken) {
logger.debug('Deleting app token', { token })
this.tokens = this.tokens.filter(({ id }) => id !== token.id)
try {
await axios.delete(`${BASE_URL}/${token.id}`)

It can be modeled after the one used for wipeToken():

async wipeToken(token: IToken) {
logger.debug('Wiping app token', { token })
try {
await confirmPassword()
if (!(await confirm())) {
logger.debug('Wipe aborted by user')
return
}
await axios.post(`${BASE_URL}/wipe/${token.id}`)

And in the controller the @PasswordConfirmationRequired (docs) annotation added. I believe here:

/**
* @NoAdminRequired
* @NoSubAdminRequired
*
* @param int $id
* @return array|JSONResponse
*/
public function destroy($id) {

Similar to here:

/**
* @NoAdminRequired
* @NoSubAdminRequired
* @PasswordConfirmationRequired
*
* @param int $id
* @return JSONResponse
* @throws InvalidTokenException
* @throws ExpiredTokenException
*/
public function wipe(int $id): JSONResponse {

It might be a good idea to add it to the update() / updateToken() too since I believe Allow filesystem access flows through there to prevent disabling that on an existing session/token.

@joshtrichards joshtrichards added the good first issue Small tasks with clear documentation about how and in which place you need to fix things in. label Jul 19, 2024
@XxTysweezyxX
Copy link
Contributor

I'd like to work on this issue if possible

@lithrel lithrel linked a pull request Sep 18, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: authentication feature: settings good first issue Small tasks with clear documentation about how and in which place you need to fix things in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants