fix(files_external/SMB): Use 'null' explicitly for no workgroup

[oblivioncth]

• Resolves: [Bug]: occ files_external:notify gives Icewind\SMB\Exception\ConnectionRefusedException on 33 #58445

Summary

Robin Appleman's SMB library is designed so that 'null' is to be used when no explicit workgroup is set:

final class BasicAuth implements IAuth {
    /** @var string */
    private $username;
    /** @var string|null */ <--------------------------
    private $workgroup;
    /** @var string */
    private $password;
//...

However, it previously was loose with its checks and would still treat any falsy value (e.g. an empty string) as "not specified" when forming arguments for the various underlying utilities, such as smbclient.

icewind/SMB@4a93467905 updated it's handling to be more strict and as such will now treat empty strings as distinct from null values for workgroups. An empty value for workgroup often doesn't make sense, unless the backend tool happens to convert it to "WORKGROUP". In the case of the smbclient utility, passing an empty string for the $workgroup paramters results in -W '' being used which is invalid syntax and so the invocation fails resulting in no connection being made and the user seeing an inaccurate [Icewind\SMB\Exception\ConnectionRefusedException] error. Therefore, it is important to always pass 'null' to this constructor for $workgroup when there isn't one.

Most code paths in the 'files_external' app already handle this correctly, but this now altered path originally just took the raw value from the app backend and passed it as-is. Assuming the user left the "Domain" box empty in the frontend, this would be an empty string. Now the the SMB library's interface is correctly honored in that case.

It's arguable that the SMB library should simply be updated to formally accept '' as a valid input meaning "no workgroup", restoring the old behavior in a canonical fashion, as there is almost no situation where one would want to pass an empty string and have it mean anything else; but regardless, there is no harm in being more explicit on the nextcloud/server side as well.

Details

Just to cover all bases, the following should be all of the uses of BasicAuth().

Corrected Path:

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

Lines 71 to 74 in a982b6c

	$smbAuth = new BasicAuth(
	$storage->getBackendOption('user'),
	$storage->getBackendOption('domain'),
	$storage->getBackendOption('password')

Kerberos fallback path:

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

Lines 92 to 111 in a982b6c

	try {
	$credentials = $credentialsStore->getLoginCredentials();
	$loginName = $credentials->getLoginName();
	$pass = $credentials->getPassword();
	preg_match('/(.*)@(.*)/', $loginName, $matches);
	$realm = $storage->getBackendOption('default_realm');
	if (empty($realm)) {
	$realm = 'WORKGROUP';
	}
	if (count($matches) === 0) {
	$username = $loginName;
	$workgroup = $realm;
	} else {
	[, $username, $workgroup] = $matches;
	}
	$smbAuth = new BasicAuth(
	$username,
	$workgroup,
	$pass
	);

Seems to never result in an empty workgroup.

Primary connection path:

server/apps/files_external/lib/Lib/Storage/SMB.php

Lines 82 to 84 in a982b6c

	} elseif (isset($parameters['user']) && isset($parameters['password']) && isset($parameters['share'])) {
	[$workgroup, $user] = $this->splitUser($parameters['user']);
	$auth = new BasicAuth($user, $workgroup, $parameters['password']);

This is the only one possibly worth making further changes to. It's mostly fine as splitUser() will correctly return null for the result that goes into $workgroup if there is no workgroup separator; however if a separator exists but with no value after it (e.g. \\user or /user) it will return an array with an empty string, which will get placed into $workgroup. Since that is an invalid string for specifying workgroup/user it's possible that such strings are rejected at some point earlier before they even reach this function, and if not, I'm not sure this is the correct place to handle invalid input like that. Though, it may not hurt to handle that case here anyway just to be safe (i.e. inspect the exploded array and swap '' for null.

server/apps/files_external/lib/Lib/Storage/SMB.php

Lines 126 to 134 in a982b6c

	private function splitUser(string $user): array {
	if (str_contains($user, '/')) {
	return explode('/', $user, 2);
	} elseif (str_contains($user, '\\')) {
	return explode('\\', $user);
	}
	return [null, $user];
	}

Thoughts?

Otherwise, the SMB library should probably be updated to throw some other kind of error when "smbclient" exits with an error, instead of "Connection Refused". Separately it could be made more robust against questionable arguments like a blank WORKGROUP (e.g. also defaulting '' to "WORKGROUP" or emitting a warning), as touched on on the commit message.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)
• Labels added where applicable (ex: bug/enhancement, 3. to review, feature component)
• Milestone added for target branch/version (ex: 32.x for stable32)

AI (if applicable)

• The content of this PR was partly or fully generated using AI

[joshtrichards]

/backport to stable34

[joshtrichards]

/backport to stable33

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

[welcome]

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22