ERDDAP / erddap

ERDDAP is a scientific data server that gives users a simple, consistent way to download subsets of gridded and tabular scientific datasets in common file formats and make graphs and maps. ERDDAP is a Free and Open Source (Apache and Apache-like) Java Servlet from NOAA NMFS SWFSC Environmental Research Division (ERD).
Creative Commons Zero v1.0 Universal
83 stars 58 forks source link

SMTP connection inappropriately requires an email address #173

Closed turnbullerin closed 1 month ago

turnbullerin commented 1 month ago

Describe the bug The SMTP connection information requires the SMTP username to be an email address, but some (typically mail relay services, notably the one required by the Government of Canada for applications) don't use an email address as the username.

To Reproduce

  1. Have an SMTP connection that uses a username that is not an email address
  2. Enter the username and password
  3. Note that the email services are disabled

Expected behavior The email service should be active and work when a non-email username is entered.

Server JDK 21, ERDDAP 2.24, using jetty development server.

Desktop (please complete the following information): Windows

Additional context Note that the issue steps from line 1844-1849 in EDStatic.java

emailIsActive = //ie if actual emails will be sent
            String2.isSomething(emailSmtpHost) &&
            emailSmtpPort > 0 &&
            String2.isEmailAddress(emailUserName) &&
            String2.isSomething(emailPassword) &&
            String2.isEmailAddress(emailFromAddress);

There appear to be no issues with making this change when I performed it on my local build.

Here's a patch for review: https://github.com/turnbullerin/erddap/tree/email_fix3

Of note, I ran the commit with --no-verify because the pre-commit hook was making significant formatting changes to the file and I didn't want to include these in the review :)

ChrisJohnNOAA commented 1 month ago

Thanks for this. I made a new branch with the change and created a pull request: https://github.com/ERDDAP/erddap/pull/174

Your repo seems to be disconnected according to GitHub so I wasn't able to make a pull directly from your repo.

As for the pre-commit hook. Yeah, that is new and I did also consider formatting the entire code base at once, but opted to start with the approach of getting files updated as they are touched.

turnbullerin commented 1 month ago

Hmm, ok I'll have to fix that! Thanks Chris!

Erin