Skip to content

Commit 4243930

Browse files
committed
Send admins a notification when a user registers
- Add log line regarding APP_URL in feature flag parser
1 parent 4f4dcfe commit 4243930

File tree

5 files changed

+92
-15
lines changed

5 files changed

+92
-15
lines changed

pydatalab/src/pydatalab/feature_flags.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,3 +164,8 @@ def _check_secret_and_warn(secret: str, error: str, environ: bool = False) -> bo
164164
LOGGER.warning(
165165
"No deployment metadata provided, please set `CONFIG.DEPLOYMENT_METADATA` to allow the UI to provide helpful information to users"
166166
)
167+
168+
if not CONFIG.APP_URL:
169+
LOGGER.warning(
170+
"Canonical URL for deployment is not set, please set `CONFIG.APP_URL` otherwise some features (redirects, email notifications) may not work correctly."
171+
)

pydatalab/src/pydatalab/routes/v0_1/auth.py

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222

2323
from pydatalab.config import CONFIG
2424
from pydatalab.errors import UserRegistrationForbidden
25-
from pydatalab.logger import LOGGER, logged_route
25+
from pydatalab.feature_flags import FEATURE_FLAGS
26+
from pydatalab.logger import LOGGER
2627
from pydatalab.login import get_by_id
2728
from pydatalab.models.people import AccountStatus, Identity, IdentityType, Person
2829
from pydatalab.mongo import flask_mongo, insert_pydantic_model_fork_safe
@@ -202,7 +203,6 @@ def set_applocal_session():
202203
orcid = LocalProxy(lambda: g.flask_dance_orcid)
203204

204205

205-
@logged_route
206206
def wrapped_login_user(*args, **kwargs):
207207
login_user(*args, **kwargs)
208208

@@ -273,7 +273,6 @@ def _check_email_domain(email: str, allow_list: list[str] | None) -> bool:
273273
return True
274274

275275

276-
@logged_route
277276
def find_user_with_identity(
278277
identifier: str,
279278
identity_type: str | IdentityType,
@@ -332,7 +331,6 @@ def find_create_or_modify_user(
332331
333332
"""
334333

335-
@logged_route
336334
def attach_identity_to_user(
337335
user_id: str,
338336
identity: Identity,
@@ -416,7 +414,13 @@ def attach_identity_to_user(
416414
user_model = get_by_id(str(user.immutable_id))
417415
if user is None:
418416
raise RuntimeError("Failed to insert user into database")
417+
419418
wrapped_login_user(user_model)
419+
# Send email notification to admins
420+
_send_admin_email_notification(user)
421+
422+
if user is not None and user.account_status == AccountStatus.DEACTIVATED:
423+
_send_admin_email_notification(user)
420424

421425
# Log the user into the session with this identity
422426
if user is not None:
@@ -477,6 +481,56 @@ def _check_user_registration_allowed(email: str) -> None:
477481
)
478482

479483

484+
def _send_admin_email_notification(user: Person) -> None:
485+
"""Sends an email notification to the admin email address when an unverified user
486+
attempts to register an account.
487+
488+
"""
489+
if not FEATURE_FLAGS.email_notifications:
490+
LOGGER.info("Email notifications are disabled; not sending unverified user notification.")
491+
return
492+
493+
admins = flask_mongo.db.users.aggregate(
494+
[
495+
{
496+
"$lookup": {
497+
"from": "roles",
498+
"localField": "_id",
499+
"foreignField": "_id",
500+
"as": "role",
501+
}
502+
},
503+
{"$unwind": "$role"},
504+
{"$match": {"role.role": "admin"}},
505+
]
506+
)
507+
508+
admin_emails = [a["contact_email"] for a in admins if a.get("contact_email")]
509+
510+
# Get contact emails of admin users via lookup in the roles and users collections
511+
if user.account_status == AccountStatus.UNVERIFIED:
512+
subject = "User Registration Notification"
513+
subject += f": {CONFIG.APP_URL}" if CONFIG.APP_URL else ""
514+
body = (
515+
f"A new user with display name {user.display_name} attempted to register an account on {CONFIG.APP_URL}.\n\n"
516+
"Please review the registration in your admin panel and verify the user if appropriate."
517+
)
518+
519+
if user.account_status == AccountStatus.DEACTIVATED:
520+
subject = "Deactivated User Login Attempt Notification"
521+
subject += f": {CONFIG.APP_URL}" if CONFIG.APP_URL else ""
522+
body = (
523+
f"A deactivated user with display name {user.display_name} attempted to log in to the datalab instance at {CONFIG.APP_URL}.\n\n"
524+
"No action is required unless you wish to reactivate this user."
525+
)
526+
527+
try:
528+
for email in admin_emails:
529+
send_mail(email, subject, body)
530+
except Exception as exc:
531+
LOGGER.warning("Failed to send unverified user notification email: %s", exc)
532+
533+
480534
def _send_magic_link_email(
481535
email: str, token: str, referrer: str | None, purpose: str = "authorize"
482536
) -> None:

pydatalab/src/pydatalab/send_email.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def send_mail(recipient: str, subject: str, body: str):
2121
body (str): The body of the email.
2222
2323
"""
24-
LOGGER.debug("Sending email to %s", recipient)
24+
LOGGER.debug("Sending email to %s with subject %s", recipient, subject)
2525

2626
sender = None
2727
if CONFIG.EMAIL_AUTH_SMTP_SETTINGS is not None:

pydatalab/tests/server/conftest.py

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,14 @@ def app_config(tmp_path_factory, secret_key):
6666
"EMAIL_AUTH_SMTP_SETTINGS": {
6767
"MAIL_SERVER": "smtp.example.com",
6868
"MAIL_PORT": 587,
69-
"MAIL_USERNAME": "",
70-
"MAIL_PASSWORD": "",
69+
"MAIL_USERNAME": "test",
7170
"MAIL_USE_TLS": True,
7271
"MAIL_DEFAULT_SENDER": "[email protected]",
7372
},
7473
"EMAIL_DOMAIN_ALLOW_LIST": ["example.org", "ml-evs.science"],
7574
"MAIL_DEBUG": True,
7675
"MAIL_SUPPRESS_SEND": True,
76+
"MAIL_PASSWORD": "test",
7777
# Set to 10 MB to check that larger files fail; this should be larger than all of our example files.
7878
# Elsewhere, we can generate an artificial large file to check that it fails.
7979
"MAX_CONTENT_LENGTH": 10 * 1000**2,
@@ -240,13 +240,20 @@ def deactivated_user_id():
240240
yield ObjectId(24 * "3")
241241

242242

243-
def insert_user(id, api_key, role, real_mongo_client, status: AccountStatus = AccountStatus.ACTIVE):
243+
def insert_user(
244+
id,
245+
api_key,
246+
role,
247+
real_mongo_client,
248+
display_name: str = "Test Admin",
249+
status: AccountStatus = AccountStatus.ACTIVE,
250+
):
244251
from hashlib import sha512
245252

246253
demo_user = {
247254
"_id": id,
248255
"contact_email": "[email protected]",
249-
"display_name": "Test Admin",
256+
"display_name": display_name,
250257
"account_status": status,
251258
}
252259
real_mongo_client.get_database(TEST_DATABASE_NAME).users.insert_one(demo_user)
@@ -272,21 +279,29 @@ def insert_demo_users(
272279
unverified_user_api_key,
273280
real_mongo_client,
274281
):
275-
insert_user(user_id, user_api_key, "user", real_mongo_client)
276-
insert_user(another_user_id, another_user_api_key, "user", real_mongo_client)
277-
insert_user(admin_user_id, admin_api_key, "admin", real_mongo_client)
282+
insert_user(user_id, user_api_key, "user", real_mongo_client, display_name="Test User")
283+
insert_user(
284+
another_user_id,
285+
another_user_api_key,
286+
"user",
287+
real_mongo_client,
288+
display_name="Another User",
289+
)
290+
insert_user(admin_user_id, admin_api_key, "admin", real_mongo_client, display_name="Test Admin")
278291
insert_user(
279292
deactivated_user_id,
280293
deactivated_user_api_key,
281294
"user",
282295
real_mongo_client,
296+
display_name="Deactivated User",
283297
status=AccountStatus.DEACTIVATED,
284298
)
285299
insert_user(
286300
unverified_user_id,
287301
unverified_user_api_key,
288302
"user",
289303
real_mongo_client,
304+
display_name="Unverified User",
290305
status=AccountStatus.UNVERIFIED,
291306
)
292307

pydatalab/tests/server/test_auth.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,12 @@ def test_magic_link_account_creation(unauthenticated_client, app, database):
2626
doc = database.magic_links.find_one()
2727
assert "jwt" in doc
2828

29-
response = unauthenticated_client.get(f"/login/email?token={doc['jwt']}")
30-
assert response.status_code == 307
31-
assert database.users.find_one({"contact_email": "[email protected]"})
29+
with app.extensions["mail"].record_messages() as outbox:
30+
response = unauthenticated_client.get(f"/login/email?token={doc['jwt']}")
31+
assert response.status_code == 307
32+
assert database.users.find_one({"contact_email": "[email protected]"})
33+
34+
assert len(outbox) == 1 # Should be a notification to admins
3235

3336

3437
def test_magic_links_expected_failures(unauthenticated_client, app):

0 commit comments

Comments
 (0)