codecasts / laravel-jwt

Dead simple, plug and play JWT API Authentication for Laravel (5.4+)
MIT License
234 stars 27 forks source link

Added accessor to the detected token #20

Closed imikemiller closed 6 years ago

imikemiller commented 6 years ago

I have need of this on my project as I am baking permissions into the aud claim and need to get hold of them in a middleware to do further authorisation checks. I think with support for adding custom claims to the token there needs to be a simple way to get hold of the token and this made most sense to me (unless I missed an obvious alternative?!)

hernandev commented 6 years ago

@imikemiller Actually, It would be better to make this method: https://github.com/codecasts/laravel-jwt/blob/master/src/Auth/Guard.php#L263

protected function detectedToken()

Into a public one:

public function detectedToken()

hernandev commented 6 years ago

The way you implemented the token could be not yet parsed, and will return null right away without trying to decode it.

imikemiller commented 6 years ago

It's kind of the same thing as detectedToken can return a null. By just exposing the detectedToken method we have to run that code every time we want to access the token. Maybe the solution is to do both that way if the token is null then detectedToken can be called to try and populate it. If the library is used as intented then by the time app code is hit the token will have been detected if it is present.

On Oct 17, 2017 10:00 AM, "Diego Hernandes" notifications@github.com wrote:

The way you implemented the token could be not yet parsed, and will return null right away without trying to decode it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/codecasts/laravel-jwt/pull/20#issuecomment-337150197, or mute the thread https://github.com/notifications/unsubscribe-auth/AXtHNsDYG9SLXkSfdkqi4eWSXOrgQGUfks5stF6QgaJpZM4P7sb4 .

hernandev commented 6 years ago

I have no problem with that. But we're talking PHP, and the Token parsing operation is somehow little heavy.

So, the right approach would be lazy-loading the token instance, this way it will parse if not already present.

Just notice on this, the actual $token attribute must be Token class instance or null, even tough the code actually does that, the comments on the attribute indicates it's string.

I'm going to merge other smaller requests and work on top of this.

Thanks for now!

imikemiller commented 6 years ago

Sounds sensible, thanks!

Do you want me to update the PR with the corrected annotation?

hernandev commented 6 years ago

ill just do it myself based on what you did, releasing it in a bit