Merge pull request #1966 from charltonaustin/j_code_review_comments

Adding in security tests and docs.

data/model/message.py

@@ -2,8 +2,12 @@ from data.database import Messages
 
 
 def get_messages():
+  """Query the data base for messages and returns a container of database message objects"""
   return Messages.select()
 
 def create(messages):
+  """Insert messages into the database."""
+  inserted = []
   for message in messages:
-    Messages.create(content=message['content'])
+    inserted.append(Messages.create(content=message['content']))
+  return inserted

endpoints/api/superuser.py

@@ -873,19 +873,21 @@ class SuperUserMessages(ApiResource):
   @nickname('getMessages')
   def get(self):
     """ Return a super users messages """
-    messages = list(model.message.get_messages())
     return {
-      'messages': [message_view(m) for m in messages],
+      'messages': [message_view(m) for m in model.message.get_messages()],
     }
 
-  @require_scope(scopes.SUPERUSER)
   @verify_not_prod
   @nickname('createMessages')
   @validate_json_request('CreateMessage')
+  @require_scope(scopes.SUPERUSER)
   def post(self):
     """ Create a message """
-    body = request.get_json()
+    if SuperUserPermission().can():
-    model.message.create([body['message']])
+      model.message.create([request.get_json()['message']])
+      return make_response('', 201)
+    abort(403)
+
 
 def message_view(message):
   return {'id': message.id, 'content': message.content}

test/test_api_security.py

@@ -51,7 +51,7 @@ from endpoints.api.superuser import (SuperUserLogs, SuperUserList, SuperUserMana
                                      SuperUserOrganizationManagement, SuperUserOrganizationList,
                                      SuperUserAggregateLogs, SuperUserServiceKeyManagement,
                                      SuperUserServiceKey, SuperUserServiceKeyApproval,
-                                     SuperUserTakeOwnership)
+                                     SuperUserTakeOwnership, SuperUserMessages)
 from endpoints.api.secscan import RepositoryImageSecurity
 from endpoints.api.manifest import RepositoryManifestLabels, ManageRepositoryManifestLabel
 
@@ -4200,6 +4200,36 @@ class TestSuperUserManagement(ApiTestCase):
   def test_delete_devtable(self):
     self._run_test('DELETE', 204, 'devtable', None)
 
+class TestSuperUserMessages(ApiTestCase):
+  def setUp(self):
+    ApiTestCase.setUp(self)
+    self._set_url(SuperUserMessages, username='freshuser')
+
+  def test_get_anonymous(self):
+    self._run_test('GET', 200, None, None)
+
+  def test_get_freshuser(self):
+    self._run_test('GET', 200, 'freshuser', None)
+
+  def test_get_reader(self):
+    self._run_test('GET', 200, 'reader', None)
+
+  def test_get_devtable(self):
+    self._run_test('GET', 200, 'devtable', None)
+
+
+  def test_post_anonymous(self):
+    self._run_test('POST', 403, None, dict(message={"content": "new message"}))
+
+  def test_post_freshuser(self):
+    self._run_test('POST', 403, 'freshuser', dict(message={"content": "new message"}))
+
+  def test_post_reader(self):
+    self._run_test('POST', 403, 'reader', dict(message={"content": "new message"}))
+
+  def test_post_devtable(self):
+    self._run_test('POST', 201, 'devtable', dict(message={"content": "new message"}))
+
 
 class TestUserInvoiceFieldList(ApiTestCase):
   def setUp(self):

test/test_api_usage.py

@@ -4283,7 +4283,7 @@ class TestSuperUserManagement(ApiTestCase):
     self.login(ADMIN_ACCESS_USER)
 
     # Create a message
-    self.postJsonResponse(SuperUserMessages, data=dict(message={"content": "new message"}))
+    self.postResponse(SuperUserMessages, data=dict(message={"content": "new message"}), expected_code=201)
 
     json = self.getJsonResponse(SuperUserMessages)