Skip to content

Custom from email #13

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Custom from email #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0c7699
allow the 'from' e-mail to be overridden
Mar 1, 2022
bd961ad
changing how email settings are acquired
Mar 1, 2022
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
10 changes: 7 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

Passwordless Authentication for Django with Magic Links.

This package was created with a focus on [ease of setup](#steps-to-impliment), [security](#security) and testing (coverage is currently at 100%). The idea is to use sane defaults to quickly create secure single-use token authentication for Django.
This package was created with a focus on [ease of setup](#configuration), [security](#security) and testing (coverage is currently at 100%). The idea is to use sane defaults to quickly create secure single-use token authentication for Django.

![](example.gif)

Expand Down Expand Up @@ -185,7 +185,7 @@ When overriding this template please ensure the following content is included:
<p>Already have an account? <a href='{% url 'magiclink:login' %}'>Log in here</a></p>
```

There are several forms made avalible in the context on this page depending on what information you want to collect:
There are several forms made available in the context on this page depending on what information you want to collect:
* **SignupFormEmailOnly** - Only includes an `email` field
* **SignupForm** - Includes `name` and `email` fields
* **SignupFormWithUsername** - Includes `username` and `email` fields
Expand Down Expand Up @@ -319,8 +319,12 @@ MAGICLINK_ANTISPAM_FIELD_TIME = 1
MAGICLINK_LOGIN_VERIFY_URL = 'magiclink:login_verify'

# If an email address has been added to the unsubscribe table but is also
# assocaited with a Django user, should a login email be sent
# associated with a Django user, should a login email be sent
MAGICLINK_IGNORE_UNSUBSCRIBE_IF_USER = False

# By default, the sign in e-mail is sent from the DEFAULT_FROM_EMAIL. set
# this if you would like to override the from e-mail.
MAGICLINK_FROM_EMAIL = '[email protected]'
```

## Magic Link cleanup
Expand Down
2 changes: 1 addition & 1 deletion magiclink/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def send(self, request: HttpRequest) -> None:
subject=settings.EMAIL_SUBJECT,
message=plain,
recipient_list=[user.email],
from_email=djsettings.DEFAULT_FROM_EMAIL,
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add a new test to actually check this value is used?
Something like the test_send_email_pass_email_in_unsubscribe test in test_models.py without the unsubscribe bits - https://github.com/pyepye/django-magiclink/blob/master/tests/test_models.py#L114-L135

Copy link
Owner

Choose a reason for hiding this comment

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

I think the djsettings import at the top is now no longer used, can you remove it please?

from_email=settings.FROM_EMAIL,
html_message=html,
)

Expand Down
4 changes: 3 additions & 1 deletion magiclink/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,6 @@

IGNORE_UNSUBSCRIBE_IF_USER = getattr(settings, 'MAGICLINK_IGNORE_UNSUBSCRIBE_IF_USER', False)
if not isinstance(IGNORE_UNSUBSCRIBE_IF_USER, bool):
raise ImproperlyConfigured('"MAGICLINK_IGNORE_UNSUBSCRIBE_IF_USER" must be a boolean')
raise ImproperlyConfigured('"MAGICLINK_IGNORE_UNSUBSCRIBE_IF_USER" must be a boolean')

FROM_EMAIL = getattr(settings, 'MAGICLINK_FROM_SETTINGS', settings.DEFAULT_FROM_EMAIL)
Copy link
Owner

Choose a reason for hiding this comment

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

I think there is a typo here which will cause this functionality not to work. A test will fail with:

AssertionError: assert '[email protected]' == '[email protected]'
Suggested change
FROM_EMAIL = getattr(settings, 'MAGICLINK_FROM_SETTINGS', settings.DEFAULT_FROM_EMAIL)
FROM_EMAIL = getattr(settings, 'MAGICLINK_FROM_EMAIL', settings.DEFAULT_FROM_EMAIL)

Copy link
Owner

@pyepye pyepye Mar 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It could also be good to validate this and default back if it's not valid.

Something like (untested):

from django.core.exceptions import ValidationError
from django.core.validators import validate_email

...

FROM_EMAIL = getattr(settings, 'MAGICLINK_FROM_EMAIL', settings.DEFAULT_FROM_EMAIL)
try:
    validate_email(FROM_EMAIL)
except ValidationError:
    FROM_EMAIL = settings.DEFAULT_FROM_EMAIL
    warning = ('MAGICLINK_FROM_EMAIL is not a valid email address, '
               'using DEFAULT_FROM_EMAIL instead')
    warnings.warn(warning, RuntimeWarning)

Then please include a test to check the email gets defaulted back to DEFAULT_FROM_EMAIL and the warning. There is also an example of a warning test here - https://github.com/pyepye/django-magiclink/blob/master/tests/test_settings.py#L92-L97

2 changes: 2 additions & 0 deletions tests/settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,5 @@
LOGOUT_REDIRECT_URL = 'no_login'
MAGICLINK_LOGIN_SENT_REDIRECT = 'magiclink:login_sent'
MAGICLINK_SIGNUP_LOGIN_REDIRECT = 'no_login'

DEFAULT_FROM_EMAIL = '[email protected]'
13 changes: 13 additions & 0 deletions tests/test_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,3 +371,16 @@ def test_ignore_unsubscribe_if_user_bad_value(settings):
with pytest.raises(ImproperlyConfigured):
from magiclink import settings
reload(settings)


def test_from_settings(settings):
settings.MAGICLINK_FROM_EMAIL = '[email protected]'
from magiclink import settings as mlsettings
reload(mlsettings)
assert mlsettings.FROM_EMAIL == settings.MAGICLINK_FROM_EMAIL


def test_default_email_if_no_magiclink_email(settings):
from magiclink import settings as mlsettings
reload(mlsettings)
assert mlsettings.FROM_EMAIL == '[email protected]'