[Bug]: Members of circle cannot access shared folders. Removed user can still access/modify files

gonzalo commented 1 year ago

⚠️ This issue respects the following points: ⚠️

[X] This is a bug, not a question or a configuration/webserver/proxy issue.
[X] This issue is not already reported on Github (I've searched it).
[X] Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
[X] Nextcloud Server is running on 64bit capable CPU, PHP and OS.
[X] I agree to follow Nextcloud's Code of Conduct.

Bug description

Using Nextcloud 25.0.6.1 almost fresh installation. Create a Circle and add users to it as members, create a folder and share with circle members. Then access with regular member of a circle…and share is not shown in user space.

Then promote user to “Moderator” and check again, share is now shown. Then the most worrying part comes: demote the user again to “Member” and the share is still shown! But things can even go worse!! I REMOVE user from circle and share is still available, user can read, add or remove files and folders.

I consider this is an extremely severe security issue as user can alter contents.

This behaviour has been reported in the past to the github repository but never answered.

(FYI I’ve found that despite the “delete user” from circle request returns a 200 code and removes it from the UI, the user is not truly removed. Refresh page shows it again with same level, no errors shown in nextcloud log)

Our only “strange” app that we use is SSO & SAML authentication.

Steps to reproduce

Create User (UserA) to manage the circle
Create circle CircleA (UserA)
Add another user (UserB) as "member" to the circleA
Create a folder ShareA and share it with CircleA
Access to nextcloud with UserB, folder ShareA is not shown
Promote UserB to Moderator folder ShareA is now shown in UserB filesystem
Demote UserB to Member folder ShareA is still shown in UserB filesystem
Remove UserB from CircleA, code 200 received, user is removed from UI circle list, but files are still available. If you refresh circle page the user is shown again. User has not been removed from circle (I tried with an admin and a circle owner, same result)

Expected behavior

Member users of a circle should access the shared resources without having to be moderators.
Admin/Owners/Moderator should be able to remove a user from circle
If UI confirms a user is removed it should be effectively removed, not dependaing on refresh page

Installation method

Community Web installer on a VPS or web space

Nextcloud Server version

25

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.1

Web server

Apache (supported)

Database engine version

MariaDB

Is this bug present after an update or on a fresh install?

Fresh Nextcloud Server install

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

What user-backends are you using?

[ ] Default user-backend (database)
[ ] LDAP/ Active Directory
[X] SSO - SAML
[ ] Other

Configuration report

{
    "system": {
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "***REMOVED SENSITIVE VALUE***"
        ],
        "default_language": "es",
        "default_locale": "es_ES",
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "tempdirectory": "\/***REMOVED SENSITIVE VALUE***\/temp",
        "version": "25.0.6.1",
        "overwrite.cli.url": "https:\/\/***REMOVED SENSITIVE VALUE***",
        "htaccess.RewriteBase": "\/",
        "dbtype": "mysql",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "filelocking.enabled": true,
        "memcache.local": "\\OC\\Memcache\\APCu",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": 0,
            "dbindex": 0
        },
        "default_phone_region": "ES",
        "maintenance": false,
        "theme": "",
        "loglevel": 3,
        "log_type": "file",
        "logfile": "\/***REMOVED SENSITIVE VALUE***\/nextcloud_logs\/nextcloud.log",
        "log_type_audit": "file",
        "logfile_audit": "\/***REMOVED SENSITIVE VALUE***\/nextcloud_logs\/audit.log",
        "log.condition": {
            "apps": [
                "admin_audit"
            ]
        },
        "installed": true,
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpmode": "smtp",
        "mail_sendmailmode": "smtp",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "mail_smtphost": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpport": "25",
        "updater.release.channel": "stable",
        "lost_password_link": "disabled",
        "defaultapp": "files",
        "allow_user_to_change_display_name": false,
        "hide_login_form": false,
        "skeletondirectory": "\/aplica\/nextcloud\/www\/assets\/skeleton",
        "templatedirectory": "\/aplica\/nextcloud\/www\/assets\/templates",
        "activity_expire_days": 90,
        "config_is_read_only": true
    }
}

List of activated Apps

Enabled:
  - activity: 2.17.0
  - admin_audit: 1.15.0
  - circles: 25.0.0
  - cloud_federation_api: 1.8.0
  - comments: 1.15.0
  - contacts: 5.2.0
  - contactsinteraction: 1.6.0
  - dashboard: 7.5.0
  - dav: 1.24.0
  - federatedfilesharing: 1.15.0
  - federation: 1.15.0
  - files: 1.20.1
  - files_accesscontrol: 1.15.1
  - files_pdfviewer: 2.6.0
  - files_rightclick: 1.4.0
  - files_sharing: 1.17.0
  - files_trashbin: 1.15.0
  - files_versions: 1.18.0
  - logreader: 2.10.0
  - lookup_server_connector: 1.13.0
  - nextcloud_announcements: 1.14.0
  - notifications: 2.13.1
  - oauth2: 1.13.0
  - photos: 2.0.1
  - privacy: 1.9.0
  - provisioning_api: 1.15.0
  - recommendations: 1.4.0
  - related_resources: 1.0.4
  - richdocuments: 7.1.3
  - serverinfo: 1.15.0
  - settings: 1.7.0
  - sharebymail: 1.15.0
  - support: 1.8.0
  - systemtags: 1.15.0
  - text: 3.6.0
  - theming: 2.0.1
  - twofactor_backupcodes: 1.14.0
  - updatenotification: 1.15.0
  - user_saml: 5.1.2
  - user_status: 1.5.0
  - viewer: 1.9.0
  - workflowengine: 2.7.0
Disabled:
  - bruteforcesettings
  - encryption
  - files_external
  - firstrunwizard: 2.14.0
  - password_policy: 1.15.0
  - survey_client: 1.13.0
  - suspicious_login
  - twofactor_totp
  - user_ldap
  - weather_status: 1.5.0

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

(no significant information on logs as last errors refer to older times)

Error   activity    OCP\RichObjectStrings\InvalidObjectExeption: Parameter is undefined     2023-05-09T13:05:06+0200
Error   activity    OCP\RichObjectStrings\InvalidObjectExeption: Parameter is undefined     2023-05-09T13:04:55+0200
Error   activity    OCP\RichObjectStrings\InvalidObjectExeption: Parameter is undefined     2023-05-09T13:04:21+0200
Error   activity    OCP\RichObjectStrings\InvalidObjectExeption: Parameter is undefined     2023-05-09T13:04:10+0200
Error   activity    OCP\RichObjectStrings\InvalidObjectExeption: Parameter is undefined     2023-05-09T12:35:52+0200
Error   activity    OCP\RichObjectStrings\InvalidObjectExeption: Parameter is undefined     2023-05-09T11:56:38+0200

Additional info

--

gonzalo commented 1 year ago

I can confirm that using occ commad has the same resul

xxxxx@xxxxx:/_my_path/www/nextcloud/config# occ circles:members:list xxxxxxxxxxxxxxxsQZCkS1abjQKl5qM
+---------------------------------+---------------------------------+---------------------------------+---------------------------------+------+-------------------+------------------------------+-----------+-------------------+
| Circle Id                       | Circle Name                     | Member Id                       | Single Id                       | Type | Source            | Username                     | Level     | Invited By        |
+---------------------------------+---------------------------------+---------------------------------+---------------------------------+------+-------------------+------------------------------+-----------+-------------------+
| xxxxxxxxxxxxxxxsQZCkS1abjQKl5qM | UC_xxxxxxxxxxxxxxxloud_miembros | xxxxxxxxxxxxxxxxxxxxxxxxxzyhxJS | xxxxxxxxxxxxxxxxxxxxxxxxxxsnbwy | user | Nextcloud Account | UC_xxxxxxxxxxxxxxxloud       | Owner     | occ               |
| xxxxxxxxxxxxxxxsQZCkS1abjQKl5qM | UC_xxxxxxxxxxxxxxxloud_miembros | xxxxxxxxxxxxxxxxxxxxxxxxxeFKFS4 | xxxxxxxxxxxxxxxxxxxxxxxxxxPP1n2 | user | Nextcloud Account | yyyyy.yy@yy.yy               | Admin     | occ               |
| xxxxxxxxxxxxxxxsQZCkS1abjQKl5qM | UC_xxxxxxxxxxxxxxxloud_miembros | xxxxxxxxxxxxxxxxxxxxxxxxxGz5Cra | xxxxxxxxxxxxxxxxxxxxxxxxxxLS7wb | user | Nextcloud Account | xxxxxxxxxx@xxxxx             | Moderator | yyyyy.yy@yy.yy    |
+---------------------------------+---------------------------------+---------------------------------+---------------------------------+------+-------------------+------------------------------+-----------+-------------------+

xxxxx@xxxxx:/_my_path/www/nextcloud/config# occ circles:members:remove xxxxxxxxxxxxxxxxxxxxxxxxxGz5Cra
xxxxx@xxxxx:/_my_path/www/nextcloud/config# occ circles:members:list xxxxxxxxxxxxxxxsQZCkS1abjQKl5qM
+---------------------------------+---------------------------------+---------------------------------+---------------------------------+------+-------------------+------------------------------+-----------+-------------------+
| Circle Id                       | Circle Name                     | Member Id                       | Single Id                       | Type | Source            | Username                     | Level     | Invited By        |
+---------------------------------+---------------------------------+---------------------------------+---------------------------------+------+-------------------+------------------------------+-----------+-------------------+
| xxxxxxxxxxxxxxxsQZCkS1abjQKl5qM | UC_xxxxxxxxxxxxxxxloud_miembros | xxxxxxxxxxxxxxxxxxxxxxxxxzyhxJS | xxxxxxxxxxxxxxxxxxxxxxxxxxsnbwy | user | Nextcloud Account | UC_xxxxxxxxxxxxxxxloud       | Owner     | occ               |
| xxxxxxxxxxxxxxxsQZCkS1abjQKl5qM | UC_xxxxxxxxxxxxxxxloud_miembros | xxxxxxxxxxxxxxxxxxxxxxxxxeFKFS4 | xxxxxxxxxxxxxxxxxxxxxxxxxxPP1n2 | user | Nextcloud Account | yyyyy.yy@yy.yy               | Admin     | occ               |
| xxxxxxxxxxxxxxxsQZCkS1abjQKl5qM | UC_xxxxxxxxxxxxxxxloud_miembros | xxxxxxxxxxxxxxxxxxxxxxxxxGz5Cra | xxxxxxxxxxxxxxxxxxxxxxxxxxLS7wb | user | Nextcloud Account | xxxxxxxxxx@xxxxx             | Moderator | yyyyy.yy@yy.yy    |
+---------------------------------+---------------------------------+---------------------------------+---------------------------------+------+-------------------+------------------------------+-----------+-------------------+

gonzalo commented 1 year ago

As we have a similar instance without the same problem I was able to find and fix the issue, but I still consider this is a bug. The problem seems to come from the loopback address. Initially it was set to "mydomain.xxx/nextcloud" but after a while we move it "mydomain.xxx". Despite we set it properly in config.php, circles app is not using it.

I used occ circles:check command to detect & fix the issue

$ occ circles:check
### Checking loopback address.
. The loopback setting is mandatory and can be checked locally.
. The address you need to define here must be a reachable url of your Nextcloud from the hosting server itself.
. By default, the App will use the entry 'overwrite.cli.url' from 'config/config.php'.    <- WRONG "overwrite.cli.url" is properly set in config.php**

* testing current address: https://MYDOMAIN.XXX/nextcloud   <- WRONG/OLD URL 
- GET request on https://MYDOMAIN.XXX/nextcloud/index.php/csrftoken: 302

- You do not have a valid loopback address setup right now.

Please write down a new loopback address to test: https://MYDOMAIN.XXX      <- HERE I SET THE RIGHT URL
* testing address: https://MYDOMAIN.XXX
- GET request on https://MYDOMAIN.XXX/index.php/csrftoken: 200
- POST request on https://MYDOMAIN.XXX/index.php/apps/circles/async/test-dummy-token/: 200
- Creating async FederatedEvent f166bd2d-e3c6-43f9-9cf6-9e64c56ceab9 (took 57ms)
- Waiting for async process to finish (5s)
- Checking status on FederatedEvent verify=17 manage=42 
- Do you want to save https://MYDOMAIN.XXX as your loopback address ? (y/N) y.  <- STORE NEW URL
- Address https://MYDOMAIN.XXX is now used as loopback

### Testing internal address.
. The internal setting should only be enabled if you are willing to use Circles in a GlobalScale setup on a local network.
. The address you need to define here is the local address of your Nextcloud, reachable by all other instances of our GlobalScale.
- Do you want to enable this feature ? (y/N) n
skipping.

So finally happy end, BUT I still we have an important security issue here. I observed during this incident that you can try to delete the user member and the circle and rececive 200 codes and apparently have it removed. But if you don't verify, both user membership and circles remain active: so shared resources, etc. A complete nightmare for an organization.

smlanger commented 1 year ago

@gonzalo How were you able to enter the loopback address with only the domain? Every time I ommit a path in the URL it will automatically append "/nextcloud".

### Checking loopback address.
. The loopback setting is mandatory and can be checked locally.
. The address you need to define here must be a reachable url of your Nextcloud from the hosting server itself.
. By default, the App will use the entry 'overwrite.cli.url' from 'config/config.php'.

* testing current address: http://localhost/nextcloud
- GET request on http://localhost/nextcloud/index.php/csrftoken: 404

- You do not have a valid loopback address setup right now.

Please write down a new loopback address to test: https://HOST.MY.DOMAIN/
* testing address: https://HOST.MY.DOMAIN
- GET request on https://HOST.MY.DOMAIN/nextcloud/index.php/csrftoken: 302

Please write down a new loopback address to test: https://HOST.MY.DOMAIN
* testing address: https://HOST.MY.DOMAIN
- GET request on https://HOST.MY.DOMAIN/nextcloud/index.php/csrftoken: 302

Please write down a new loopback address to test: ^C
exiting.

When entering a bogus path it will try using that instead of "nextcloud" but it won't accept a missing path

Please write down a new loopback address to test: https://HOST.MY.DOMAIN/index.php
* testing address: https://HOST.MY.DOMAIN/index.php
- GET request on https://HOST.MY.DOMAIN/index.php/index.php/csrftoken: 302

Please write down a new loopback address to test: https://HOST.MY.DOMAIN/
* testing address: https://HOST.MY.DOMAIN
- GET request on https://HOST.MY.DOMAIN/nextcloud/index.php/csrftoken: 302

The only way I could get occ circles:check to work correctly was changing the vhost to make nextcloud reachable on "https://HOST.MY.DOMAIN/nextcloud".

gonzalo commented 1 year ago

I just entered "https://MYDOMAIN.XXX", have you checked if this url is aligned with the ones on config.php and vhosts? (also for overwrite.cli.url)

smlanger commented 1 year ago

I tried again and made sure that overwrite.cli.url in config.php is set to the same address I want to use for loopback address but no chance. But looking through some other open issues I found you can set app configs directly with occ config:app:set So I did:

occ config:app:set circles loopback_cloud_path
occ config:app:set --value https circles loopback_cloud_scheme
occ config:app:set --value HOST.MY.DOMAIN circles loopback_cloud_id

And now the circles app seems to be working properly being able to delete a circle, remove members and share files with regular members instead of only circle moderators.

Also after setting the loopback address manually occ circles:check is detecting the current configuration as valid without asking for input.

joshtrichards commented 3 weeks ago

I REMOVE user from circle and share is still available, user can read, add or remove files and folders.

Tested with current release v29 and this does not happen today. Are you still able to reproduce this behavior today in >=28?


By default, the App will use the entry 'overwrite.cli.url' from 'config/config.php'.    <- WRONG "overwrite.cli.url" is properly set in config.php**

* testing current address: https://MYDOMAIN.XXX/nextcloud   <- WRONG/OLD URL

It's accurate in the sense that the overwrite.cli.url value is what is used during the initial setup. After that, the app polls it's own configuration.

https://github.com/nextcloud/circles/blob/bf0a812a617140bc751be651ff049015a85125b3/lib/Service/ConfigService.php#L491-L509

I can see there may be some room for improvement here in terms of language + documentation.

nextcloud / circles