Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate ext::auth::SMTPConfig -> cfg::EmailProvider #7942

Merged
merged 11 commits into from
Nov 21, 2024
Merged

Migrate ext::auth::SMTPConfig -> cfg::EmailProvider #7942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
460971f
Migrate `SMTPConfig` -> `SMTPProviderConfig`
scotttrinh Oct 24, 2024
1c07815
Move EmailProvider into main cfg
scotttrinh Oct 30, 2024
623c2d1
Add server `smtp` module and switch auth
scotttrinh Nov 5, 2024
57fe0bc
Fix refactor miss
scotttrinh Nov 6, 2024
df5a071
Remove auth SMTP config from other tests
scotttrinh Nov 6, 2024
3121c31
cqa
scotttrinh Nov 7, 2024
381d3e9
Rewrite old smtp config commands during dump restore
msullivan Nov 8, 2024
8c06f01
Microoptimization in the email provider lookup loop
msullivan Nov 8, 2024
08c444a
Add EDGEDB_MAGIC_SMTP_CONFIG for sidechannel SMTP configs
msullivan Nov 8, 2024
3f8e89d
Convert frozenset of email_providers to list
scotttrinh Nov 12, 2024
a7303bc
Connect to SMTP server before sending message
scotttrinh Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions edb/lib/cfg.edgeql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,71 @@ CREATE TYPE cfg::Auth EXTENDING cfg::ConfigObject {
};
};

CREATE SCALAR TYPE cfg::SMTPSecurity EXTENDING enum<
PlainText,
TLS,
STARTTLS,
STARTTLSOrPlainText,
>;

CREATE ABSTRACT TYPE cfg::EmailProviderConfig EXTENDING cfg::ConfigObject {
CREATE REQUIRED PROPERTY name -> std::str {
CREATE CONSTRAINT std::exclusive;
CREATE ANNOTATION std::description :=
"The name of the email provider.";
};
};

CREATE TYPE cfg::SMTPProviderConfig EXTENDING cfg::EmailProviderConfig {
CREATE PROPERTY sender -> std::str {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should make sender required, I'm not sure why we didn't have that before.

CREATE ANNOTATION std::description :=
"\"From\" address of system emails sent for e.g. \
password reset, etc.";
};
CREATE PROPERTY host -> std::str {
CREATE ANNOTATION std::description :=
"Host of SMTP server to use for sending emails. \
If not set, \"localhost\" will be used.";
};
CREATE PROPERTY port -> std::int32 {
CREATE ANNOTATION std::description :=
"Port of SMTP server to use for sending emails. \
If not set, common defaults will be used depending on security: \
465 for TLS, 587 for STARTTLS, 25 otherwise.";
};
CREATE PROPERTY username -> std::str {
CREATE ANNOTATION std::description :=
"Username to login as after connected to SMTP server.";
};
CREATE PROPERTY password -> std::str {
SET secret := true;
CREATE ANNOTATION std::description :=
"Password for login after connected to SMTP server.";
};
CREATE REQUIRED PROPERTY security -> cfg::SMTPSecurity {
SET default := cfg::SMTPSecurity.STARTTLSOrPlainText;
CREATE ANNOTATION std::description :=
"Security mode of the connection to SMTP server. \
By default, initiate a STARTTLS upgrade if supported by the \
server, or fallback to PlainText.";
};
CREATE REQUIRED PROPERTY validate_certs -> std::bool {
SET default := true;
CREATE ANNOTATION std::description :=
"Determines if SMTP server certificates are validated.";
};
CREATE REQUIRED PROPERTY timeout_per_email -> std::duration {
SET default := <std::duration>'60 seconds';
CREATE ANNOTATION std::description :=
"Maximum time to send an email, including retry attempts.";
};
CREATE REQUIRED PROPERTY timeout_per_attempt -> std::duration {
SET default := <std::duration>'15 seconds';
CREATE ANNOTATION std::description :=
"Maximum time for each SMTP request.";
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Max concurrency config here?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think so. Though this would become a per-provider config, instead of a server/tenant-wide config. It should be fine for now, but in the long run we may want to have both: per-provider config makes sure of not overwhelming the provider, while the server/tenant-wide config makes sure e.g. the local number of FDs is not exceeded.

};

CREATE ABSTRACT TYPE cfg::AbstractConfig extending cfg::ConfigObject;

CREATE ABSTRACT TYPE cfg::ExtensionConfig EXTENDING cfg::ConfigObject {
Expand Down Expand Up @@ -158,6 +223,16 @@ ALTER TYPE cfg::AbstractConfig {
CREATE ANNOTATION cfg::system := 'true';
};

CREATE MULTI LINK email_providers -> cfg::EmailProviderConfig {
CREATE ANNOTATION std::description :=
'The list of email providers that can be used to send emails.';
};

CREATE PROPERTY current_email_provider_name -> std::str {
CREATE ANNOTATION std::description :=
'The name of the current email provider.';
};

CREATE PROPERTY allow_dml_in_functions -> std::bool {
SET default := false;
CREATE ANNOTATION cfg::affects_compilation := 'true';
Expand Down
52 changes: 0 additions & 52 deletions edb/lib/ext/auth.edgeql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,58 +469,6 @@ CREATE EXTENSION PACKAGE auth VERSION '1.0' {
};
};

create scalar type ext::auth::SMTPSecurity extending enum<PlainText, TLS, STARTTLS, STARTTLSOrPlainText>;

create type ext::auth::SMTPConfig extending cfg::ExtensionConfig {
create property sender: std::str {
create annotation std::description :=
"\"From\" address of system emails sent for e.g. \
password reset, etc.";
};
create property host: std::str {
create annotation std::description :=
"Host of SMTP server to use for sending emails. \
If not set, \"localhost\" will be used.";
};
create property port: std::int32 {
create annotation std::description :=
"Port of SMTP server to use for sending emails. \
If not set, common defaults will be used depending on security: \
465 for TLS, 587 for STARTTLS, 25 otherwise.";
};
create property username: std::str {
create annotation std::description :=
"Username to login as after connected to SMTP server.";
};
create property password: std::str {
set secret := true;
create annotation std::description :=
"Password for login after connected to SMTP server.";
};
create required property security: ext::auth::SMTPSecurity {
set default := ext::auth::SMTPSecurity.STARTTLSOrPlainText;
create annotation std::description :=
"Security mode of the connection to SMTP server. \
By default, initiate a STARTTLS upgrade if supported by the \
server, or fallback to PlainText.";
};
create required property validate_certs: std::bool {
set default := true;
create annotation std::description :=
"Determines if SMTP server certificates are validated.";
};
create required property timeout_per_email: std::duration {
set default := <std::duration>'60 seconds';
create annotation std::description :=
"Maximum time to send an email, including retry attempts.";
};
create required property timeout_per_attempt: std::duration {
set default := <std::duration>'15 seconds';
create annotation std::description :=
"Maximum time for each SMTP request.";
};
};

create function ext::auth::signing_key_exists() -> std::bool {
using (
select exists cfg::Config.extensions[is ext::auth::AuthConfig]
Expand Down
82 changes: 80 additions & 2 deletions edb/server/compiler/compiler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,62 @@ def describe_database_dump(
blocks=descriptors,
)

def _reprocess_restore_config(
self,
stmts: list[qlast.Base],
) -> list[qlast.Base]:
'''Do any rewrites to the restore script needed.

This is intended to patch over certain backwards incompatible
changes to config. We try not to do that too much, but when we
do, dumps still need to work.
'''

new_stmts = []
smtp_config = {}

for stmt in stmts:
# ext::auth::SMTPConfig got removed and moved into a cfg
# object, so intercept those and rewrite them.
if (
isinstance(stmt, qlast.ConfigSet)
and stmt.name.module == 'ext::auth::SMTPConfig'
):
smtp_config[stmt.name.name] = stmt.expr
else:
new_stmts.append(stmt)

if smtp_config:
# Do the rewrite of SMTPConfig
smtp_config['name'] = qlast.Constant.string('_default')

new_stmts.append(
qlast.ConfigInsert(
scope=qltypes.ConfigScope.DATABASE,
name=qlast.ObjectRef(
module='cfg', name='SMTPProviderConfig'
),
shape=[
qlast.ShapeElement(
expr=qlast.Path(steps=[qlast.Ptr(name=name)]),
compexpr=expr,
)
for name, expr in smtp_config.items()
],
)
)
new_stmts.append(
qlast.ConfigSet(
scope=qltypes.ConfigScope.DATABASE,
name=qlast.ObjectRef(
name='current_email_provider_name'
),
expr=qlast.Constant.string('_default'),
)
)

return new_stmts

def describe_database_restore(
self,
user_schema_pickle: bytes,
Expand Down Expand Up @@ -984,7 +1040,11 @@ def describe_database_restore(

# The state serializer generated below is somehow inappropriate,
# so it's simply ignored here and the I/O process will do it on its own
units = compile(ctx=ctx, source=ddl_source).units
statements = edgeql.parse_block(ddl_source)
statements = self._reprocess_restore_config(statements)
units = _try_compile_ast(
ctx=ctx, source=ddl_source, statements=statements
).units

_check_force_database_error(ctx, scope='restore')

Expand Down Expand Up @@ -2363,8 +2423,26 @@ def _try_compile(
if text.startswith(sentinel):
time.sleep(float(text[len(sentinel):text.index("\n")]))

default_cardinality = enums.Cardinality.NO_RESULT
statements = edgeql.parse_block(source)
return _try_compile_ast(statements=statements, source=source, ctx=ctx)


def _try_compile_ast(
*,
ctx: CompileContext,
statements: list[qlast.Base],
source: edgeql.Source,
) -> dbstate.QueryUnitGroup:
if _get_config_val(ctx, '__internal_testmode'):
# This is a bad but simple way to emulate a slow compilation for tests.
# Ideally, we should have a testmode function that is hooked to sleep
# as `simple_special_case`, or wait for a notification from the test.
sentinel = "# EDGEDB_TEST_COMPILER_SLEEP = "
text = source.text()
if text.startswith(sentinel):
time.sleep(float(text[len(sentinel):text.index("\n")]))

default_cardinality = enums.Cardinality.NO_RESULT
statements_len = len(statements)

if not len(statements): # pragma: no cover
Expand Down
28 changes: 8 additions & 20 deletions edb/server/protocol/auth_ext/email.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
import random

from typing import Any, Coroutine
from edb.server import tenant
from edb.server import tenant, smtp

from . import util, ui, smtp
from . import util, ui


async def send_password_reset_email(
Expand All @@ -15,7 +15,6 @@ async def send_password_reset_email(
reset_url: str,
test_mode: bool,
) -> None:
from_addr = util.get_config(db, "ext::auth::SMTPConfig::sender")
app_details_config = util.get_app_details_config(db)
if app_details_config is None:
email_args = {}
Expand All @@ -27,16 +26,13 @@ async def send_password_reset_email(
brand_color=app_details_config.brand_color,
)
msg = ui.render_password_reset_email(
from_addr=from_addr,
to_addr=to_addr,
reset_url=reset_url,
**email_args,
)
coro = smtp.send_email(
db,
smtp_provider = smtp.SMTP(db)
coro = smtp_provider.send(
msg,
sender=from_addr,
recipients=to_addr,
test_mode=test_mode,
)
await _protected_send(coro, tenant)
Expand All @@ -51,7 +47,6 @@ async def send_verification_email(
provider: str,
test_mode: bool,
) -> None:
from_addr = util.get_config(db, "ext::auth::SMTPConfig::sender")
app_details_config = util.get_app_details_config(db)
verification_token_params = urllib.parse.urlencode(
{
Expand All @@ -71,16 +66,13 @@ async def send_verification_email(
brand_color=app_details_config.brand_color,
)
msg = ui.render_verification_email(
from_addr=from_addr,
to_addr=to_addr,
verify_url=verify_url,
**email_args,
)
coro = smtp.send_email(
db,
smtp_provider = smtp.SMTP(db)
coro = smtp_provider.send(
msg,
sender=from_addr,
recipients=to_addr,
test_mode=test_mode,
)
await _protected_send(coro, tenant)
Expand All @@ -93,7 +85,6 @@ async def send_magic_link_email(
link: str,
test_mode: bool,
) -> None:
from_addr = util.get_config(db, "ext::auth::SMTPConfig::sender")
app_details_config = util.get_app_details_config(db)
if app_details_config is None:
email_args = {}
Expand All @@ -105,16 +96,13 @@ async def send_magic_link_email(
brand_color=app_details_config.brand_color,
)
msg = ui.render_magic_link_email(
from_addr=from_addr,
to_addr=to_addr,
link=link,
**email_args,
)
coro = smtp.send_email(
db,
smtp_provider = smtp.SMTP(db)
coro = smtp_provider.send(
msg,
sender=from_addr,
recipients=to_addr,
test_mode=test_mode,
)
await _protected_send(coro, tenant)
Expand Down
Loading