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.

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

Passwordless Authentication #645

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions api/desecapi/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ class EmailPasswordPayloadAuthentication(BaseAuthentication):
def authenticate(self, request):
serializer = EmailPasswordSerializer(data=request.data)
serializer.is_valid(raise_exception=True)
if "password" not in serializer.data:
return None
return self.authenticate_credentials(
serializer.data["email"], serializer.data["password"], request
)
Expand Down
10 changes: 10 additions & 0 deletions api/desecapi/models/authenticated_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from .domains import Domain
from .mfa import TOTPFactor
from .tokens import Token


class AuthenticatedAction(models.Model):
Expand Down Expand Up @@ -112,6 +113,15 @@ def _state_fields(self):
return super()._state_fields + [str(self.user.id)]


class AuthenticatedCreateLoginTokenAction(AuthenticatedBasicUserAction):
"""
Action to create a login token.
"""

def _act(self):
return Token.create_login_token(self.user)


class AuthenticatedEmailUserAction(AuthenticatedBasicUserAction):
"""
Abstract AuthenticatedAction involving a user instance with unmodified email address.
Expand Down
13 changes: 13 additions & 0 deletions api/desecapi/models/tokens.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
from django_prometheus.models import ExportModelOperationsMixin
from netfields import CidrAddressField, NetManager

from .users import User


class Token(ExportModelOperationsMixin("Token"), rest_framework.authtoken.models.Token):
@staticmethod
Expand Down Expand Up @@ -100,6 +102,17 @@ def delete(self):
self.tokendomainpolicy_set.filter(domain__isnull=True).delete()
return super().delete()

@classmethod
def create_login_token(cls, user: User):
token = cls.objects.create(
user=user,
perm_manage_tokens=True,
max_age=timedelta(days=7),
max_unused_period=timedelta(hours=1),
mfa=False,
)
return token


@pgtrigger.register(
# Ensure that token_user is consistent with token
Expand Down
1 change: 1 addition & 0 deletions api/desecapi/models/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ def send_email(
"delete-account": fast_lane,
"domain-dyndns": fast_lane,
"renew-domain": immediate_lane,
"create-login-token": immediate_lane,
}
if reason not in lanes:
raise ValueError(
Expand Down
1 change: 1 addition & 0 deletions api/desecapi/serializers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
AuthenticatedChangeEmailUserActionSerializer,
AuthenticatedChangeOutreachPreferenceUserActionSerializer,
AuthenticatedConfirmAccountUserActionSerializer,
AuthenticatedCreateLoginTokenActionSerializer,
AuthenticatedCreateTOTPFactorUserActionSerializer,
AuthenticatedDeleteUserActionSerializer,
AuthenticatedRenewDomainBasicUserActionSerializer,
Expand Down
11 changes: 11 additions & 0 deletions api/desecapi/serializers/authenticated_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,17 @@ class Meta(AuthenticatedBasicUserActionSerializer.Meta):
model = models.AuthenticatedDeleteUserAction


class AuthenticatedCreateLoginTokenActionSerializer(
AuthenticatedBasicUserActionSerializer
):
reason = "create-login-token"
validity_period = timedelta(minutes=10)

class Meta(AuthenticatedBasicUserActionSerializer.Meta):
model = models.AuthenticatedCreateLoginTokenAction
fields = AuthenticatedBasicUserActionSerializer.Meta.fields


class AuthenticatedDomainBasicUserActionSerializer(
AuthenticatedBasicUserActionSerializer
):
Expand Down
2 changes: 1 addition & 1 deletion api/desecapi/serializers/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class EmailSerializer(serializers.Serializer):


class EmailPasswordSerializer(EmailSerializer):
password = serializers.CharField()
password = serializers.CharField(required=False)


class ChangeEmailSerializer(serializers.Serializer):
Expand Down
12 changes: 12 additions & 0 deletions api/desecapi/templates/emails/create-login-token/content.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{% extends "emails/content.txt" %}
{% block content %}{% load action_extras %}Hi,

someone request a login link for your account. To log in, please use the following link (valid for {% action_link_expiration_hours action_serializer %} hours):
Copy link
Member

Choose a reason for hiding this comment

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

I think we should at least have the requesting IP address and perhaps the user agent here, so users can associate the request with themselves. (Geo-mapping seems not very reliable, so I'd vote for not having that.)


{% action_link action_serializer %}

If you did not request this, please contact [email protected].

Stay secure,
The deSEC Team
{% endblock %}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[deSEC] Login information
61 changes: 52 additions & 9 deletions api/desecapi/tests/test_user_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,11 @@ def register(self, email, password, captcha=None, **kwargs):
reverse("v1:register"), {"email": email, "password": password, **kwargs}
)

def login_user(self, email, password):
return self.post(
reverse("v1:login"),
{
"email": email,
"password": password,
},
)
def login_user(self, email, password=None):
payload = {"email": email}
if password is not None:
payload["password"] = password
return self.post(reverse("v1:login"), payload)

def logout(self, token):
return self.post(reverse("v1:logout"), HTTP_AUTHORIZATION=f"Token {token}")
Expand Down Expand Up @@ -127,7 +124,7 @@ def register_user(self, email=None, password=None, late_captcha=False, **kwargs)
self.client.register(email, password, captcha, **kwargs),
)

def login_user(self, email, password):
def login_user(self, email, password=None):
return self.client.login_user(email, password)

def logout(self, token):
Expand Down Expand Up @@ -550,6 +547,52 @@ def _test_delete_account(self, email, password):
self.assertUserDoesNotExist(email)


class PasswordlessUserTestCase(UserManagementTestCase):

def assertLoginSuccessResponse(self, response):
return self.assertContains(
response=response, text="instructions", status_code=status.HTTP_202_ACCEPTED
)

def assertCreateLoginTokenVerificationEmail(self, reset=True):
return self.assertEmailSent(
subject_contains="Login information",
body_contains="login link for your account",
recipient=[self.email],
reset=reset,
pattern=r"following link[^:]*:\s+([^\s]*)",
)

def assertCreateLoginTokenVerificationSuccessResponse(self, response):
return self.assertContains(
response=response,
text=f"token",
status_code=status.HTTP_200_OK,
)

def setUp(self):
self.email, _, _ = self.register_user(self.random_username())
confirmation_link = self.assertRegistrationEmail(self.email)
self.assertConfirmationLinkRedirect(confirmation_link)
response = self.client.verify(confirmation_link)
self.assertRegistrationVerificationSuccessResponse(response)
self.assertTrue(User.objects.get(email=self.email).is_active)
self.assertEmailSent(reset=True)

def test_login_successful(self):
response = self.login_user(self.email, None)
self.assertLoginSuccessResponse(response)
link = self.assertCreateLoginTokenVerificationEmail()
self.assertCreateLoginTokenVerificationSuccessResponse(
self.client.verify(link)
)

def test_login_unsuccessful(self):
response = self.login_user("[email protected]", None)
self.assertLoginSuccessResponse(response) # no disclosure that this account does not exist!
self.assertNoEmailSent()


class UserLifeCycleTestCase(UserManagementTestCase):
def test_life_cycle(self):
self.email, self.password = self._test_registration(
Expand Down
5 changes: 5 additions & 0 deletions api/desecapi/urls/version_1.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@
views.AuthenticatedRenewDomainBasicUserActionView.as_view(),
name="confirm-renew-domain",
),
path(
"v/create-login-token/<code>/",
views.AuthenticatedCreateLoginTokenActionView.as_view(),
name="confirm-create-login-token",
),
# CAPTCHA
path("captcha/", views.CaptchaView.as_view(), name="captcha"),
]
Expand Down
1 change: 1 addition & 0 deletions api/desecapi/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
AuthenticatedChangeEmailUserActionView,
AuthenticatedChangeOutreachPreferenceUserActionView,
AuthenticatedConfirmAccountUserActionView,
AuthenticatedCreateLoginTokenActionView,
AuthenticatedCreateTOTPFactorUserActionView,
AuthenticatedDeleteUserActionView,
AuthenticatedRenewDomainBasicUserActionView,
Expand Down
11 changes: 11 additions & 0 deletions api/desecapi/views/authenticated_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,17 @@ def post(self, request, *args, **kwargs):
return Response(serializer.data)


class AuthenticatedCreateLoginTokenActionView(AuthenticatedActionView):
html_url = "/confirm/create-login-token/{code}/"
serializer_class = serializers.AuthenticatedCreateLoginTokenActionSerializer

def post(self, request, *args, **kwargs):
super().post(request, *args, **kwargs)
token = self.authenticated_action.act()
serializer = serializers.TokenSerializer(token, include_plain=True)
return Response(serializer.data)


class AuthenticatedResetPasswordUserActionView(AuthenticatedActionView):
html_url = "/confirm/reset-password/{code}/"
serializer_class = serializers.AuthenticatedResetPasswordUserActionSerializer
Expand Down
37 changes: 22 additions & 15 deletions api/desecapi/views/users.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from datetime import timedelta

from django.conf import settings
from django.contrib.auth import user_logged_in
from rest_framework import generics, mixins, status
Expand Down Expand Up @@ -93,23 +91,32 @@ def post(self, request, *args, **kwargs):

class AccountLoginView(generics.GenericAPIView):
authentication_classes = (authentication.EmailPasswordPayloadAuthentication,)
permission_classes = (IsAuthenticated,)
serializer_class = serializers.TokenSerializer
throttle_scope = "account_management_passive"

def post(self, request, *args, **kwargs):
user = self.request.user
token = Token.objects.create(
user=user,
perm_manage_tokens=True,
max_age=timedelta(days=7),
max_unused_period=timedelta(hours=1),
mfa=False,
)
user_logged_in.send(sender=user.__class__, request=self.request, user=user)

data = self.get_serializer(token, include_plain=True).data
return Response(data)
if request.user and request.user.is_authenticated:
# password was provided
user = self.request.user
data = self.get_serializer(
Token.create_login_token(user), include_plain=True
).data
user_logged_in.send(sender=user.__class__, request=request, user=user)
return Response(data)
else:
# password was not provided
message = "Login instructions have been sent to the email address associated with your user account."
response = Response(data={"detail": message}, status=status.HTTP_202_ACCEPTED)

# TODO how to determine user ID properly? Use a serializer?
try:
user = User.objects.get(email=request.data.get('email'))
except User.DoesNotExist:
pass
else:
serializers.AuthenticatedCreateLoginTokenActionSerializer.build_and_save(user=user)
Copy link
Member

Choose a reason for hiding this comment

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

This allows spamming a user with log-in emails.

To avoid such scenarios with sign-up emails, we store user pending registrations, and clean them up after 24h unless activated. During that time, no other registration email can be sent. -- Is there a way to deal with this here? (One way could be to only allow passwordless login if no password is set. This would need UI to switch between password and passwordless login.)

Password reset seems to have the same problem, though.


return response


class AccountLogoutView(APIView, mixins.DestroyModelMixin):
Expand Down
24 changes: 24 additions & 0 deletions www/webapp/src/components/CreateLoginTokenActionHandler.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<script>
import GenericActionHandler from "./GenericActionHandler"
import {HTTP} from "../utils";

export default {
name: 'CreateLoginTokenActionHandler',
extends: GenericActionHandler,
data: () => ({
auto_submit: true,
}),
watch: {
success(value) {
if(value) {
HTTP.defaults.headers.common.Authorization = `Token ${value.token}`;
this.$store.commit('login', value);
if (this.useSessionStorage) {
sessionStorage.setItem('token', JSON.stringify(value));
}
this.$router.replace({ name: 'domains' });
}
}
}
};
</script>
18 changes: 8 additions & 10 deletions www/webapp/src/views/Login.vue
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,11 @@
/>
<v-text-field
v-model="password"
label="Password"
label="Password (leave empty to login via email)"
:append-icon="hide_password ? 'mdi-eye' : 'mdi-eye-off'"
:type="hide_password ? 'password' : 'text'"
outline
required
:disabled="working"
:rules="password_rules"
tabindex="2"
@click:append="() => (hide_password = !hide_password)"
/>
Expand Down Expand Up @@ -104,9 +102,6 @@ export default {
useSessionStorage: false,
email_rules: [v => !!v || 'Please enter the email address associated with your account'],
email_errors: [],
password_rules: [
v => !!v || 'Enter your password to log in',
],
hide_password: true,
errors: [],
}),
Expand All @@ -115,10 +110,13 @@ export default {
this.working = true;
this.errors.splice(0, this.errors.length);
try {
const response = await HTTP.post('auth/login/', {
email: this.email,
password: this.password,
});
let payload = {
email: this.email
};
if (this.password) {
payload['password'] = this.password;
}
const response = await HTTP.post('auth/login/', payload);
HTTP.defaults.headers.common.Authorization = `Token ${response.data.token}`;
this.$store.commit('login', response.data);
if (this.useSessionStorage) {
Expand Down