Open blizzz opened 4 years ago
The user folder initialization should be unbound from the login process. It shall be possible that this can happen at a later point, and/or be able to be triggered by another user backend.
@rullzer and I had been discussing something like https://laravel.com/docs/7.x/queues for these cases. We could then make this initialization happen asynchronously.
The user folder initialization should be unbound from the login process. It shall be possible that this can happen at a later point, and/or be able to be triggered by another user backend.
@rullzer and I had been discussing something like https://laravel.com/docs/7.x/queues for these cases. We could then make this initialization happen asynchronously.
That's essentially background jobs on steroids. You'll have some priority queue to get the folder done asap, because otherwise the user will end up on an error page – unless we catch and inform the user that the dir is still being prepared. If SSE is disabled (that might even work in the cases, where we have the password - local backends or creation triggered from within nextcloud), the job could be done earlier, upon user creation. Here the exceptions are external backends that cannot list (thus announce) users in time, e.g. SAML and IMAP. You will be stuck with the same problem on very limited, webspace-type installations in theory. In practice those use cases most likely do not apply there. And you will have user directories before their first login, which might lead to complaints. When attaching a big external backend that can announce users, it could clog the queue.
=> Having it async can be a good approach, provided we do it in a reliable and quick manner (and not forgot the simple setups)
Moving to 21
How to use GitHub
Is your feature request related to a problem? Please describe.
User Alice logs in for the first time with user_saml. As expected their file system is prepared with the contents of the skeleton directory. What they are perhaps not aware about is, that user_saml is duplicating folder generation code, because it has to override the login mechanism.
User Bob logs in for the first time with user_ldap. Due to the password policy they have to change the initial password after first login. Having done that Bob is redirected to the files app - but is greeted with a web page of death, because the file system was not generated. Bob calls his sysadmin.
User Cecile does the same as Bob. But because their sysadmin has patched the code, Nextcloud works for them, alas the directory is not filled with the default content. Cecile cannot read the "first 7 things to do after your first login.pdf" from their organization as instructed.
Initialization of the user directory happens in \OC\User\Session::prepareUserLogin() and is called after the postLogin events are handled. user_saml is beyond that, the described user_ldap scenario forces a redirect to its password renew controller. There are other issues reported, where this issue occured even with the local backend.
User backends following the regular login chain do not know whether the user logs in for the first time or not, because the information is withhold and the new login date is already written to the database. This is also questionable, because as apps can cancel the login during postLogin event handling.
Describe the solution you'd like
OC\Authentication\Login\Chain
. It needs to be ensured that a delayed setting of this timestamp does not cause regressions.OC\Authentication\Login\Chain
(just before the login timestamp is set).handleLogin
. The relevant parts might need to switch to using theOC\Authentication\Login\Chain
.OC\User\Session
, but become public and part ofOCP\IUserSession
, so it can be called out of context. Or, it shall be evaluated whether apps like user_saml should get a chance to trigger the relevant parts of the login chain handling tasks after the login was irrevocably successful.exit(0)
to solve this case, after sending headers themselves. The LoginController ought to catch it.Additional context
This is eventually driven by a one line patch:
@rullzer @ChristophWurst this is my brain dump for today, and it well might not be the final state. I'll certainly sleep over it. Do you have thoughts about it?