ViViDboarder / vaultwarden_ldap

Automate LDAP invites to Vaultwarden
GNU General Public License v3.0
160 stars 26 forks source link

Issue with Invalid Email Address Format (e.g. "+" in Local Part) #169

Open 3srv opened 4 days ago

3srv commented 4 days ago

Hello,

I've encountered an issue where the script fails when the email address contains an invalid format. In my case, the email address has a "+" in the local part before the "@" symbol. While this is technically a valid character in many email systems, it appears that the script is not handling it correctly and results in an error.

Would it be possible to update the script to handle such cases, or at least provide a clearer error message when it encounters invalid or non-standard email formats, and allow the script to continue its execution without crashing?

Here’s an example of an email that caused the failure: username+test@domain.com

Thank you for your attention and for maintaining this project!

Best regards,

ViViDboarder commented 4 days ago

Please share the error from the log.

ViViDboarder commented 4 days ago

I wrote a test case and it seems to run fine. If you have a log I can dig further.

3srv commented 3 days ago

Unfortunately, I lost the error message, and I tried to recover it, but that machine's log doesn't have the older errors.

To "fix" the issue, I added the following:

if user_email.contains('+') {
    println!("Skipping user with email containing '+': {}", user_email);
    continue; 
}

right after:

for ldap_user in search_entries(config)? {
    // Safely get first email from list of emails in field
    if let Some(user_email) = ldap_user
        .attrs
        .get(mail_field.as_str())
        .and_then(|l| (l.first()))
    {

in main.rs.

I'm sorry I couldn't be of more help.

ViViDboarder commented 3 days ago

Are you running a binary you compiled with that patch? Would you be willing to revert to the latest release to reproduce? If not, I’m probably going to clue the issue until it can be reproduced.

3srv commented 3 days ago

If your test passes, then the problem must occur when the email is being sent, and at that moment, the sending server is unable to complete the action, causing it to fail. The issue isn't necessarily the failure itself, as that might be related to our own sending server. However, what I do see as a problem is that the script stops executing, meaning that any remaining users with correct email formats won't receive their invitation emails either.

As I mentioned, I tried to reproduce the error, but I can’t modify production right now to run this test, and we don't have another LDAP environment available for testing.

If you feel you need to close the issue, I understand. I’ve done what I could.

As a final note, and in general, I want to thank you for your work. It would be ideal if the script could handle each email individually, so that if an error occurs with one, it moves on to the next one.

ViViDboarder commented 3 days ago

The email is sent from Vaultwarden. If that is the case, manually inviting someone with a plus should be enough to trigger an error and then we could open an issue over there.

Regarding continuing to invite on error. This is something I considered, however the risk is silent failure. In that case the user would not have been invited and you, as the operator, would probably never have known. Generally I prefer noisy failures because they at least allow users to mitigate the issue. Perhaps an approach could be to continue with the rest of invites for that iteration and then crash.

Thanks for the appreciation! I actually wrote this because I planned to use it, but never did. So I just keep it working for folks like you using it. It’s good to know it’s appreciated.