nextcloud / registration

User registration app for Nextcloud
GNU Affero General Public License v3.0
137 stars 78 forks source link

Registration is failing with broken old records in the database #250

Open dimm0 opened 3 years ago

dimm0 commented 3 years ago

Steps to reproduce

  1. Upgrade to NC 20
  2. Try to register

Expected behaviour

Registering works fine

Actual behaviour

Fails with 500 error.

Logs:

{"reqId":"7lOAXu5KXnH6rwCveKuM","level":3,"time":"2020-10-16T20:02:48+00:00","remoteAddr":"10.244.33.7","user":"--","app":"index","method":"POST","url":"/apps/registration/","message":{"Exception":"Symfony\\Component\\Routing\\Exception\\InvalidParameterException","Message":"Parameter \"secret\" for route \"registration.register.showUserForm\" must match \"[^/]++\" (\"\" given) to generate a corresponding URL.","Code":0,"Trace":[{"file":"/var/www/html/3rdparty/symfony/routing/Generator/UrlGenerator.php","line":160,"function":"doGenerate","class":"Symfony\\Component\\Routing\\Generator\\UrlGenerator","type":"->","args":[{"secret":0,"token":1},{"caller":["registration","RegisterController","showUserForm"],"action":null},[],[["variable","/","[^/]++","token"],["variable","/","[^/]++","secret"],["text","/apps/registration/register"]],{"secret":null,"token":"qKNPiJzzHt"},"registration.register.showUserForm",1,[],[]]},{"file":"/var/www/html/lib/private/Route/Router.php","line":362,"function":"generate","class":"Symfony\\Component\\Routing\\Generator\\UrlGenerator","type":"->","args":["registration.register.showUserForm",{"secret":null,"token":"qKNPiJzzHt"},1]},{"file":"/var/www/html/lib/private/Route/CachingRouter.php","line":60,"function":"generate","class":"OC\\Route\\Router","type":"->","args":["registration.register.showUserForm",{"secret":null,"token":"qKNPiJzzHt"},false]},{"file":"/var/www/html/lib/private/URLGenerator.php","line":83,"function":"generate","class":"OC\\Route\\CachingRouter","type":"->","args":["registration.register.showUserForm",{"secret":null,"token":"qKNPiJzzHt"}]},{"file":"/var/www/html/lib/private/URLGenerator.php","line":95,"function":"linkToRoute","class":"OC\\URLGenerator","type":"->","args":["registration.register.showUserForm",{"secret":null,"token":"qKNPiJzzHt"}]},{"file":"/var/www/html/custom_apps/registration/lib/Service/MailService.php","line":86,"function":"linkToRouteAbsolute","class":"OC\\URLGenerator","type":"->","args":["registration.register.showUserForm",{"secret":null,"token":"qKNPiJzzHt"}]},{"file":"/var/www/html/custom_apps/registration/lib/Controller/RegisterController.php","line":130,"function":"sendTokenByMail","class":"OCA\\Registration\\Service\\MailService","type":"->","args":[{"id":12,"__class__":"OCA\\Registration\\Db\\Registration"}]},{"file":"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php","line":169,"function":"submitEmailForm","class":"OCA\\Registration\\Controller\\RegisterController","type":"->","args":["dimm0@yandex.ru"]},{"file":"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php","line":100,"function":"executeController","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->","args":[{"__class__":"OCA\\Registration\\Controller\\RegisterController"},"submitEmailForm"]},{"file":"/var/www/html/lib/private/AppFramework/App.php","line":152,"function":"dispatch","class":"OC\\AppFramework\\Http\\Dispatcher","type":"->","args":[{"__class__":"OCA\\Registration\\Controller\\RegisterController"},"submitEmailForm"]},{"file":"/var/www/html/lib/private/Route/Router.php","line":308,"function":"main","class":"OC\\AppFramework\\App","type":"::","args":["OCA\\Registration\\Controller\\RegisterController","submitEmailForm",{"__class__":"OC\\AppFramework\\DependencyInjection\\DIContainer"},{"action":null,"_route":"registration.register.submitEmailForm"}]},{"file":"/var/www/html/lib/base.php","line":1009,"function":"match","class":"OC\\Route\\Router","type":"->","args":["/apps/registration/"]},{"file":"/var/www/html/index.php","line":37,"function":"handleRequest","class":"OC","type":"::","args":[]}],"File":"/var/www/html/3rdparty/symfony/routing/Generator/UrlGenerator.php","Line":193,"CustomMessage":"--"},"userAgent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/85.0.4183.121 Safari/537.36 OPR/71.0.3770.228","version":"20.0.0.9"}

Server configuration

Operating system: Container

Nextcloud version: (see Nextcloud admin page) 20.0.0

Updated from an older Nextcloud/ownCloud or fresh install: Updated from 19.0.4

Signing status:

Signing status ``` Technical information ===================== The following list covers which files have failed the integrity check. Please read the previous linked documentation to learn more about the errors and how to fix them. Results ======= - core - EXTRA_FILE - aria2c.sess Raw output ========== Array ( [core] => Array ( [EXTRA_FILE] => Array ( [aria2c.sess] => Array ( [expected] => [current] => 4605678dd6e205b54d85a43a6c7a2e491f5d9e4271965ef5cba6eac811facdcf1296559baebc5d018c15818b9dd3b36aa2dbcc6cc1a6f171cf546cf9dd483950 ) ) ) ) ```

List of activated apps:

App list ``` Enabled: - accessibility: 1.6.0 - activity: 2.13.1 - apporder: 0.11.0 - bruteforcesettings: 2.0.1 - camerarawpreviews: 0.7.8 - cloud_federation_api: 1.3.0 - comments: 1.10.0 - contacts: 3.4.0 - contactsinteraction: 1.1.0 - dashboard: 7.0.0 - dav: 1.16.0 - dicomviewer: 1.2.2 - federatedfilesharing: 1.10.1 - federation: 1.10.1 - files: 1.15.0 - files_external: 1.11.1 - files_markdown: 2.3.1 - files_pdfviewer: 2.0.1 - files_photospheres: 1.20.0 - files_rightclick: 0.17.0 - files_sharing: 1.12.0 - files_trashbin: 1.10.1 - files_videoplayer: 1.9.0 - firstrunwizard: 2.9.0 - guests: 1.6.0 - lookup_server_connector: 1.8.0 - metadata: 0.12.0 - news: 15.0.4 - nextcloud_announcements: 1.9.0 - notes: 4.0.0 - notifications: 2.8.0 - oauth2: 1.8.0 - ocdownloader: 1.7.8 - password_policy: 1.10.1 - photos: 1.2.0 - privacy: 1.4.0 - provisioning_api: 1.10.0 - recommendations: 0.8.0 - registration: 0.5.1 - serverinfo: 1.10.0 - settings: 1.2.0 - sharebymail: 1.10.0 - sociallogin: 3.4.1 - support: 1.3.0 - systemtags: 1.10.0 - tasks: 0.13.4 - text: 3.1.0 - theming: 1.11.0 - twofactor_backupcodes: 1.9.0 - twofactor_totp: 5.0.0 - twofactor_u2f: 6.0.0 - user_status: 1.0.0 - viewer: 1.4.0 - weather_status: 1.0.0 - workflowengine: 2.2.0 Disabled: - admin_audit - analytics - announcementcenter - calendar - cookbook - dashboardcharts - encryption - event_update_notification - external - extract - files_3d - files_accesscontrol - files_versions - flowupload - gpgmailer - groupfolders - logreader - maps - ocr - radio - survey_client - twofactor_nextcloud_notification - unsplash - updatenotification - user_ldap - workflow_pdf_converter ```

The content of config/config.php:

Config report ``` '/', 'memcache.local' => '\\OC\\Memcache\\APCu', 'memcache.distributed' => '\\OC\\Memcache\\Redis', 'memcache.locking' => '\\OC\\Memcache\\Redis', 'redis' => array ( 'host' => 'nextclouddb', 'port' => 6379, 'timeout' => 3, ), 'apps_paths' => array ( 0 => array ( 'path' => '/var/www/html/apps', 'url' => '/apps', 'writable' => false, ), 1 => array ( 'path' => '/var/www/html/custom_apps', 'url' => '/custom_apps', 'writable' => true, ), ), 'instanceid' => '', 'passwordsalt' => '', 'secret' => '', 'trusted_domains' => array ( ... ), 'datadirectory' => '/var/www/html/data', 'overwriteprotocol' => 'https', 'overwrite.cli.url' => '...', 'dbtype' => 'mysql', 'version' => '20.0.0.9', 'dbname' => 'nextclouddb', 'dbhost' => 'nextclouddb', 'dbport' => '', 'dbtableprefix' => '', 'mysql.utf8mb4' => true, 'dbuser' => 'nextcloud', 'dbpassword' => '...', 'installed' => true, mail... 'maintenance' => false, 'updater.release.channel' => 'stable', 'loglevel' => 0, 'enabledPreviewProviders' => array ( 0 => 'OC\\Preview\\PNG', 1 => 'OC\\Preview\\JPEG', 2 => 'OC\\Preview\\GIF', 3 => 'OC\\Preview\\BMP', 4 => 'OC\\Preview\\XBitmap', 5 => 'OC\\Preview\\MP3', 6 => 'OC\\Preview\\TXT', 7 => 'OC\\Preview\\MarkDown', 8 => 'OC\\Preview\\TIFF', ), 'twofactor_enforced' => 'false', 'twofactor_enforced_groups' => array ( ), 'twofactor_enforced_excluded_groups' => array ( ), 'app_install_overwrite' => array ( 0 => 'camerarawpreviews', 1 => 'dicomviewer', 2 => 'radio', 3 => 'extract', 4 => 'unsplash', 5 => 'files_photospheres', 6 => 'files_3d', 7 => 'ocdownloader', 8 => 'registration', ), 'mail_sendmailmode' => 'smtp', 'mail_smtpsecure' => 'tls', 'theme' => '', ); ```

Are you using external storage, if yes which one: local/smb/sftp/... cephFS

Are you using encryption: yes/no no

Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/... no

Browser log

Browser log ``` Internal Server Error The server was unable to complete your request. If this happens again, please send the technical details below to the server administrator. More details can be found in the server log. ```
nickvergessen commented 3 years ago

There should not be a trailing slash on that url

dimm0 commented 3 years ago

Still getting the error if I remove the slash from the path

dimm0 commented 3 years ago

Is it only broken for me, or nobody upgraded yet? I still can't register anyone

nickvergessen commented 3 years ago

I just tried it again on my local test instance and it works pretty fine here. Can you share a link to your instance so I can test it there and see what happens? If you don't want to make your instance publicly known you can also email me the link to <my github name>@nextcloud.com

dimm0 commented 3 years ago

Done

nickvergessen commented 3 years ago

Can you try the following change? https://github.com/nextcloud/registration/pull/251/files

I can see the error happening on your instance, but I can not reproduce an issue locally.

dimm0 commented 3 years ago

Still giving the same error

Tried disabling the OIDC, no effect

dimm0 commented 3 years ago

Ahh! CILogon now works, but not the regular registration

MarkPartlett commented 3 years ago

Same issue. Updated from 19.0.4. to 20.0.1

MarkPartlett commented 3 years ago

No issue on fresh install, seems to be upgrade.

MarkPartlett commented 3 years ago

Found the issue: If registration email address has been used previously and the 'secret' field in the oc_registration.client_secret is null, you receive an error 500. If you remove the oc_registration record and allow it to be re-created then everything is fine.

dimm0 commented 3 years ago

Cleared a bunch of stale records from the table, and it works now. Wouldn't it make sense to expire the stale records at some point?

nickvergessen commented 3 years ago

Found the issue: If registration email address has been used previously and the 'secret' field in the oc_registration.client_secret is null, you receive an error 500. If you remove the oc_registration record and allow it to be re-created then everything is fine.

The question is why you have no client secret on those entries anyway. Because it's always created and never reset, as we only ever delete the full row.

Wouldn't it make sense to expire the stale records at some point?

Sure, there is a requested field which has the datetime of the registration moment. We could set up a background job and purge entries older than 2 days. Do you want to send a Pull Request to do that?

AlbanBedel commented 3 years ago

I hit the same bug on my instance, there are quiet a few rows in the oc_registration table with secret set to NULL. Most of them are more than a year old, there is really a need for a job to purge old entries.

Looking at this I'm also asking myself if there is any limit on the number of open request, or can an attacker just let open request pile up until the disk is full?

nickvergessen commented 3 years ago

There will never be a limit. There are otherways you could create problems like this.