Commit 90e2c8ab authored by Kostas Papadimitriou's avatar Kostas Papadimitriou
Browse files

Change email process improvements

- Allow replacement of previously email change requests
- Cleanup expired email change requests in change email view
- Log email change
- Warn user for existing pending requests
- Redirect to profile on change email actions (failed or succeded)
parent 3617eceb
......@@ -511,6 +511,7 @@ class ExtendedPasswordResetForm(PasswordResetForm):
class EmailChangeForm(forms.ModelForm):
class Meta:
model = EmailChange
fields = ('new_email_address',)
......@@ -533,6 +534,7 @@ class EmailChangeForm(forms.ModelForm):
class SignApprovalTermsForm(forms.ModelForm):
class Meta:
model = AstakosUser
fields = ("has_signed_terms",)
......@@ -548,6 +550,7 @@ class SignApprovalTermsForm(forms.ModelForm):
class InvitationForm(forms.ModelForm):
username = forms.EmailField(label=_("Email"))
def __init__(self, *args, **kwargs):
......
......@@ -260,8 +260,7 @@ def send_feedback(msg, data, user, email_template_name='im/feedback_mail.txt'):
def send_change_email(ec, request, email_template_name='registration/email_change_email.txt'):
try:
url = reverse('email_change_confirm',
kwargs={'activation_key': ec.activation_key})
url = ec.get_url()
url = request.build_absolute_uri(url)
t = loader.get_template(email_template_name)
c = {'url': url, 'site_name': SITENAME}
......
......@@ -72,7 +72,8 @@ GENERIC_ERROR = 'Something wrong has happened. \
MAX_INVITATION_NUMBER_REACHED = 'There are no invitations left.'
GROUP_MAX_PARTICIPANT_NUMBER_REACHED = 'Group maximum participant number has been reached.'
NO_APPROVAL_TERMS = 'There are no approval terms.'
PENDING_EMAIL_CHANGE_REQUEST = 'There is already a pending change email request.'
PENDING_EMAIL_CHANGE_REQUEST = 'There is already a pending change email request. ' + \
'Submiting a new email will cancel any previous requests.'
OBJECT_CREATED_FAILED = 'The %(verbose_name)s creation failed: %(reason)s.'
GROUP_JOIN_FAILURE = 'Failed to join group.'
GROUPKIND_UNKNOWN = 'There is no such a group kind'
......
......@@ -570,6 +570,9 @@ class AstakosUser(User):
if q.count() != 0:
raise ValidationError({'__all__': [_(astakos_messages.UNIQUE_EMAIL_IS_ACTIVE_CONSTRAIN_ERR)]})
def email_change_is_pending(self):
return self.emailchanges.count() > 0
@property
def signed_terms(self):
term = get_latest_terms()
......@@ -960,6 +963,7 @@ class Invitation(models.Model):
class EmailChangeManager(models.Manager):
@transaction.commit_on_success
def change_email(self, activation_key):
"""
......@@ -993,9 +997,13 @@ class EmailChangeManager(models.Manager):
raise ValueError(_(astakos_messages.NEW_EMAIL_ADDR_RESERVED))
# update user
user = AstakosUser.objects.get(pk=email_change.user_id)
old_email = user.email
user.email = email_change.new_email_address
user.save()
email_change.delete()
msg = "User %d changed email from %s to %s" % (user.pk, old_email,
user.email)
logger.log(LOGGING_LEVEL, msg)
return user
except EmailChange.DoesNotExist:
raise ValueError(_(astakos_messages.INVALID_ACTIVATION_KEY))
......@@ -1005,13 +1013,17 @@ class EmailChange(models.Model):
new_email_address = models.EmailField(_(u'new e-mail address'),
help_text=_(astakos_messages.EMAIL_CHANGE_NEW_ADDR_HELP))
user = models.ForeignKey(
AstakosUser, unique=True, related_name='emailchange_user')
AstakosUser, unique=True, related_name='emailchanges')
requested_at = models.DateTimeField(default=datetime.now())
activation_key = models.CharField(
max_length=40, unique=True, db_index=True)
objects = EmailChangeManager()
def get_url(self):
return reverse('email_change_confirm',
kwargs={'activation_key': self.activation_key})
def activation_key_expired(self):
expiration_date = timedelta(days=EMAILCHANGE_ACTIVATION_DAYS)
return self.requested_at + expiration_date < datetime.now()
......
......@@ -47,6 +47,9 @@ from urllib import quote
from astakos.im import messages
astakos_settings.EMAILCHANGE_ENABLED = True
class ShibbolethClient(Client):
"""
A shibboleth agnostic client.
......@@ -624,3 +627,82 @@ class LocalUserTests(TestCase):
self.assertContains(r, "Password change for this account is not"
" supported")
class UserActionsTests(TestCase):
def setUp(self):
kind = GroupKind.objects.create(name="default")
AstakosGroup.objects.create(name="default", kind=kind)
def test_email_change(self):
# to test existing email validation
existing_user = get_local_user('existing@grnet.gr')
# local user
user = get_local_user('kpap@grnet.gr')
# login as kpap
self.client.login(username='kpap@grnet.gr', password='password')
r = self.client.get('/im/profile', follow=True)
user = r.context['request'].user
self.assertTrue(user.is_authenticated())
# change email is enabled
r = self.client.get('/im/email_change')
self.assertEqual(r.status_code, 200)
self.assertFalse(user.email_change_is_pending())
# request email change to an existing email fails
data = {'new_email_address': 'existing@grnet.gr'}
r = self.client.post('/im/email_change', data)
self.assertContains(r, messages.EMAIL_USED)
# proper email change
data = {'new_email_address': 'kpap@gmail.com'}
r = self.client.post('/im/email_change', data, follow=True)
self.assertRedirects(r, '/im/profile')
self.assertContains(r, messages.EMAIL_CHANGE_REGISTERED)
change1 = EmailChange.objects.get()
# user sees a warning
r = self.client.get('/im/email_change')
self.assertEqual(r.status_code, 200)
self.assertContains(r, messages.PENDING_EMAIL_CHANGE_REQUEST)
self.assertTrue(user.email_change_is_pending())
# link was sent
self.assertEqual(len(get_mailbox('kpap@grnet.gr')), 0)
self.assertEqual(len(get_mailbox('kpap@gmail.com')), 1)
# proper email change
data = {'new_email_address': 'kpap@yahoo.com'}
r = self.client.post('/im/email_change', data, follow=True)
self.assertRedirects(r, '/im/profile')
self.assertContains(r, messages.EMAIL_CHANGE_REGISTERED)
self.assertEqual(len(get_mailbox('kpap@grnet.gr')), 0)
self.assertEqual(len(get_mailbox('kpap@yahoo.com')), 1)
change2 = EmailChange.objects.get()
r = self.client.get(change1.get_url())
self.assertEquals(r.status_code, 302)
self.client.logout()
r = self.client.post('/im/local?next=' + change2.get_url(),
{'username': 'kpap@grnet.gr',
'password': 'password',
'next': change2.get_url()},
follow=True)
self.assertRedirects(r, '/im/profile')
user = r.context['request'].user
self.assertEquals(user.email, 'kpap@yahoo.com')
self.assertEquals(user.username, 'kpap@yahoo.com')
self.client.logout()
r = self.client.post('/im/local?next=' + change2.get_url(),
{'username': 'kpap@grnet.gr',
'password': 'password',
'next': change2.get_url()},
follow=True)
self.assertContains(r, "Please enter a correct username and password")
self.assertEqual(user.emailchanges.count(), 0)
......@@ -670,6 +670,8 @@ def change_email(request, activation_key=None,
confirm_template_name='registration/email_change_done.html',
extra_context=None):
extra_context = extra_context or {}
if activation_key:
try:
user = EmailChange.objects.change_email(activation_key)
......@@ -679,34 +681,50 @@ def change_email(request, activation_key=None,
auth_logout(request)
response = prepare_response(request, user)
transaction.commit()
return response
return HttpResponseRedirect(reverse('edit_profile'))
except ValueError, e:
messages.error(request, e)
transaction.rollback()
return HttpResponseRedirect(reverse('index'))
return render_response(confirm_template_name,
modified_user=user if 'user' in locals(
) else None,
context_instance=get_context(request,
modified_user=user if 'user' in locals() \
else None, context_instance=get_context(request,
extra_context))
if not request.user.is_authenticated():
path = quote(request.get_full_path())
url = request.build_absolute_uri(reverse('index'))
return HttpResponseRedirect(url + '?next=' + path)
# clean up expired email changes
if request.user.email_change_is_pending():
change = request.user.emailchanges.get()
if change.activation_key_expired():
change.delete()
transaction.commit()
return HttpResponseRedirect(reverse('email_change'))
form = EmailChangeForm(request.POST or None)
if request.method == 'POST' and form.is_valid():
try:
# delete pending email changes
request.user.emailchanges.all().delete()
ec = form.save(email_template_name, request)
except SendMailError, e:
msg = e
messages.error(request, msg)
transaction.rollback()
except IntegrityError, e:
msg = _(astakos_messages.PENDING_EMAIL_CHANGE_REQUEST)
messages.error(request, msg)
return HttpResponseRedirect(reverse('edit_profile'))
else:
msg = _(astakos_messages.EMAIL_CHANGE_REGISTERED)
messages.success(request, msg)
transaction.commit()
return HttpResponseRedirect(reverse('edit_profile'))
if request.user.email_change_is_pending():
messages.warning(request, astakos_messages.PENDING_EMAIL_CHANGE_REQUEST)
return render_response(
form_template_name,
form=form,
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment