Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Access to Moderation Queue #1028

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions contrib/isso-dev.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ notify = stdout
reply-notifications = false
log-file =
latest-enabled = true
pending-enabled = true

[admin]
enabled = true
Expand Down
4 changes: 4 additions & 0 deletions isso/isso.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ gravatar-url = https://www.gravatar.com/avatar/{}?d=identicon&s=55
# needing to previously know the posts URIs)
latest-enabled = false

# enable the "/pending" endpoint, that works likes "/latest" but only
# for comments waiting moderation
Copy link
Member

Choose a reason for hiding this comment

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

"that works like"
"awaiting"

pending-enabled = false

[admin]
enabled = false

Expand Down
78 changes: 78 additions & 0 deletions isso/tests/test_comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
import re
import tempfile
import unittest
import base64

from urllib.parse import urlencode

from werkzeug.wrappers import Response
from werkzeug.datastructures import Headers

from isso import Isso, core, config
from isso.utils import http
Expand Down Expand Up @@ -689,6 +691,20 @@ def testLatestNotEnabled(self):
response = self.get('/latest?limit=5')
self.assertEqual(response.status_code, 404)

def testPendingNotFound(self):
# load some comments in a mix of posts
saved = []
for idx, post_id in enumerate([1, 2, 2, 1, 2, 1, 3, 1, 4, 2, 3, 4, 1, 2]):
text = 'text-{}'.format(idx)
post_uri = 'test-{}'.format(post_id)
self.post('/new?uri=' + post_uri, data=json.dumps({'text': text}))
saved.append((post_uri, text))

response = self.get('/pending?limit=5')

# If the admin interface was not enabled we should get a 404.
self.assertEqual(response.status_code, 404)


class TestHostDependent(unittest.TestCase):

Expand Down Expand Up @@ -763,13 +779,17 @@ def setUp(self):
conf.set("moderation", "enabled", "true")
conf.set("guard", "enabled", "off")
conf.set("hash", "algorithm", "none")
conf.set("admin", "enabled", "true")
self.conf = conf

class App(Isso, core.Mixin):
pass

self.app = App(conf)
self.app.wsgi_app = FakeIP(self.app.wsgi_app, "192.168.1.1")
self.client = JSONClient(self.app, Response)
self.post = self.client.post
self.get = self.client.get

def tearDown(self):
os.unlink(self.path)
Expand Down Expand Up @@ -844,6 +864,64 @@ def testModerateComment(self):
# Comment should no longer exist
self.assertEqual(self.app.db.comments.get(id_), None)

def testPendingWithoutAdmin(self):
self.conf.set("admin", "enabled", "false")
Copy link
Member

Choose a reason for hiding this comment

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

Are these tests guaranteed to be run in order? I'd prefer you un-set the admin.enabled value at test end.

response = self.get('/pending?limit=5')
self.assertEqual(response.status_code, 404)

def testPendingUnauthorized(self):
response = self.get('/pending?limit=5')
self.assertEqual(response.status_code, 401)

def getAuthenticated(self, url, username, password):
credentials = f"{username}:{password}"
encoded_credentials = base64.b64encode(credentials.encode('utf-8')).decode('utf-8')
headers = Headers()
headers.add('Authorization', f'Basic {encoded_credentials}')

return self.client.get(url, headers=headers)

def testPendingNotEnabled(self):
password = "s3cr3t"
self.conf.set("admin", "enabled", "true")
self.conf.set("admin", "password", password)
response = self.getAuthenticated('/pending?limit=5', 'admin', password)
self.assertEqual(response.status_code, 404)

def testPendingNotEnabled(self):
Copy link
Member

Choose a reason for hiding this comment

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

should be testPendingEnabled

password = "s3cr3t"
self.conf.set("admin", "enabled", "true")
self.conf.set("admin", "password", password)
self.conf.set("general", "pending-enabled", "true")
response = self.getAuthenticated('/pending?limit=5', 'admin', password)
self.assertEqual(response.status_code, 200)

body = loads(response.data)
self.assertEqual(body, [])

def testPendingPosts(self):
# load some comments in a mix of posts
saved = []
for idx, post_id in enumerate([1, 2, 2, 1, 2, 1, 3, 1, 4, 2, 3, 4, 1, 2]):
text = 'text-{}'.format(idx)
post_uri = 'test-{}'.format(post_id)
self.post('/new?uri=' + post_uri, data=json.dumps({'text': text}))
saved.append((post_uri, text))

password = "s3cr3t"
self.conf.set("admin", "enabled", "true")
self.conf.set("admin", "password", password)
self.conf.set("general", "pending-enabled", "true")
response = self.getAuthenticated('/pending?limit=5', 'admin', password)
self.assertEqual(response.status_code, 200)

body = loads(response.data)
expected_items = saved[-5:] # latest 5
for reply, expected in zip(body, expected_items):
expected_uri, expected_text = expected
self.assertIn(expected_text, reply['text'])
self.assertEqual(expected_uri, reply['uri'])


class TestUnsubscribe(unittest.TestCase):

Expand Down
101 changes: 100 additions & 1 deletion isso/views/comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,33 @@ def get_uri_from_url(url):
return uri


def requires_auth(method):
def decorated(self, *args, **kwargs):
request = args[1]
auth = request.authorization
if not auth:
return Response(
"Unauthorized", 401,
{'WWW-Authenticate': 'Basic realm="Authentication Required"'})
if not self.check_auth(auth.username, auth.password):
return Response(
"Wrong username or password", 401,
{'WWW-Authenticate': 'Basic realm="Authentication Required"'})
return method(self, *args, **kwargs)
return decorated


def requires_admin(method):
def decorated(self, *args, **kwargs):
if not self.isso.conf.getboolean("admin", "enabled"):
return NotFound(
"Unavailable because 'admin' not enabled by site admin"
)

return method(self, *args, **kwargs)
return decorated


class API(object):

FIELDS = set(['id', 'parent', 'text', 'author', 'website',
Expand All @@ -146,6 +173,7 @@ class API(object):
('counts', ('POST', '/count')),
('feed', ('GET', '/feed')),
('latest', ('GET', '/latest')),
('pending', ('GET', '/pending')),
('view', ('GET', '/id/<int:id>')),
('edit', ('PUT', '/id/<int:id>')),
('delete', ('DELETE', '/id/<int:id>')),
Expand Down Expand Up @@ -1562,6 +1590,77 @@ def latest(self, environ, request):
"Unavailable because 'latest-enabled' not set by site admin"
)

return self._latest(environ, request, "1")


def check_auth(self, username, password):
admin_password = self.isso.conf.get("admin", "password")

return username == 'admin' and password == admin_password


"""
@api {get} /pending pending
@apiGroup Comment
@apiName pending
@apiVersion 0.13.0
@apiDescription
Get the latest comments from the system waiting moderation, no matter which thread. Only available if `[general] pending-enabled` is set to `true` in server config.

@apiQuery {Number} limit
The quantity of last comments to retrieve

@apiExample {curl} Get the latest 5 pending comments
curl 'https://comments.example.com/pending?limit=5'

@apiUse commentResponse

@apiSuccessExample Example result:
[
{
"website": null,
"uri": "/some",
"author": null,
"parent": null,
"created": 1464912312.123416,
"text": " &lt;p&gt;I want to use MySQL&lt;/p&gt;",
"dislikes": 0,
"modified": null,
"mode": 2,
"id": 3,
"likes": 1
},
{
"website": null,
"uri": "/other",
"author": null,
"parent": null,
"created": 1464914341.312426,
"text": " &lt;p&gt;I want to use MySQL&lt;/p&gt;",
"dislikes": 0,
"modified": null,
"mode": 2,
"id": 4,
"likes": 0
}
]
"""
# If the admin interface is not enabled, people may have not changed
# the default password. We therefore disallow the /pending endpoint,
# as well.
@requires_admin
@requires_auth
def pending(self, environ, request):
# if the feature is not allowed, don't present the endpoint
if not self.conf.getboolean("pending-enabled"):
return NotFound(
"Unavailable because 'pending-enabled' not set by site admin"
)

return self._latest(environ, request, "2")
Copy link
Member

Choose a reason for hiding this comment

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

Rather than adding a new endpoint, wouldn't it make more sense to instead pass through a mode GET parameter? It would remain at "1" as a default for /latest and could be set to "2" to get pending comments (which would then require auth).



def _latest(self, environ, request, mode):
# get and check the limit
bad_limit_msg = "Query parameter 'limit' is mandatory (integer, >0)"
try:
Expand All @@ -1572,7 +1671,7 @@ def latest(self, environ, request):
return BadRequest(bad_limit_msg)

# retrieve the latest N comments from the DB
all_comments_gen = self.comments.fetchall(limit=None, order_by='created', mode='1')
all_comments_gen = self.comments.fetchall(limit=None, order_by='created', mode=mode)
comments = collections.deque(all_comments_gen, maxlen=limit)

# prepare a special set of fields (except text which is rendered specifically)
Expand Down