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

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

Open
dimm0 opened this issue Oct 16, 2020 · 15 comments
Open

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

dimm0 opened this issue Oct 16, 2020 · 15 comments

Comments

@dimm0
Copy link

dimm0 commented Oct 16, 2020

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":["[email protected]"]},{"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
<?php
$CONFIG = array (
  'htaccess.RewriteBase' => '/',
  '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.
@dimm0 dimm0 changed the title Registration is faining on nc 20 Registration is failing on nc 20 Oct 16, 2020
@nickvergessen
Copy link
Member

There should not be a trailing slash on that url

@dimm0
Copy link
Author

dimm0 commented Oct 16, 2020

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

@dimm0
Copy link
Author

dimm0 commented Oct 19, 2020

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

@nickvergessen
Copy link
Member

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
Copy link
Author

dimm0 commented Oct 21, 2020

Done

@nickvergessen
Copy link
Member

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
Copy link
Author

dimm0 commented Oct 23, 2020

Still giving the same error

Tried disabling the OIDC, no effect

@dimm0
Copy link
Author

dimm0 commented Oct 27, 2020

Ahh! CILogon now works, but not the regular registration

@MarkPartlett
Copy link
Contributor

Same issue. Updated from 19.0.4. to 20.0.1

@MarkPartlett
Copy link
Contributor

No issue on fresh install, seems to be upgrade.

@MarkPartlett
Copy link
Contributor

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
Copy link
Author

dimm0 commented Nov 3, 2020

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
Copy link
Member

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?

@nickvergessen nickvergessen changed the title Registration is failing on nc 20 Registration is failing with broken old records in the database Nov 17, 2020
@AlbanBedel
Copy link

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants