Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

chore: Send email verification for signing up #1118

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
5 changes: 5 additions & 0 deletions vms/registration/forms.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Django
from django import forms
from django.contrib.auth.models import User
from django.contrib.auth.forms import UserCreationForm
from django.core.exceptions import ValidationError


Expand Down Expand Up @@ -33,3 +34,7 @@ class Meta:
model = User
fields = ('username', 'password')

class SignUpForm(UserCreationForm):
class Meta:
model = User
fields = ('email', 'first_name', 'last_name', 'username')
2 changes: 2 additions & 0 deletions vms/registration/urls.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Django
from django.conf.urls import url
from django.urls import path

# local Django
from registration.views import (activate, AdministratorSignupView,
Expand All @@ -22,5 +23,6 @@
url(r'^check_states/$', check_states, name='check_states'),
url(r'^load_cities/$', load_cities, name='load_cities'),
url(r'^load_states/$', load_states, name='load_states'),
path('activate/<uidb64>/<token>/',views.activate, name='activate'),
]

27 changes: 27 additions & 0 deletions vms/registration/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,33 @@ def post(self, request):
'country_list': country_list,
})

def signup(request):
if request.method == 'GET':
return render(request, 'registration/signup_administrator.html')
if request.method == 'POST':
form = SignUpForm(request.POST)
# print(form.errors.as_data())
if form.is_valid():
user = form.save(commit=False)
user.is_active = False
user.save()
current_site = get_current_site(request)
mail_subject = 'Activate your account.'
message = render_to_string('registration/acc_active_email.html', {
'user': user,
'domain': current_site.domain,
'uid': urlsafe_base64_encode(force_bytes(user.pk)),
'token': default_token_generator.make_token(user),
})
to_email = form.cleaned_data.get('email')
email = EmailMessage(
mail_subject, message, to=[to_email]
)
email.send()
return HttpResponse('Please confirm your email address to complete the registration')
else:
form = SignUpForm()
return render(request, 'accounts/signup.html', {'form': form})

def activate(request, uidb64, token):
"""
Expand Down
15 changes: 11 additions & 4 deletions vms/vms/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,17 @@

# Instead of sending out real email, during development the emails will be sent
# to stdout, where from they can be inspected.
if DEBUG:
EMAIL_HOST = config('EMAIL_HOST', default='localhost')
EMAIL_PORT = config('EMAIL_PORT', default=1025, cast=int)
EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'
Copy link
Member

Choose a reason for hiding this comment

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

keep this in comments for better understanding rather than just removing it completely


# if DEBUG:
# EMAIL_HOST = config('EMAIL_HOST', default='localhost')
# EMAIL_PORT = config('EMAIL_PORT', default=1025, cast=int)
# EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'
EMAIL_BACKEND = 'django.core.mail.backends.smtp.EmailBackend'
EMAIL_HOST = 'smtp.gmail.com'
EMAIL_HOST_PASSWORD = 'your_password'
EMAIL_HOST_USER = 'your_email'
EMAIL_PORT = 587
EMAIL_USE_TSL = True

Choose a reason for hiding this comment

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

I don't understand this change 🤔

It's supposed to be like that so we can test easily during local dev

Copy link
Member

Choose a reason for hiding this comment

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

But I don't think while pushing the changes we usually keep the email addresses and the passwords. The one who clones it, needs to add their personal ones before running this. This is what I learnt in my Internship. If I am wrong, do correct me :)

Choose a reason for hiding this comment

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

Sorry for the late response but could you elaborate on that? I do not get you.

Copy link
Member

Choose a reason for hiding this comment

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

Sure. What we used to do like whenever we used to push the changes in authentication part regarding login or signups, we deleted the dummy email Id used. Because after the changes are merged this email ID and password wont be valid for the other person who tries to clone and run. So kept that field empty and whenever someone tries to run, s/he must first put in their id and password in order to try the emails sent.

Copy link
Member

Choose a reason for hiding this comment

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

If we are not supposed to do that way here then do let me know if I am wrong :)

Choose a reason for hiding this comment

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

Oh I see. I will try to explain.

So basically, first things first, for anything that depends on an external service, it's generally much more efficient to keep it configurable, and that's exactly what's done here. If you can imagine, if for an external service like mail, we keep it hardcoded it can lead to all sorts of abuse, that is why the config() function is used (from a 3rd party package) which basically loads the required settings from a .env file.


LOGIN_REDIRECT_URL = reverse_lazy('home:index')
RECOVER_ONLY_ACTIVE_USERS = False
Expand Down