Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

Added security configuration #47

Merged
merged 1 commit into from
Feb 1, 2017
Merged

Added security configuration #47

merged 1 commit into from
Feb 1, 2017

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Jan 30, 2017

/fixes #45

This makes it easier to secure the API routes exposed by this bundle. By default, no access is granted (already in master). Users can configure which access is granted to which roles (added in this PR). For instance:

cmf_resource_rest:
  security:
    access_control:
      # All visitors can read from /cms/public
      - { path: ^/cms/public, attributes: [CMF_RESOURCE_READ], require: IS_AUTHENTICATED_ANONYMOUSLY }

      # Only users can read from /cms/members-only
      - { path: ^/cms/members-only, attributes: [CMF_RESOURCE_READ], require: ROLE_USER }

      # Only admins can write
      - { path: ^/, attributes: [CMF_RESOURCE_WRITE], require: ROLE_ADMIN }

@dbu
Copy link
Member

dbu commented Jan 30, 2017

looks good to me. just to be sure, can you clarify why its not enough to configure routing security in the security.yml? are there operations where the resource path is not part of the url?

please rebase on master to get the style fixes, and then let styleci fix the new ones.

the ConfigurationTest needs to be adjusted for the new config

@wouterj wouterj force-pushed the issue-45/default-voter branch from 04b317b to 95eaad9 Compare January 30, 2017 13:33
@wouterj wouterj force-pushed the issue-45/default-voter branch from 95eaad9 to 2dd330c Compare January 30, 2017 13:33
@wouterj
Copy link
Member Author

wouterj commented Jan 30, 2017

@dbu the big difference is whitelisting vs blacklisting. The default Symfony security works using blacklisting, while this bundle's security works using whitelisting.

Security is extremely important for this bundle: If not configured correctly, people will have access to the complete CR backend and all data that's stored inside it. That's why I think it's extremely important to not allow any access by default, forcing the developer to think correctly about what and who to allow accessing the API.

@dbu
Copy link
Member

dbu commented Jan 30, 2017

thanks, agree with that! lets use this in the introduction to the bundle documentation...

@wouterj wouterj added this to the 1.0 milestone Jan 30, 2017
Copy link
Member

@ElectricMaxxx ElectricMaxxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you don't like them, but i would expect a unit test case for the security map (from configuration into param and as arguemnt into service), cause you expect a behavior in the unit test for the voter depending on a map you hopefully get through the configiguration.

@wouterj
Copy link
Member Author

wouterj commented Feb 1, 2017

@ElectricMaxxx there is already a configuration test in this PR. It should cover all cases.

@dbu dbu merged commit 548e312 into master Feb 1, 2017
@dbu dbu deleted the issue-45/default-voter branch February 1, 2017 09:13
@dbu
Copy link
Member

dbu commented Feb 1, 2017

created symfony-cmf/symfony-cmf-docs#813 about the documentation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add configuration option for security paths
3 participants