Skip to content

Commit 1904ec0

Browse files
sidhochou
authored andcommitted
Fix IP fencing
[finishes #99446152] Signed-off-by: Jenny Chou <[email protected]> Moving IPs to ENV variable Signed-off-by: Sid Ho <[email protected]> Signed-off-by: Jenny Chou <[email protected]> Signed-off-by: Sid Ho <[email protected]> Signed-off-by: Jenny Chou <[email protected]>
1 parent 76db89d commit 1904ec0

File tree

7 files changed

+82
-25
lines changed

7 files changed

+82
-25
lines changed

.gitignore

+2
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,5 @@ manifest.yml
2525
cf-cli.deb
2626

2727
coverage
28+
29+
config/ips.yml
+11-4
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,22 @@
11
class ApplicationController < ActionController::Base
22
protect_from_forgery
3-
before_filter :boxed
43
before_filter :require_login
4+
before_filter :boxed
55

66
def require_login
7-
redirect_to '/login' unless session[:logged_in]
7+
mapper = IpFencer.new(hydrate_ips)
8+
redirect_to '/login' unless session[:logged_in] || mapper.authorized?(request.remote_ip)
89
end
910

10-
# Adds an outer container element around any yielded HTML.
11-
# TODO: Get rid of this. We shouldn't toggle an HTML wrapping in the controller.
1211
def boxed
1312
@boxed = true
1413
end
14+
15+
def hydrate_ips
16+
ips = ENV['IP_WHITELIST'].split(',')
17+
18+
ips.map do |ip|
19+
IPAddr.new(ip)
20+
end
21+
end
1522
end

app/models/ip_fencer.rb

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
class IpFencer
2+
def initialize (authorized_ips)
3+
@authorized_ips = authorized_ips
4+
end
5+
6+
def authorized?(ip_address_string)
7+
begin
8+
ip_address = IPAddr.new(ip_address_string)
9+
return @authorized_ips.any? { |ip| ip.include? ip_address }
10+
11+
rescue
12+
13+
Rails.logger.debug "Rescued from exception while authenticating IP: '#{ip_address_string}'"
14+
return false
15+
end
16+
end
17+
18+
alias_method :includes?, :authorized?
19+
end

db/seeds.rb

-7
This file was deleted.

script/deploy-staging.sh

+1
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,6 @@ if [[ "$TRAVIS_BRANCH" == "master" ]]; then
1010
git checkout .
1111
cf set-env whiteboard-acceptance OKTA_SSO_TARGET_URL $OKTA_SSO_TARGET_URL
1212
cf set-env whiteboard-acceptance OKTA_CERT_FINGERPRINT $OKTA_CERT_FINGERPRINT
13+
cf set-env whiteboard-acceptance IP_WHITELIST $IP_WHITELIST
1314
bundle exec rake SPACE=whiteboard cf:deploy:staging
1415
fi

spec/features/authentication_spec.rb

+30-14
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,47 @@
11
require 'rails_helper'
22

33
describe 'Authenticating', type: :feature do
4-
describe 'Logging In' do
4+
describe 'Logging Out' do
55
before do
66
log_in_to_okta('[email protected]')
7+
visit '/login'
8+
click_on 'Log in with Okta'
79
end
810

911
it 'allows you to log in and view the dashboard' do
10-
visit '/login'
11-
expect(page).to have_content('Log in with Okta')
12-
click_on 'Log in with Okta'
1312
expect(page).to have_content('Whiteboard')
14-
expect(page).to have_content('Choose a Standup'.upcase)
13+
click_on 'Log Out'
14+
expect(page).to have_content('You have been logged out.')
1515
end
1616
end
1717

18-
describe 'Logging Out' do
19-
before do
20-
log_in_to_okta('[email protected]')
21-
visit '/login'
22-
click_on 'Log in with Okta'
18+
context 'from a non-whitelisted ip' do
19+
describe 'Logging In' do
20+
before do
21+
log_in_to_okta('[email protected]')
22+
end
23+
24+
it 'allows you to log in and view the dashboard' do
25+
visit '/login'
26+
expect(page).to have_content('Log in with Okta')
27+
click_on 'Log in with Okta'
28+
expect(page).to have_content('Whiteboard')
29+
expect(page).to have_content('Choose a Standup'.upcase)
30+
end
2331
end
32+
end
2433

25-
it 'allows you to login and view the dashboard' do
26-
expect(page).to have_content('Whiteboard')
27-
click_on 'Log Out'
28-
expect(page).to have_content('You have been logged out.')
34+
context 'from a whitelisted ip' do
35+
describe 'logging in' do
36+
before do
37+
page.driver.header('X-Forwarded-For', '50.194.143.46')
38+
end
39+
40+
it 'should not force user to authenticate' do
41+
visit '/'
42+
expect(page).not_to have_content('Log in with Okta')
43+
expect(page).to have_content('Choose a Standup'.upcase)
44+
end
2945
end
3046
end
3147
end

spec/models/ip_fencer_spec.rb

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
require 'rails_helper'
2+
3+
describe IpFencer do
4+
describe '#authorized?' do
5+
let(:validIp1) { IPAddr.new('127.0.0.8') }
6+
let(:validIp2) { IPAddr.new('168.2.1.3') }
7+
let(:authorized_ips) { [validIp1, validIp2] }
8+
let(:mapper) { IpFencer.new(authorized_ips) }
9+
10+
it 'can tell if a ip is authorized' do
11+
expect(mapper.authorized?('127.0.0.8')).to eq true
12+
expect(mapper.authorized?('168.2.1.3')).to eq true
13+
end
14+
15+
it 'returns false with an invalid address' do
16+
expect(mapper.authorized?('100.255.13.88')).to eq false
17+
end
18+
end
19+
end

0 commit comments

Comments
 (0)