Browse Source

Don't pass the whole user object to celery

Only pass the needed attributes to the task. Fixes #397

Also includes some PEP8 fixes.
Peter Justin 7 years ago
parent
commit
17de9746fb

+ 52 - 35
flaskbb/auth/views.py

@@ -53,14 +53,16 @@ def check_rate_limiting():
 def login_rate_limit_error(error):
     """Register a custom error handler for a 'Too Many Requests'
     (HTTP CODE 429) error."""
-    return render_template("errors/too_many_logins.html", timeout=error.description)
+    return render_template("errors/too_many_logins.html",
+                           timeout=error.description)
 
 
 def login_rate_limit():
     """Dynamically load the rate limiting config from the database."""
     # [count] [per|/] [n (optional)] [second|minute|hour|day|month|year]
     return "{count}/{timeout}minutes".format(
-        count=flaskbb_config["AUTH_REQUESTS"], timeout=flaskbb_config["AUTH_TIMEOUT"]
+        count=flaskbb_config["AUTH_REQUESTS"],
+        timeout=flaskbb_config["AUTH_TIMEOUT"]
     )
 
 
@@ -105,13 +107,9 @@ class Login(MethodView):
             try:
                 user = User.authenticate(form.login.data, form.password.data)
                 if not login_user(user, remember=form.remember_me.data):
-                    flash(
-                        _(
-                            "In order to use your account you have to activate it "
-                            "through the link we have sent to your email "
-                            "address."
-                        ), "danger"
-                    )
+                    flash(_("In order to use your account you have to "
+                            "activate it through the link we have sent to "
+                            "your email address."), "danger")
                 return redirect_or_next(url_for("forum.index"))
             except AuthenticationError:
                 flash(_("Wrong username or password."), "danger")
@@ -162,16 +160,16 @@ class Register(MethodView):
             if flaskbb_config["ACTIVATE_ACCOUNT"]:
                 # Any call to an expired model requires a database hit, so
                 # accessing user.id would cause an DetachedInstanceError.
-                # This happens because the `user`'s session does no longer exist.
-                # So we just fire up another query to make sure that the session
-                # for the newly created user is fresh.
+                # This happens because the `user`'s session does no longer
+                # exist. So we just fire up another query to make sure that
+                # the session for the newly created user is fresh.
                 # PS: `db.session.merge(user)` did not work for me.
                 user = User.query.filter_by(email=user.email).first()
-                send_activation_token.delay(user)
-                flash(
-                    _("An account activation email has been sent to %(email)s", email=user.email),
-                    "success"
+                send_activation_token.delay(
+                    user_id=user.id, username=user.username, email=user.email
                 )
+                flash(_("An account activation email has been sent to "
+                        "%(email)s", email=user.email), "success")
             else:
                 login_user(user)
                 flash(_("Thanks for registering."), "success")
@@ -194,16 +192,14 @@ class ForgotPassword(MethodView):
             user = User.query.filter_by(email=form.email.data).first()
 
             if user:
-                send_reset_token.delay(user)
+                send_reset_token.delay(
+                    user_id=user.id, username=user.username, email=user.email
+                )
                 flash(_("Email sent! Please check your inbox."), "info")
                 return redirect(url_for("auth.forgot_password"))
             else:
-                flash(
-                    _(
-                        "You have entered an username or email address that is "
-                        "not linked with your account."
-                    ), "danger"
-                )
+                flash(_("You have entered an username or email address that "
+                        "is not linked with your account."), "danger")
         return render_template("auth/forgot_password.html", form=form)
 
 
@@ -219,7 +215,9 @@ class ResetPassword(MethodView):
     def post(self, token):
         form = self.form()
         if form.validate_on_submit():
-            expired, invalid, user = get_token_status(form.token.data, "reset_password")
+            expired, invalid, user = get_token_status(
+                form.token.data, "reset_password"
+            )
 
             if invalid:
                 flash(_("Your password token is invalid."), "danger")
@@ -244,20 +242,24 @@ class RequestActivationToken(MethodView):
     form = RequestActivationForm
 
     def get(self):
-        return render_template("auth/request_account_activation.html", form=self.form())
+        return render_template("auth/request_account_activation.html",
+                               form=self.form())
 
     def post(self):
         form = self.form()
         if form.validate_on_submit():
             user = User.query.filter_by(email=form.email.data).first()
-            send_activation_token.delay(user)
+            send_activation_token.delay(
+                user_id=user.id, username=user.username, email=user.email
+            )
             flash(
                 _("A new account activation token has been sent to "
                   "your email address."), "success"
             )
             return redirect(url_for("auth.activate_account"))
 
-        return render_template("auth/request_account_activation.html", form=form)
+        return render_template("auth/request_account_activation.html",
+                               form=form)
 
 
 class ActivateAccount(MethodView):
@@ -267,7 +269,9 @@ class ActivateAccount(MethodView):
     def get(self, token=None):
         expired = invalid = user = None
         if token is not None:
-            expired, invalid, user = get_token_status(token, "activate_account")
+            expired, invalid, user = get_token_status(
+                token, "activate_account"
+            )
 
         if invalid:
             flash(_("Your account activation token is invalid."), "danger")
@@ -288,17 +292,23 @@ class ActivateAccount(MethodView):
             flash(_("Your account has been activated."), "success")
             return redirect(url_for("forum.index"))
 
-        return render_template("auth/account_activation.html", form=self.form())
+        return render_template(
+            "auth/account_activation.html", form=self.form()
+        )
 
     def post(self, token=None):
         expired = invalid = user = None
         form = self.form()
 
         if token is not None:
-            expired, invalid, user = get_token_status(token, "activate_account")
+            expired, invalid, user = get_token_status(
+                token, "activate_account"
+            )
 
         elif form.validate_on_submit():
-            expired, invalid, user = get_token_status(form.token.data, "activate_account")
+            expired, invalid, user = get_token_status(
+                form.token.data, "activate_account"
+            )
 
         if invalid:
             flash(_("Your account activation token is invalid."), "danger")
@@ -325,19 +335,26 @@ class ActivateAccount(MethodView):
 register_view(auth, routes=['/logout'], view_func=Logout.as_view('logout'))
 register_view(auth, routes=['/login'], view_func=Login.as_view('login'))
 register_view(auth, routes=['/reauth'], view_func=Reauth.as_view('reauth'))
-register_view(auth, routes=['/register'], view_func=Register.as_view('register'))
 register_view(
-    auth, routes=['/reset-password'], view_func=ForgotPassword.as_view('forgot_password')
+    auth,
+    routes=['/register'],
+    view_func=Register.as_view('register')
+)
+register_view(
+    auth,
+    routes=['/reset-password'],
+    view_func=ForgotPassword.as_view('forgot_password')
 )
 register_view(
-    auth, routes=['/reset-password/<token>'], view_func=ResetPassword.as_view('reset_password')
+    auth,
+    routes=['/reset-password/<token>'],
+    view_func=ResetPassword.as_view('reset_password')
 )
 register_view(
     auth,
     routes=['/activate'],
     view_func=RequestActivationToken.as_view('request_activation_token')
 )
-
 register_view(
     auth,
     routes=['/activate/confirm', '/activate/confirm/<token>'],

+ 16 - 12
flaskbb/email.py

@@ -21,46 +21,50 @@ logger = logging.getLogger(__name__)
 
 
 @celery.task
-def send_reset_token(user):
+def send_reset_token(user_id, username, email):
     """Sends the reset token to the user's email address.
 
-    :param user: The user object to whom the email should be sent.
+    :param user_id: The user id. Used to generate the reset token.
+    :param username: The username to whom the email should be sent.
+    :param email:  The email address of the user
     """
-    token = make_token(user=user, operation="reset_password")
+    token = make_token(user_id=user_id, operation="reset_password")
     send_email(
         subject=_("Password Recovery Confirmation"),
-        recipients=[user.email],
+        recipients=[email],
         text_body=render_template(
             "email/reset_password.txt",
-            user=user,
+            username=username,
             token=token
         ),
         html_body=render_template(
             "email/reset_password.html",
-            user=user,
+            username=username,
             token=token
         )
     )
 
 
 @celery.task
-def send_activation_token(user):
+def send_activation_token(user_id, username, email):
     """Sends the activation token to the user's email address.
 
-    :param user: The user object to whom the email should be sent.
+    :param user_id: The user id. Used to generate the reset token.
+    :param username: The username to whom the email should be sent.
+    :param email:  The email address of the user
     """
-    token = make_token(user=user, operation="activate_account")
+    token = make_token(user_id=user_id, operation="activate_account")
     send_email(
         subject=_("Account Activation"),
-        recipients=[user.email],
+        recipients=[email],
         text_body=render_template(
             "email/activate_account.txt",
-            user=user,
+            username=username,
             token=token
         ),
         html_body=render_template(
             "email/activate_account.html",
-            user=user,
+            username=username,
             token=token
         )
     )

+ 1 - 1
flaskbb/templates/email/activate_account.html

@@ -1,6 +1,6 @@
 {% set link = url_for('auth.activate_account', token=token, _external=True) %}
 
-<p>{% trans user=user.username %}Dear {{ user }},{% endtrans %}</p>
+<p>{% trans %}Dear {{ username }},{% endtrans %}</p>
 
 <p>{% trans %}Click the link below to activate your account:{% endtrans %}</p>
 

+ 1 - 1
flaskbb/templates/email/activate_account.txt

@@ -1,6 +1,6 @@
 {% set link = url_for('auth.activate_account', token=token, _external=True) %}
 
-{% trans user=user.username, link=link %}Dear {{ user }},
+{% trans link=link %}Dear {{ username }},
 
 Click the link below to activate your account:
 

+ 1 - 1
flaskbb/templates/email/reset_password.html

@@ -1,5 +1,5 @@
 {% set link = url_for('auth.reset_password', token=token, _external=True) %}
-<p>{% trans user=user.username %}Dear {{ user }},{% endtrans %}</p>
+<p>{% trans %}Dear {{ username }},{% endtrans %}</p>
 
 <p>{% trans %}Click the link below to reset your password:{% endtrans %}</p>
 

+ 1 - 1
flaskbb/templates/email/reset_password.txt

@@ -1,6 +1,6 @@
 {% set link = url_for('auth.reset_password', token=token, _external=True) %}
 
-{% trans user=user.username, link=link %}Dear {{ user }},
+{% trans link=link %}Dear {{ username }},
 
 Click the link below to reset your password:
 

+ 3 - 3
flaskbb/utils/tokens.py

@@ -20,12 +20,12 @@ from flaskbb.user.models import User
 logger = logging.getLogger(__name__)
 
 
-def make_token(user, operation, expire=3600):
+def make_token(user_id, operation, expire=3600):
     """Generates a JSON Web Signature (JWS).
     See `RFC 7515 <https://tools.ietf.org/html/rfc7515>` if you want to know
     more about JWS.
 
-    :param user: The user object for whom the token should be generated.
+    :param user_id: The user_id for which the token should be generated.
     :param operation: The function of the token. For example, you might want
                       to generate two different tokens. One for a
                       password reset link, which you hypothetically want
@@ -38,7 +38,7 @@ def make_token(user, operation, expire=3600):
     s = TimedJSONWebSignatureSerializer(
         current_app.config['SECRET_KEY'], expire
     )
-    data = {"id": user.id, "op": operation}
+    data = {"id": user_id, "op": operation}
     return s.dumps(data)
 
 

+ 2 - 2
tests/unit/test_email.py

@@ -8,7 +8,7 @@ def test_send_reset_token_to_user(default_settings, user):
 
     with current_app.test_request_context():
         with mail.record_messages() as outbox:
-            send_reset_token(user)
+            send_reset_token(user.id, user.username, user.email)
 
             assert len(outbox) == 1
             # from /auth/reset-password/<token>
@@ -21,7 +21,7 @@ def test_send_activation_token_to_user(default_settings, user):
 
     with current_app.test_request_context():
         with mail.record_messages() as outbox:
-            send_activation_token(user)
+            send_activation_token(user.id, user.username, user.email)
 
             assert len(outbox) == 1
             # from /auth/activate/<token>

+ 5 - 5
tests/unit/utils/test_tokens.py

@@ -4,7 +4,7 @@ from flaskbb.utils.tokens import make_token, get_token_status
 
 
 def test_make_token(user):
-    token = make_token(user, "test")
+    token = make_token(user.id, "test")
     s = TimedJSONWebSignatureSerializer(current_app.config['SECRET_KEY'])
     unpacked_token = s.loads(token)
     assert user.id == unpacked_token["id"]
@@ -12,7 +12,7 @@ def test_make_token(user):
 
 
 def test_valid_token_status(user):
-    token = make_token(user, "valid_test")
+    token = make_token(user.id, "valid_test")
     expired, invalid, token_user = get_token_status(token, "valid_test")
 
     assert not expired
@@ -21,7 +21,7 @@ def test_valid_token_status(user):
 
 
 def test_token_status_with_data(user):
-    token = make_token(user, "test_data")
+    token = make_token(user.id, "test_data")
     expired, invalid, token_user, data = \
         get_token_status(token, "test_data", return_data=True)
     assert user.id == data["id"]
@@ -29,7 +29,7 @@ def test_token_status_with_data(user):
 
 
 def test_token_operation(user):
-    token = make_token(user, "operation_test")
+    token = make_token(user.id, "operation_test")
     expired, invalid, token_user = get_token_status(token, "invalid_op")
     assert invalid
     assert not expired
@@ -48,7 +48,7 @@ def test_invalid_token_status(user):
 
 
 def test_expired_token_status(user):
-    token = make_token(user, "expired_test", -1)
+    token = make_token(user.id, "expired_test", -1)
     expired, invalid, token_user = get_token_status(token, "expired_test")
     assert expired
     assert not invalid