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

Allow ips #2998

Merged
merged 2 commits into from
Sep 12, 2023
Merged

Allow ips #2998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ef07f3a
add ip addresses to allowed_hosts if servername is nil
johrstrom Aug 31, 2023
782ab1d
update test cases
johrstrom Aug 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion ood-portal-generator/lib/ood_portal_generator/view.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require "digest/sha1"
require "erb"
require 'socket'

module OodPortalGenerator
# A view class that renders an OOD portal Apache configuration file
Expand Down Expand Up @@ -131,13 +132,31 @@ def log_filename(value,log_type)

def allowed_hosts
config_hosts = []
config_hosts << @servername unless @servername.nil?

add_servername_or_ips(config_hosts)
config_hosts << @proxy_server unless @proxy_server.nil?
config_hosts.concat(@server_aliases)

return nil if config_hosts.empty?

config_hosts.sort.uniq
end

def add_servername_or_ips(config_hosts)
# if @servername is nil, they're trying to use ip addresses
if @servername.nil?
config_hosts.concat(ip_addresses)
else
config_hosts << @servername
end
end

def ip_addresses
Socket.ip_address_list.select(&:ipv4?)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not clear to me if we should allow ipv6 addresses.

What's more is, if I do enable ipv6 address - I'm not really sure how to use them. They seem to have the interface name appended to it, so I'm not entirely sure if we need to cut that off or what.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure need to worry too much about what IPs people will get since the best practice is to supply DNS names like ondemand.example.com that correspond to certificate CNs, etc. I think IPv4 is good enough as that's most common.

.reject(&:ipv4_loopback?)
.map(&:ip_address)
end

# Helper method to escape IP for maintenance rewrite condition
def escape_ip(value)
# Value is already escaped
Expand Down
1 change: 1 addition & 0 deletions ood-portal-generator/spec/application_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
allow(OodPortalGenerator).to receive(:debian?).and_return(false)
allow_any_instance_of(OodPortalGenerator::Dex).to receive(:default_secret_path).and_return(dex_secret_path.path)
allow(SecureRandom).to receive(:uuid).and_return('83bc78b7-6f5e-4010-9d80-22f328aa6550')
allow(Socket).to receive(:ip_address_list).and_return([Addrinfo.ip("8.8.8.8")])
end

after(:each) do
Expand Down
2 changes: 1 addition & 1 deletion ood-portal-generator/spec/fixtures/ood-portal.conf.dex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
#
SetEnv OOD_PUN_STAGE_CMD "sudo /opt/ood/nginx_stage/sbin/nginx_stage"

SetEnv OOD_ALLOWED_HOSTS "example.com"
SetEnv OOD_ALLOWED_HOSTS "8.8.8.8,example.com"

#
# Below is used for sub-uri's this Open OnDemand portal supports
Expand Down
2 changes: 1 addition & 1 deletion ood-portal-generator/spec/fixtures/ood-portal.conf.maint_with_ips
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
#
SetEnv OOD_PUN_STAGE_CMD "sudo /opt/ood/nginx_stage/sbin/nginx_stage"

SetEnv OOD_ALLOWED_HOSTS "example.com"
SetEnv OOD_ALLOWED_HOSTS "8.8.8.8,example.com"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears that @proxy_server is never nil because it's populated by the fqdn. So that's why example.com almost always shows up here in OOD_ALLOWED_HOSTS.


#
# Below is used for sub-uri's this Open OnDemand portal supports
Expand Down
2 changes: 1 addition & 1 deletion ood-portal-generator/spec/fixtures/ood-portal.conf.nomaint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
#
SetEnv OOD_PUN_STAGE_CMD "sudo /opt/ood/nginx_stage/sbin/nginx_stage"

SetEnv OOD_ALLOWED_HOSTS "example.com"
SetEnv OOD_ALLOWED_HOSTS "8.8.8.8,example.com"

#
# Below is used for sub-uri's this Open OnDemand portal supports
Expand Down
2 changes: 1 addition & 1 deletion ood-portal-generator/spec/fixtures/output/auth.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
#
SetEnv OOD_PUN_STAGE_CMD "sudo /opt/ood/nginx_stage/sbin/nginx_stage"

SetEnv OOD_ALLOWED_HOSTS "example.com"
SetEnv OOD_ALLOWED_HOSTS "8.8.8.8,example.com"

#
# Below is used for sub-uri's this Open OnDemand portal supports
Expand Down
2 changes: 1 addition & 1 deletion ood-portal-generator/spec/fixtures/output/auth_deb.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
#
SetEnv OOD_PUN_STAGE_CMD "sudo /opt/ood/nginx_stage/sbin/nginx_stage"

SetEnv OOD_ALLOWED_HOSTS "example.com"
SetEnv OOD_ALLOWED_HOSTS "8.8.8.8,example.com"

#
# Below is used for sub-uri's this Open OnDemand portal supports
Expand Down
1 change: 1 addition & 0 deletions ood-portal-generator/spec/update_ood_portal_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
allow(OodPortalGenerator).to receive(:apache_group).and_return('apache')
allow(OodPortalGenerator).to receive(:debian?).and_return(false)
allow(OodPortalGenerator).to receive(:fqdn).and_return('example.com')
allow(Socket).to receive(:ip_address_list).and_return([Addrinfo.ip("8.8.8.8")])
end

after(:each) do
Expand Down