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

Refactor JWT Drivers to Laravel's provided Managers #73

Open
leon0399 opened this issue Nov 24, 2021 · 7 comments
Open

Refactor JWT Drivers to Laravel's provided Managers #73

leon0399 opened this issue Nov 24, 2021 · 7 comments
Labels
enhancement New feature or request
Milestone

Comments

@leon0399
Copy link
Member

Subject of the issue

Right now multiple drivers for JWT (lcobucci/jwt and namshi/jose), Auth and Storage uses some custom hacky solution (1, 2, when Laravel provides special solution for that (1, 2, that is better suited and already used within Laravel itself

@leon0399 leon0399 added the enhancement New feature or request label Nov 24, 2021
@Messhias
Copy link
Collaborator

We should keep both because the alg encrypt, in fact as per what I was reading (I'm having some JWS issues here in my job) this is something related to the RFC 7577 and 7575.

If I understood clearly the RFC.

@leon0399
Copy link
Member Author

We should keep both because the alg encrypt, in fact as per what I was reading (I'm having some JWS issues here in my job) this is something related to the RFC 7577 and 7575.

If I understood clearly the RFC.

I think there is some kind of misunderstandment 😅. I'm not talking about removing any features, I'm talking about internal code refactoring, that will not only use existing and provided solutions, but also allow end-users etend library entirely inside their projects more easily

@Messhias
Copy link
Collaborator

We should keep both because the alg encrypt, in fact as per what I was reading (I'm having some JWS issues here in my job) this is something related to the RFC 7577 and 7575.
If I understood clearly the RFC.

I think there is some kind of misunderstanding 😅. I'm not talking about removing any features, I'm talking about internal code refactoring, which will not only use existing and provided solutions but also allow end-users to extend library entirely inside their projects more easily

Sorry for my misunderstanding my friend hahahhaha

Anyway sounds good, but for this enhancement I believe will be quite harder since the project if looks deeply it works very attached the things between each other, so I believe if we're going forward with that let's split into small parts to keep tests/project running (mainly for us that we already have it working in our production projects/jobs).

@leon0399 leon0399 added this to the 2.0 milestone Nov 29, 2021
@mfn mfn modified the milestones: 2.0, 3.0 Sep 8, 2022
@Messhias
Copy link
Collaborator

@mfn / @eschricker can we close this one?

@eschricker
Copy link
Contributor

I have looked at the articles mentioned in the documentation. In my opinion, we should leave this open. From my point of view, this looks like a very useful revision.

@Messhias
Copy link
Collaborator

I have looked at the articles mentioned in the documentation. In my opinion, we should leave this open. From my point of view, this looks like a very useful revision.

So basically we have to choose on of the drivers and follow up with them right?

@mfn
Copy link
Contributor

mfn commented Jun 2, 2023

I think this issue makes sense and should be kept open,

I've a few comments but I'm not 110% sure the apply to a possible solution to this:

  • https://github.com/namshi/jose is effectively abandoned, see https://github.com/namshi/jose#deprecation-notice

    Hi there,

    as much as we'd like to be able to work on all of the OSS in the world, we don't actively use this library anymore This means that new features / bugfixes / etc will only be merged based on pull requests from external contributors, and we strongly recommend you look for a long-term alternative.

    If you're looking for an actively maintained library check firebase/php-jwt out!

    I mean the documentation we have does not even mention this driver at all.

    That other package https://github.com/firebase/php-jwt is not a drop-in replacement for namshi/jose and therefore would need it's own driver.

  • https://github.com/lcobucci/jwt released a major version ^5 some time ago, so it probably makes sense to start looking into this

  • one thing I don't particular like is that this package jwt-auth requires all possible supported drivers currently, see

    jwt-auth/composer.json

    Lines 42 to 43 in 5b4e3ee

    "lcobucci/jwt": "^4.0",
    "namshi/jose": "^7.0",

    This means all users of jwt-auth have an effectively abandoned package namshi/jose installed for probably no good reason. I doubt anyone uses it ("famous last words", I know).
    Not sure how this can work better, but we should only "suggest supported drivers and encourage use to make an explicit decision and we can add a recommendation which one to use etc.

Just some ideas :-}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants