Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Added JWK Caching and Support for Laravel 11 #32

Merged
merged 33 commits into from
Jan 5, 2025

Conversation

gaokevin1
Copy link
Member

@gaokevin1 gaokevin1 commented Dec 13, 2024

Related Issues

Fixes https://github.com/descope/etc/issues/7756

Related PRs

branch PR
service a PR Link to PR
service b PR Link to PR

Description

Added the following items to the PHP SDK:

  • JWK Caching
  • Support for Laravel 11 with Custom JWT Validation Logic
  • Added new Password Hashing Algorithm (MD5) to SDK

Must

  • Tests
  • Documentation (if applicable)

@gaokevin1 gaokevin1 requested review from gshriki and Bars92 December 27, 2024 20:30
@gaokevin1 gaokevin1 self-assigned this Dec 27, 2024
@gaokevin1 gaokevin1 changed the title Remove validateToken() and replace with verify() Added JWK Caching and JWT Validation Logic Dec 27, 2024
@gaokevin1 gaokevin1 changed the title Added JWK Caching and JWT Validation Logic Feat: Added JWK Caching and JWT Validation Logic Dec 27, 2024
@gaokevin1 gaokevin1 changed the title Feat: Added JWK Caching and JWT Validation Logic Chore: Added JWK Caching and JWT Validation Logic Dec 27, 2024
@gaokevin1 gaokevin1 changed the title Chore: Added JWK Caching and JWT Validation Logic chore: Added JWK Caching and JWT Validation Logic Dec 27, 2024
sample/callback.php Outdated Show resolved Hide resolved
sample/login.php Outdated Show resolved Hide resolved
@gshriki gshriki requested a review from omercnet December 31, 2024 12:51
@gshriki
Copy link
Member

gshriki commented Dec 31, 2024

Maybe I missed - but where are we caching the JWK itself so it is not fetched on every request? adding @omercnet to review as well

src/SDK/DescopeSDK.php Outdated Show resolved Hide resolved
@omercnet
Copy link
Member

Maybe I missed - but where are we caching the JWK itself so it is not fetched on every request? adding @omercnet to review as well

it's cached only in the same verification attempt, see https://github.com/descope/descope-php/pull/32/files#diff-a47473cebf5ae187c13efe92b1fec1ab8ca735408b5a393de87a85ade0174312R73-R111
if it fails it busts cache, but it will not use the cache between requests

if we want to cache the jwks between requests need to use APCu or Memcached, depends on what's enabled by the end user
for laravel and other frameworks we can utilize the framework caching mechanism

@gaokevin1 gaokevin1 changed the title chore: Added JWK Caching and JWT Validation Logic chore: Added JWK Caching and Support for Laravel 11 Jan 2, 2025
@gaokevin1 gaokevin1 changed the title chore: Added JWK Caching and Support for Laravel 11 feat: Added JWK Caching and Support for Laravel 11 Jan 2, 2025
@gaokevin1
Copy link
Member Author

gaokevin1 commented Jan 2, 2025

I feel like the best option is to use APCu by default (since it's supported by Laravel as well) and then build some LaravelCache class that will instead use the built-in Laravel caching for Laravel apps, all within the same SDK. Then the customer could just decide which one to use in the initialization of the SDKConfig.

Wdyt @omercnet ?

@omercnet
Copy link
Member

omercnet commented Jan 2, 2025

I feel like the best option is to use APCu by default (since it's supported by Laravel as well) and then build some LaravelCache class that will instead use the built-in Laravel caching for Laravel apps, all within the same SDK. Then the customer could just decide which one to use in the initialization of the SDKConfig.

Wdyt @omercnet ?

if you could put in an abstract class with default implementation for acpu that'd be great
then in each framework you can override it with the framework relevant implementation

@gaokevin1 gaokevin1 requested review from omercnet and gshriki January 2, 2025 22:37
@gaokevin1 gaokevin1 merged commit eb3f69b into main Jan 5, 2025
4 checks passed
@gaokevin1 gaokevin1 deleted the fix-validateToken branch January 5, 2025 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants