[Bug]: Password is required for log-in credentials in External storage config

[zc-devs]

⚠️ This issue respects the following points: ⚠️

• This is a bug, not a question or a configuration/webserver/proxy issue.
• This issue is not already reported on Github OR Nextcloud Community Forum (I've searched it).
• Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
• I agree to follow Nextcloud's Code of Conduct.

Bug description

Cannot change External storage settings because of required password field when using Log-in credentials, save in session or Log-in credentials, save in database. This settings should not request password at all.

Steps to reproduce

1. Run Nextcloud

docker run -p 8080:80 nextcloud:31.0

2. Initialize it: go to http://localhost:8080, enter admin's login and password, hit Install
3. Enable External storage app: go to Menu - Apps - Disabled apps, enable External storage support
4. Go to Administration settings - External storage
5. Add new WebDAV storage, for example
6. Choose Log-in credentials, save in session or Log-in credentials, save in database in Authentication column
7. See that either of them requires some password

Expected behavior

1. Run Nextcloud

docker run -p 8080:80 nextcloud:28.0

2. Initialize it: go to http://localhost:8080, enter admin's login and password, hit Install
3. Enable External storage app: go to Menu - Apps - Disabled apps, enable External storage support
4. Go to Administration settings - External storage
5. Add new WebDAV storage, for example
6. Choose Log-in credentials, save in session or Log-in credentials, save in database in Authentication column
7. See that there is no password field and you can save the settings

Nextcloud Server version

31

Operating system

None

PHP engine version

None

Web server

Apache (supported)

Database engine version

None

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

None

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

What user-backends are you using?

• Default user-backend (database)
• LDAP/ Active Directory
• SSO - SAML
• Other

Configuration report

List of activated Apps

Nextcloud Signing status

Nextcloud Logs

Additional info

It was discovered on version 29 for SMB storage, using docker image, Postgres database and LDAP authentication. This instance is very old and was migrated from virtual machine a couple of years ago. SMB storage works fine there, I just can't change the settings now (or create new entries).

However, the issue appears for WebDAV (as well as FTP, SFTP, Nextcloud) external storages also using apache docker images starting from the latest 29 through 31, fresh install with SQLite database and default authentication.

The latest 28 version works great.

[zc-devs]

SMB storage works fine there

That is my database state at the moment:

nextcloud=# select * from oc_external_config;
 config_id | mount_id |     key     |   value
-----------+----------+-------------+-----------
         1 |        1 | host        | 192.168.11.11
         2 |        1 | share       | profiles
         3 |        1 | root        | $user
         4 |        1 | domain      | DOMAIN
         5 |        1 | show_hidden |
         6 |        1 | check_acl   |
         7 |        1 | timeout     |
(7 rows)

nextcloud=# select * from oc_external_mounts;
 mount_id | mount_point | storage_backend |        auth_backend        | priority | type
----------+-------------+-----------------+----------------------------+----------+------
        1 | /Profile    | smb             | password::logincredentials |      100 |    1
(1 row)

nextcloud=# select * from oc_external_options;
 option_id | mount_id |           key            | value
-----------+----------+--------------------------+-------
         1 |        1 | encrypt                  | true
         2 |        1 | previews                 | true
         4 |        1 | filesystem_check_changes | 1
         5 |        1 | encoding_compatibility   | false
         3 |        1 | enable_sharing           | true
         6 |        1 | readonly                 | true
(6 rows)

⬆ Log-in credentials, save in database authentication in UI.

I just wanted to flip the readonly flag via UI :)

[skjnldsv]

This settings should not request password at all.

It's a security measure that was recently added. But there was a slight bug on some specific configurations, especially LDAP and OIDC.
It's fixed in the latest version and an additional fix will be coming in the upcoming ones, in May

[zc-devs]

It's a security measure that was recently added. But there was a slight bug on some specific configurations, especially LDAP and OIDC.

Are you talking about #51919?
I'm not sure it is duplicate, because

1. In [Bug]: External storage will prompt for password but always tell you it's the wrong password #51919 there is/was a bug in a pop-up password prompt, if I understand correctly. And here it seems like a password field from backend config. And it prevents me from saving config => I cannot reach those admin's password confirmation prompt/pop-up.

Note, that it is system-wide config, I cannot know user name or password - it's per user.

2. this really seems like it's been an issue since 30.0.4 (IIRC) before that it just worked, after that version it started doing this and I have an issue starting from 29.
3. As you can see in steps to reproduce, I didn't use LDAP or OIDC.

It's fixed in the latest version and an additional fix will be coming in the upcoming ones, in May

But I might be wrong and it might be resolved in the main. Sure, I'll test it on upcoming releases, but maybe there is some dev/PR based containers, that I could test now?

[skjnldsv]

Hey Thomas, sorry it looks like you're right!: 💯
I totally misread your report 👀

[skjnldsv]

Seems to be a regression from #51019 ?

@provokateurin the passwords are now required for LoginCredentials and SessionCredentials, what's the reason behind it ? I don't think it make sense since the password issupposed to be provided by the Login or Session, no ?

[zc-devs]

No problem.
I've just tested further, speaking of version 29, it was introduced in nextcloud:29.0.13-apache docker image, 29.0.13.2 version of Nextcloud. Probably, the cause is #51392 (or #50930, or #51105 as you have mentioned already).

[provokateurin]

Seems to be a regression from #51019 ?

This was a fix because the password field was specified in the wrong place. It was originally added to ensure that the password value is marked as sensitive and is not returned to the frontend, but that was added to only one backend and not the login methods which actually specify the password field.

[skjnldsv]

#51392 & #50930, were what confused me initially. But they actually are different, they are about security and asking the current user to validate their password to save the settings. We already have that in other areas, like apps or user management. This is different from the storage password :)

[provokateurin]

I think you might be right that this is a regression of my PR, but I have little knowledge about this stuff. Maybe @ArtificialOwl can provide some more insight as he did the original fix?
It's probably right that we only need to add the password fields in case it's a storage configured by the user.