Skip to content

Conversation

@alacn1
Copy link
Contributor

@alacn1 alacn1 commented Nov 10, 2021

When using external mount with "Global credentials, user entered", users without it set won't be able to login.
This happens because BasicAuth requires username and password as string, but will be null when user didn't set it yet.

It happens because since the commit: 66781e7 update icewind/smb to 3.4.0
the file apps/files_external/3rdparty/icewind/smb/src/BasicAuth.php was changed to use php 7.4 typed properties.

-	public function __construct($username, $workgroup, $password) {
+	public function __construct(string $username, ?string $workgroup, string $password) {

BasicAuth with username as null throws TypeError:

Argument 1 passed to Icewind\\SMB\\BasicAuth::__construct() must be of the type string, null given, called in apps/files_external/lib/Lib/Backend/SMB.php on line 82

apps/files_external/lib/Lib/Backend/SMB.php

public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) {
[...]
$smbAuth = new BasicAuth(
	$storage->getBackendOption('user'),
	$storage->getBackendOption('domain'),
	$storage->getBackendOption('password')
);

TypeError is a subclass of Error, not a subclass of Exception, it won't catch it at FailedStorage:

apps/files_external/lib/Config/ConfigAdapter.php

public function getMountsForUser(IUser $user, IStorageFactory $loader) {
	[...]
	$storages = array_map(function (StorageConfig $storageConfig) use ($user) {
		try {
			$this->prepareStorageConfig($storageConfig, $user);
			return $this->constructStorage($storageConfig);
		} catch (\Exception $e) {
			// propagate exception into filesystem
			return new FailedStorage(['exception' => $e]);
		}
	}, $storageConfigs);

The proposed fix is to check if user and password is string instead of null, and throw InvalidArgumentException if it's not.

apps/files_external/lib/Lib/Backend/SMB.php

public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) {
	[...]
	if(!is_string($storage->getBackendOption('user')) || !is_string($storage->getBackendOption('password')))
		throw new \InvalidArgumentException('user or password is not set');

This commit can be cherry-picked to master, stable22, stable21.

Fix #28229 .
Fix #26697 .

Signed-off-by: Anderson Luiz Alves <alacn1@gmail.com>
@alacn1 alacn1 changed the title Fix users can't login external mount user entered credentials not set [stable22] Fix users can't login external mount user entered credentials not set Nov 10, 2021
@PVince81
Copy link
Member

code makes sense, but we can't merge this before having it on master first, otherwise we have discrepancies on branches (also stable23 is the most recent after master)

do you mind resending this to master ?

@alacn1
Copy link
Contributor Author

alacn1 commented Nov 26, 2021

code makes sense, but we can't merge this before having it on master first, otherwise we have discrepancies on branches (also stable23 is the most recent after master)

do you mind resending this to master ?

did.

PR #29923 .

@solracsf
Copy link
Member

Closing in flavor of #29923 (will be backported than).

@solracsf solracsf closed this Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants