fsi-open / admin-security-bundle Goto Github PK

https://symfony.com/blog/new-in-symfony-4-3-simpler-event-dispatching

When the option is set to true, element manager should check if all elements implement that interface.

Currently, if an disabled or locked user tries to log in or reset his password, he will receive a message containing information on his current status in the form errors. This should not happen, since it makes it possible to discover which users exist in the database.

in features/fixtures/project/src/FSi/FixturesBundle/Admin/PageSettings.php, line 43

TODO comments are left in the code when a feature (or a bug) isn't completely developed (or fixed). You should complete the implementation and remove the comment.

        return 'Page settings';
    }

    protected function initDataGrid(DataGridFactoryInterface $factory)
    {
        // TODO: Implement initDataGrid() method.
    }

    protected function initDataSource(DataSourceFactoryInterface $factory)
    {
        // TODO: Implement initDataSource() method.

Posted from SensioLabsInsight

There is an assumption in the change password command that the user entity will use the repository provided by admin security bundle, which may not always be the case. I think that it would be better to create a container repository with a dependency on user's repository, that would use the known methods. That way we aren't limited by actions of end-user.

This will allow making user repository service private as it should be

EnforcePasswordChangeListener redirects user to password change page, but it is invoked after symfony's Firewall listener which has higher priority. When the user clicks on link to page which they don't have permission, access denied exception will be thrown before EnforcePasswordChangeListener run.

Simply changing priority is not enough. Before firewall starts we cannot use $authorizationChecker->isGranted() and we have no token. Maybe we can also listen for kernel.exception ?

Continuation of #226

Routing like admin_security.controller.activation:activateAction has been deprecated in Symfony 4.1 and it is recommended to change it to admin_security.controller.activation::activateAction. Unfortunately, in the 2.x versions this notation is not supported, so we will have to wait with this change until next major version.

Currently user form always shows a list of roles, which is not always desirable. An option to hide it would be prudent.

Continuation of #228.

We need to specify a sensible way of asserting access to specific object instances in form and display elements.

SecuredElementListener should not call SecuredElementInterface::isAllowed() when there is no token, because it finally throws exception here

SecurityController should use https://github.com/symfony/security-http/blob/master/Authentication/AuthenticationUtils.php#L44 and https://github.com/symfony/security-csrf/blob/master/CsrfTokenManager.php

If you define a chain provider for login form with at least two entities not having the same table, only the class defined in the fsi_admin_security.model.user parameter will be checked against the provided credentials. Obviously this should be changed, so all defined entities will be checked against.

Example - ChangePasswordController.

If application throw exception, symfony's ExceptionHandler will make sub-request. In that sub-request processing EnforcePasswordChangeListener will check if route is right (it's null) and redirect.

Probably we need just check if request is master?

Current services IDs should be recreated as aliases for new ones, with class names as IDs.

User Deprecated: The "FSiBundleAdminSecurityBundleSecurityUserUserInterface" interface extends "SymfonyComponentSecurityCoreUserAdvancedUserInterface" that is deprecated since Symfony 4.1

It should be rather omitted during (menu / actions / batch actions) generation than completely removed. Current implementation causes confusion during debugging. Another problematic use case is when list element is allowed but action or batch action should not be allowed. Then additional permission check is required before adding action or batch action to list's datagrid.

The SetEmailAsUsernameListener, which is being called on admin save form event, always overwrites the username value, no matter what you provide in the form. I think it should be controlled via a configuration flag, so a compiler pass adds this listener only when it is set to do so.

With conjunction with #330, we could create separate security contexts with dedicated paths, security routing and user providers.

in features/fixtures/project/src/FSi/FixturesBundle/Admin/News.php, line 48

TODO comments are left in the code when a feature (or a bug) isn't completely developed (or fixed). You should complete the implementation and remove the comment.

        // TODO: Implement initDataGrid() method.
    }

    protected function initDataSource(DataSourceFactoryInterface $factory)
    {
        // TODO: Implement initDataSource() method.
    }

    protected function initForm(FormFactoryInterface $factory, $data = null)
    {
        // TODO: Implement initForm() method.

Posted from SensioLabsInsight

Currently:

"Purpose of this bundle is to provide simple authentication & authorization mechanisms for fsi/admin-bundle"

It should say:

"Purpose of this bundle is to provide simple authentication & authorization mechanisms for fsi-open/admin-bundle"

Notice the fsi-open in fsi-open/admin-bundle

When the user enters a element to which it don't have permission (because of SecuredElementInterface), AccessDeniedException or similar should be thrown instead of current NotFoundHttpException.
There is no easy way to distinguish between those 2 situations currently

SecurityController should use flashmessage service from admin-bundle to display errors

obviously this should not happen :)

in features/fixtures/project/src/FSi/FixturesBundle/Admin/PageSettings.php, line 48

TODO comments are left in the code when a feature (or a bug) isn't completely developed (or fixed). You should complete the implementation and remove the comment.

        // TODO: Implement initDataGrid() method.
    }

    protected function initDataSource(DataSourceFactoryInterface $factory)
    {
        // TODO: Implement initDataSource() method.
    }

    protected function initForm(FormFactoryInterface $factory, $data = null)
    {
        // TODO: Implement initForm() method.

Posted from SensioLabsInsight

in features/fixtures/project/src/FSi/FixturesBundle/Admin/News.php, line 43

TODO comments are left in the code when a feature (or a bug) isn't completely developed (or fixed). You should complete the implementation and remove the comment.

        return 'News';
    }

    protected function initDataGrid(DataGridFactoryInterface $factory)
    {
        // TODO: Implement initDataGrid() method.
    }

    protected function initDataSource(DataSourceFactoryInterface $factory)
    {
        // TODO: Implement initDataSource() method.

Posted from SensioLabsInsight

in features/fixtures/project/src/FSi/FixturesBundle/Admin/News.php, line 53

TODO comments are left in the code when a feature (or a bug) isn't completely developed (or fixed). You should complete the implementation and remove the comment.

        // TODO: Implement initDataSource() method.
    }

    protected function initForm(FormFactoryInterface $factory, $data = null)
    {
        // TODO: Implement initForm() method.
    }
}

Posted from SensioLabsInsight

• Removal of dependency on FOSUserBundle.
• Added default administration element for managing users.
• Added email activation.
• Changed translation domain for messages displayed on login form page (#84).
• Changed routes for activation password reset actions (#93)
• Added console commands for user actions.
• Requirement of user model class repository to implement bundles' interface.
• The fact, that during user creation username is begin overwritten by a listener

in features/fixtures/project/src/FSi/FixturesBundle/Admin/PageSettings.php, line 53

TODO comments are left in the code when a feature (or a bug) isn't completely developed (or fixed). You should complete the implementation and remove the comment.

        // TODO: Implement initDataSource() method.
    }

    protected function initForm(FormFactoryInterface $factory, $data = null)
    {
        // TODO: Implement initForm() method.
    }
}

Posted from SensioLabsInsight

Currently, if I understand correctly, when a user requests a password change, the token is being created only for existing users. So if you send a request for the same email twice, you will know if the current user exists in the database. That happens because you will receive a message about a token being already generated for this email only if the user exists. Perhaps we could create a password reset token regardless of user existence and make the user - token relation optional?