When adding comment, respond 403 if user is not authed

Since 2e6326e, which removed the code allowing anonymous
comments with a captcha, we have effectively been assuming the
session is properly authenticated when adding a comment - we
look at `request.user` and `request.user.name` for the 'author'
and don't do any checks, we just assume they were set. But if
we have a cached session that has expired, or something like
that, they will *not* be set. Without an explicit check, the
error the client ultimately gets when trying to add the comment
fails is not one which the client code recognizes as meaning
'reauthentication needed', so the client just sort of bails out.
This was breaking apps like fedora-easy-karma which just
retrieves a bunch of update info then tries to leave comments,
once the cached session for the client expired.

To fix this, we'll restore the check (removed in 2e6326e) that
`author` actually gets set, and tweak it a bit to return 403 not
400 if it wasn't set. The client code *does* recognize a 403
response as meaning 'reauth needed', so this fixes the problem,
and it's a more correct response than 400 anyway.

Resolves: fedora-infra#3298

Signed-off-by: Adam Williamson <awilliam@redhat.com>

bodhi/server/services/comments.py

@@ -21,6 +21,7 @@
 
 from cornice import Service
 from cornice.validators import colander_body_validator, colander_querystring_validator
+from pyramid.httpexceptions import HTTPForbidden
 from sqlalchemy import func, distinct
 from sqlalchemy.sql import or_, and_
 
@@ -214,6 +215,13 @@ def new_comment(request):
 
     update = data.pop('update')
     author = request.user and request.user.name
+    if not author:
+        # this can happen if we have a stale cached session, a 403
+        # response will trigger the client to reauth:
+        # https://github.com/fedora-infra/bodhi/issues/3298
+        request.errors.add('body', 'email', 'You must provide an author')
+        request.errors.status = HTTPForbidden.code
+        return
 
     try:
         comment, caveats = update.comment(session=request.db, author=author, **data)

bodhi/tests/server/services/test_comments.py

@@ -18,12 +18,15 @@
 
 from datetime import datetime, timedelta
 from unittest import mock
+import copy
 
 from fedora_messaging import api, testing as fml_testing
+import webtest
 
 from bodhi.messages.schemas import update as update_schemas
 from bodhi.server.models import (Build, Comment, Release, RpmBuild, RpmPackage, Update,
                                  UpdateRequest, UpdateStatus, UpdateType, User)
+from bodhi.server import main
 from bodhi.tests.server import base
 
 
@@ -674,3 +677,29 @@ def test_no_comment_on_stable_pushed_update(self):
         up = self.db.query(Build).filter_by(nvr=up2).one().update
         assert len(up.comments) == 0
         assert up.karma == 0
+
+    def test_comment_not_loggedin(self):
+        """
+        Test that 403 error is returned if a non-authenticated 'post comment'
+        request is received. It's important that we return 403 here so the
+        client will know to re-authenticate
+        """
+        from bodhi.server import main
+        anonymous_settings = copy.copy(self.app_settings)
+        anonymous_settings.update({
+            'authtkt.secret': 'whatever',
+            'authtkt.secure': True,
+        })
+        with mock.patch('bodhi.server.Session.remove'):
+            app = webtest.TestApp(main({}, session=self.db, **anonymous_settings))
+
+        csrf = app.get('/csrf', headers={'Accept': 'application/json'}).json_body['csrf_token']
+        update = Build.query.filter_by(nvr='bodhi-2.0-1.fc17').one().update.alias
+        comment = {
+            'update': update,
+            'text': 'Test',
+            'karma': 0,
+            'csrf_token': csrf,
+        }
+        res = app.post_json('/comments/', comment, status=403)
+        assert 'errors' in res.json_body