fix(connector): allow SMTP login auth without user/pass#8888
Merged
Conversation
COMPARE TO
|
| Name | Diff |
|---|---|
| .changeset/fix-smtp-auth-empty-credentials.md | 📈 +190 Bytes |
| packages/connectors/connector-smtp/src/index.test.ts | 📈 +326 Bytes |
| packages/connectors/connector-smtp/src/index.ts | 📈 +277 Bytes |
| packages/connectors/connector-smtp/src/types.ts | 📈 +589 Bytes |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes SMTP connector configuration for relays that authorize by source (IP/VLAN) and don't require credentials. Previously, the route-layer cleanDeep stripped empty-string user/pass from auth, leaving { type: 'login' }, which then failed the connector's Zod validation on subsequent reads.
Changes:
- Made
userandpassoptional onloginAuthGuardand added.strict()to prevent the looser shape from incorrectly matching OAuth2 payloads first in the union. - Added a documented type assertion at the Nodemailer boundary since
AuthenticationTypeLogintypesuser/passas required strings while runtime treats them as optional. - Added a unit test covering the post-
cleanDeepshapeauth: { type: 'login' }.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/connectors/connector-smtp/src/types.ts | Loosen loginAuthGuard (optional user/pass) and add .strict() to disambiguate union with OAuth2 guards. |
| packages/connectors/connector-smtp/src/index.ts | Type-assert config to SMTPTransport.Options with eslint-disable + rationale comment. |
| packages/connectors/connector-smtp/src/index.test.ts | New test for login auth missing user/pass. |
| .changeset/fix-smtp-auth-empty-credentials.md | Patch bump changeset for @logto/connector-smtp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wangsijie
approved these changes
May 28, 2026
gao-sun
approved these changes
May 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #6754.
Context
SMTP relays that authorize by source (IP/VLAN) — and don't take credentials — can't be configured because the connector route calls
cleanDeepon the config before persisting, which strips empty-stringuser/passfromauth. The resulting{ "type": "login" }then fails the connector's zod guard on subsequent reads, the UI loses the fields, and the connector stops working.What changed
packages/connectors/connector-smtp/src/types.ts— madeuserandpassoptional onloginAuthGuard, and added.strict()so the now-looser shape doesn't silently swallow oauth2 payloads in theauthGuardunion (zod would otherwise match login first and drop the oauth2-only keys).packages/connectors/connector-smtp/src/index.ts— type-assert at the nodemailer boundary. Nodemailer'sAuthenticationTypeLogintypesuser/passas required strings, but at runtime treats them as optional (skipping auth when absent). Bypass is documented inline.packages/connectors/connector-smtp/src/index.test.ts— added one test covering the post-cleanDeepshape (auth: { type: 'login' })..changeset/fix-smtp-auth-empty-credentials.md— patch bump for@logto/connector-smtp.Expected result
user/passcontinue to validate and work unchanged (optional widens, doesn't break).cleanDeeppass, and nodemailer skips authentication as it already does for the standalone case shown in bug: empty strings are removed from connector configuration (prevents use of SMTP relay without user/password) #6754.cleanDeepround-trip dropping empty-stringuser/passfrom the UI form is unchanged — it's now harmless rather than fatal.Reviewer notes
.strict()is the minimal disambiguation for the union here. Az.discriminatedUnion('type', …)would be cleaner long-term but would require makingtypenon-optional on all three variants, which is a breaking change to stored configs. Happy to revisit as a follow-up.cleanDeepis left alone — it's shared across all connectors, so changing its empty-string behavior has a much larger blast radius than warranted here.Testing
Unit tests
Checklist
.changeset