GCTC-NTGC / gc-digital-talent

GC Digital Talent is the new recruitment platform for digital and tech jobs in the Government of Canada. // Talents numériques du GC est la nouvelle plateforme de recrutement pour les emplois numériques et technologiques au gouvernement du Canada.
https://talent.canada.ca
GNU Affero General Public License v3.0
21 stars 8 forks source link

♻ Consolidate php JWT management libs in API app #2835

Open patcon opened 2 years ago

patcon commented 2 years ago

🕵️ Details

Re-ticketed from https://github.com/GCTC-NTGC/gc-digital-talent/pull/2813#discussion_r877400297

Not urgent.

From the code comment that @petertgiles +1'd:

We're using two JWT management libraries here (Jose & Lcobucci), which each offer different functionality related to constraints and JWKS. TODO: Consider consolidating into a single library, or migrating to a new

The file in which the new web-token/jwt-core will appear: https://github.com/GCTC-NTGC/gc-digital-talent/blob/main/api/app/Services/OpenIdBearerTokenService.php

The two libraries we're using right now, plus another front-runner: (after merging #2813)

  1. prior: https://github.com/lcobucci/jwt
    • gives us advanced constraint management to explicitly set constraints (not sure value?)
    • lacking jwks management
    • no system deps
  2. new: https://github.com/web-token/jwt-framework (specifically web-token/jwt-core pkg)
    • does jwks management
    • no constraint management (that I could see when skimming)
    • requires either bcmath or gmp PHP extension
      1. alt: https://github.com/firebase/php-jwt
        • does some jwks management
        • maintained by Google
        • lots of open issues
        • uses libsodium, a highly auditted crypto package
        • if no libsodium, a polyfill lib can be installed to do pure-php crypto ops (slower)
Package ⭐ Stars 🧑‍🤝‍🧑 Contributors Started
lcobucci/jwt 6,600 48 2014
web-token/jwt-core 714 14 2018
firebase/php-jwt 8,800 50 2011

✅ Acceptance Criteria

A set of assumptions which, when tested, verify that the bug was addressed.

mnigh commented 7 months ago

@petertgiles

  1. have there been any major changes since this issue was created that warrant closing this issue?
  2. is there any way forward on consolidation of libraries?
  3. is there any acceptance criteria for this issue?
petertgiles commented 7 months ago

There has been a recent development actually, ♻️ Package web-token/jwt-core is abandoned #9494 . I think that using a single library is still something we want to do, which would be the AC.