[style] improve naming and fixes from code review

-- Related: #8867

11 files changed, 64 insertions, 58 deletions

diff --git a/docs/auth.rst b/docs/auth.rst
index 06427a01..ad454005 100644
--- a/docs/auth.rst
+++ b/docs/auth.rst
@@ -13,12 +13,12 @@ client.
 There are currently two distinct authenticated entry points:
 
 * A public TLS encrypted **Users API**, providing the *Synchronization*,
-  *Blobs* and *Incoming* services, verified against the Leap Platform
+  *Blobs* services, verified against the Leap Platform
   ``tokens`` database.
 
 * A local plaintext **Services API**, providing the delivery part of the
-  *Incoming* service, authenticated against tokens defined in the server
-  configuration file.
+  *Incoming* service, authenticated against tokens defined in a file specified
+  on the server configuration file.
 
 Authorization header
 --------------------
@@ -34,13 +34,9 @@ server (as the version of the server and runtime configuration options).
 Special credentials for local services
 --------------------------------------
 
-Some special credentials can be configured in the Soledad Server configuration
-file. Currently, the only special credential provided is for the `/incoming`
-API, and defaults to the value `mx:default_mx_token`.
-
-If a credential header is sent in the request and the uuid is not one in a
-special credential configured in the Soledad Server configuration file, then a
-CouchDB database called `tokens` is consulted to check for a valid token.
+Some special credentials can be added into a file and then configured in the
+Soledad Server configuration file. Currently, the only special credential
+provided is for the `/incoming` API.
 
 Implementation
 --------------
@@ -50,8 +46,10 @@ daemon that loads a `.tac file
 <https://twistedmatrix.com/documents/12.2.0/core/howto/application.html#auto5>`_.
 When the server is started, two services are spawned:
 
-* A local entrypoint for services (serving on localhost only on port 2323).
-* A public entrypoint for users (serving on public IP on port 2424).
+* A local entrypoint for services (serving on localhost only).
+* A public entrypoint for users (serving on public IP).
+* Localhost and public IP ports are configurable. Default is 2424 for public IP
+  and 2525 for localhost.
 
 .. code-block:: none
 
@@ -61,7 +59,7 @@ When the server is started, two services are spawned:
     '------------------------------------------------------'
        |                                                |
     .--------------.                      .----------------.
-    | 0.0.0.0:2424 |                      | 127.0.0.1:2323 |
+    | 0.0.0.0:2424 |                      | 127.0.0.1:2525 |
     |     (TLS)    |                      |     (TCP)      |
     '--------------'                      '----------------'
        |                                                |
@@ -79,8 +77,4 @@ When the server is started, two services are spawned:
        |  '-------'                | (delivery only) |
        |  .--------.               '-----------------'
        '->| /blobs |
-       |  '--------'
-       |  .-------------.
-       '->|  /incoming  |
-          | (users API) |
-          '-------------'
+          '--------'
diff --git a/pkg/server/soledad-server.service b/pkg/server/soledad-server.service
index 73cc5920..0e17f245 100644
--- a/pkg/server/soledad-server.service
+++ b/pkg/server/soledad-server.service
@@ -3,6 +3,7 @@ Description=Soledad Server
 
 [Service]
 Environment=PATH=/sbin:/bin:/usr/sbin:/usr/bin
+# TODO: Change this dist-packages path as it's Debian specific
 Environment=TAC=/usr/lib/python2.7/dist-packages/leap/soledad/server/server.tac
 Environment=HTTPS_PORT=2424
 Environment=LOCAL_SERVICES_PORT=2525
@@ -17,8 +18,7 @@ ExecStart=/usr/bin/twistd \
   --pidfile= \
   --syslog \
   --prefix=soledad-server \
-  --python \
-  ${TAC}
+  --python=${TAC}
 
 WorkingDirectory=/var/lib/soledad/
 
diff --git a/src/leap/soledad/server/_config.py b/src/leap/soledad/server/_config.py
index 3f3d7640..12c286f5 100644
--- a/src/leap/soledad/server/_config.py
+++ b/src/leap/soledad/server/_config.py
@@ -31,7 +31,7 @@ CONFIG_DEFAULTS = {
         'batching': True,
         'blobs': False,
         'blobs_path': '/srv/leap/soledad/blobs',
-        'services_tokens_file': '/dev/null',
+        'services_tokens_file': '/etc/soledad/incoming.tokens',
     },
     'database-security': {
         'members': ['soledad'],
diff --git a/src/leap/soledad/server/_resource.py b/src/leap/soledad/server/_resource.py
index a9f854b6..7b326fef 100644
--- a/src/leap/soledad/server/_resource.py
+++ b/src/leap/soledad/server/_resource.py
@@ -24,7 +24,7 @@ from ._incoming import IncomingResource
 from ._wsgi import get_sync_resource
 
 
-__all__ = ['SoledadResource', 'SoledadAnonResource']
+__all__ = ['PublicResource', 'SoledadAnonResource']
 
 
 class _Robots(Resource):
@@ -60,7 +60,7 @@ class LocalResource(Resource):
         self.putChild('incoming', IncomingResource())
 
 
-class SoledadResource(Resource):
+class PublicResource(Resource):
     """
     This is a dummy twisted resource, used only to allow different entry points
     for the Soledad Server.
diff --git a/src/leap/soledad/server/auth.py b/src/leap/soledad/server/auth.py
index 4dbe9a6d..89626ead 100644
--- a/src/leap/soledad/server/auth.py
+++ b/src/leap/soledad/server/auth.py
@@ -17,6 +17,7 @@
 """
 Twisted http token auth.
 """
+import os
 import binascii
 import time
 
@@ -38,7 +39,7 @@ from twisted.web.resource import IResource
 
 from leap.soledad.common.couch import couch_server
 
-from ._resource import SoledadResource, SoledadAnonResource
+from ._resource import PublicResource, SoledadAnonResource
 from ._resource import LocalResource
 from ._blobs import BlobsResource
 from ._config import get_config
@@ -59,7 +60,7 @@ class SoledadRealm(object):
                                        conf['blobs_path']) if blobs else None
         self.anon_resource = SoledadAnonResource(
             enable_blobs=blobs)
-        self.auth_resource = SoledadResource(
+        self.auth_resource = PublicResource(
             blobs_resource=blobs_resource,
             sync_pool=sync_pool)
 
@@ -81,9 +82,8 @@ class SoledadRealm(object):
 @implementer(IRealm)
 class LocalServicesRealm(object):
 
-    def __init__(self, conf=None):
-        if conf is None:
-            conf = get_config()
+    def __init__(self):
+        conf = get_config()
         self.anon_resource = SoledadAnonResource(
             enable_blobs=conf['blobs'])
         self.auth_resource = LocalResource()
@@ -108,12 +108,16 @@ class FileTokenChecker(object):
     credentialInterfaces = [IUsernamePassword, IAnonymous]
 
     def __init__(self, conf=None):
+        # conf parameter is only used during tests
         conf = conf or get_config()
         self._trusted_services_tokens = {}
         self._tokens_file_path = conf['services_tokens_file']
         self._reload_tokens()
 
     def _reload_tokens(self):
+        if not os.path.isfile(self._tokens_file_path):
+            log.warn("No local token auth file at %s" % self._tokens_file_path)
+            return
         with open(self._tokens_file_path) as tokens_file:
             for line in tokens_file.readlines():
                 line = line.strip()
@@ -128,6 +132,7 @@ class FileTokenChecker(object):
         service = credentials.username
         token = credentials.password
 
+        # TODO: Use constant time comparison
         if self._trusted_services_tokens[service] != token:
             return defer.fail(error.UnauthorizedLogin())
 
@@ -221,16 +226,17 @@ class TokenCredentialFactory(object):
             raise error.LoginFailed('Invalid credentials')
 
 
-def portalFactory(public=True, sync_pool=None):
+def publicPortal(sync_pool):
     database_checker = CouchDBTokenChecker()
+    realm = SoledadRealm(sync_pool=sync_pool)
+    auth_checkers = [database_checker]
+    return Portal(realm, auth_checkers)
+
+
+def localPortal():
     file_checker = FileTokenChecker()
-    if public:
-        assert sync_pool
-        realm = SoledadRealm(sync_pool=sync_pool)
-        auth_checkers = [database_checker]
-    else:
-        realm = LocalServicesRealm()
-        auth_checkers = [file_checker, database_checker]
+    realm = LocalServicesRealm()
+    auth_checkers = [file_checker]
     return Portal(realm, auth_checkers)
 
 
diff --git a/src/leap/soledad/server/entrypoint.py b/src/leap/soledad/server/entrypoints.py
index 7115007b..ff2f333a 100644
--- a/src/leap/soledad/server/entrypoint.py
+++ b/src/leap/soledad/server/entrypoints.py
@@ -24,7 +24,7 @@ or the systemd script.
 from twisted.internet import reactor
 from twisted.python import threadpool
 
-from .auth import portalFactory
+from .auth import localPortal, publicPortal
 from .session import SoledadSession
 from ._config import get_config
 from ._wsgi import init_couch_state
@@ -40,14 +40,14 @@ class SoledadEntrypoint(SoledadSession):
         pool = threadpool.ThreadPool(name='wsgi')
         reactor.callWhenRunning(pool.start)
         reactor.addSystemEventTrigger('after', 'shutdown', pool.stop)
-        portal = portalFactory(public=True, sync_pool=pool)
+        portal = publicPortal(sync_pool=pool)
         SoledadSession.__init__(self, portal)
 
 
 class LocalServicesEntrypoint(SoledadSession):
 
     def __init__(self):
-        portal = portalFactory(public=False)
+        portal = localPortal()
         SoledadSession.__init__(self, portal)
 
 # see the comments in application.py recarding why couch state has to be
diff --git a/src/leap/soledad/server/server.tac b/src/leap/soledad/server/server.tac
index b443e632..1a4e53ee 100644
--- a/src/leap/soledad/server/server.tac
+++ b/src/leap/soledad/server/server.tac
@@ -5,14 +5,14 @@ from twisted.application import service, strports
 from twisted.web import server
 from twisted.python import log
 
-from leap.soledad.server import entrypoint
+from leap.soledad.server import entrypoints
 
 application = service.Application('soledad-server')
 
 # local entrypoint
-local_port = os.getenv('LOCAL_SERVICES_PORT', 2323)
+local_port = os.getenv('LOCAL_SERVICES_PORT', 2525)
 local_description = 'tcp:%s:interface=127.0.0.1' % local_port
-local_site = server.Site(entrypoint.LocalServicesEntrypoint())
+local_site = server.Site(entrypoints.LocalServicesEntrypoint())
 
 local_server = strports.service(local_description, local_site)
 local_server.setServiceParent(application)
@@ -33,9 +33,13 @@ if port:
         'privateKey=' + privateKey,
         'certKey=' + certKey,
         'sslmethod=' + sslmethod])
-else:
+elif os.getenv('DEBUG_SERVER', False):
     public_description = 'tcp:port=2424:interface=0.0.0.0'
-public_site = server.Site(entrypoint.SoledadEntrypoint())
+else:
+    log.err("HTTPS_PORT env var is required to be set!")
+    sys.exit(20)
+
+public_site = server.Site(entrypoints.SoledadEntrypoint())
 
 public_server = strports.service(public_description, public_site)
 public_server.setServiceParent(application)
diff --git a/testing/tests/conftest.py b/testing/tests/conftest.py
index 817a43b0..87742dfe 100644
--- a/testing/tests/conftest.py
+++ b/testing/tests/conftest.py
@@ -126,7 +126,7 @@ class SoledadServer(object):
             '--logfile=%s' % self._logfile,
             '--pidfile=%s' % self._pidfile,
             'web',
-            '--class=leap.soledad.server.entrypoint.SoledadEntrypoint',
+            '--class=leap.soledad.server.entrypoints.SoledadEntrypoint',
             '--port=tcp:2424'
         ])
 
diff --git a/testing/tests/server/test__resource.py b/testing/tests/server/test__resource.py
index f5331e35..a43ac19f 100644
--- a/testing/tests/server/test__resource.py
+++ b/testing/tests/server/test__resource.py
@@ -23,7 +23,7 @@ from twisted.web.wsgi import WSGIResource
 from twisted.web.resource import getChildForRequest
 from twisted.internet import reactor
 
-from leap.soledad.server._resource import SoledadResource
+from leap.soledad.server._resource import PublicResource
 from leap.soledad.server._resource import LocalResource
 from leap.soledad.server._server_info import ServerInfo
 from leap.soledad.server._blobs import BlobsResource
@@ -34,11 +34,11 @@ from leap.soledad.server.gzip_middleware import GzipMiddleware
 _pool = reactor.getThreadPool()
 
 
-class SoledadResourceTestCase(unittest.TestCase):
+class PublicResourceTestCase(unittest.TestCase):
 
     def test_get_root(self):
         blobs_resource = None  # doesn't matter
-        resource = SoledadResource(
+        resource = PublicResource(
             blobs_resource=blobs_resource, sync_pool=_pool)
         request = DummyRequest([''])
         child = getChildForRequest(resource, request)
@@ -46,7 +46,7 @@ class SoledadResourceTestCase(unittest.TestCase):
 
     def test_get_blobs_enabled(self):
         blobs_resource = BlobsResource("filesystem", '/tmp')
-        resource = SoledadResource(
+        resource = PublicResource(
             blobs_resource=blobs_resource, sync_pool=_pool)
         request = DummyRequest(['blobs'])
         child = getChildForRequest(resource, request)
@@ -54,7 +54,7 @@ class SoledadResourceTestCase(unittest.TestCase):
 
     def test_get_blobs_disabled(self):
         blobs_resource = None
-        resource = SoledadResource(
+        resource = PublicResource(
             blobs_resource=blobs_resource, sync_pool=_pool)
         request = DummyRequest(['blobs'])
         child = getChildForRequest(resource, request)
@@ -64,7 +64,7 @@ class SoledadResourceTestCase(unittest.TestCase):
 
     def test_get_sync(self):
         blobs_resource = None  # doesn't matter
-        resource = SoledadResource(
+        resource = PublicResource(
             blobs_resource=blobs_resource, sync_pool=_pool)
         request = DummyRequest(['user-db', 'sync-from', 'source-id'])
         child = getChildForRequest(resource, request)
@@ -72,9 +72,10 @@ class SoledadResourceTestCase(unittest.TestCase):
         self.assertIsInstance(child._application, GzipMiddleware)
 
     def test_no_incoming_on_public_resource(self):
-        resource = SoledadResource(None, sync_pool=_pool)
+        resource = PublicResource(None, sync_pool=_pool)
         request = DummyRequest(['incoming'])
         child = getChildForRequest(resource, request)
+        # WSGIResource is returned if a path is unknown
         self.assertIsInstance(child, WSGIResource)
 
     def test_get_incoming(self):
diff --git a/testing/tests/server/test_auth.py b/testing/tests/server/test_auth.py
index 85dd5ecf..78cf20ab 100644
--- a/testing/tests/server/test_auth.py
+++ b/testing/tests/server/test_auth.py
@@ -36,7 +36,7 @@ from leap.soledad.server.auth import SoledadRealm
 from leap.soledad.server.auth import CouchDBTokenChecker
 from leap.soledad.server.auth import FileTokenChecker
 from leap.soledad.server.auth import TokenCredentialFactory
-from leap.soledad.server._resource import SoledadResource
+from leap.soledad.server._resource import PublicResource
 
 
 class SoledadRealmTestCase(unittest.TestCase):
@@ -47,7 +47,7 @@ class SoledadRealmTestCase(unittest.TestCase):
         pool = reactor.getThreadPool()
         realm = SoledadRealm(conf=conf, sync_pool=pool)
         iface, avatar, logout = realm.requestAvatar('any', None, IResource)
-        self.assertIsInstance(avatar, SoledadResource)
+        self.assertIsInstance(avatar, PublicResource)
         self.assertIsNone(logout())
 
 
@@ -69,7 +69,7 @@ def dummy_server(token):
     yield collections.defaultdict(lambda: DummyServer(token))
 
 
-class DatabaseTokenCheckerTestCase(unittest.TestCase):
+class CouchDBTokenCheckerTestCase(unittest.TestCase):
 
     @inlineCallbacks
     def test_good_creds(self):
diff --git a/testing/tests/server/test_tac.py b/testing/tests/server/test_tac.py
index e825a9ce..7bb50e35 100644
--- a/testing/tests/server/test_tac.py
+++ b/testing/tests/server/test_tac.py
@@ -45,7 +45,7 @@ class TacServerTestCase(unittest.TestCase):
     @defer.inlineCallbacks
     def test_local_public_default_ports_on_server_tac(self):
         yield self._spawnServer()
-        result = yield self._get('http://localhost:2323/incoming')
+        result = yield self._get('http://localhost:2525/incoming')
         fail_msg = "Localhost endpoint must require authentication!"
         self.assertEquals(401, result.code, fail_msg)
 
@@ -56,7 +56,7 @@ class TacServerTestCase(unittest.TestCase):
         result = yield self._get(public_endpoint_url + 'other')
         self.assertEquals(401, result.code, "public server lacks auth!")
 
-        public_using_local_port_url = 'http://%s:2323/' % self._get_public_ip()
+        public_using_local_port_url = 'http://%s:2525/' % self._get_public_ip()
         with pytest.raises(Exception):
             yield self._get(public_using_local_port_url)
 
@@ -66,7 +66,8 @@ class TacServerTestCase(unittest.TestCase):
         executable = os.path.join(env, 'bin', 'twistd')
         no_pid_argument = '--pidfile='
         args = [executable, no_pid_argument, '-noy', TAC_FILE_PATH]
-        t = reactor.spawnProcess(protocol, executable, args)
+        env = {'DEBUG_SERVER': 'yes'}
+        t = reactor.spawnProcess(protocol, executable, args, env=env)
         self.addCleanup(os.kill, t.pid, signal.SIGKILL)
         self.addCleanup(t.loseConnection)
         return self._sleep(1)  # it takes a while to start server