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.

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

Fix initializer "web_console.permissions" #279

Merged
merged 3 commits into from
May 28, 2019
Merged

Fix initializer "web_console.permissions" #279

merged 3 commits into from
May 28, 2019

Conversation

patorash
Copy link

config.web_console.permissions is always exists.
As a result, config.web_console.whitelisted_ips never used.

config.web_console.permissions is always exists.
As a result, config.web_console.whitelisted_ips never used.
@gsamokovarov
Copy link
Collaborator

Thanks for the catch! Can you add a test to guard for it as well?

config.web_console.permissions initial value is not need.
Because Webconsole::Permissions have ALWAYS_PERMITTED_NETWORKS.
@patorash
Copy link
Author

@gsamokovarov
Sorry I couldn't add test. I challenged set config.web_console.whitelist_ips without new_uninitialized_app method. but I don't know reinitialize Rails app.

Railtieclass initialize config.web_console.permissions.

module WebConsole
class Railtie < ::Rails::Railtie
config.web_console = ActiveSupport::OrderedOptions.new
config.web_console.permissions = %w( 127.0.0.1 ::1 )

But new_uninitialized_app method set only app.config.web_console. It is not same value.

app = Class.new(Rails::Application)
app.config.web_console = ActiveSupport::OrderedOptions.new

At the beginning, I add app.config.web_console.permissions = %w( 127.0.0.1 ::1 ), but something is wrong...🤔
I read Permission source. I think config.web_console.permissions initial value is not need. Because Permission always includes localhost.

Therefore, I remove config.web_console.permissions initial value.

@gsamokovarov
Copy link
Collaborator

Thanks! Will include this in 4.0.1, which will be released a bit before Rails 6.

@gsamokovarov gsamokovarov merged commit 95bcf59 into rails:master May 28, 2019
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.

2 participants