Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: smtp_user in config.inc.php does not allow pre/post strings before/after %u placeholder #9162

Closed
ruudpen opened this issue Oct 8, 2023 · 4 comments
Milestone

Comments

@ruudpen
Copy link

ruudpen commented Oct 8, 2023

After an upgrade from roundcube 1.4.14 (Debian bullseye) to roundcube 1.6.3 (Debian bookworm) SMTP authentication fails.
All my users log in to roundcube using a short name (without @<domain_name>) and my config.inc.php contained:

$config['smtp_user'] = '%u@ourdomain.nl';

This worked perfectly before due to the %u substring replacement in roundcube/program/lib/Roundcube/rcube_smtp.php of version 1.4.14 which contained:

$smtp_user = str_replace('%u', $rcube->get_user_name(), $CONFIG['smtp_user']);

This was broken by the strict replacement algororithm introduced by issue #8435 :

if ($CONFIG['smtp_user'] == '%u') {
    $smtp_user = (string) $rcube->get_user_name();
} else {
    $smtp_user = $CONFIG['smtp_user'];
}

Note that the fix of issue #8435 sadly also modified handling the smtp_user (%u) and not only the smtp_password(%p) ...

@johndoh
Copy link
Contributor

johndoh commented Oct 8, 2023

May be something like this:

$smtp_user = preg_replace('/(?<!\\\)%u/', $rcube->get_user_name(), $CONFIG['smtp_user']);

Then for issues like #8435 the macro could be escaped in the config var like

$config['smtp_user'] = '\%u';

@ruudpen
Copy link
Author

ruudpen commented Oct 8, 2023

AFAIK the POSIX standard states that user names cannot contain the % character.
So, replacing %u by the name of the logged in user using string substitution should never be a problem for the config smtp_user field like 'prefix%upostfix'.

Passwords on the other hand may contain % characters and thus should be treated differently.

@alecpl
Copy link
Member

alecpl commented Oct 22, 2023

I don't think it was ever intentional to support this case. The description of the option was always:

// SMTP username (if required) if you use %u as the username Roundcube
// will use the current username for login
$config['smtp_user'] = '%u';

That being said, I suppose it should be safe to add support for this. I agree that it's unlikely to have % character in a username.

@alecpl alecpl added this to the 1.6.5 milestone Oct 22, 2023
alecpl added a commit that referenced this issue Oct 29, 2023
alecpl added a commit that referenced this issue Oct 29, 2023
@alecpl
Copy link
Member

alecpl commented Oct 29, 2023

Fixed.

@alecpl alecpl closed this as completed Oct 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants