Merge pull request #135 from freedomofpress/fix-auth-setup

Fix TOTP retry logic, improve cassette handling; document

README.md

@@ -80,12 +80,23 @@ The steps to generate new cassettes are split into two sections based on communi
     NUM_SOURCES=5 make dev
     ```
 
-2. [Skip if adding a new test] Delete the cassettes you wish to regenerate or just delete all yaml files by running:
+2. Delete the cassettes you wish to regenerate or just delete all yaml files by running:
 
     ```bash
     rm data/*.yml
     ```
 
+   If you are only adding a new test and not modifying existing ones, you can
+   skip this step, but you still need to remove the authentication setup during
+   cassette generation. Otherwise you will get 403 errors for API endpoints that
+   require a valid token. Remove the setup cassette by running:
+
+   ```bash
+   rm data/test-setup.yml
+   ```
+
+   (You can reinstate the unmodified version later.)
+
 3. Generate new cassettes that make API calls over HTTP by running:
 
     ```bash
@@ -174,7 +185,7 @@ name=sd-dev-proxy
 sd-dev sd-dev-proxy allow
 ```
 
-8. Modify `/etc/qubes-rpc/qubes.Filecopy` in **dom0** by adding the following line so that the proxy can send files over qrexec to the sdk:
+8. Modify `/etc/qubes-rpc/policy/qubes.Filecopy` in **dom0** by adding the following line to the top of the file so that the proxy can send files over qrexec to the sdk:
 
 ```
 sd-dev-proxy sd-dev allow

tests/test_api.py

@@ -14,39 +14,46 @@
 from sdclientapi import API, RequestTimeoutError
 from sdclientapi.sdlocalobjects import (
     AuthError,
-    BaseError,
     Reply,
     ReplyError,
     Source,
     Submission,
     WrongUUIDError,
 )
-from utils import load_auth_for_http, save_auth_for_http
 
 NUM_REPLIES_PER_SOURCE = 2
 
 
 class TestAPI(unittest.TestCase):
-    @vcr.use_cassette("data/test-setup.yml")
     def setUp(self):
         self.totp = pyotp.TOTP("JHCOGO7VCER3EJ4L")
         self.username = "journalist"
         self.password = "correct horse battery staple profanity oil chewy"
         self.server = "http://127.0.0.1:8081/"
-        self.api = API(self.server, self.username, self.password, str(self.totp.now()))
-        for i in range(3):
-            try:
-                self.api.authenticate()
-            except BaseError:
-                token = load_auth_for_http()
-                if token:
-                    self.api.token = token
-                    self.api.update_auth_header()
-                    break
-                time.sleep(31)
-
-            save_auth_for_http(self.api.token)
-            break
+
+        # Because we may be using a TOTP code from a previous run that has since
+        # been invalidated (or that may be invalid because of bad timing),
+        # we retry repeatedly to get the token with a new TOTP code.
+        #
+        # It doesn't matter if these intermittent 403s are captured in the
+        # cassette as we ignore them during playback.
+        auth_result = None
+        with vcr.use_cassette("data/test-setup.yml") as cassette:
+            for i in range(3):
+                totp = self.totp.now()
+                self.api = API(self.server, self.username, self.password, str(totp))
+                try:
+                    auth_result = self.api.authenticate()
+                except AuthError:
+                    # Don't sleep on final retry attempt or during playback
+                    if i < 2 and cassette.play_count == 0:
+                        time.sleep(31)
+                    continue
+                # No error, let's move on
+                break
+
+        if auth_result is None:
+            raise AuthError("Could not obtain API token during test setup.")
 
     @vcr.use_cassette("data/test-baduser.yml")
     def test_auth_baduser(self):

tests/utils.py

@@ -123,16 +123,3 @@ def load_auth():
 def save_auth(token):
     with open("testtoken.json", "w") as fobj:
         json.dump(token, fobj)
-
-
-def load_auth_for_http():
-    "Helper function to load token"
-    if os.path.exists("testtoken_http.json"):
-        with open("testtoken_http.json") as fobj:
-            return json.load(fobj)
-    return None
-
-
-def save_auth_for_http(token):
-    with open("testtoken_http.json", "w") as fobj:
-        json.dump(token, fobj)