logto-io / logto

🧑‍🚀 The better identity infrastructure for developers and the open-source alternative to Auth0.
https://logto.io
Mozilla Public License 2.0
8.97k stars 448 forks source link

bug: empty strings are removed from connector configuration (prevents use of SMTP relay without user/password) #6754

Open bpow opened 3 weeks ago

bpow commented 3 weeks ago

Describe the bug

Trying to set up a SMTP relay with the SMTP connector that doesn't require user/password does not work because the api endpoint removes JSON key/value pairs from the auth parameter.

Expected behavior

It should be possible to provide auth parameter like the following:

{
  "type": "login",
  "user": "",
  "pass": ""
}

And to have this configuration passed along to nodemailer.

How to reproduce?

  1. Create a SMTP connector from the admin console
  2. Put the configuration described above in the Auth text input for the SMTP connector.
  3. Upon saving the changes, this will validate (if everything else is valid), but the empty strings associated with user/pass will not be saved, so the connector won't work. Furthermore, the contents of the Auth box will change to just have "type": "login".

It looks like this happens because of the call to cleanDeep in packages/core/src/routes/connector/index.ts, which includes the config object.

      await insertConnector({
        id: insertConnectorId,
        connectorId,
        ...cleanDeep({ syncProfile, config, metadata }),
      });

This also leads to weird behavior where if you have a relay that doesn't require user/pass, then when you first put the full info in the Auth input block, you can successfully "send" to test the email connector, but after it is saved and the user/pass items are removed, the connector won't work and send/test won't work anymore.

Context

Screenshots

darcyYe commented 3 weeks ago

Hi @bpow, would you like to let us know which email service are you using? I think the problem you're encountering might be a special case requiring specific handling. If you could tell us which email service it is, we might be able to find a better solution.

bpow commented 3 weeks ago

Thanks for looking into this, @darcyYe. The SMTP server involved is institutional email relay-- I'm not sure what specific software it is running, but I'm pretty sure it's RFC-standard SMTP. Just with access controlled by source (e.g., on a VLAN) rather than by credentials.

I can say that I am able to send test emails directly using nodemailer with something like the following:

let nodemailer = require('nodemailer');

console.log('getting started');

let transporter = nodemailer.createTransport({
  host: "relay.test.com",
  port: 25,
  auth: {
    user: "",
    pass: "",
  },
});

transporter.verify(function (error, success) {
  if (error) {
    console.log(error);
  } else {
    console.log("Server is ready for us!");
  }
});

const message = {
  from: "bpow@test.com",
  to: "bpow@test.com",
  subject: "nodemailer test",
  text: "Let's see if this message makes its way through!",
};

transporter.sendMail(message, function(err, info) {
  if (err) {
    console.log(`err: ${err}`);
  }
});

(with the test.com parts obviously replaced by something else...)

So it works to be able to send messages if user and pass are empty strings.

However, it also works if I change the auth block to just be {} (as per the nodemailer docs, nodemailer just assumes that auth has already taken place if this is the case). So rather than changing the use of cleanDeep, it may suffice for my purposes to just make user and pass optional in authGuard of types.ts-- in this case it is validateConfig that leads to an error (after cleanDeep removes the key-values with empty-string values), but nodemailer does not actually require user and pass.

If you want to see the unexpected behavior when empty strings are provided for user and pass, then the following docker-compose.yml file can replicate it, using mailpit as the SMTP server:

# This compose file is for demonstration only, do not use in prod.
version: "3.9"
services:
  app:
    depends_on:
      postgres:
        condition: service_healthy
    image: ghcr.io/logto-io/logto:1.21.0
    entrypoint: ["sh", "-c", "npm run cli db seed -- --swe && npm start"]
    ports:
      - 3001:3001
      - 3002:3002
    environment:
      - TRUST_PROXY_HEADER=1
      - DB_URL=postgres://postgres:changeme@postgres:5432/logto
      # Mandatory for GitPod to map host env to the container, thus GitPod can dynamically configure the public URL of Logto;
      # Or, you can leverage it for local testing.
      - ENDPOINT
      - ADMIN_ENDPOINT
  postgres:
    image: postgres:14-alpine
    user: postgres
    environment:
      POSTGRES_USER: postgres
      POSTGRES_PASSWORD: changeme
    healthcheck:
      test: ["CMD-SHELL", "pg_isready"]
      interval: 10s
      timeout: 5s
      retries: 5
  mailpit:
    image: 'axllent/mailpit:latest'
    expose:
        - '${FORWARD_MAILPIT_PORT:-1025}:1025'
    ports:
        - '${FORWARD_MAILPIT_DASHBOARD_PORT:-8025}:8025'

It's not a perfect example since mailpit in this configuration will accept any user/pass combination, but it will accept empty strings. To show the behavior using this setup:

  1. docker compose up -d
  2. Go to the admin console at http://localhost:3002
  3. Setup a new SMTP connector, using configuration:
    • Host: mailpit
    • Port: 1025
    • Auth:
      {
      "type": "login",
      "user": "",
      "pass": ""
      }
    • Put something that looks like an email address in From Email and Reply To
    • Everything else can remain as is.
  4. Put any email address in the Test your email connector box and press "Send"
  5. You should be able to see the email in the mailpit web interface (at http://localhost:8025)
  6. If you press "Save changes", the connector code does save things (it validates OK with the empty strings for user/pass, but then it calls cleanDeep after validation, so what gets saved doesn't include the user/pass values at all.
  7. Now if you scroll back up to the auth section, user and pass are gone! And now testing send or saving again won't work because of the validation failing!

I hope this outlines things clearly enough. I can make a short screen recording if you think that would help.

But TL;DR, could we maybe just make the user and pass parts of authGuard optional?