Improve comments following review

apel_rest/settings.py

@@ -163,7 +163,7 @@
 
 PROVIDERS_URL = 'provider_url'
 
-# The Hostnames of IAMs that can issue access tokens for the REST interface
+# List of hostnames of IAMs that can issue access tokens for the REST interface
 IAM_HOSTNAME_LIST = ['allowed_iams']
 SERVER_IAM_ID = 'server_iam_id'
 SERVER_IAM_SECRET = 'server_iam_secret'

api/tests/test_token_checker.py

@@ -30,7 +30,7 @@ def test_token_cache(self, mock_check_token_not_revoked,
         Check a cached token is granted access.
 
         Method does this by checking a token is valid twice, the first time
-        the token is validate and stored in a cache, the second time access
+        the token is validated and stored in a cache, the second time access
         should be granted because the token is in the cache, not because the
         token is valid.
         """
@@ -63,7 +63,7 @@ def test_token_cache_mis_match(self, mock_check_token_not_revoked,
         """
         Check tokens with the same subject are handled correctly.
 
-        Having a token cached for the sub should not be sufficent to grant
+        Having a token cached for the subject should not be sufficent to grant
         access, the tokens must match.
         """
         # Mock the external call to retrieve the IAM public key
@@ -78,7 +78,7 @@ def test_token_cache_mis_match(self, mock_check_token_not_revoked,
 
         # This payload has a subject that will be in the cache, but this
         # new token is not. We need to ensure this invalid token does not
-        # get granted rights based only on it's sub being in the cache
+        # get granted rights based only on it's subject being in the cache
         payload2 = {
             'iss': 'https://iam-test.idc.eu/',
             'jti': '098cb343-c45e-490d-8aa0-ce1873cdc5f8',
@@ -168,18 +168,16 @@ def test_is_token_issuer_trusted(self):
         """
         payload_list = []
 
-        # Add a payload without 'iss' field.
-        # to test we reject these as we cannot
-        # tell where it came from (so can't verify it)
+        # Test we reject a payload without 'iss' field
+        # as we cannot tell where it came from (so can't verify it)
         payload_list.append({
             'jti': '098cb343-c45e-490d-8aa0-ce1873cdc5f8',
             'iat': int(time.time()) - 2000000,
             'sub': CLIENT_ID,
             'exp': int(time.time()) + 200000})
 
-        # Add a payload with a malicious 'iss' field.
-        # to test we reject these as we do not wantt
-        # to attempt to verify it
+        # Test we reject a payload with a malicious 'iss' field
+        # as we do not want to attempt to verify it
         payload_list.append({
             'iss': 'https://malicious-iam.idc.biz/',
             'jti': '098cb343-c45e-490d-8aa0-ce1873cdc5f8',
@@ -212,42 +210,40 @@ def test_is_token_json_temporally_valid(self):
         """
         payload_list = []
 
-        # Add a payload wihtout 'iat' or 'exp' to the payload list
-        # to test we reject these (as we are choosing to)
+        # Test that we reject a payload without 'iat' or 'exp'
+        # as the tokens should have a lifetime
         payload_list.append({
             'sub': CLIENT_ID,
             'iss': 'https://iam-test.indigo-datacloud.eu/',
             'jti': '714892f5-014f-43ad-bea0-fa47579db222'})
 
-        # Add a payload without 'exp' to the payload_list
-        # to test we reject these (as we are choosing to)
+        # Test that we reject a payload without 'exp'
+        # as such a token would never expire
         payload_list.append({
             'iss': 'https://iam-test.indigo-datacloud.eu/',
             'jti': '098cb343-c45e-490d-8aa0-ce1873cdc5f8',
             'iat': int(time.time()) - 2000000,
             'sub': CLIENT_ID})
 
-        # Add a payload without 'iat'
-        # to test we reject these (as we are choosing to)
+        # Test that we reject a payload without 'iat'
+        # as all tokens should indicate when they were issued
         payload_list.append({
             'iss': 'https://iam-test.indigo-datacloud.eu/',
             'jti': '098cb343-c45e-490d-8aa0-ce1873cdc5f8',
             'sub': CLIENT_ID,
             'exp': int(time.time()) + 200000})
 
-        # Add a payload with an 'iat' and 'exp' in the past
-        # (e.g. they have expired) to test we are
-        # rejecting these
+        # Test that we reject a payload with an 'iat' and 'exp'
+        # in the past (e.g. they have expired)
         payload_list.append({
             'iss': 'https://iam-test.indigo-datacloud.eu/',
             'jti': '098cb343-c45e-490d-8aa0-ce1873cdc5f8',
             'iat': int(time.time()) - 2000000,
             'sub': CLIENT_ID,
             'exp': int(time.time()) - 200000})
 
-        # Add a payload with an 'iat' and 'exp' in the future
-        # to test we are rejecting these (as we should as they
-        # are not yet valid)
+        # Test that we reject a payload with an 'iat' and 'exp'
+        # in the future (as we should as they are not yet valid)
         payload_list.append({
             'iss': 'https://iam-test.indigo-datacloud.eu/',
             'jti': '098cb343-c45e-490d-8aa0-ce1873cdc5f8',
@@ -263,7 +259,7 @@ def test_is_token_json_temporally_valid(self):
                 self._token_checker._is_token_json_temporally_valid(payload),
                 "Payload %s should not be accepted!" % payload)
 
-            # Assert the wrapper method valid_token_to_id reutrns
+            # Assert the wrapper method valid_token_to_id returns
             # None when passed temporally invalid tokens
             token = self._create_token(payload, PRIVATE_KEY)
             self.assertEqual(
@@ -277,7 +273,7 @@ def test_garbage_token(self):
         self.assertEqual(result, None)
 
     def test_http_issuer_ban(self):
-        """Test a a HTTP issuer is rejected."""
+        """Test a HTTP issuer is rejected."""
         self.assertEqual(
             self._token_checker._check_token_not_revoked(None,
                                                          'http://idc.org'),
@@ -292,7 +288,7 @@ def _create_token(self, payload, key):
         return jwt.encode(payload, key, algorithm='RS256')
 
     def _standard_token(self):
-        """Return a token that will ve valid if properly signed."""
+        """Return a token that will be valid if properly signed."""
         return {
             'iss': 'https://iam-test.idc.eu/',
             'jti': '098cb343-c45e-490d-8aa0-ce1873cdc5f8',

api/utils/TokenChecker.py

@@ -20,7 +20,7 @@ def __init__(self):
         self.logger = logging.getLogger(__name__)
 
     def valid_token_to_id(self, token):
-        """Introspect a token to determine it's origin."""
+        """Introspect a token to determine its origin."""
         try:
             jwt_unverified_json = jwt.get_unverified_claims(token)
         except JWTError:
@@ -46,7 +46,7 @@ def valid_token_to_id(self, token):
 
         issuer = jwt_unverified_json['iss']
         # we can pass issuer because the previous 'if' statement
-        #  returns if jwt_unverified_json['iss'] is missing.
+        # returns if jwt_unverified_json['iss'] is missing.
         issuer = jwt_unverified_json['iss']
         if not self._verify_token(token, issuer):
             return None
@@ -58,10 +58,10 @@ def valid_token_to_id(self, token):
             return None
 
         self.logger.info('Token validated')
-        # if the execution gets here, we can cache the token
-        # cache is a key: value like structure with an optional timeout
-        # as we only need the token stored we use that as the key
-        # and None as the value.
+        # If the execution gets here, we can cache the token.
+        # The cache variable is a key: value like structure
+        # with an optional timeout, we use the token subject
+        # as the key and the token as the value.
         # Cache timeout set to 300 seconds to enable quick revocation
         # but also limit the number of requests to the IAM instance
         # Caching is also done after token validation to ensure