Skip to content

Add SMTP migration#187

Open
premtsd-code wants to merge 2 commits into
add-policies-migrationfrom
add-smtp-migration
Open

Add SMTP migration#187
premtsd-code wants to merge 2 commits into
add-policies-migrationfrom
add-smtp-migration

Conversation

@premtsd-code
Copy link
Copy Markdown
Contributor

Adds the project SMTP configuration as a new settings resource. Source reads via the typed Project::get() model; destination writes the project doc's smtp map directly (read-then-merge to preserve destination password — source API never exposes it). Stacks on top of #186.

Adds a single SMTP settings resource carrying the project's custom SMTP
configuration. Source reads via the typed Project model
(Project::get()->smtp*); destination writes the smtp attribute on the
project document directly, matching the pattern used by the other 5
settings resources.

Password is intentionally not migrated — the source API only exposes
smtpPassword as an empty string (write-only field). The destination's
existing password is preserved via read-then-merge of the smtp map.
@premtsd-code premtsd-code changed the base branch from main to add-policies-migration May 21, 2026 06:39
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 21, 2026

Greptile Summary

This PR introduces SMTP configuration as a new Settings migration resource. The source reads SMTP fields from Project::get() and the destination applies a read-then-merge strategy to avoid clobbering the destination's stored password (which the source API never exposes).

  • SMTP resource class (Resources/Settings/SMTP.php) is a clean singleton following the exact same constructor / getter / fromArray / jsonSerialize pattern as Services, Labels, and Protocols.
  • Source side (exportSMTP) and destination side (createSMTP) are wired into the existing exportGroupSettings / destination switch, with the type registered in Resource.php and Transfer.php.

Confidence Score: 4/5

The change is otherwise clean but createSMTP unconditionally copies the source's enabled flag to the destination even when no destination password is present, which will leave a destination with enabled=true, a populated username, but no password — causing every outbound email attempt to fail silently with an auth error.

The enabled flag is written without checking whether the destination actually has a stored password. On a fresh destination where the smtp attribute is empty, the merge preserves only the password that doesn't exist, so the resulting document can end up in an inconsistent state where SMTP appears enabled but cannot authenticate. This defect is on the hot path of every migration that uses custom SMTP.

src/Migration/Destinations/Appwrite.php — specifically the createSMTP method's unconditional enabled assignment.

Important Files Changed

Filename Overview
src/Migration/Resources/Settings/SMTP.php New singleton resource class for SMTP config — constructor, getters, fromArray, and jsonSerialize all follow the established Settings resource pattern correctly.
src/Migration/Sources/Appwrite.php Adds exportSMTP() and wires it into exportGroupSettings; outer try/catch uses $e->getCode() without (int) cast or ?: Exception::CODE_INTERNAL fallback, inconsistent with exportGroupAuth's pattern (previously flagged).
src/Migration/Destinations/Appwrite.php createSMTP() correctly does a read-then-merge to preserve the destination's password, but unconditionally copies the enabled flag even when no destination password exists — already flagged in a previous thread.
src/Migration/Resource.php Adds TYPE_SMTP constant and registers it in the all-types array, correctly placed alongside other Settings types.
src/Migration/Transfer.php Adds TYPE_SMTP to both GROUP_SETTINGS_RESOURCES and the supported-types list, consistent with how other settings resources are registered.

Reviews (2): Last reviewed commit: "chore: fix import ordering (Pint)" | Re-trigger Greptile

$project = $this->dbForPlatform->getDocument('projects', $this->projectId);
$smtp = $project->getAttribute('smtp', []);

$smtp['enabled'] = $resource->getEnabled();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 SMTP enabled flag copied without a destination password

createSMTP sets $smtp['enabled'] = $resource->getEnabled() unconditionally, but it intentionally skips writing $smtp['password']. On a fresh destination where $project->getAttribute('smtp', []) returns [], the resulting document will have enabled = true alongside a populated username but no password key. Any email delivery attempt will fail with an SMTP AUTH error, with no warning to the user.

The safest fix is to suppress the enabled flag when the destination has no stored password: if empty($smtp['password']), force $smtp['enabled'] = false regardless of the source value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant