Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Commit

Permalink
bug: change "enable_simplepush" flag to "disable_simplepush"
Browse files Browse the repository at this point in the history
Due to a last minute change in the prior patch, the initial state of the
config option was improperly set. In addition the simplepush
registration was succeeding when it should have been rejected. Added
testing to verify both conditions are satisfied.

Closes #994
  • Loading branch information
jrconlin committed Aug 22, 2017
1 parent 2193a95 commit f2cdbf0
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 16 deletions.
4 changes: 2 additions & 2 deletions autopush/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class AutopushConfig(object):
# Use the cryptography library
use_cryptography = attrib(default=False) # type: bool
# Allow Simplepush Protocol (deprecated)
enable_simplepush = attrib(default=False) # type: bool
disable_simplepush = attrib(default=False) # type: bool

def __attrs_post_init__(self):
"""Initialize the Settings object"""
Expand Down Expand Up @@ -314,7 +314,7 @@ def from_argparse(cls, ns, **kwargs):
connect_timeout=ns.connection_timeout,
memusage_port=ns.memusage_port,
use_cryptography=ns.use_cryptography,
enable_simplepush=ns.enable_simplepush,
disable_simplepush=ns.disable_simplepush,
router_table=dict(
tablename=ns.router_tablename,
read_throughput=ns.router_read_throughput,
Expand Down
2 changes: 1 addition & 1 deletion autopush/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def __init__(self,
routers, # type: Dict[str, IRouter]
**kwargs):
# type: (...) -> None
if conf.enable_simplepush:
if not conf.disable_simplepush:
self.ap_handlers += (
(r"/spush/(?:(?P<api_ver>v\d+)\/)?(?P<token>[^\/]+)",
SimplePushHandler),
Expand Down
8 changes: 4 additions & 4 deletions autopush/main_argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ def add_shared_args(parser):
help="Use the cryptography library vs. JOSE",
action="store_true",
default=False, env_var="USE_CRYPTOGRAPHY")
parser.add_argument('--enable_simplepush',
help="Enable the deprecated Simplepush protocol",
action="store_true", default=True,
env_var="ENABLE_SIMPLEPUSH")
parser.add_argument('--disable_simplepush',
help="Disable the deprecated Simplepush protocol",
action="store_true", default=False,
env_var="DISABLE_SIMPLEPUSH")
# No ENV because this is for humans
_add_external_router_args(parser)
_obsolete_args(parser)
Expand Down
2 changes: 1 addition & 1 deletion autopush/router/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def routers_from_config(conf, db, agent):
routers = dict(
webpush=WebPushRouter(conf, None, db, agent)
)
if conf.enable_simplepush:
if not conf.disable_simplepush:
routers['simplepush'] = SimpleRouter(
conf, router_conf.get("simplepush"), db, agent)
if 'apns' in router_conf:
Expand Down
9 changes: 7 additions & 2 deletions autopush/tests/test_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def setUp(self):
hostname="localhost",
statsd_host=None,
crypto_key='AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=',
enable_simplepush=True,
disable_simplepush=False,
)
db = test_db()
self.message_mock = db.message = Mock(spec=Message)
Expand Down Expand Up @@ -139,7 +139,7 @@ def setUp(self):
hostname="localhost",
statsd_host=None,
bear_hash_key='AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB=',
enable_simplepush=True,
disable_simplepush=False,
)
self.fernet_mock = conf.fernet = Mock(spec=Fernet)

Expand Down Expand Up @@ -188,6 +188,11 @@ def test_base_tags(self):
tags = self.reg.base_tags()
eq_(tags, ['user_agent:test', 'host:example.com:8080'])

def test_disable_simplepush(self):
self.conf.disable_simplepush = True
routers = routers_from_config(self.conf, self.db, Mock())
ok_("simplepush" not in routers)

def _check_error(self, resp, code, errno, error, message=None):
d = json.loads(resp.content)
eq_(d.get("code"), code)
Expand Down
6 changes: 3 additions & 3 deletions autopush/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,12 @@ def hello(self, uaid=None):
self.ws.send(msg)
result = json.loads(self.ws.recv())
log.debug("Recv: %s", result)
eq_(result["status"], 200)
if self.uaid and self.uaid != result["uaid"]: # pragma: nocover
log.debug("Mismatch on re-using uaid. Old: %s, New: %s",
self.uaid, result["uaid"])
self.channels = {}
self.uaid = result["uaid"]
eq_(result["status"], 200)
return result

def register(self, chid=None, key=None):
Expand Down Expand Up @@ -309,7 +309,7 @@ class IntegrationBase(unittest.TestCase):
storage_table=dict(tablename=STORAGE_TABLE),
message_table=dict(tablename=MESSAGE_TABLE),
use_cryptography=True,
enable_simplepush=True,
disable_simplepush=False,
)

_conn_defaults = dict(
Expand All @@ -323,7 +323,7 @@ class IntegrationBase(unittest.TestCase):
storage_table=dict(tablename=STORAGE_TABLE),
message_table=dict(tablename=MESSAGE_TABLE),
use_cryptography=True,
enable_simplepush=True,
disable_simplepush=False,
)

def setUp(self):
Expand Down
2 changes: 1 addition & 1 deletion autopush/tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ class TestArg:

proxy_protocol_port = None
memusage_port = None
enable_simplepush = False
disable_simplepush = True
use_cryptography = False

def setUp(self):
Expand Down
10 changes: 10 additions & 0 deletions autopush/tests/test_websocket.py
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,16 @@ def test_hello_check_fail(self):
eq_(msg["status"], 500)
eq_(msg["reason"], "already_connected")

@inlineCallbacks
def test_hello_no_simplepush(self):
state = self.conf.disable_simplepush
self.conf.disable_simplepush = True
self._connect()
self._send_message(dict(messageType="hello", channelIDs=[]))
msg = yield self.get_response()
eq_(msg["status"], 401)
self.conf.disable_simplepush = state

@inlineCallbacks
def test_hello_dupe(self):
self._connect()
Expand Down
9 changes: 7 additions & 2 deletions autopush/websocket.py
Original file line number Diff line number Diff line change
Expand Up @@ -735,8 +735,13 @@ def process_hello(self, data):
if self.ps.uaid:
return self.returnError("hello", "duplicate hello", 401)

conn_type = "webpush" if data.get("use_webpush", False) else \
"simplepush"
if data.get("use_webpush", False):
conn_type = "webpush"
else:
if self.conf.disable_simplepush:
return self.returnError("hello", "Simplepush not supported",
401)
conn_type = "simplepush"
self.ps.set_connection_type(conn_type)

uaid = data.get("uaid")
Expand Down

0 comments on commit f2cdbf0

Please sign in to comment.