-
Notifications
You must be signed in to change notification settings - Fork 74
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
Added support for cache #51
Conversation
Great thanks, let me look into adding this repo to Travis, Although I see that you're sneaking in 5.5 again as the min requirement 😄 (I can understand why). Let me check with @glena |
I actually did't sneak it "up" to PHP 5.5. Previous requirements was symfony/symfony:3.0 which require php 5.5.9 (source). I lowered it to php 5.5.0. |
Apologies, I was looking in the other PR history which showed Symfony 2.8 as the requirement. The This should be fine then. |
composer.json
Outdated
|
||
"suggest": { | ||
"phpunit/phpunit": "^5.1", | ||
"nyholm/symfony-bundle-test": "^1.0" | ||
}, | ||
|
||
"autoload": { | ||
"psr-4": { | ||
"Auth0\\JWTAuthBundle\\": "src/" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing comma
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
src/Security/Auth0Service.php
Outdated
{ | ||
$this->api_secret = $api_secret; | ||
$this->domain = $domain; | ||
$this->api_identifier = $api_identifier; | ||
$this->authorized_issuer = $authorized_issuer; | ||
$this->secret_base64_encoded = $secret_base64_encoded; | ||
$this->supported_algs = $supported_algs; | ||
$this->cache = $cache; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we did we lose supported algs? (used in decodeJWT)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry. I did not indent to do that.
Thank you for the review. Also, awesome that you enabled Travis =) |
@cocojoe LGTM :) |
Anything else I can do to get this PR merged? |
Thx for reminding me, I'll need to set this repo up for a proper release cycle. Let me sort that out. |
Thank you for merging |
This will fix #50.
I see that you are not running any test on travis. And you barely have any tests... I added some smoke tests.
I also allowed Symfony 2.8 since it is the same as 3.0.