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

Session handling: api and webdav requests shall not start a session #7628

Closed
martinlanger90 opened this issue Dec 26, 2017 · 23 comments · Fixed by #28311
Closed

Session handling: api and webdav requests shall not start a session #7628

martinlanger90 opened this issue Dec 26, 2017 · 23 comments · Fixed by #28311
Labels
1. to develop Accepted and waiting to be taken care of bug feature: dav

Comments

@martinlanger90
Copy link

Steps to reproduce

  1. Use NextCloud with macOS Calendar
  2. do find phptmp/* | wc -l
  3. get: 3825 -> more than 3800 session files in about 1-2 weeks

Expected behaviour

For WebDav connections there should be no session file generated.

Actual behaviour

for every webdav connection there is a session file

For the rest, take a look at:
owncloud/core#29779

and the old one:
owncloud/core#5383

and last but not least, be faster as the oc-team. nah just kidding, merry christmas and a happy new year :-)

@MorrisJobke MorrisJobke added enhancement papercut Annoying recurring issue with possibly simple fix. labels Jan 2, 2018
@martinlanger90
Copy link
Author

shouldn't this behaviour considered as a bug because the system get's full of a ton of session files? 😄

@martinlanger90
Copy link
Author

martinlanger90 commented Apr 19, 2018

Is there any progress on this issue?
I have to clear my phptmp folder every day, because after 2 minutes I have about 17 files generated. For an experienced coder this should not be hard to get this fixed? As this issue has the papercut label it must be easy for anyone whos contributing to NC

@nextcloud-bot nextcloud-bot added the stale Ticket or PR with no recent activity label Jun 20, 2018
@martinlanger90
Copy link
Author

I really don't want to bother someone, but "stale" on this issue? really?
1400 files per day is really a point.

@nextcloud-bot nextcloud-bot removed the stale Ticket or PR with no recent activity label Aug 2, 2018
@martinlanger90
Copy link
Author

on papercut label, but not fixed since 2 years. very annoying bug.
again 12000 files in phptmp directory.

@kesselb
Copy link
Contributor

kesselb commented Nov 29, 2019

I think "papercut" is not appropriate ;) It's probably only moving two lines of code around but a lot of code is build with the assumption that a session is available. I would not touch it without a good reason.

If username + password used for the calendar application than using app passwords could reduce the number of created sessions.

#  This purges session files in session.save_path older than X,
#  where X is defined in seconds as the largest value of
#  session.gc_maxlifetime from all your SAPI php.ini files
#  or 24 minutes if not defined.  The script triggers only
#  when session.save_handler=files.
#
#  WARNING: The scripts tries hard to honour all relevant
#  session PHP options, but if you do something unusual
#  you have to disable this script and take care of your
#  sessions yourself.

# Look for and purge old sessions every 30 minutes
09,39 *     * * *     root   [ -x /usr/lib/php/sessionclean ] && if [ ! -d /run/systemd/system ]; then /usr/lib/php/sessionclean; fi

Please make sure that your session.save_path is purged from time to time. Above is from a debian system. By default the sessions path is checked for old files every 30 minutes.

I agree to the issue itself but your figures seems to be a configuration problem. Fair enough it's not possible to use app passwords everywhere but cleaning the sessions path is.

@martinlanger90
Copy link
Author

I'm using a lot of calendar apps with webdav connection (macOS Calendar / Contacts are using this connection type for example). Everytime webdav connection is established a session file is generated. Should not be when using webdav or oauth2 connections. Purging files is just a workaroung but I'll try using that.

@kesselb
Copy link
Contributor

kesselb commented Nov 29, 2019

I'm using a lot of calendar apps with webdav connection (macOS Calendar / Contacts are using this connection type for example). Everytime webdav connection is established a session file is generated.

I get that. Please make sure to setup calendar / contacts like described here: https://docs.nextcloud.com/server/latest/user_manual/pim/sync_osx.html https://docs.nextcloud.com/server/latest/user_manual/en/pim/sync_osx.html (use app password instead of your actual password).

Purging files is just a workaroung but I'll try using that.

It's anything but not a workaround. There is a good reason that this is enabled by default (at least for debian/ubuntu).

@skjnldsv skjnldsv added the 0. Needs triage Pending check for reproducibility or if it fits our roadmap label Aug 20, 2020
@nooblag
Copy link
Contributor

nooblag commented Oct 25, 2020

... Please make sure to setup calendar / contacts like described here: https://docs.nextcloud.com/server/latest/user_manual/pim/sync_osx.html (use app password instead of your actual password).

Hi there, this link is dead.

@szaimen
Copy link
Contributor

szaimen commented Jun 17, 2021

Is this Issue still valid in NC21.0.2? If not, please close this issue. Thanks! :)

@martinlanger90
Copy link
Author

Still valid. Using "app passwords" or "configure cron properly" doesnt solve this. Nobody cares. Really annoying.

@szaimen szaimen added 1. to develop Accepted and waiting to be taken care of bug feature: dav and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement needs info papercut Annoying recurring issue with possibly simple fix. labels Jun 17, 2021
@szaimen
Copy link
Contributor

szaimen commented Jun 17, 2021

"configure cron properly"

how did you configure cron?

@nooblag
Copy link
Contributor

nooblag commented Jun 17, 2021

Just chiming in to say it's still valid for us too on 21.0.2. Our cron is "system cron" with www-data user.

Cron

@szaimen
Copy link
Contributor

szaimen commented Jun 17, 2021

Thanks! please post your config.php file for further debugging

@martinlanger90
Copy link
Author

Did it like nooblag said. Running cron.php all 5 mins.

@nooblag
Copy link
Contributor

nooblag commented Jun 17, 2021

Hi there, thanks. Our config was posted in this issue #23094 which was closed... :(

@szaimen
Copy link
Contributor

szaimen commented Jun 17, 2021

Could you please post your current config of your NC21.0.2 instance nonetheless again? Thanks!

@szaimen szaimen added 0. Needs triage Pending check for reproducibility or if it fits our roadmap needs info and removed 1. to develop Accepted and waiting to be taken care of labels Jun 17, 2021
@kesselb
Copy link
Contributor

kesselb commented Jun 17, 2021

To reference as similar/related issue: #11125

This happens because the Nextcloud base system is used, which always starts the session. This is quite hard to fix without any fundamental restructuring.

#11125 (comment)

@szaimen
Copy link
Contributor

szaimen commented Jun 17, 2021

So the probably also the same answer:

It's valid but we can't easily fix this.

@szaimen szaimen added 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap needs info labels Jun 17, 2021
@nooblag
Copy link
Contributor

nooblag commented Jun 18, 2021

$ sudo -u www-data php /var/www/nextcloud/occ config:list system
{
    "system": {
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "***REMOVED SENSITIVE VALUE***"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "tempdirectory": "\/mnt\/***REMOVED SENSITIVE VALUE***\/__temp",
        "skeletondirectory": "\/mnt\/***REMOVED SENSITIVE VALUE***\/__skeleton",
        "templatedirectory": "\/mnt\/***REMOVED SENSITIVE VALUE***\/__templates",
        "dbtype": "pgsql",
        "version": "21.0.2.1",
        "overwrite.cli.url": "***REMOVED SENSITIVE VALUE***",
        "htaccess.RewriteBase": "\/",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "log_type": "file",
        "logfile": "\/var\/log\/nextcloud\/nextcloud.log",
        "loglevel": 2,
        "logtimezone": "Australia\/Melbourne",
        "mail_smtpmode": "smtp",
        "remember_login_cookie_lifetime": 0,
        "log_rotate_size": "10485760",
        "trashbin_retention_obligation": "auto, 14",
        "versions_retention_obligation": "auto, 365",
        "simpleSignUpLink.shown": false,
        "login_form_autocomplete": true,
        "filelocking.enabled": true,
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "memcache.local": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": "6379",
            "dbindex": "0",
            "timeout": "1.5"
        },
        "maintenance": false,
        "default_phone_region": "AU",
        "default_locale": "en_AU",
        "force_locale": "en_AU",
        "default_language": "en",
        "force_language": "en",
        "defaultapp": "activity,files",
        "activity_expire_days": 180,
        "session_lifetime": 14400,
        "session_keepalive": false,
        "auth.webauthn.enabled": false,
        "overwriteprotocol": "https",
        "app_install_overwrite": [
            "activitylog",
            "files_readmemd"
        ],
        "updater.release.channel": "stable",
        "theme": "",
        "connectivity_check_domains": [
            "nextcloud.com",
            "duckduckgo.com"
        ],
        "trusted_proxies": "***REMOVED SENSITIVE VALUE***",
        "forwarded_for_headers": [
            "HTTP_CF_CONNECTING_IP"
        ],
        "mail_smtpsecure": "ssl",
        "mail_sendmailmode": "smtp",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpauthtype": "LOGIN",
        "mail_smtpauth": 1,
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "465",
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***"
    }
}

@martinlanger90
Copy link
Author

Whats going on with the PR:
Do not setup a session when not required on API requests #28311??
Nothing happens since August again.

@martinlanger90
Copy link
Author

Overall much less impact than the original proposal, but should be sufficient to handle exhausted session creation for clients which always do basic auth against WebDAV.

I would test that with DAVx for Android and the MacOS Calendar apps, because they actually cause a few thousand session files a week. Is there a build to test anywhere uploaded after merging #28311 @juliushaertl ?

@juliusknorr
Copy link
Member

Help with testing this would be very much appreciated. There should be daily builds at https://download.nextcloud.com/server/daily/

Otherwise you could also apply the pull request as a patch as described in https://docs.nextcloud.com/server/latest/admin_manual/issues/applying_patch.html#getting-a-patch-from-a-github-pull-request

@martinlanger90
Copy link
Author

If it's in todays daily build, I'll test it tomorrow.
Can you call jenkins or whatever to build it with the PR included?

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: dav
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants