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

Password input time for admin reaproval to short #11224

Closed
nextgen-networks opened this issue Sep 14, 2018 · 24 comments
Closed

Password input time for admin reaproval to short #11224

nextgen-networks opened this issue Sep 14, 2018 · 24 comments
Assignees
Labels
1. to develop Accepted and waiting to be taken care of bug feature: settings medium
Milestone

Comments

@nextgen-networks
Copy link

When changing settings for nextcloud via web interface the timeframe is to short to type in ma password.
This happens if an session exists a couple of time and reaproval of credentials is forced.

In case you have an very long (and hopefully a secure) password the timeframe to type in exeeds an you face an error message.
This feature has been introduced in NC14.

I've found no option to extend the timeframe for the password input in the web interface.

Please make it configureable.

@nextcloud-bot
Copy link
Member

GitMate.io thinks possibly related issues are #10706 (Add admin password time indicator), #213 (Allow admin to define a password rules), #3416 (admin password confirmation problem), #1261 (force password change for all users by admin), and #3777 (Change of user passwords by admin fails).

@ghost
Copy link

ghost commented Sep 16, 2018

Was just having the same issue when doing it from the phone. It's very difficult to type a long password in time.

@kesselb
Copy link
Contributor

kesselb commented Sep 16, 2018

indeed. 7 seconds are a bit short.

@MorrisJobke
Copy link
Member

cc @nickvergessen @skjnldsv

@skjnldsv
Copy link
Member

We need to migrate to https://github.com/ChristophWurst/nextcloud-password-confirmation/blob/master/confirmation.js

@skjnldsv skjnldsv added 1. to develop Accepted and waiting to be taken care of medium labels Sep 27, 2018
@skjnldsv skjnldsv self-assigned this Sep 27, 2018
@skjnldsv skjnldsv added this to the Nextcloud 15 milestone Sep 27, 2018
@MorrisJobke
Copy link
Member

cc @ChristophWurst

@ChristophWurst
Copy link
Member

We need to migrate to https://github.com/ChristophWurst/nextcloud-password-confirmation/blob/master/confirmation.js

I think the problem is that the code times our at the wrong layer. There should be a timeout, but on the password confirmation dialogue itself. This timeout error should be propagated to the code that requested password confirmation.

Otherwise you still see the password confirmation dialogue, but the code that requested it might already have considered it as timed out and the callback leads to undefined behavior.

@skjnldsv
Copy link
Member

@ChristophWurst if we use await or .then it should be fine, right?

@ChristophWurst
Copy link
Member

No, because OC.PasswordConfirmation.requirePasswordConfirmation only resolves on success and never rejects on error/timeout.

@skjnldsv
Copy link
Member

@ChristophWurst Ah, I thought you worked around this.
This is the time where we need our modal component, right? :)

@ChristophWurst
Copy link
Member

Either that or adjust the existing code 😉

@FlorianFranzen
Copy link

So let me get this straight: I can only update my apps if I am being able to type my password within seven seconds (as the occ command does not support updating apps, #8773)?

What is the purpose of a timeout in the first place? What is gained by timing out the dialog?

@TheNomad11
Copy link

Still not fixed?

@kesselb
Copy link
Contributor

kesselb commented Oct 24, 2018

is the only place where requirePasswordConfirmation is used with a timeout in nextcloud\server. Why is there a timeout for password confirmation in this place? @skjnldsv @ChristophWurst

@ChristophWurst
Copy link
Member

@skjnldsv
Copy link
Member

@ChristophWurst yes, we just need to migrate to your solution or to nextcloud-axios?

@kesselb
Copy link
Contributor

kesselb commented Oct 25, 2018

Is there a technical reason why you need the timeout here? Could you just drop it and use it like anywhere else in nextcloud\server?

@skjnldsv
Copy link
Member

Is there a technical reason why you need the timeout here? Could you just drop it and use it like anywhere else in nextcloud\server?

Yes there is a valid reason behind.
And yes we can drop by using another implementation

@ChristophWurst
Copy link
Member

@ChristophWurst yes, we just need to migrate to your solution or to nextcloud-axios?

Well, https://www.npmjs.com/package/nextcloud-password-confirmation does just call OC.PasswordConfirmation.requirePasswordConfirmation and never resolves if the user does not enter a correct password. https://www.npmjs.com/package/nextcloud-axios is unrelated to this issue.

@nado
Copy link

nado commented Oct 25, 2018

Is there a technical reason why you need the timeout here? Could you just drop it and use it like anywhere else in nextcloud\server?

Yes there is a valid reason behind.

Care to explain what is the reason? I suppose myself and others dont exactly see the point of it.

@skjnldsv
Copy link
Member

Is there a technical reason why you need the timeout here? Could you just drop it and use it like anywhere else in nextcloud\server?

Yes there is a valid reason behind.

Care to explain what is the reason? I suppose myself and others dont exactly see the point of it.

Of course :)
It was a technical issue because I did not found a proper way to use an asynchronous callback on the password confirmation and we had to set a limit for the script to wait for the dialog to resolve instead of directly use the user input to detect if the user did something. With @ChristophWurst's implementation it will be all fixed! 🤗

@Scorry
Copy link

Scorry commented Nov 9, 2018

And, finally, how to disable this behavior?

@ChristophWurst
Copy link
Member

skjnldsv moved this from To do to Priorities (up next) in Skjnldsv's Tasks 2 days ago

Thanks 🙌

@MorrisJobke
Copy link
Member

Will be shipped with 14.0.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug feature: settings medium
Projects
None yet
Development

No branches or pull requests

10 participants