update pyjwt and move pyramid-jwtauth -> pyramid_jwt

the jwt secret will now be read from docker-compose.yml (with fallback for local dev) to avoid having it in the repo code

Author: lbesson

c2corg_api/__init__.py

@@ -15,6 +15,7 @@
 from c2corg_api.models import DBSession, Base
 from c2corg_api.search import configure_es_from_config, get_queue_config
 
+from pyramid.authorization import ACLAuthorizationPolicy
 from pyramid.settings import asbool
 
 log = logging.getLogger(__name__)
@@ -51,7 +52,19 @@ def main(global_config, **settings):
         bypass_auth = asbool(settings["noauthorization"])
 
     if not bypass_auth:
-        config.include("pyramid_jwtauth")
+        from c2corg_api.security.roles import groupfinder
+        from c2corg_api.security.jwt_policy import \
+            IntegerSubJWTAuthenticationPolicy
+        policy = IntegerSubJWTAuthenticationPolicy(
+            private_key=settings['jwt.private_key'],
+            algorithm='HS256',
+            auth_type='JWT',
+            callback=groupfinder,
+        )
+        config.set_authentication_policy(policy)
+        config.set_authorization_policy(ACLAuthorizationPolicy())
+        config.add_request_method(
+            policy.get_claims, 'jwt_claims', reify=True)
         # Intercept request handling to validate token against the database
         config.add_tween(
             "c2corg_api.tweens.jwt_database_validation."

c2corg_api/security/jwt_policy.py

@@ -0,0 +1,49 @@
+import re
+
+from pyramid_jwt import JWTAuthenticationPolicy
+
+# Matches token="<value>" in the Authorization header params
+_TOKEN_RE = re.compile(r'token="([^"]+)"')
+
+
+class IntegerSubJWTAuthenticationPolicy(JWTAuthenticationPolicy):
+    """Custom JWT authentication policy that handles the legacy
+    ``JWT token="<token>"`` header format used by the frontend,
+    and converts the ``sub`` claim back to an integer so that the
+    rest of the application can keep comparing
+    ``request.authenticated_userid`` with integer user ids.
+
+    PyJWT >= 2.9 enforces that ``sub`` must be a string (per the JWT
+    spec), but this application has historically stored integer user ids
+    in the ``sub`` claim.  We now encode ``sub`` as a string (see
+    :func:`c2corg_api.security.roles.create_claims`) and convert it
+    back here.
+    """
+
+    def get_claims(self, request):
+        """Extract claims from the request, supporting the legacy
+        ``JWT token="<value>"`` authorization header format.
+        """
+        try:
+            if request.authorization is None:
+                return {}
+        except ValueError:
+            return {}
+        (auth_type, params) = request.authorization
+        if auth_type != self.auth_type:
+            return {}
+        # Support legacy format: JWT token="<actual_token>"
+        match = _TOKEN_RE.search(params)
+        token = match.group(1) if match else params
+        if not token:
+            return {}
+        return self.jwt_decode(request, token)
+
+    def unauthenticated_userid(self, request):
+        sub = request.jwt_claims.get('sub')
+        if sub is not None:
+            try:
+                return int(sub)
+            except (TypeError, ValueError):
+                return sub
+        return None

c2corg_api/security/roles.py

@@ -2,6 +2,7 @@
 from pyramid.security import Authenticated
 from pyramid.interfaces import IAuthenticationPolicy
 
+import jwt as pyjwt
 from c2corg_api.models import DBSession
 from c2corg_api.models.user import User
 from c2corg_api.models.token import Token
@@ -19,9 +20,12 @@
 
 
 def extract_token(request):
-    # Extract XXX from 'JWT token="XXX"'
-    splitted = request.authorization[1].split('"')
-    return splitted[1] if len(splitted) >= 2 else None
+    # Extract token from 'JWT token="XXX"' or 'JWT XXX'
+    params = request.authorization[1]
+    if 'token="' in params:
+        splitted = params.split('"')
+        return splitted[1] if len(splitted) >= 2 else None
+    return params
 
 
 def groupfinder(userid, request):
@@ -70,7 +74,7 @@ def remove_token(token):
 
 def create_claims(user, exp):
     return {
-        'sub': user.id,
+        'sub': str(user.id),
         'username': user.username,
         'exp': int((exp - datetime.datetime(1970, 1, 1)).total_seconds())
     }
@@ -96,7 +100,8 @@ def log_validated_user_i_know_what_i_do(user, request):
     now = datetime.datetime.utcnow()
     exp = now + datetime.timedelta(days=CONST_EXPIRE_AFTER_DAYS)
     claims = create_claims(user, exp)
-    token = policy.encode_jwt(request, claims=claims).decode('utf-8')
+    token = pyjwt.encode(
+        claims, key=policy.private_key, algorithm=policy.algorithm)
     return add_or_retrieve_token(token, exp, user.id)
 
 
@@ -108,7 +113,8 @@ def renew_token(user, request):
         now = datetime.datetime.utcnow()
         exp = now + datetime.timedelta(days=CONST_EXPIRE_AFTER_DAYS)
         claims = create_claims(user, exp)
-        token_value = policy.encode_jwt(request, claims=claims).decode('utf-8')
+        token_value = pyjwt.encode(
+            claims, key=policy.private_key, algorithm=policy.algorithm)
         return add_or_retrieve_token(token_value, exp, user.id)
 
     return None

c2corg_api/tests/__init__.py

@@ -164,15 +164,14 @@ def _add_global_test_data(session):
 
     session.flush()
 
-    key = settings['jwtauth.master_secret']
+    key = settings['jwt.private_key']
     algorithm = 'HS256'
     now = datetime.datetime.utcnow()
     exp = now + datetime.timedelta(weeks=10)
 
     for user in users:
         claims = create_claims(user, exp)
-        token = jwt.encode(claims, key=key, algorithm=algorithm). \
-            decode('utf-8')
+        token = jwt.encode(claims, key=key, algorithm=algorithm)
         add_or_retrieve_token(token, exp, user.id)
         global_userids[user.username] = user.id
         global_tokens[user.username] = token

c2corg_api/tests/security/test_jwt_policy.py

@@ -0,0 +1,102 @@
+import jwt
+import unittest
+
+from webob import Request
+
+from c2corg_api.security.jwt_policy import IntegerSubJWTAuthenticationPolicy
+from c2corg_api.security.roles import extract_token
+
+
+class TestIntegerSubJWTAuthenticationPolicy(unittest.TestCase):
+
+    def setUp(self):
+        self.secret = 'a_long_enough_secret_key_for_hs256_testing'
+        self.policy = IntegerSubJWTAuthenticationPolicy(
+            private_key=self.secret,
+            algorithm='HS256',
+            auth_type='JWT',
+        )
+        self.claims = {
+            'sub': '12345',
+            'username': 'testuser',
+        }
+        self.token = jwt.encode(
+            self.claims, self.secret, algorithm='HS256')
+
+    def _make_request(self, authorization_header=None):
+        environ = {'REQUEST_METHOD': 'GET', 'PATH_INFO': '/'}
+        if authorization_header:
+            environ['HTTP_AUTHORIZATION'] = authorization_header
+        request = Request(environ)
+        # Attach jwt_claims for unauthenticated_userid
+        request.jwt_claims = self.policy.get_claims(request)
+        return request
+
+    def test_get_claims_legacy_format(self):
+        """JWT token="<token>" format used by the frontend."""
+        request = self._make_request(
+            'JWT token="' + self.token + '"')
+        claims = self.policy.get_claims(request)
+        self.assertEqual(claims['sub'], '12345')
+        self.assertEqual(claims['username'], 'testuser')
+
+    def test_get_claims_standard_format(self):
+        """JWT <token> format (standard Bearer-style)."""
+        request = self._make_request('JWT ' + self.token)
+        claims = self.policy.get_claims(request)
+        self.assertEqual(claims['sub'], '12345')
+        self.assertEqual(claims['username'], 'testuser')
+
+    def test_get_claims_no_authorization(self):
+        request = self._make_request()
+        claims = self.policy.get_claims(request)
+        self.assertEqual(claims, {})
+
+    def test_get_claims_wrong_auth_type(self):
+        request = self._make_request('Bearer ' + self.token)
+        claims = self.policy.get_claims(request)
+        self.assertEqual(claims, {})
+
+    def test_get_claims_invalid_token(self):
+        request = self._make_request('JWT not-a-valid-token')
+        claims = self.policy.get_claims(request)
+        self.assertEqual(claims, {})
+
+    def test_unauthenticated_userid_returns_int(self):
+        """sub claim is a string but unauthenticated_userid returns int."""
+        request = self._make_request(
+            'JWT token="' + self.token + '"')
+        userid = self.policy.unauthenticated_userid(request)
+        self.assertIsInstance(userid, int)
+        self.assertEqual(userid, 12345)
+
+    def test_unauthenticated_userid_standard_format(self):
+        request = self._make_request('JWT ' + self.token)
+        userid = self.policy.unauthenticated_userid(request)
+        self.assertIsInstance(userid, int)
+        self.assertEqual(userid, 12345)
+
+    def test_unauthenticated_userid_no_auth(self):
+        request = self._make_request()
+        request.jwt_claims = {}
+        userid = self.policy.unauthenticated_userid(request)
+        self.assertIsNone(userid)
+
+
+class TestExtractToken(unittest.TestCase):
+
+    def _make_request(self, authorization_header):
+        environ = {
+            'REQUEST_METHOD': 'GET',
+            'PATH_INFO': '/',
+            'HTTP_AUTHORIZATION': authorization_header,
+        }
+        return Request(environ)
+
+    def test_extract_token_legacy_format(self):
+        request = self._make_request('JWT token="my.jwt.token"')
+        self.assertEqual(extract_token(request), 'my.jwt.token')
+
+    def test_extract_token_standard_format(self):
+        request = self._make_request('JWT my.jwt.token')
+        self.assertEqual(extract_token(request), 'my.jwt.token')

common.ini.in

@@ -60,10 +60,8 @@ feed.admin_user_account = {feed_admin_user_account}
 
 logging.level = {logging_level}
 
-jwtauth.find_groups = c2corg_api.security.roles:groupfinder
-
-# FIXME: do not save the secret key on github
-jwtauth.master_secret = The master key
+# JWT signing key – MUST be at least 32 bytes for HS256
+jwt.private_key = {jwt_secret_key}
 
 # FIXME: do not save the forum key on github
 discourse.url = {discourse_url}

config/env.default

@@ -85,6 +85,10 @@ discourse_sso_secret="d836444a9e4084d5b224a60c208dce14"
 discourse_api_key="SET_API_KEY"
 image_backend_secret_key="test"
 
+# JWT signing key – MUST be at least 32 bytes for HS256.
+# Generate with: python3 -c "import secrets; print(secrets.token_urlsafe(32))"
+jwt_secret_key="CHANGE_ME_TO_A_REAL_SECRET______"
+
 skip_captcha_validation=false
 recaptcha_secret_key="6LfWUwoUAAAAABDo4_HRfru4HfLxOKmcqBRBObGj"
 

requirements.txt

@@ -13,10 +13,10 @@ kombu==5.6.2
 Markdown==3.7
 phpserialize==1.3.0 # phpserialize is only required during the migration
 psycopg2-binary==2.9.11
-pyjwt==1.7.1
+pyjwt==2.12.1
 pymdown-extensions==10.21.2
 pyproj==3.6.1
-pyramid-jwtauth==0.1.3
+pyramid_jwt==1.6.1
 pyramid==1.10.8
 pyramid_debugtoolbar==4.12.1
 pyramid_mailer==0.15.1

test.ini.in

@@ -6,7 +6,7 @@ pyramid.includes =
 sqlalchemy.url = postgresql://{tests_db_user}:{tests_db_password}@{tests_db_host}:{tests_db_port}/{tests_db_name}
 noauthorization = False
 debug_authorization = True
-jwtauth.master_secret = The master key
+jwt.private_key = {jwt_secret_key}
 elasticsearch.host = {tests_elasticsearch_host}
 elasticsearch.port = {tests_elasticsearch_port}
 elasticsearch.index = {tests_elasticsearch_index}