Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Package refactoring #4

Closed
wants to merge 4 commits into from
Closed

Package refactoring #4

wants to merge 4 commits into from

Conversation

mlocati
Copy link

@mlocati mlocati commented Jun 1, 2018

  • Add job to update IPs
  • use standard concrete.security.trusted_proxies.ips configuration key
  • use the http client if available
  • throw exceptions when something fails
  • fix return code if CLI command

Fix #2
Fix #3

mlocati added 4 commits June 1, 2018 11:30
@mlocati
php-cs-fixer
38d3797
@mlocati
Package refactoring
ccc0242 
- Add job to update IPs
- use standard concrete.security.trusted_proxies.ips configuration key
- use the http client if available
- throw exceptions when something fails
- fix return code if CLI command
@mlocati
Add install_post element
6392ded
@mlocati
cf:ip:update should not be executed as root
fa59c2d
@KorvinSzanto
Copy link
Member

This package needs to affect runtime, and not affect the state of the install. This can be achieved with $config->set(...) but what we should do is have a middleware that determines the symfony version and manages the injection itself. If this change is merged it looks like uninstalling would leave you in a state where all cloudflare IPs are trusted and you have no way to remove them. If you open this PR against the feature/symfony-3 I can merge it and we can get it closer to something we'd release.

I think in order to have this merged into master it needs:

  1. It's own config that either merges into the core config requested in Use default concrete.security.trusted_proxies.ips configuration key? #2 at runtime, or manages the process itself (which is what I'd prefer)
  2. Move the on_start service provider functionality back into a service provider

@mlocati mlocati mentioned this pull request Jun 14, 2018
Merged
@mlocati mlocati deleted the switch-to-core-ip-list branch June 14, 2018 15:11
@mlocati
Copy link
Author

mlocati commented Jun 14, 2018

If you open this PR against the feature/symfony-3 I can merge it and we can get it closer to something we'd release.

Done: see #5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@mlocati @KorvinSzanto