Skip to content

Commit

Permalink
Merge branch 'force-change-pw' into 'main'
Browse files Browse the repository at this point in the history
Add user option to force password change on next login

See merge request reportcreator/reportcreator!782
  • Loading branch information
MWedl committed Nov 27, 2024
2 parents 61a302c + 13672ee commit b100761
Show file tree
Hide file tree
Showing 14 changed files with 261 additions and 63 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* Plugin: webhooks at certain events
* Plugin: automatically assign project numbers
* Run periodic tasks in background
* Add user option to force change password on next login
* UI: fix line break in logo text on Firefox


Expand Down
41 changes: 39 additions & 2 deletions api/src/reportcreator_api/tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def assert_login(self, user, password=None, success=True, status='success'):
self.assert_api_access(False)
return res

def assert_mfa_login(self, mfa_method, data=None, user=None, success=True):
def assert_mfa_login(self, mfa_method, data=None, user=None, success=True, status='success'):
self.assert_login(user=user or self.user_mfa, status='mfa-required')
if mfa_method.method_type == MFAMethodType.BACKUP:
res = self.client.post(reverse('auth-login-code'), data={
Expand All @@ -62,12 +62,23 @@ def assert_mfa_login(self, mfa_method, data=None, user=None, success=True):

if success:
assert res.status_code == 200
self.assert_api_access(True)
assert res.data['status'] == status
if status == 'success':
self.assert_api_access(True)
else:
assert res.status_code in [400, 403]
self.assert_api_access(False)
return res

def assert_change_password(self, password=None, success=True):
res = self.client.post(reverse('auth-change-password'), data={'password': password or get_random_string(32)})
if success:
assert res.status_code == 200
assert res.data['status'] == 'success'
else:
assert res.status_code in [400, 403]
return res

def test_login(self):
self.assert_login(user=self.user)
self.assert_api_access(True)
Expand Down Expand Up @@ -116,6 +127,32 @@ def test_login_mfa_method_of_other_user(self):
other_mfa = MFAMethod.objects.create_totp(user=other_user)
self.assert_mfa_login(user=self.user_mfa, mfa_method=other_mfa, success=False)

def test_must_change_password(self):
self.user.must_change_password = True
self.user.save()

self.assert_login(self.user, status='password-change-required')
self.assert_api_access(False)
self.assert_change_password(password=get_random_string(3), success=False)
self.assert_api_access(False)
self.assert_change_password()
self.assert_api_access(True)

self.user.refresh_from_db()
assert not self.user.must_change_password

def test_must_change_password_mfa(self):
self.user_mfa.must_change_password = True
self.user_mfa.save()

self.assert_mfa_login(mfa_method=self.mfa_totp, user=self.user_mfa, status='password-change-required')
self.assert_api_access(False)
self.assert_change_password()
self.assert_api_access(True)

self.user.refresh_from_db()
assert not self.user.must_change_password

@override_settings(REMOTE_USER_AUTH_ENABLED=True, REMOTE_USER_AUTH_HEADER='Remote-User')
def test_login_remoteuser(self):
AuthIdentity.objects.create(user=self.user_mfa, provider=AuthIdentity.PROVIDER_REMOTE_USER, identifier='[email protected]')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ def handle(self, username, password, is_superuser, is_system_user, is_initial_sa
user.is_staff = True
if is_system_user:
user.is_system_user = True
if is_initial_saas_user:
user.must_change_password = True
user.save()

self.stdout.write("User created or updated")
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 5.1.3 on 2024-11-27 09:11

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('users', '0013_pentestuser_is_project_admin'),
]

operations = [
migrations.AddField(
model_name='pentestuser',
name='must_change_password',
field=models.BooleanField(default=False, verbose_name='Must change password at next login'),
),
]
1 change: 1 addition & 0 deletions api/src/reportcreator_api/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

class PentestUser(BaseModel, AbstractUser):
password = EncryptedField(base_field=models.CharField(_("password"), max_length=128))
must_change_password = models.BooleanField(_('Must change password at next login'), default=False)

middle_name = models.CharField(_('Middle name'), max_length=255, null=True, blank=True)
title_before = models.CharField(_('Title (before)'), max_length=255, null=True, blank=True)
Expand Down
16 changes: 11 additions & 5 deletions api/src/reportcreator_api/users/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class Meta:
fields = [
'id', 'created', 'updated', 'last_login', 'is_active',
'username', 'name', 'title_before', 'first_name', 'middle_name', 'last_name', 'title_after',
'email', 'phone', 'mobile',
'email', 'phone', 'mobile', 'must_change_password',
'scope', 'is_superuser', 'is_project_admin', 'is_designer', 'is_template_editor', 'is_user_manager', 'is_guest', 'is_system_user', 'is_global_archiver',
'is_mfa_enabled', 'can_login_local', 'can_login_sso',
]
Expand Down Expand Up @@ -125,7 +125,7 @@ def get_choices(self, cutoff=None):
return OrderedDict([(str(item.pk), self.display_value(item)) for item in queryset])


class ResetPasswordSerializer(serializers.ModelSerializer):
class ChangePasswordSerializer(serializers.ModelSerializer):
password = serializers.CharField(write_only=True)

class Meta:
Expand All @@ -137,9 +137,15 @@ def validate_password(self, value):
return value

def update(self, instance, validated_data):
instance.set_password(validated_data['password'])
instance.save()
return instance
instance.set_password(validated_data.pop('password'))
return super().update(instance, {
'must_change_password': False,
} | validated_data)


class ResetPasswordSerializer(ChangePasswordSerializer):
class Meta(ChangePasswordSerializer.Meta):
fields = ChangePasswordSerializer.Meta.fields + ['must_change_password']


class MFAMethodSerializer(serializers.ModelSerializer):
Expand Down
81 changes: 63 additions & 18 deletions api/src/reportcreator_api/users/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
APITokenCreateSerializer,
APITokenSerializer,
AuthIdentitySerializer,
ChangePasswordSerializer,
CreateUserSerializer,
LoginMFACodeSerializer,
LoginSerializer,
Expand Down Expand Up @@ -104,8 +105,10 @@ def get_object(self):
return super().get_object()

def get_serializer_class(self):
if self.action in ['change_password', 'reset_password']:
if self.action == 'reset_password':
return ResetPasswordSerializer
elif self.action == 'change_password':
return ChangePasswordSerializer
elif self.action == 'create':
return CreateUserSerializer
elif (getattr(self.request.user, 'is_admin', False) or getattr(self.request.user, 'is_user_manager', False)) or \
Expand Down Expand Up @@ -272,6 +275,8 @@ def get_serializer_class(self):
return LoginSerializer
elif self.action == 'login_code':
return LoginMFACodeSerializer
elif self.action == 'change_password':
return ChangePasswordSerializer
else:
return serializers.Serializer

Expand All @@ -284,20 +289,7 @@ def login(self, request, *args, **kwargs):
serializer.is_valid(raise_exception=True)
user = serializer.validated_data

mfa_methods = list(user.mfa_methods.all().default_order())
if not mfa_methods:
# MFA disabled
return self.perform_login(request, user)
else:
request.session['login_state'] = request.session.get('login_state', {}) | {
'status': 'mfa-required',
'user_id': str(user.id),
'start': timezone.now().isoformat(),
}
return Response({
'status': 'mfa-required',
'mfa': MFAMethodSerializer(mfa_methods, many=True).data,
}, status=200)
return self.perform_login_local(request, user)

@action(detail=False, methods=['post'], authentication_classes=api_settings.DEFAULT_AUTHENTICATION_CLASSES)
def logout(self, request, *args, **kwargs):
Expand All @@ -310,7 +302,7 @@ def login_code(self, request, *args, **kwargs):

serializer = self.get_serializer(data=request.data)
serializer.is_valid(raise_exception=True)
return self.perform_login(request, request.user)
return self.perform_login_local(request, request.user, step='mfa')

@action(detail=False, url_path='login/fido2/begin', methods=['post'], authentication_classes=[MFALoginInProgressAuthentication], permission_classes=[LocalUserAuthPermissions])
def login_fido2_begin(self, request, *args, **kwargs):
Expand Down Expand Up @@ -338,7 +330,7 @@ def login_fido2_complete(self, request, *args, **kwargs):
raise serializers.ValidationError(ex.args[0], 'fido2') from ex
else:
raise ex
return self.perform_login(request, request.user)
return self.perform_login_local(request, request.user, step='mfa')

def _verify_mfa_preconditions(self, request):
login_state = request.session.get('login_state', {})
Expand All @@ -347,11 +339,64 @@ def _verify_mfa_preconditions(self, request):
elif datetime.fromisoformat(login_state.get('start')) + settings.MFA_LOGIN_TIMEOUT < timezone.now():
raise APIBadRequestError('Login timeout. Please restart login.')

def perform_login(self, request, user, can_reauth=True):
@action(detail=False, url_path='login/change-password', methods=['post'], authentication_classes=[MFALoginInProgressAuthentication], permission_classes=[LocalUserAuthPermissions])
def change_password(self, request, *args, **kwargs):
# verify login state
login_state = request.session.get('login_state', {})
if login_state.get('status') != 'password-change-required':
raise APIBadRequestError('Password change not allowed')

serializer = self.get_serializer(instance=request.user, data=request.data)
serializer.is_valid(raise_exception=True)
serializer.save()

return self.perform_login_local(request, request.user, step='change-password')

def perform_login_local(self, request, user, step=None, can_reauth=True):
is_reauth = bool(request.session.get('authentication_info', {}).get('login_time')) and str(user.id) == request.session.get(SESSION_KEY)
if not step:
# After username+password successful
request.session['login_state'] = request.session.get('login_state', {}) | {
'status': 'mfa-required',
'user_id': str(user.id),
'start': timezone.now().isoformat(),
}

mfa_methods = list(user.mfa_methods.all().default_order())
if not mfa_methods:
# MFA disabled: skip MFA setp
return self.perform_login_local(request, user, step='mfa')
else:
return Response({
'status': 'mfa-required',
'mfa': MFAMethodSerializer(mfa_methods, many=True).data,
}, status=200)
elif step == 'mfa':
# After MFA successful
self.validate_login_allowed(user)
request.session['login_state'] = request.session.get('login_state', {}) | {
'status': 'password-change-required',
}

if not user.must_change_password or is_reauth:
# Continue with next stage
return self.perform_login_local(request, user, step='change-password')
else:
return Response({
'status': 'password-change-required',
}, status=200)
else:
# After all other steps: perform actual login
return self.perform_login(request, user, can_reauth=can_reauth)

def validate_login_allowed(self, user):
if not user.is_active:
raise APIBadRequestError('User is inactive')
license.validate_login_allowed(user)

def perform_login(self, request, user, can_reauth=True):
self.validate_login_allowed(user)

request.session.pop('login_state', None)
first_login = not user.last_login
is_reauth = bool(request.session.get('authentication_info', {}).get('login_time')) and str(user.id) == request.session.get(SESSION_KEY)
Expand Down
49 changes: 48 additions & 1 deletion packages/frontend/src/components/LoginForm.vue
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
<template>
<s-card>
<v-toolbar color="header" flat>
<v-toolbar-title><slot name="title">Login</slot></v-toolbar-title>
<v-toolbar-title>
<slot name="title">
<template v-if="step === LoginStep.CHANGE_PASSWORD">Change Password</template>
<template v-else>Login</template>
</slot>
</v-toolbar-title>
</v-toolbar>

<template v-if="step === LoginStep.USERNAME">
Expand Down Expand Up @@ -125,6 +130,29 @@
</v-list>
</v-card-text>
</template>

<template v-else-if="step === LoginStep.CHANGE_PASSWORD">
<v-form ref="form" @submit.prevent="changePassword">
<v-card-text>
<s-password-field
v-model="formChangePassword.password"
confirm show-strength
label="New Password"
/>

<p v-if="errorMessage" class="text-error">
{{ errorMessage }}
</p>
</v-card-text>
<v-card-actions>
<v-spacer />
<s-btn-primary
type="submit"
text="Change Password"
/>
</v-card-actions>
</v-form>
</template>
</s-card>
</template>

Expand All @@ -146,6 +174,7 @@ enum LoginStep {
USERNAME = 'username',
MFA = 'mfa',
MFA_SELECT = 'mfa-select',
CHANGE_PASSWORD = 'change-password',
}
const step = ref<LoginStep>(LoginStep.USERNAME);
Expand All @@ -161,6 +190,9 @@ const formCode = ref({
code: '',
});
const otpRef = ref<VOtpInput|null>(null);
const formChangePassword = ref({
password: '',
});
async function loginStep(fn: () => Promise<LoginResponse|null>) {
if (actionInProgress.value) {
Expand All @@ -183,12 +215,16 @@ async function loginStep(fn: () => Promise<LoginResponse|null>) {
} else if (res.status === LoginResponseStatus.MFA_REQUIRED) {
mfaMethods.value = res.mfa!;
beginMfaLogin(mfaMethods.value!.find(m => m.is_primary) || mfaMethods.value![0]!)
} else if (res.status === LoginResponseStatus.PASSWORD_CHANGE_REQUIRED) {
step.value = LoginStep.CHANGE_PASSWORD;
}
} catch (error: any) {
if (error?.data?.detail) {
errorMessage.value = error.data.detail;
} else if (error?.data?.non_field_errors) {
errorMessage.value = error.data.non_field_errors[0];
} else if (error?.data?.password) {
errorMessage.value = error.data.password.join(', ');
} else if (error instanceof DOMException) {
errorMessage.value = error.message;
} else {
Expand Down Expand Up @@ -242,4 +278,15 @@ async function loginCode() {
});
}
async function changePassword() {
await loginStep(async () => {
return await $fetch('/api/v1/auth/login/change-password/', {
method: 'POST',
body: {
password: formChangePassword.value.password,
},
});
})
}
</script>
2 changes: 2 additions & 0 deletions packages/frontend/src/pages/login/auto.vue
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ const { error } = useAsyncData(async () => {
});
if (res.status === LoginResponseStatus.MFA_REQUIRED) {
throw new Error('MFA required, but not supported for autologin');
} else if (res.status !== LoginResponseStatus.SUCCESS) {
throw new Error(`Login failed: ${res.status}`);
}
await auth.fetchUser();
await auth.redirect();
Expand Down
Loading

0 comments on commit b100761

Please sign in to comment.