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: 4 additions & 1 deletion vms/registration/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
from django.contrib.auth.models import User
from django.core.exceptions import ValidationError

class useremail(forms.ModelForm):
Email = forms.CharField(widget=forms.EmailInput())
def __str__(self):
return self.Email

class UserForm(forms.ModelForm):
# password not visible when user types it out
Expand Down Expand Up @@ -32,4 +36,3 @@ def clean_password(self):
class Meta:
model = User
fields = ('username', 'password')

11 changes: 10 additions & 1 deletion vms/registration/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.contrib.sites.shortcuts import get_current_site
from django.core.exceptions import ObjectDoesNotExist
from django.core.mail import EmailMessage
from django.core.mail import send_mail
from django.http import JsonResponse
from django.http import HttpResponseBadRequest
from django.shortcuts import render
Expand All @@ -15,6 +16,7 @@
from django.views.generic import TemplateView

# local Django
from vms.settings import EMAIL_HOST_USER
from administrator.forms import AdministratorForm
from cities_light.models import City, Region, Country
from organization.models import Organization
Expand Down Expand Up @@ -316,7 +318,6 @@ def post(self, request):
'country_list': country_list,
})


def activate(request, uidb64, token):
"""
Checks token, if valid, then user will active and login
Expand All @@ -333,6 +334,14 @@ def activate(request, uidb64, token):
user = None
if user is not None and account_activation_token.check_token(user, token):
user.is_active = True
sub = forms.registration()
if request.method == 'POST':
sub = forms.registration(request.POST)
subject = 'Account Verification'
message = 'Thank you for confirmation of your account'
recepient = str(sub['Email'].value())
send_mail(subject,
message, EMAIL_HOST_USER, [recepient], fail_silently = False)
user.save()
return render(request, 'home/confirmed_email.html')
else:
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