[Security] Deprecate current system in favor of a JWTTokenAuthenticator (Guard)

[chalasr]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations	yes
Fixed tickets	~
Tests pass?	yes

See #132 discussion.

@slashfan after you started to work on, I made some changes.
I'm really not an expert in the Guard component itself, but it seems make our life easier from all sides.

Should we propose it as an alternative or deprecate the "legacy" way in favor of this?

What's next?

---

• Add the Guard JWTAuthenticator
• Test it
• Add a full functional test case
• Depreciate old security system
• Exclude legacy tests from CI Mute deprecations using symfony/phpunit-bridge
• Update the documentation

[chalasr]

@weaverryan Your help would be really appreciated, as your are the father of this great tool (guard).

[slashfan]

@chalasr The main idea for V2 is to replace the legacy system by a new one based on the Guard Component. It would help transition but it would also mean too much work to maintain both systems.

[chalasr]

@slashfan Agreed, though upgrading can stay soft using the JWTGuardFactory that could reproduce the same configuration as for the legacy system (lexik_jwt: ~).
I would just change the return of getKey() to lexik_jwt rather than lexik_jwt_guard, sort as we don't have any breaking changes in security configuration, except for people making the change themselves (as documented). What do you think?

We also even need some security options such as throw_exceptions, create_entry_point and the choice of token extractors, right? They could be kept through this way.

[slashfan]

@chalasr Agreed ! Let's try to keep the transition smooth, we'll remove the old system in a V3

[chalasr]

@slashfan I'm facing an issue.

One of the main advantages of Guard is that the end user can have several authenticators per firewall.
The problem is: how to apply our current security configuration on the new guard authenticator?
It can be done using the factory I added (an override of the original GuardAuthenticationFactory that's not the normal usage of this component) that would be used in the same way as our current system:

firewalls:
    api:
        lexik_jwt_guard: ~

Instead of the "normal" way:

firewalls:
    api: 
        guard:
            lexik_jwt_authenticator.jwt_authenticator

Because like this (second way), we can't pass any config option.
I'm afraid that Guard cannot fit our needs because of this.

I'll investigate, then keep you informed.
Please let me know if you have any input.

[chalasr]

See symfony/symfony#19175

[slashfan]

We could move the security configuration to the bundle configuration (extractors, prefix) and use it to configure the lexik_jwt_authenticator.jwt_authenticator service.

By default the bundle would provide one preconfigured authenticator (this one) and if another firewall needs to use JWT with a different configuration the developer would just have to create another authenticator service extending this one and overriding its configuration.

What do you think ? That would simplify the factory.

[chalasr]

@slashfan It seems to be the better alternative, good idea!
The authenticator service would take all our current per-firewall configuration.
I'll try to keep the configuration simple so the service is easily extensible, then document it as well as possible.

Now that we go in this way, I'm not sure about the usefulness of the factory added here, I'm afraid that it could be more confusing than useful, let me know what do you think.

BTW, the branch of this PR is part of the main repo, not on my fork, do not hesitate to make changes as you want.

[slashfan]

Even better if we can get rid of the new factory class ! Less code = less problems :)

[chalasr]

I would like to discuss about some of our current config options, in order to keep them or not during the migration to Guard.

The primary point of the throw_exceptions option is that an end user setting the option to true is able to catch an authentication exception (right?). IMO the authentication success/failure listeners are here for that. Plus, now that we are going to use Guard, an exception can be thrown in getUser (as documented in the GuardAuthenticatorInterface) causing the authentication fail and returning the common 401 (json) response.
I think we can remove it, then if an user need this feature, we could explain how to get around the need.

Next, the create_entry_point option. IMO this option is no more useful (now that we are going to use Guard too), guard has its own entry_point option, taking the service id of one of the configured authenticators that should be used as entry point (GuardAuthenticator::start()). Of course, we could even keep it and, if set to false, make JWTTokenAuthenticator::start() not returning a failure response, but it is IMO the exact role of the anonymous firewall option, letting the request continue if anonymous requests are allowed, otherwise call the entry point. Having both causes a kind of conflict in our conception.

That's all.

All opinions are really welcome.
@slashfan What do you think?

[chalasr]

I plan to merge this one in the next days