From f4bb278835bb895dab6fc58cc79753bbded39738 Mon Sep 17 00:00:00 2001
From: Ian Gulliver <ian@isinglasstheater.org>
Date: Mon, 2 Jun 2014 21:53:51 -0700
Subject: [PATCH] Impose access controls for subscribe even without a channel
 open.

---
 api.py        | 25 ++++++++++++++-----------
 lib/models.py | 12 ++++++------
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/api.py b/api.py
index a945605..be83784 100644
--- a/api.py
+++ b/api.py
@@ -118,6 +118,15 @@ def Subscribe(google_user, client, instance_id, args):
   messages = args.get('messages', 0)
   last_id = args.get('last_id', None)
 
+  try:
+    subject.VerifyReadable(models.Client.profile.get_value_for_datastore(client))
+  except models.AccessDenied:
+    logging.warning('Subscribe access denied')
+    return {
+      'result': 'access_denied',
+    }
+
+
   if not instance or not instance.active:
     # Probably a race with the channel opening
     return {
@@ -125,17 +134,11 @@ def Subscribe(google_user, client, instance_id, args):
       'events': subject.GetEvents(messages, last_id),
     }
 
-  try:
-    return {
-      'result': 'ok',
-      'events': models.Subscription.FindOrCreate(
-          subject, client, instance, messages, last_id),
-    }
-  except models.AccessDenied:
-    logging.warning('Subscribe access denied')
-    return {
-      'result': 'access_denied',
-    }
+  return {
+    'result': 'ok',
+    'events': models.Subscription.FindOrCreate(
+        subject, client, instance, messages, last_id),
+  }
 
 
 def Unpin(google_user, client, instance_id, args):
diff --git a/lib/models.py b/lib/models.py
index 36aa347..bbeac49 100644
--- a/lib/models.py
+++ b/lib/models.py
@@ -223,6 +223,12 @@ class Subject(db.Model):
         writable_only_by != sender):
       raise AccessDenied
 
+  def VerifyReadable(self, reader):
+    readable_only_by = Subject.readable_only_by.get_value_for_datastore(self)
+    if (readable_only_by and
+        readable_only_by != reader):
+      raise AccessDenied
+
   def SendMessage(self, message, sender, sender_message_id):
     self.VerifyWritable(sender)
     obj, subscriptions = self.PutMessage(message, sender, sender_message_id)
@@ -315,12 +321,6 @@ class Subscription(db.Model):
   @classmethod
   @db.transactional()
   def FindOrCreate(cls, subject, client, instance, messages=0, last_id=None):
-    readable_only_by = (
-        Subject.readable_only_by.get_value_for_datastore(subject))
-    if (readable_only_by and
-        readable_only_by != Client.profile.get_value_for_datastore(client)):
-      raise AccessDenied
-
     subscriptions = (
         cls.all(keys_only=True)
         .ancestor(subject)