Fix JWT token parsing and generation.

sciactive · Jan 12, 2021 · 1410c75 · 1410c75
1 parent d6b0308
commit 1410c75
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 54 deletions.
diff --git a/conf/defaults.php b/conf/defaults.php
@@ -7,10 +7,14 @@
  * @see http://tilmeld.org/
  */
 
-use Lcobucci\JWT\Builder;
+use Lcobucci\JWT\Signer\Key\InMemory;
 use Lcobucci\JWT\Signer\Hmac\Sha256;
-use Lcobucci\JWT\Parser;
-use Lcobucci\JWT\ValidationData;
+use Lcobucci\JWT\Configuration;
+use Lcobucci\JWT\Token\Plain;
+use Lcobucci\JWT\Validation\Constraint\SignedWith;
+use Lcobucci\Clock\SystemClock;
+use Lcobucci\JWT\Validation\Constraint\ValidAt;
+use Lcobucci\JWT\Validation\RequiredConstraintsViolated;
 use Respect\Validation\Validator as v;
 
 // phpcs:disable Generic.Files.LineLength.TooLong,Squiz.WhiteSpace.ObjectOperatorSpacing.Before
@@ -182,36 +186,37 @@
    * JWT Expire
    * How long from current time, in seconds, the JWT token expires.
    */
-  'jwt_expire' => 60 * 60 * 24 * 14, // Two weeks(ish)
+  'jwt_expire' => 60 * 60 * 24 * 7 * 8, // 8 weeks
   /*
    * JWT Renew
    * How long before the JWT token expires to give the user a new one.
    */
-  'jwt_renew' => 60 * 60 * 24 * 8, // 8 days(ish)
+  'jwt_renew' => 60 * 60 * 24 * 7 * 3, // 3 weeks
   /*
    * JWT Builder
    * Function to build the JWT for user sessions.
    */
   'jwt_builder' => function ($user) {
-    $erPrev = \error_reporting();
-    // Workaround for a deprecated warning in Lcobucci\JWT.
-    \error_reporting($erPrev & ~E_DEPRECATED & ~E_NOTICE);
     $secret = \Tilmeld\Tilmeld::$config['jwt_secret'];
     if (!isset($secret)) {
       throw new \Exception('JWT secret is not configured.');
     }
 
-    $signer = new Sha256();
-    $token = (new Builder())
-      ->setIssuedAt(time())
-      ->setNotBefore(time())
-      ->setExpiration(time() + \Tilmeld\Tilmeld::$config['jwt_expire'])
-      ->set('guid', $user->guid)
-      ->set('xsrfToken', uniqid('TILMELDXSRF-', true))
-      ->sign($signer, $secret)
-      ->getToken();
-    \error_reporting($erPrev);
-    return $token;
+    $key = InMemory::plainText($secret);
+    $config = Configuration::forSymmetricSigner(
+      new Sha256(),
+      $key
+    );
+
+    $now = new \DateTimeImmutable('now', new DateTimeZone('UTC'));
+    $token = $config->builder()
+      ->issuedAt($now)
+      ->canOnlyBeUsedAfter($now)
+      ->expiresAt($now->modify('+'.\Tilmeld\Tilmeld::$config['jwt_expire'].' seconds'))
+      ->relatedTo($user->guid)
+      ->withClaim('xsrfToken', uniqid('TILMELDXSRF-', true))
+      ->getToken($config->signer(), $config->signingKey());
+    return $token->toString();
   },
   /*
    * JWT Extract
@@ -223,41 +228,48 @@
    * Return false if the JWT is not valid, or an array of GUID and expire
    * timestamp otherwise.
    */
-  'jwt_extract' => function ($token, $xsrfToken = null) {
-    $erPrev = \error_reporting();
-    // Workaround for a deprecated warning in Lcobucci\JWT.
-    \error_reporting($erPrev & ~E_DEPRECATED & ~E_NOTICE);
+  'jwt_extract' => function ($jwt, $xsrfToken = null) {
     $secret = \Tilmeld\Tilmeld::$config['jwt_secret'];
     if (!isset($secret)) {
       throw new \Exception('JWT secret is not configured.');
     }
 
-    $token = (new Parser())->parse($token);
-    $signer = new Sha256();
-    if (!$token->verify($signer, $secret)) {
-      return false;
-    }
+    $key = InMemory::plainText($secret);
+    $config = Configuration::forSymmetricSigner(
+      new Sha256(),
+      $key
+    );
+    $config->setValidationConstraints(
+      new SignedWith($config->signer(), $config->signingKey()),
+      new ValidAt(new SystemClock(new DateTimeZone('UTC')))
+    );
 
-    $data = new ValidationData();
-    if (!$token->validate($data)) {
+    $token = $config->parser()->parse($jwt);
+    $constraints = $config->validationConstraints();
+    try {
+      !$config->validator()->assert($token, ...$constraints);
+    } catch (RequiredConstraintsViolated $e) {
       return false;
     }
 
-    $token->getClaims();
-
-    $jwtXsrfToken = $token->getClaim('xsrfToken');
+    $jwtXsrfToken = $token->claims()->get('xsrfToken');
     if (isset($xsrfToken) && $xsrfToken !== $jwtXsrfToken) {
       return false;
     }
 
-    $guid = $token->getClaim('guid');
+    if ($token->claims()->has('guid')) {
+      // This check is only needed for as long as 'jwt_expire'.
+      $guid = intval($token->claims()->get('guid'));
+    } else {
+      $guid = intval($token->claims()->get('sub'));
+    }
     if (!is_numeric($guid) || $guid <= 0) {
       return false;
     }
 
-    $ret = ['guid' => $guid, 'expire' => $token->getClaim('exp')];
-    \error_reporting($erPrev);
-    return $ret;
+    $exp = $token->claims()->get('exp')->getTimestamp();
+
+    return ['guid' => $guid, 'expire' => $exp];
   },
   /*
    * Group Validator

diff --git a/src/Entities/Group.php b/src/Entities/Group.php
@@ -530,9 +530,15 @@ public function save() {
 
     // Formatting.
     $this->groupname = trim($this->groupname);
-    $this->email = trim($this->email);
-    $this->name = trim($this->name);
-    $this->phone = trim($this->phone);
+    if (isset($this->email)) {
+      $this->email = trim($this->email);
+    }
+    if (isset($this->name)) {
+      $this->name = trim($this->name);
+    }
+    if (isset($this->phone)) {
+      $this->phone = trim($this->phone);
+    }
 
     // Verification.
     $unCheck = $this->checkGroupname();

diff --git a/src/Entities/User.php b/src/Entities/User.php
@@ -1104,9 +1104,15 @@ public function register($data) {
       // Generate a new primary group for the user.
       $primaryGroup = Group::factory();
       $primaryGroup->groupname = $this->username;
-      $primaryGroup->avatar = $this->avatar;
-      $primaryGroup->name = $this->name;
-      $primaryGroup->email = $this->email;
+      if (isset($this->avatar)) {
+        $primaryGroup->avatar = $this->avatar;
+      }
+      if (isset($this->name)) {
+        $primaryGroup->name = $this->name;
+      }
+      if (isset($this->email)) {
+        $primaryGroup->email = $this->email;
+      }
       $primaryGroup->parent = Nymph::getEntity(
         ['class' => '\Tilmeld\Entities\Group'],
         ['&',
@@ -1282,14 +1288,24 @@ public function save() {
     // Formatting.
     $this->username = trim($this->username);
     // Setting username sets both username and email if email_usernames is on.
-    if (!Tilmeld::$config['email_usernames']) {
+    if (!Tilmeld::$config['email_usernames'] && isset($this->email)) {
       $this->email = trim($this->email);
     }
-    $this->nameFirst = trim($this->nameFirst);
-    $this->nameMiddle = trim($this->nameMiddle);
-    $this->nameLast = trim($this->nameLast);
-    $this->phone = trim($this->phone);
-    $this->name = $this->nameFirst.(
+    if (isset($this->nameFirst)) {
+      $this->nameFirst = trim($this->nameFirst);
+    }
+    if (isset($this->nameMiddle)) {
+      $this->nameMiddle = trim($this->nameMiddle);
+    }
+    if (isset($this->nameLast)) {
+      $this->nameLast = trim($this->nameLast);
+    }
+    if (isset($this->phone)) {
+      $this->phone = trim($this->phone);
+    }
+    $this->name = (
+        !empty($this->nameFirst) ? $this->nameFirst : ''
+      ).(
         !empty($this->nameMiddle) ? ' '.$this->nameMiddle : ''
       ).(
         !empty($this->nameLast) ? ' '.$this->nameLast : ''
@@ -1398,9 +1414,21 @@ public function save() {
     if (isset($this->group->user) && $this->is($this->group->user)) {
       // Update the user's generated primary group.
       $this->group->groupname = $this->username;
-      $this->group->avatar = $this->avatar;
-      $this->group->email = $this->email;
-      $this->group->name = $this->name;
+      if (isset($this->avatar)) {
+        $this->group->avatar = $this->avatar;
+      } else {
+        unset($this->group->avatar);
+      }
+      if (isset($this->email)) {
+        $this->group->email = $this->email;
+      } else {
+        unset($this->group->email);
+      }
+      if (isset($this->name)) {
+        $this->group->name = $this->name;
+      } else {
+        unset($this->group->name);
+      }
       $this->group->saveSkipAC();
     }
 

diff --git a/src/Tilmeld.php b/src/Tilmeld.php
@@ -486,8 +486,8 @@ public static function authenticate($skipXsrfToken = false) {
       }
 
       $extract = self::$config['jwt_extract'](
-          $authToken,
-          $_SERVER['HTTP_X_XSRF_TOKEN']
+        $authToken,
+        $_SERVER['HTTP_X_XSRF_TOKEN']
       );
     }