Skip to content

Enhanced Firewall Restrictions docs#3681

Merged
weaverryan merged 1 commit intosymfony:masterfrom
danez:restrict-firewall-change
Mar 19, 2014
Merged

Enhanced Firewall Restrictions docs#3681
weaverryan merged 1 commit intosymfony:masterfrom
danez:restrict-firewall-change

Conversation

@danez
Copy link
Copy Markdown
Contributor

@danez danez commented Mar 14, 2014

see symfony/symfony#10404

And as I'm not native english, someone please read my changes :)

fabpot added a commit to symfony/symfony that referenced this pull request Mar 14, 2014
…wall config (danez)

This PR was submitted for the 2.4 branch but it was merged into the 2.5-dev branch instead (closes #10404).

Discussion
----------

[Security] Match request based on HTTP methods in firewall config

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        | symfony/symfony-docs#3681

For an api I had to work on, I was in the need to have different firewalls for different HTTP Methods. I started building my own ```RequestMatcher``` until I discovered, that the regular RequestMatcher is able to handle HTTP Methods. The only thing that is missing is the possibility to configure it in the firewall section of the configuration. (For access_control it is already possible)

With this PR it is possible to do things like this:
```yaml
security:
    firewalls:
        api_options:
            pattern:    ^/
            methods:    [OPTIONS]
            security:   false
        api:
            pattern:    ^/
            some_auth:  true
```

I think this integrates quite nicely. Or is there any downside you can think of?

If it is good to go, I'll open a PR for the docs.

Commits
-------

2878757 Make it possible to match the request based on HTTP methods in the firewall configuration
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is there a specific reason you did remove this article? I can't find something that says this is deprecated now in your PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I renamed it to firewall_restrictions
That's because I added all information on "how to restrict/match a route in the firewall" in this file and the name was not explaining is content anymore, because it has not only information about host restriction, but also http methods and pattern.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, now I see why :) You moved this more specific usecase to an article with a general overview, great choice!

However, this will break sites linking to this article. Can you please readd it, but then without text and just linking to the new article? You can see an example of this here: http://symfony.com/doc/2.3/cookbook/form/use_virtuals_forms.html

@wouterj
Copy link
Copy Markdown
Member

wouterj commented Mar 14, 2014

Thank you for submitting such a detailed documentation!

@danez
Copy link
Copy Markdown
Contributor Author

danez commented Mar 14, 2014

I changed everything you mentioned in new commit. (to see the actual fixes) I can squash the commits before merging if desired.

@wouterj
Copy link
Copy Markdown
Member

wouterj commented Mar 14, 2014

except from #3681 (comment), I'm 👍 !

@danez
Copy link
Copy Markdown
Contributor Author

danez commented Mar 14, 2014

Ok the redirect is now also there.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

these

@weaverryan
Copy link
Copy Markdown
Contributor

@danez Looks great - I just added one tiny comment :).

Squashed commits:
[2081fc4] Enhanced Firewall Restrictions docs for PR symfony/symfony#10404
[8f065bc] Add redirect to new page
[b42e80f] fixed typos
[cc6b07d] tiny update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants