Skip to content
This repository was archived by the owner on Jul 24, 2020. It is now read-only.

[1422] Create helper method to check ENV variables #1583

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ def markdown_help
# modify redirect after signing in
def after_sign_in_path_for(user)
# CODE FOR CAS LOGIN --> NEW USER
if ENV['CAS_AUTH'] && current_user && current_user.id.nil? &&
if env?('CAS_AUTH') && current_user && current_user.id.nil? &&
current_user.username
# store username in session since there's a request in between
session[:new_username] = current_user.username
Expand Down
3 changes: 1 addition & 2 deletions app/controllers/import_users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ def valid_input_file?(imported_users, file) # rubocop:disable all
end

# make sure we have username data (otherwise all will always fail)
unless imported_users.first.keys.include?(:username) ||
ENV['CAS_AUTH'].nil?
unless imported_users.first.keys.include?(:username) || !env?('CAS_AUTH')
flash[:error] = 'Unable to import CSV file. None of the users will be '\
'able to log in without specifying \'username\' data.'
redirect_to(:back) && return
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def set_user
end

def check_cas_auth
@cas_auth = ENV['CAS_AUTH']
@cas_auth = env?('CAS_AUTH')
end

# ------------ end before filter methods ------------ #
Expand Down
12 changes: 6 additions & 6 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class User < ActiveRecord::Base
# :cas_authenticatable module. If not, we implement password authentcation
# using the :database_authenticatable module and also allow for password
# resets.
if ENV['CAS_AUTH']
if env?('CAS_AUTH')
devise :cas_authenticatable
else
devise :database_authenticatable, :recoverable, :rememberable
Expand Down Expand Up @@ -38,7 +38,7 @@ class User < ActiveRecord::Base
presence: true, uniqueness: true,
format: { with: /\A([\w\.%\+\-]+)@([\w\-]+\.)+([\w]{2,})\z/i }
# validations for CAS authentication
if ENV['CAS_AUTH']
if env?('CAS_AUTH')
validates :cas_login, presence: true, uniqueness: true
# validations for password authentication
else
Expand Down Expand Up @@ -92,9 +92,9 @@ def equipment_items
# rubocop:disable CyclomaticComplexity
def self.search_ldap(login)
return nil if login.blank?
return nil unless ENV['USE_LDAP']
return nil unless env?('USE_LDAP')

filter_param = if ENV['CAS_AUTH']
filter_param = if env?('CAS_AUTH')
Rails.application.secrets.ldap_login
else
Rails.application.secrets.ldap_email
Expand Down Expand Up @@ -139,7 +139,7 @@ def self.search_ldap(login)
.select { |s| s && !s.empty? }.join(' ')

# define username based on authentication method
out[:username] = if ENV['CAS_AUTH']
out[:username] = if env?('CAS_AUTH')
result[Rails.application.secrets.ldap_login.to_sym][0]
else
out[:email]
Expand All @@ -158,7 +158,7 @@ def self.select_options
end

def render_name
ENV['CAS_AUTH'] ? "#{name} #{username}" : name.to_s
env?('CAS_AUTH') ? "#{name} #{username}" : name.to_s
end

# ---- Reservation methods ---- #
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/_find_user.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<%= label_tag :searched_id, 'Find User' %>
<div class="input-group">
<%= autocomplete_field_tag 'fake_searched_id', '', autocomplete_user_last_name_users_path,
size: 30, update_elements: {id: '#searched_id'}, placeholder: "Enter a name or #{ENV['CAS_AUTH'] ? 'username' : 'email'}", class: 'submittable form-control' %>
size: 30, update_elements: {id: '#searched_id'}, placeholder: "Enter a name or #{env?('CAS_AUTH') ? 'username' : 'email'}", class: 'submittable form-control' %>
<%= hidden_field_tag 'searched_id' %>
<span class="input-group-btn">
<%= button_tag sanitize("#{content_tag :i, nil, class: "fa fa-search"}"), class: "btn btn-default", rel: 'tooltip', title: 'Search Users' %>
Expand Down
12 changes: 9 additions & 3 deletions config/environments/production.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true
Rails.application.configure do
ENV_FALSE = [0, '0', false, 'false', nil, ''].freeze

# Settings specified here will take precedence over those in
# config/application.rb

Expand All @@ -24,7 +26,8 @@

# Disable serving static files from the `/public` folder by default since
# Apache or NGINX already handles this.
config.serve_static_files = ENV['RAILS_SERVE_STATIC_FILES'].present?
config.serve_static_files =
!ENV_FALSE.include?(ENV['RAILS_SERVE_STATIC_FILES'])

# Compress JavaScripts and CSS.
config.assets.js_compressor = :uglifier
Expand Down Expand Up @@ -77,7 +80,8 @@
# config.action_controller.asset_host = 'http://assets.example.com'

# Disable e-mails if environment variable is set
config.action_mailer.perform_deliveries = ENV['DISABLE_EMAILS'].nil?
config.action_mailer.perform_deliveries =
ENV_FALSE.include?(ENV['DISABLE_EMAILS'])

# Disable delivery errors, bad email addresses will be ignored
# config.action_mailer.raise_delivery_errors = false
Expand Down Expand Up @@ -105,5 +109,7 @@
config.assets.precompile += %w(print.css)

# set up PartyFoul
config.middleware.use('PartyFoul::Middleware') if ENV['PARTY_FOUL_TOKEN']
unless ENV_FALSE.include?(ENV['PARTY_FOUL_TOKEN'])
config.middleware.use('PartyFoul::Middleware')
end
end
2 changes: 2 additions & 0 deletions config/initializers/000_env_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# frozen_string_literal: true
include EnvironmentHandler
4 changes: 2 additions & 2 deletions config/initializers/00_devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
config.skip_session_storage = [:http_auth]

# ==> Configuration for :database_authenticatable
unless ENV['CAS_AUTH']
unless env?('CAS_AUTH')
# For bcrypt, this is the cost for hashing the password and defaults to 10.
# If using other encryptors, it sets how many times you want the password
# re-encrypted.
Expand Down Expand Up @@ -72,7 +72,7 @@
end

# ==> devise_cas_authenticatable configuration
if ENV['CAS_AUTH']
if env?('CAS_AUTH')
# configure the base URL of your CAS server
config.cas_base_url = Rails.application.secrets.cas_base_url

Expand Down
6 changes: 3 additions & 3 deletions config/initializers/authentication.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
# frozen_string_literal: true
# Check for authentication method and copy data over if necessary (ENV variable
# to skip if necessary, skip if migrating from a pre-v4.1.0 DB or no table)
unless ENV['SKIP_AUTH_INIT'] || !User.table_exists? ||
unless env?('SKIP_AUTH_INIT') || !User.table_exists? ||
!User.column_names.include?('username')

user = User.first

# if we want to use CAS authentication and the username parameter doesn't
# match the cas_login parameter, we need to copy that over
if ENV['CAS_AUTH'] && user && (user.username != user.cas_login)
if env?('CAS_AUTH') && user && (user.username != user.cas_login)
# if there are any users that don't have cas_logins, we can't use CAS
if User.where(cas_login: ['', nil]).count > 0
raise 'There are users missing their CAS logins, you cannot use CAS '\
Expand All @@ -18,7 +18,7 @@
end
# if we want to use password authentication all users can reset their
# passwords so it doesn't matter if they already have them or not
elsif ENV['CAS_AUTH'].nil? && user && (user.username != user.email)
elsif env?('CAS_AUTH')
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be inverted?

User.update_all 'username = email'
end
end
4 changes: 2 additions & 2 deletions config/initializers/setup_mail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
}

# optional server authentication
if ENV['RES_SMTP_AUTH']
if env?('RES_SMTP_AUTH')
ActionMailer::Base.smtp_settings[:authentication] = :login
ActionMailer::Base.smtp_settings[:user_name] =
Rails.application.secrets.smtp_username
Expand All @@ -20,7 +20,7 @@
# logging of automatically sent emails
class MailObserver
def self.delivered_email(message)
if ENV['LOG_EMAILS']
if env?('LOG_EMAILS')
Rails.logger.info "Sent #{message.subject} to #{message.to}"
end
end
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/20150323013431_add_cas_login_to_users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ def change
add_column :users, :cas_login, :string

# copy username to cas_login if you use CAS already
if ENV['CAS_AUTH']
if env?('CAS_AUTH')
ActiveRecord::Base.connection
.execute('update users set cas_login=username')
end
Expand Down
4 changes: 2 additions & 2 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
end

# GLOBAL VARIABLES
MANUAL = ENV['manual'].present? || ENV['friendly'].present?
NO_PICS = ENV['no_pics'].present? || !MANUAL
MANUAL = env?('manual') || env?('friendly')
NO_PICS = env?('no_pics') || !MANUAL
IMAGES = Dir.glob(File.join(Rails.root, 'db', 'seed_images', '*'))

# run script
Expand Down
6 changes: 3 additions & 3 deletions lib/csv_import.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def import_users(array_of_user_data, overwrite = false, user_type = 'normal')
user_data[:role] = user_type
user_data[:csv_import] = true
next if attempt_save_with_csv_data?(user_data)
if ENV['USE_LDAP']
if env?('USE_LDAP')
attempt_save_with_ldap(user_data)
else
@array_of_fail << [user_data, 'Invalid user parameters.']
Expand All @@ -65,7 +65,7 @@ def import_users(array_of_user_data, overwrite = false, user_type = 'normal')
# otherwise it replaces the keys in the data hash with the ldap data.
def import_with_ldap(user_data)
# use username if using cas, email otherwise
ldap_param = user_data[ENV['CAS_AUTH'] ? :username : :email]
ldap_param = user_data[env?('CAS_AUTH') ? :username : :email]

# check LDAP for missing data
ldap_user_hash = User.search_ldap(ldap_param)
Expand All @@ -85,7 +85,7 @@ def import_with_ldap(user_data)
# tries to save using only the csv data. This method will return
# false if the data specified in the csv is invalid on the user model.
def attempt_save_with_csv_data?(user_data)
if ENV['CAS_AUTH']
if env?('CAS_AUTH')
# set the cas login
user_data[:cas_login] = user_data[:username]
else
Expand Down
8 changes: 8 additions & 0 deletions lib/environment_handler.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# frozen_string_literal: true
module EnvironmentHandler
FALSE = [0, '0', false, 'false', nil, ''].freeze

def env?(var)
!FALSE.include? ENV[var]
end
end
6 changes: 3 additions & 3 deletions lib/tasks/setup_application.rake
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace :app do
phone = STDIN.gets.chomp
puts 'Email Address:'
email = STDIN.gets.chomp
if ENV['CAS_AUTH']
if env?('CAS_AUTH')
puts 'Username (i.e. NetID, ensure this is correct):'
cas_login = STDIN.gets.chomp
username = cas_login
Expand All @@ -56,12 +56,12 @@ namespace :app do
u.last_name = last_name
u.phone = phone
u.email = email
u.cas_login = cas_login if ENV['CAS_AUTH']
u.cas_login = cas_login if env?('CAS_AUTH')
u.username = username
u.affiliation = affiliation
u.role = 'superuser'
u.view_mode = 'superuser'
unless ENV['CAS_AUTH']
unless env?('CAS_AUTH')
u.password = password
u.password_confirmation = password_confirmation
end
Expand Down
2 changes: 1 addition & 1 deletion spec/factories/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
role 'normal'
view_mode 'normal'

if ENV['CAS_AUTH']
if env?('CAS_AUTH')
username { cas_login }
else
username { email }
Expand Down
28 changes: 28 additions & 0 deletions spec/lib/environment_handler_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# frozen_string_literal: true
require 'spec_helper'
include EnvironmentHandler

describe EnvironmentHandler do
describe '.env?' do
Copy link
Contributor

Choose a reason for hiding this comment

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

As stated above, I think that we should include an empty string as being "falsey". We should also add a test for that.

it 'returns false when variable not set' do
allow(ENV).to receive(:[]).with('CAS_AUTH').and_return(nil)
expect(env?('CAS_AUTH')).to be_falsey
end
it 'returns false when variable set to 0' do
allow(ENV).to receive(:[]).with('CAS_AUTH').and_return('0')
expect(env?('CAS_AUTH')).to be_falsey
end
it 'returns false when variable set to false' do
allow(ENV).to receive(:[]).with('CAS_AUTH').and_return('false')
expect(env?('CAS_AUTH')).to be_falsey
end
it 'returns false when variable set to empty string' do
allow(ENV).to receive(:[]).with('CAS_AUTH').and_return('')
expect(env?('CAS_AUTH')).to be_falsey
end
it 'returns true when variable set to anything except 0 or false' do
allow(ENV).to receive(:[]).with('CAS_AUTH').and_return('true')
expect(env?('CAS_AUTH')).to be_truthy
end
end
end
2 changes: 1 addition & 1 deletion spec/mailers/user_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
end

it "doesn't log if the env is not set" do
expect(ENV['LOG_EMAILS']).to be_nil
expect(env?('LOG_EMAILS')).to be_falsey
expect(Rails.logger).to receive(:info).with(/Sent/).exactly(0).times
# force a request email; there is not an email for a basic reservation
@mail = UserMailer.reservation_status_update(@res,
Expand Down