nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
26.45k stars 3.98k forks source link

[Bug]: Contact Search: System Address Book Avatar (Image) URL issue with IMAP #42072

Open PeteKersker opened 8 months ago

PeteKersker commented 8 months ago

⚠️ This issue respects the following points: ⚠️

Bug description

When searching for a system address book user using the contact search button, the user's image avatar url needs to be escaped when using UserExternal_IMAP authentication.

This is what the system generates, which does not work: https://my.nextcloud.installation/remote.php/dav/addressbooks/system/system/system/OCA\UserExternal\IMAP:usercard.vcf?photo&size=32

Manually editing the url to escape each '\' to '%5c' does work: https://my.nextcloud.installation/remote.php/dav/addressbooks/system/system/system/OCA%5cUserExternal%5cIMAP:usercard.vcf?photo&size=32

Steps to reproduce

  1. Log in to the web interface.
  2. Click on the contact search button and search for a user with a profile picture.
  3. The user avatar does not appear. AvatarMissing

Expected behavior

The user's avatar image should be displayed. (Security is set correctly to allow it. The image displays correctly in the contacts app.)

Installation method

Community Manual installation with Archive

Nextcloud Server version

27

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.2

Web server

None

Database engine version

MariaDB

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

Upgraded to a MAJOR version (ex. 22 to 23)

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

What user-backends are you using?

Configuration report

{
    "system": {
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "tend.agardenwalk.net"
        ],
        "overwrite.cli.url": "https:\/\/tend.agardenwalk.net",
        "htaccess.RewriteBase": "\/",
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "objectstore": {
            "class": "\\OC\\Files\\ObjectStore\\S3",
            "arguments": {
                "bucket": "tend.agardenwalk.net",
                "autocreate": true,
                "key": "***REMOVED SENSITIVE VALUE***",
                "secret": "***REMOVED SENSITIVE VALUE***",
                "hostname": "s3.us-east-1.wasabisys.com",
                "region": "us-east-1",
                "port": 443,
                "use_ssl": true,
                "use_path_style": true
            }
        },
        "quota_include_external_storage": false,
        "share_folder": "\/Shared with Me",
        "dbtype": "mysql",
        "version": "27.1.4.1",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "memcache.local": "\\OC\\Memcache\\APCu",
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "filesystem_check_changes": 0,
        "filelocking.enabled": "true",
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": 0,
            "dbindex": 0,
            "password": "***REMOVED SENSITIVE VALUE***",
            "timeout": 1.5
        },
        "enable_previews": true,
        "enabledPreviewProviders": [
            "OC\\Preview\\TXT",
            "OC\\Preview\\MarkDown",
            "OC\\Preview\\OpenDocument",
            "OC\\Preview\\PDF",
            "OC\\Preview\\MSOffice2003",
            "OC\\Preview\\MSOfficeDoc",
            "OC\\Preview\\Image",
            "OC\\Preview\\Photoshop",
            "OC\\Preview\\TIFF",
            "OC\\Preview\\SVG",
            "OC\\Preview\\Font",
            "OC\\Preview\\MP3",
            "OC\\Preview\\Movie",
            "OC\\Preview\\MKV",
            "OC\\Preview\\MP4",
            "OC\\Preview\\AVI"
        ],
        "remember_login_cookie_lifetime": 1296000,
        "session_lifetime": 86400,
        "session_relaxed_expiry": false,
        "session_keepalive": true,
        "auto_logout": false,
        "default_language": "en_US",
        "default_locale": "en_US",
        "default_phone_region": "en_US",
        "mail_smtpmode": "smtp",
        "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": "587",
        "mail_smtpname": "***REMOVED SENSITIVE VALUE***",
        "mail_smtppassword": "***REMOVED SENSITIVE VALUE***",
        "maintenance": false,
        "simpleSignUpLink.shown": false,
        "user_backends": [
            {
                "class": "\\OCA\\UserExternal\\IMAP",
                "arguments": [
                    "email.agardenwalk.net",
                    993,
                    "ssl",
                    "agardenwalk.net",
                    true,
                    true
                ]
            }
        ],
        "theme": "",
        "loglevel": 2,
        "knowledgebaseenabled": false
    }
}

List of activated Apps

Enabled:
  - activity: 2.19.0
  - analytics: 4.11.1
  - announcementcenter: 6.7.0
  - bookmarks: 13.1.1
  - bruteforcesettings: 2.7.0
  - calendar: 4.6.0
  - calendar_resource_management: 0.5.0
  - circles: 27.0.1
  - cloud_federation_api: 1.10.0
  - comments: 1.17.0
  - contacts: 5.4.2
  - contactsinteraction: 1.8.0
  - cookbook: 0.10.3
  - dashboard: 7.7.0
  - dav: 1.27.0
  - deck: 1.11.2
  - drawio: 2.1.4
  - epubviewer: 1.5.3
  - external: 5.2.1
  - federatedfilesharing: 1.17.0
  - federation: 1.17.0
  - files: 1.22.0
  - files_3dmodelviewer: 0.0.12
  - files_accesscontrol: 1.17.1
  - files_antivirus: 5.4.0
  - files_archive: 1.1.3
  - files_external: 1.19.0
  - files_markdown: 2.4.1
  - files_mindmap: 0.0.30
  - files_pdfviewer: 2.8.0
  - files_reminders: 1.0.0
  - files_rightclick: 1.6.0
  - files_sharing: 1.19.0
  - files_texteditor: 2.15.1
  - files_trashbin: 1.17.0
  - files_versions: 1.20.0
  - files_zip: 1.4.0
  - firstrunwizard: 2.16.0
  - groupfolders: 15.3.1
  - integration_peertube: 1.0.2
  - logreader: 2.12.0
  - lookup_server_connector: 1.15.0
  - mail: 3.4.5
  - maps: 1.1.1
  - music: 1.9.1
  - news: 24.0.0
  - nextcloud_announcements: 1.16.0
  - notes: 4.8.1
  - notifications: 2.15.0
  - oauth2: 1.15.1
  - oidc: 0.7.1
  - password_policy: 1.17.0
  - passwords: 2023.11.30
  - photos: 2.3.0
  - previewgenerator: 5.4.0
  - privacy: 1.11.0
  - provisioning_api: 1.17.0
  - quota_warning: 1.18.0
  - recommendations: 1.6.0
  - related_resources: 1.2.0
  - richdocuments: 8.2.3
  - serverinfo: 1.17.0
  - settings: 1.9.0
  - sharebymail: 1.17.0
  - side_menu: 3.11.2
  - sketch_picker: 1.0.1
  - spreed: 17.1.3
  - support: 1.10.0
  - survey_client: 1.15.0
  - systemtags: 1.17.0
  - tasks: 0.15.0
  - text: 3.8.0
  - theming: 2.2.0
  - theming_customcss: 1.15.0
  - twofactor_backupcodes: 1.16.0
  - updatenotification: 1.17.0
  - user_external: 3.2.0
  - user_status: 1.7.0
  - viewer: 2.1.0
  - weather_status: 1.7.0
  - welcome: 1.0.10
  - workflowengine: 2.9.0
Disabled:
  - admin_audit: 1.17.0
  - encryption: 2.15.0
  - suspicious_login: 5.0.0
  - twofactor_totp: 9.0.0
  - user_ldap: 1.17.0

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

empty

Additional info

The issue is the same in Brave, Chrome, and Firefox Escaping the '\' in the inspector/debugger displays the image correctly.

kesselb commented 8 months ago

Thanks for letting us know :+1:

Index: apps/dav/lib/CardDAV/AddressBookImpl.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/apps/dav/lib/CardDAV/AddressBookImpl.php b/apps/dav/lib/CardDAV/AddressBookImpl.php
--- a/apps/dav/lib/CardDAV/AddressBookImpl.php  (revision f49cff2f5af060963e6d5980d73f91a51562fa44)
+++ b/apps/dav/lib/CardDAV/AddressBookImpl.php  (date 1701895491151)
@@ -274,13 +274,14 @@

        foreach ($vCard->children() as $property) {
            if ($property->name === 'PHOTO' && in_array($property->getValueType(), ['BINARY', 'URI'])) {
+               [$userBackend, $userId] = explode(':', $uri, 2);
                $url = $this->urlGenerator->getAbsoluteURL(
                    $this->urlGenerator->linkTo('', 'remote.php') . '/dav/');
                $url .= implode('/', [
                    'addressbooks',
                    substr($this->addressBookInfo['principaluri'], 11), //cut off 'principals/'
                    $this->addressBookInfo['uri'],
-                   $uri
+                   urlencode($userBackend) . ':' . $userId,
                ]) . '?photo';

                $result['PHOTO'] = 'VALUE=uri:' . $url;

The above patch may fix it.

The current code does not consider that the user backend name could contain a backslash. That's usually not the case for the integrated backends.

user_external should implement the IUserBackend to signal a proper backend name.

PeteKersker commented 8 months ago

Wow! Thank you for such a quick response!

Yes, it fixed the problem, but it was a breaking change. The avatars for my regular contacts ended up with a malformed url (an extra ':').

Since you pointed me to the right place, I fiddled with the code and was able to get everything to work by implementing your suggestion for the user_external code, only in this section instead.

Here's what worked:

Replace $uri With urlencode($uri) No other changes

I'm new to all this, so what should I do to have this change considered for a future revision?

PeteKersker commented 5 months ago

Update: Nextcloud 28

I upgraded to 28.0.3 and the contact photos now all come back as a circle with a ? in the middle. No url is returning at all. The patch above no longer works.

Reminder: I'm using User Authentication (IMAP) to authenticate. I'm willing to be a tester for my environment if that helps.

PeteKersker commented 3 months ago

Nextcloud 28.0.5 continues with the same pattern I mentioned in March. No version of the patch discussed above helps.

Please let me know if there is any way I can help.