Roadmap #47

rosstuck · 2017-06-02T10:07:48Z

chalasr · 2017-06-09T13:20:50Z

tasks number 4 and 5 of Road to 1.0 addressed in #48

tyx · 2017-06-09T15:56:26Z

I give a try to the integration test of the SecurityMiddleware but with the following lines

$authenticationManager = static::$kernel->getContainer()->get('security.authentication.manager');
$authenticationManager->authenticate(new UsernamePasswordToken('admin', 'kitten', 'main', ['ROLE_ADMIN']));

I get
Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException: The token storage contains no authentication token. One possible reason may be that there is no firewall configured for this URL.

I don't get how to handle that. I should admit I have a very small experience with this component. If anyone know how to simply authenticate a user with a single password without Request

rosstuck · 2017-06-10T19:51:45Z

Hmm, I haven't had this problem myself, but this might help?

http://symfony.com/doc/current/testing/http_authentication.html

The WebTestCase base class used above is just a thin addition to the same KernelTestCase base classes we use, some I imagine the same solutions apply.

RonRademaker · 2017-06-19T08:12:33Z

Addressed number 2 in #49

chalasr · 2017-06-20T08:33:55Z

Fix incorrect docblocks (a number of them are missing type annotations, have old params, etc)

Fixed in #51

chalasr · 2017-06-20T12:04:35Z

Add SecurityMiddlewarePassTest
Add ValidatorMiddlewarePassTest

fixed in #52 , good to have

chalasr · 2017-06-20T12:09:12Z

increasing full test coverage of the security middleware

@RonRademaker Would you mind looking at this? Otherwise I can pick it, but I would first have to look at this middleware.

RonRademaker · 2017-06-20T12:20:58Z

I'll have some time to look at that in the upcoming days 👍

chalasr · 2017-06-21T15:45:47Z

Am I mistaken or do we only need to review the docs for a v1 to be out?

rosstuck · 2017-06-21T15:49:53Z

@chalasr I'm poring over the new security tests as we speak but it's starting to look that way.... 🌞

rosstuck · 2017-06-21T16:31:47Z

Unfortunately, I found some issues with the security unit tests, documented in #54. I have to go offline for a family event, I'm blocking in some time to go forward with review on Saturday though.

chalasr · 2017-06-21T16:43:00Z

@rosstuck I'll have a look at these meanwhile, maybe submit a PR targetting your cleanup branch

rosstuck · 2017-07-18T12:08:12Z

I'm on vacation right now but since we're not making much progress on getting the security issues figured out, I propose we disable/remove them for the time being so we can ship a release that will help the issues 3.3 users are experiencing. I feel bad doing this but I think it's the best option right now. If there's no complaints or alternate suggestions, I'll start on this next week

chalasr · 2017-07-18T20:09:59Z

I'm fine with postponing security related features.

RonRademaker · 2017-07-19T07:42:21Z

I'd like to see the security middleware in the next release (as I'm already using it in a project that is currently in development, but will be in production in a few months). I'm not exactly clear what the security issues are ATM. Are these clear so I can invest some time to address them?

rosstuck · 2017-07-19T10:09:59Z

I'd like to see it there as well because you have really been put through the wringer over it. The issues are entirely the test suite, I'm very hesitant to ship a security related piece without full integration tests.

…

On Jul 19, 2017 9:42 AM, "Ron Rademaker" ***@***.***> wrote: I'd like to see the security middleware in the next release (as I'm already using it in a project that is currently in development, but will be in production in a few months). I'm not exactly clear what the security issues are ATM. Are these clear so I can invest some time to address them? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#47 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAI9To7uXF9VQOA9nxjEOq8nDkb1X7qJks5sPbNegaJpZM4NuFaX> .

rosstuck · 2017-08-02T14:40:12Z

Moving again on #54, will do a bit more tomorrow as well (look at Scrutinizier issues, etc)

rosstuck · 2017-08-03T11:12:12Z

Okay, I've finished tests, updated the docs and walked through all the README examples in a fresh Symfony install. So far it looks really good!

The only thing that smacked me in the face was when I tried to run a command as an anon user. Symfony doesn't automatically give a anon ROLE_USER role so there's no way to configure some commands right now to be runnable by anon users (say, sign me up for the newsletter) and some to be authenticated users (bill this user) without splitting them into two separate buses or overwriting the token in token storage. We might want to address this or at least give a way of mitigating it in the docs.

Still, I don't see that as a blocker, so I'm thinking I'll tag an RC for 1.0 today and we can get that out there for some install and feedback! Sound good? :)

RonRademaker · 2017-08-03T11:20:49Z

👍

chalasr · 2017-08-03T11:29:11Z

👍

RonRademaker · 2017-08-09T09:14:44Z

The only thing that smacked me in the face was when I tried to run a command as an anon user. Symfony doesn't automatically give a anon ROLE_USER role so there's no way to configure some commands right now to be runnable by anon users (say, sign me up for the newsletter) and some to be authenticated users (bill this user) without splitting them into two separate buses or overwriting the token in token storage. We might want to address this or at least give a way of mitigating it in the docs.

I've also ran into this issue and worked around it using my own authorization checker with a dedicated CLI role, something like:

use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;

class WorkaroundAuthorizationChecker implements AuthorizationCheckerInterface
{
    /**
     * Container to retrieve the actual authorization checker from.
     *
     * @var Container
     */
    private $container;

    /**
     * AuthorizationCheckerFacade constructor.
     *
     * @param Container $container
     */
    public function __construct(Container $container)
    {
        $this->container = $container;
    }

    /**
     * Proxy to the authorization checker in the container.
     *
     * @param mixed $attributes
     * @param null $object
     *
     * @return bool
     */
    public function isGranted($attributes, $object = null): bool
    {
        $tokenStorage = $this->container->get('security.token_storage');

        if (null === ($token = $tokenStorage->getToken()) && php_sapi_name() ==='cli') {
            // workaround because cli wouldn't have a token
            $tokenStorage->setToken(
                new AnonymousToken('cli', 'anon', [new Role('ROLE_CLI'), new Role('IS_AUTHENTICATED_ANONYMOUSLY')])
            );
        } 

        return $this->container->get('security.authorization_checker')->isGranted($attributes, $object);
    }
}

This is the auth checker I inject in the security middleware.

rosstuck · 2017-08-28T10:34:10Z

It's been a couple weeks and I haven't heard of any major issues except the auth ones above, so I'll tag 1.0 today.

tyx · 2017-08-28T10:55:12Z

Indeed ! we are using the RC1 since the release and everything is fine.

rosstuck · 2017-08-28T11:04:22Z

Released, congrats and thanks again to everyone for all your hard work! 😄

Going forward, I think #44 and #60 are high on my priority list. Also, if y'all are interested, someone's also opened a PR for this bundle in the Symfony recipes repo symfony/recipes-contrib#71 (comment)

rosstuck · 2017-10-09T15:04:11Z

I've just merged #67 into master. This adds the ability to have automatic wiring/mapping strategies. If y'all can give this a shot in your applications and open bugs for anything broken, I would greatly appreciate it!

See https://github.com/thephpleague/tactician-bundle#configuring-command-handlers to get started

Also, if you try using this in combination with the Symfony autowiring strategies, please post a sample of your config. I'd love to see what this looks like for most folks and get some samples into the documentation.

Thanks a ton!

rosstuck · 2017-10-09T15:05:05Z

Oh and I've updated the roadmap to push the new tactician:debug command to the next release since it's not a must-have IMHO and I don't have the energy to pick it up right now. If anyone is interested in doing so, please post on #71.

dmecke · 2017-10-12T07:18:03Z

Also, if you try using this in combination with the Symfony autowiring strategies, please post a sample of your config. I'd love to see what this looks like for most folks and get some samples into the documentation.

I have about 100 commands in my app, so I have a seperate command_handler.yml for the handler service definitions. Here is a before and after:

Before

services:
    onlinetennis.command_handler.cancel_showmatch_request:
        class: AppBundle\CommandHandler\CancelShowmatchRequestHandler
        arguments: ['@onlinetennis.repository.player', '@onlinetennis.repository.showmatch_request']
        tags:
            - { name: tactician.handler, command: Domain\Command\CancelShowmatchRequest }
    // 500 more lines

After

services:
    AppBundle\CommandHandler\:
        resource: "../../CommandHandler"
        autowire: true
        autoconfigure: true
        public: true
        tags:
            - { name: tactician.handler, typehints: true }

dmecke · 2017-10-21T06:57:27Z

@rosstuck Are you planning to tag the v1.1 in the near future? If you want to give it some more time, would it be possible to tag a v1.1-RC1 as you did with v1.0?

rosstuck · 2017-10-21T08:37:20Z

Hey @dmecke, I was hoping #73 would land first (since that would be good to have for folks struggling with the new system) but if that's gonna take a bit longer, then I can tag an RC-ish thing tomorrow.

rosstuck · 2017-10-22T12:51:28Z

Tagged v1.1-RC1

rosstuck · 2017-11-02T06:54:06Z

#73 has landed, so there's now a debug command for seeing which commands are mapped to which services! Will review docs and update changelog, etc in next couple days then cut 1.1.0 if there's no blockers.

In the meantime, if anyone is interested in giving the debug command a spin on their full app, let me know if it works out for you. 👍

ogizanagi · 2017-11-02T08:06:37Z

Just tried on an app with ~70 handlers and 3 buses, works fine 👍 (nice work @kejwmen)
I'm just wondering if debug:tactician would've been acceptable instead of tactician:debug as for other debug commands, even if it's not a symfony core one.

rosstuck · 2017-11-05T13:38:47Z

Good idea! Changed the command name for consistency. I've also tweaked a couple docs and cut the release, v1.1.0 is out. If anyone finds any bugs, please open an issue.

Otherwise, thanks for all your hardwork and testing, everyone :)

rosstuck · 2017-11-11T07:24:58Z

Tagged 1.1.1 (fixes an error using the default bus when absolutely no config is defined, improved version constraints, better docs, no BC breaks). Thanks for picking up the hard work on this one folks :)

lcobucci · 2017-11-13T13:22:42Z

Split validator middleware into separate package?

@rosstuck we're using the validator middleware with tactician in a Zend Expressive application and we just have some SF packages installed because of this bundle. With that said, could we help with splitting that middleware to another package? It should be pretty straightforward but what are your thoughts on this?

ogizanagi · 2017-11-13T15:11:29Z

@lcobucci : there is a pending PR regarding the validator middleware extraction -> #74

rosstuck added the help wanted label Jun 2, 2017

Roadmap #47

Roadmap #47

Comments

rosstuck commented Jun 2, 2017 • edited Loading

Road to 1.0

1.1

1.2

2.0

Misc

chalasr commented Jun 9, 2017 • edited Loading

tyx commented Jun 9, 2017

rosstuck commented Jun 10, 2017

RonRademaker commented Jun 19, 2017

chalasr commented Jun 20, 2017

chalasr commented Jun 20, 2017

chalasr commented Jun 20, 2017

RonRademaker commented Jun 20, 2017

chalasr commented Jun 21, 2017

rosstuck commented Jun 21, 2017 • edited Loading

rosstuck commented Jun 21, 2017 • edited Loading

chalasr commented Jun 21, 2017 • edited Loading

rosstuck commented Jul 18, 2017

chalasr commented Jul 18, 2017

RonRademaker commented Jul 19, 2017

rosstuck commented Jul 19, 2017 via email

rosstuck commented Aug 2, 2017

rosstuck commented Aug 3, 2017 • edited Loading

RonRademaker commented Aug 3, 2017

chalasr commented Aug 3, 2017

RonRademaker commented Aug 9, 2017

rosstuck commented Aug 28, 2017

tyx commented Aug 28, 2017

rosstuck commented Aug 28, 2017

rosstuck commented Oct 9, 2017

rosstuck commented Oct 9, 2017

dmecke commented Oct 12, 2017

Before

After

dmecke commented Oct 21, 2017

rosstuck commented Oct 21, 2017

rosstuck commented Oct 22, 2017

rosstuck commented Nov 2, 2017

ogizanagi commented Nov 2, 2017 • edited Loading

rosstuck commented Nov 5, 2017

rosstuck commented Nov 11, 2017 • edited Loading

lcobucci commented Nov 13, 2017

ogizanagi commented Nov 13, 2017

rosstuck commented Jun 2, 2017 •

edited

Loading

chalasr commented Jun 9, 2017 •

edited

Loading

rosstuck commented Jun 21, 2017 •

edited

Loading

rosstuck commented Jun 21, 2017 •

edited

Loading

chalasr commented Jun 21, 2017 •

edited

Loading

rosstuck commented Aug 3, 2017 •

edited

Loading

ogizanagi commented Nov 2, 2017 •

edited

Loading

rosstuck commented Nov 11, 2017 •

edited

Loading