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

Ext storage mode save in session doesn't work without cookies #25511

Closed
PVince81 opened this issue Jul 18, 2016 · 14 comments · Fixed by #25553
Closed

Ext storage mode save in session doesn't work without cookies #25511

PVince81 opened this issue Jul 18, 2016 · 14 comments · Fixed by #25553

Comments

@PVince81
Copy link
Contributor

See owncloud/android#1674 for details.

From the symptoms it looks like basic auth is happening for the curl calls, but the credentials aren't available yet in the session. Maybe that part of the code runs too late.

This means that potentially any client that doesn't support cookies will not work properly with the "save in session" mode.

CC @Xenopathic @jvillafanez

@PVince81
Copy link
Contributor Author

I just tested this in 9.0.3 with session mode and running curl with my username/password properly does a PROPFIND on the "sftp" or "sftp/" folder.

Works for me.

@QuentinC
Copy link

Does this works also in ownCloud 9.1.0 RC1 (beta) ?

@PVince81
Copy link
Contributor Author

I just tested with v9.1.0RC1 and it also works with curl.

@PVince81
Copy link
Contributor Author

Am able to reproduce a similar issue on v9.0.3, need to have LDAP enabled and login as a LDAP user who has access to SFTP storage using "save in session".

v9.0.3:

  • web UI PROPFIND: doesn't contain the mount point at all !
  • Webdav PROPFIND on "/": contains "sftp" mount point
  • Webdav PROPFIND on "/sftp": 503 Storage not available

Will check stable9.1 later.

@PVince81
Copy link
Contributor Author

There are two pieces of code:

  1. Store the password in session
  2. Read the password from session when mounting SFTP

The two are run in the wrong order when doing a curl PROPFIND.

Read password from session happens here:

0  OCA\Files_External\Lib\Auth\Password\SessionCredentials->manipulateStorageConfig() /srv/www/htdocs/owncloud/apps/files_external/lib/auth/password/sessioncredentials.php:72
1  OCA\Files_External\Config\ConfigAdapter->prepareStorageConfig() /srv/www/htdocs/owncloud/apps/files_external/lib/config/configadapter.php:90
2  OCA\Files_External\Config\ConfigAdapter->getMountsForUser() /srv/www/htdocs/owncloud/apps/files_external/lib/config/configadapter.php:128
3  OC\Files\Config\MountProviderCollection->OC\Files\Config\{closure}() /srv/www/htdocs/owncloud/lib/private/files/config/mountprovidercollection.php:70
4  array_map()     /srv/www/htdocs/owncloud/lib/private/files/config/mountprovidercollection.php:71
5  OC\Files\Config\MountProviderCollection->getMountsForUser() /srv/www/htdocs/owncloud/lib/private/files/config/mountprovidercollection.php:71
6  OC\Files\Filesystem::initMountPoints() /srv/www/htdocs/owncloud/lib/private/files/filesystem.php:432
7  OC\AvatarManager->getAvatar() /srv/www/htdocs/owncloud/lib/private/avatarmanager.php:90
8  OC\User\User->getAvatarImage() /srv/www/htdocs/owncloud/lib/private/user/user.php:380
9  OCA\DAV\CardDAV\Converter->getAvatarImage() /srv/www/htdocs/owncloud/apps/dav/lib/carddav/converter.php:164
10 OCA\DAV\CardDAV\Converter->updateCard() /srv/www/htdocs/owncloud/apps/dav/lib/carddav/converter.php:75
11 OCA\DAV\CardDAV\SyncService->updateUser() /srv/www/htdocs/owncloud/apps/dav/lib/carddav/syncservice.php:228
12 OCA\DAV\HookManager->changeUser() /srv/www/htdocs/owncloud/apps/dav/lib/hookmanager.php:99
13 call_user_func:{/srv/www/htdocs/owncloud/lib/private/hook.php:105}() /srv/www/htdocs/owncloud/lib/private/hook.php:105
14 OC_Hook::emit() /srv/www/htdocs/owncloud/lib/private/hook.php:105
15 OC\Server->OC\{closure}() /srv/www/htdocs/owncloud/lib/private/server.php:244
16 call_user_func_array:{/srv/www/htdocs/owncloud/lib/private/hooks/emittertrait.php:98}() /srv/www/htdocs/owncloud/lib/private/hooks/emittertrait.php:98
17 OC\Hooks\BasicEmitter->emit() /srv/www/htdocs/owncloud/lib/private/hooks/emittertrait.php:98
18 OC\Hooks\PublicEmitter->emit() /srv/www/htdocs/owncloud/lib/private/hooks/publicemitter.php:32
19 OC\User\User->triggerChange() /srv/www/htdocs/owncloud/lib/private/user/user.php:417
20 OC\User\User->setEMailAddress() /srv/www/htdocs/owncloud/lib/private/user/user.php:163
21 OCA\user_ldap\lib\user\User->updateEmail() /srv/www/htdocs/owncloud/apps/user_ldap/lib/user/user.php:436
22 OCA\user_ldap\lib\user\User->processAttributes() /srv/www/htdocs/owncloud/apps/user_ldap/lib/user/user.php:167
23 OCA\user_ldap\lib\Access->batchApplyUserAttributes() /srv/www/htdocs/owncloud/apps/user_ldap/lib/access.php:734
24 OCA\user_ldap\lib\Access->fetchListOfUsers() /srv/www/htdocs/owncloud/apps/user_ldap/lib/access.php:706
25 OCA\user_ldap\lib\Access->fetchUsersByLoginName() /srv/www/htdocs/owncloud/apps/user_ldap/lib/access.php:679
26 OCA\user_ldap\USER_LDAP->getLDAPUserByLoginName() /srv/www/htdocs/owncloud/apps/user_ldap/user_ldap.php:103
27 OCA\user_ldap\USER_LDAP->checkPassword() /srv/www/htdocs/owncloud/apps/user_ldap/user_ldap.php:120
28 OC\User\Manager->checkPassword() /srv/www/htdocs/owncloud/lib/private/user/manager.php:189
29 OC\User\Session->login() /srv/www/htdocs/owncloud/lib/private/user/session.php:219
30 OCA\DAV\Connector\Sabre\Auth->validateUserPass() /srv/www/htdocs/owncloud/apps/dav/lib/connector/sabre/auth.php:106
31 Sabre\DAV\Auth\Backend\AbstractBasic->check() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Auth/Backend/AbstractBasic.php:105
32 OCA\DAV\Connector\Sabre\Auth->auth() /srv/www/htdocs/owncloud/apps/dav/lib/connector/sabre/auth.php:220
33 OCA\DAV\Connector\Sabre\Auth->check() /srv/www/htdocs/owncloud/apps/dav/lib/connector/sabre/auth.php:127
34 Sabre\DAV\Auth\Plugin->beforeMethod() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Auth/Plugin.php:166
35 call_user_func_array:{/srv/www/htdocs/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php:105}() /srv/www/htdocs/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php:105
36 Sabre\Event\EventEmitter->emit() /srv/www/htdocs/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php:105
37 Sabre\DAV\Server->invokeMethod() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php:446
38 Sabre\DAV\Server->exec() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php:248
39 require_once()  /srv/www/htdocs/owncloud/apps/dav/appinfo/v1/webdav.php:55
40 {main}          /srv/www/htdocs/owncloud/remote.php:138

and store password only happens later in the "postLogin" hook:

0  OCA\Files_External\Lib\Auth\Password\SessionCredentials->authenticate() /srv/www/htdocs/owncloud/apps/files_external/lib/auth/password/sessioncredentials.php:68
1  call_user_func:{/srv/www/htdocs/owncloud/lib/private/hook.php:105}() /srv/www/htdocs/owncloud/lib/private/hook.php:105
2  OC_Hook::emit() /srv/www/htdocs/owncloud/lib/private/hook.php:105
3  OC\Server->OC\{closure}() /srv/www/htdocs/owncloud/lib/private/server.php:237
4  call_user_func_array:{/srv/www/htdocs/owncloud/lib/private/hooks/emittertrait.php:98}() /srv/www/htdocs/owncloud/lib/private/hooks/emittertrait.php:98
5  OC\Hooks\BasicEmitter->emit() /srv/www/htdocs/owncloud/lib/private/hooks/emittertrait.php:98
6  OC\Hooks\PublicEmitter->emit() /srv/www/htdocs/owncloud/lib/private/hooks/publicemitter.php:32
7  OC\User\Session->login() /srv/www/htdocs/owncloud/lib/private/user/session.php:225
8  OCA\DAV\Connector\Sabre\Auth->validateUserPass() /srv/www/htdocs/owncloud/apps/dav/lib/connector/sabre/auth.php:106
9  Sabre\DAV\Auth\Backend\AbstractBasic->check() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Auth/Backend/AbstractBasic.php:105
10 OCA\DAV\Connector\Sabre\Auth->auth() /srv/www/htdocs/owncloud/apps/dav/lib/connector/sabre/auth.php:220
11 OCA\DAV\Connector\Sabre\Auth->check() /srv/www/htdocs/owncloud/apps/dav/lib/connector/sabre/auth.php:127
12 Sabre\DAV\Auth\Plugin->beforeMethod() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Auth/Plugin.php:166
13 call_user_func_array:{/srv/www/htdocs/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php:105}() /srv/www/htdocs/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php:105
14 Sabre\Event\EventEmitter->emit() /srv/www/htdocs/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php:105
15 Sabre\DAV\Server->invokeMethod() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php:446
16 Sabre\DAV\Server->exec() /srv/www/htdocs/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php:248
17 require_once()  /srv/www/htdocs/owncloud/apps/dav/appinfo/v1/webdav.php:55
18 {main}          /srv/www/htdocs/owncloud/remote.php:138

From what I see is that the avatar fetching code from LDAP is triggering the filesystem setup too early.

@PVince81
Copy link
Contributor Author

Additional note: the LDAP users need to have an email address set in the LDAP "mail" field for setEmailAddress to get triggered. This is similar to #24423 where the same logic triggered other issues.

@PVince81
Copy link
Contributor Author

@DeepDiver1975 @jvillafanez ideally checkPassword from LDAP should not trigger the full vcard fetching. Not sure if possible without changing the API to add an override flag and causing other side effects.

Possible solutions:

  1. Find a way to prevent checkPassword to trigger setEmailAddress and other vcard data fetching
  2. Find a way to prevent setupFS to be run at the time before the login fully completed ? will likely break stuff
  3. Find a way to make files_external still publish the faulty mount, and retry getting the auth params from the session at access time
  4. Make files_external use pre-login hook instead of post-login ?

So far it feels like 3) is the best way to go. Even when creds are not available, we do want the mount point to be always visible.

@PVince81
Copy link
Contributor Author

Just tried solution 4 using pre_login instead of post_login. It works but it has the drawback that it stores the credentials before the login could even be verified. Unfortunately there is no hook for "login failed" in which case we could delete the credentials from the session again. Not sure how bad this is.
POC PR here: #25530

Next step: find out whether there is a better solution, hopefully one that involves preventing all this early setupFS stuff

@PVince81
Copy link
Contributor Author

@QuentinC can you try with #25531 which might work better ?

@QuentinC
Copy link

@PVince81 Yeah it works with this !

Thanks !!!

@PVince81
Copy link
Contributor Author

@QuentinC glad to hear

@QuentinC
Copy link

@PVince81 Well, I seem to have some "less important but still annoying" problem using LDAP + Session credentials.

I can browse every folder now, but it seems that owncloud won't scan external storages in order to update folder sizes. It will always stay on "Waiting", "En attente" in fact in French, and only update the size when manually browsing folders.

Is it a behavior that you can reproduce ?

@PVince81
Copy link
Contributor Author

Hmm, this is a separate issue. Can you make one ?
I think I know why: the auto-scanning used to be triggered from the web UI and was moved to a background (cron) job. But that background job doesn't have the user's password (no session), so cannot perform the deep scan.

@lock
Copy link

lock bot commented Aug 3, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants