diff --git a/app/controllers/mappings_controller.rb b/app/controllers/mappings_controller.rb index d69bcb1..3690132 100644 --- a/app/controllers/mappings_controller.rb +++ b/app/controllers/mappings_controller.rb @@ -3,6 +3,7 @@ class MappingsController < ApplicationController def index + @directory_permissions_command = Mapping.directory_permissions_command end @@ -15,23 +16,27 @@ def create @mapping = Mapping.new(mapping_params) if @mapping.save_and_set_facls - flash[:success] = @mapping.save_message + flash[:success] = 'Mapping successfully created.' redirect_to action: :index else - flash[:warning] = 'Unable to create new mapping. ' + @mapping.save_message - redirect_to new_mapping_path, locals: params + flash.now[:warning] = 'Unable to create new mapping. ' + @mapping.errors.full_messages.join(' ') + render :new end end # POST /mappings - def destroy - if Mapping.destroy_and_remove_facls(params[:id]) - flash[:success] = 'Successfully deleted mapping.' + def destroy + @mapping = Mapping.find(params[:id]) + if @mapping.destroy_and_remove_facls() + flash[:success] = 'Mapping successfully removed.' redirect_to action: :index else - flash[:danger] = 'Unable to delete mapping ' + params[:id] + flash[:danger] = 'Unable to remove mapping. ' + @mapping.errors.full_messages.join(' ') redirect_to action: :index end + rescue ActiveRecord::RecordNotFound + flash[:warning] = "Unable to find mapping #{params[:id]} to remove it." + redirect_to action: :index end diff --git a/app/helpers/mappings_helper.rb b/app/helpers/mappings_helper.rb index a7e9429..2f1ceec 100644 --- a/app/helpers/mappings_helper.rb +++ b/app/helpers/mappings_helper.rb @@ -1,23 +1,23 @@ -require 'uri' +require 'etc' +require 'pathname' module MappingsHelper def user_list - users_from_group = ENV['USERS_FROM_GROUP'] - result = `getent group #{users_from_group}` + Etc.getgrnam(Configuration.users_from_group).mem.sort + end - # example output - # "wiagstf:*:5362:mrodgers,efranz\n" - result.strip.split(':')[3].split(',').sort + def user_select_list + user_list.map { |user| [display_username(user), user] } end def user_list_help - "Users list is built from users in group: #{ENV['USERS_FROM_GROUP']}" + "Users list is built from users in group: #{Configuration.users_from_group}" end # Get a list of the various Shiny apps def app_list - Dir.glob( Configuration.shared_apps_root.join('bc_shiny_*')).sort + Dir.glob( Configuration.shared_apps_root.join('bc_shiny_*')).sort.map{|path| Pathname.new(path)} end def app_list_help @@ -26,7 +26,7 @@ def app_list_help def get_app_name(app_path) - URI(app_path).path.split('/').last + app_path.basename end @@ -44,4 +44,13 @@ def known_datasets def known_datasets_help "Known datasets include files or directories under #{Configuration.app_dataset_root.to_s} and arbitrary paths already added to this database" end + + # Attempt to get a full name for the user + # @return [String] + def display_username(user) + full_name = Etc.getpwnam(user).gecos.strip + full_name = full_name.empty? ? user : full_name + + "#{full_name} - #{user}" + end end diff --git a/app/models/mapping.rb b/app/models/mapping.rb index 3d0fbb6..0bcebea 100644 --- a/app/models/mapping.rb +++ b/app/models/mapping.rb @@ -6,19 +6,20 @@ class Mapping < ActiveRecord::Base - attr_accessor :save_message - validates :user, :app, :dataset, presence: true + attr_accessor :dataset_non_std_location_value + validates :user, :app, :dataset, presence: { message: "A %{attribute} must be selected." } validate :dataset_path_must_exist - validates_uniqueness_of :user, scope: [:user, :app], message: "Unable to create a second mapping between user and app." + validate :app_path_may_not_be_blank + validates_uniqueness_of :user, scope: [:user, :app], message: "may only be mapped once to a given app." # Type dataset as a Pathname def dataset - Pathname.new(super) + Pathname.new(super.to_s) end # Type app as a Pathname def app - Pathname.new(super) + Pathname.new(super.to_s) end # @return [Array] @@ -46,7 +47,17 @@ def self.dump_to_yaml # Ensure that a user can use a given mapping # @return [Boolean] def is_still_valid? - return app.exist? && dataset.exist? && user_has_permissions_on_both? + app.exist? && dataset.exist? && user_has_permissions_on_both? + end + + # Explain why a given mapping is not valid + # @return [String] + def reason_invalid + [ + app.exist? ? nil : "#{app} is no longer visible on the file system.", + dataset.exist? ? nil : "#{dataset} is no longer visible on the file system.", + user_has_permissions_on_both? ? nil : "User #{user} does not have read/write permissions on both `#{app}` and `#{dataset}`." + ].select{ |value| ! value.nil? }.join(' ') end # @return [Hash] @@ -55,36 +66,34 @@ def to_hash end # Custom destructor - def self.destroy_and_remove_facls(id) + def destroy_and_remove_facls begin - mapping = find(id) - mapping.remove_rx_facl(mapping.dataset) - mapping.remove_rx_facl(mapping.app) - mapping.destroy - dump_to_yaml - @save_message = 'Mapping successfully destroyed.' + remove_rx_facl(dataset) + remove_rx_facl(app) + destroy + Mapping.dump_to_yaml return true rescue OodSupport::InvalidPath, OodSupport::BadExitCode => e - @save_message = 'Unable to destroy mapping because ' + e.to_s + @errors[:base] << 'Unable to destroy mapping because ' + e.to_s - return false - rescue ActiveRecord::RecordNotFound # User is probably mashing the delete return false end end - # Create a user readable string from the error.messages hash - def format_error_messages - @save_message = errors.messages.map {|_, message| message}.join(' ') + # Builds an Array of apps that need their permissions changed to include the C attribute for GROUP + # + # An example of acceptable FACL entry for GROUP: + # A:g:GROUP@:rwaDxtTnNcCy + # + # @return [Pathname] + def self.installed_apps_with_busted_permissions? + ApplicationController.helpers.app_list.select { |app| ! can_modify_facl?(app) } end # Custom save method def save_and_set_facls - unless valid? - format_error_messages - return false - end + return false unless valid? begin success = save(:validate => false) @@ -92,15 +101,10 @@ def save_and_set_facls add_rx_facl(app) add_rx_facl(dataset) Mapping.dump_to_yaml - @save_message = 'Mapping successfully created.' return true - rescue ActiveRecord::RecordNotUnique => e - @save_message = "Unable to create duplicate mapping between #{user}, #{app}, and #{dataset}" - - return false rescue OodSupport::InvalidPath, OodSupport::BadExitCode => e - @save_message = "Unable to set FACLS because " + e.to_s + @errors[:base] << "Unable to set FACLS because " + e.to_s return false end @@ -126,13 +130,13 @@ def self.permission_sensitive_dirs # @return [String] def self.directory_permissions_command - permission_sensitive_dirs.map{|directory| "chmod 0775 #{directory.to_s}"}.join(' && ') + permission_sensitive_dirs.map{|directory| "chmod 2775 #{directory.to_s}"}.join(' && ') end # @return [Boolean] def should_add_facl?(pathname) # Calling owned first protects rx_facl_exists? from throwing InvalidPath - pathname.owned? && ! rx_facl_exists?(pathname) + Mapping.can_modify_facl?(pathname) && ! rx_facl_exists?(pathname) end # Idempotently add a RX entry to the ACL for a file @@ -141,6 +145,8 @@ def add_rx_facl(pathname) entry = build_facl_entry_for_user(user, Configuration.facl_user_domain) OodSupport::ACLs::Nfs4ACL.add_facl(path: pathname, entry: entry) + + logger.debug { "add_rx_facl(#{pathname}) succeeded for #{user} and #{pathname.to_s}" } end # Check if pathname / user combination occurs once or less in the database @@ -154,7 +160,7 @@ def pathname_uniq_for_user?(pathname) # @return [Boolean] def should_remove_facl?(pathname) - pathname.owned? && rx_facl_exists?(pathname) && pathname_uniq_for_user?(pathname) + Mapping.can_modify_facl?(pathname) && rx_facl_exists?(pathname) && pathname_uniq_for_user?(pathname) end # Conditionally remove RX FACLs @@ -166,6 +172,8 @@ def remove_rx_facl(pathname) entry = build_facl_entry_for_user(user, Configuration.facl_user_domain) OodSupport::ACLs::Nfs4ACL.rem_facl(path: pathname, entry: entry) + + logger.debug { "remove_rx_facl(#{pathname}) succeeded for #{user} and #{pathname.to_s}" } end # Build FACL for user and domain combination @@ -188,7 +196,7 @@ def rx_facl_exists?(pathname) expected = build_facl_entry_for_user(user, Configuration.facl_user_domain) return acl.entries.include?(expected) - rescue + rescue OodSupport::InvalidPath, OodSupport::BadExitCode return false end end @@ -199,9 +207,10 @@ def user_has_permissions_on_both? ood_user = OodSupport::User.new(user) required_permissions = [:r, :x] - app_facl = OodSupport::ACLs::Nfs4ACL.get_facl(path: app) + app_facl = nil dataset_facl = nil begin + app_facl = OodSupport::ACLs::Nfs4ACL.get_facl(path: app) dataset_facl = OodSupport::ACLs::Nfs4ACL.get_facl(path: dataset) rescue OodSupport::InvalidPath, OodSupport::BadExitCode return false @@ -221,9 +230,56 @@ def dataset_path_must_exist errors.add(:base, "Dataset must exist.") unless dataset.exist? end + # Validator for app + def app_path_may_not_be_blank + errors.add(:base, "You must select an app.") unless app.exist? + end + # Directories between $HOME and the configured database directory must be set to 775 # @return [Boolean] def self.directory_perms_are_775?(directory) directory.stat.mode.to_s(8).end_with?('775') end + + # Checks to see if FACLs are modifiable by the user + # + # Note that this check assumes that no negative permissions have been set. + # Also we are not using an ownership check since that could mask problems + # with permissions that would impact other users. + # + # @return [Boolean] + def self.can_modify_facl?(pathname) + pathname.exist? && group_facl_entry_has_C_set?(pathname) + end + + # Get the group owner's name + # @return [String] + def self.get_groupname_for_pathname(pathname) + Etc.getgrgid(pathname.stat.gid).name + end + + # Check if pathname is owned by the admin_group + # @return [Boolean] + def self.app_has_correct_group_ownership?(pathname) + get_groupname_for_pathname(pathname) == Configuration.admin_group + end + + # Does the app have permission modification enabled for the GROUP principle + # @return [Boolean] + def self.group_facl_entry_has_C_set?(pathname) + result = false + begin + result = OodSupport::ACLs::Nfs4ACL.get_facl( + path: pathname + ).entries.select{ + |entry| entry.principle == 'GROUP' + }.first.permissions.include?(:C) + rescue OodSupport::InvalidPath, OodSupport::BadExitCode + logger.fatal { "Unable to check FACL for #{pathname} - is it on an NFS4 file system?" } + end + + logger.debug { "group_facl_entry_has_C_set?(#{pathname}) == #{result}" } + + result + end end diff --git a/app/views/mappings/index.html.erb b/app/views/mappings/index.html.erb index c5e64d5..b46f6ad 100644 --- a/app/views/mappings/index.html.erb +++ b/app/views/mappings/index.html.erb @@ -1,7 +1,7 @@ <% if Mapping.has_directory_permission_errors? %> <% end %>
@@ -19,10 +19,10 @@ <% Mapping.find_each do |mapping| %> - <%= mapping.user %> + <%= display_username(mapping.user) %> <%= mapping.app.split.last %> <%= mapping.dataset %> - <%= mapping.is_still_valid? ? fa_icon('check') : fa_icon('exclamation-triangle') %> + <%= mapping.is_still_valid? ? fa_icon('check') : fa_icon('exclamation-triangle') + ' ' + mapping.reason_invalid %> <%= mapping.updated_at %> <%= link_to 'Destroy', mapping_path(mapping), method: :delete, class: 'btn btn-danger', data: { confirm: 'Are you sure you want to delete this mapping?' } %> diff --git a/app/views/mappings/new.html.erb b/app/views/mappings/new.html.erb index 00e3d12..7c62759 100644 --- a/app/views/mappings/new.html.erb +++ b/app/views/mappings/new.html.erb @@ -4,8 +4,8 @@ <%= bootstrap_form_for(:mapping, url: mappings_path) do |f| %> <%= f.alert_message 'Please fix the errors below.' %> - <%= f.select :user, user_list, :include_blank => true, help: user_list_help %> - <%= f.select :app, app_list.map{|uri| [get_app_name(uri), uri]}, :include_blank => true, help: app_list_help %> + <%= f.select :user, user_select_list, :include_blank => true, help: user_list_help %> + <%= f.select :app, app_list.map{ |uri| [get_app_name(uri), uri]}, :include_blank => true, help: app_list_help %> <%= f.form_group :dataset, label: { text: 'Dataset (absolute path)' }, help: known_datasets_help do %> <% known_datasets.each do |dataset| %> diff --git a/app/views/mappings/no_apps_installed.html.erb b/app/views/mappings/no_apps_installed.html.erb index 2224ff7..cb10b72 100644 --- a/app/views/mappings/no_apps_installed.html.erb +++ b/app/views/mappings/no_apps_installed.html.erb @@ -1,8 +1,8 @@

No Apps Installed

-

Please install one or more apps to <%= ENV['SHARED_APPS_ROOT'] =%>.

-

Datasets should be installed to <%= ENV['APP_DATASET_ROOT'] =%>.

+

Please install one or more apps to <%= Configuration.shared_apps_root =%>.

+

Datasets should be installed to <%= Configuration.app_dataset_root =%>.

<%= link_to 'Back', mappings_path, class: 'btn btn-default' %>
diff --git a/config/configuration_singleton.rb b/config/configuration_singleton.rb index 1d4f37c..f0f1be3 100644 --- a/config/configuration_singleton.rb +++ b/config/configuration_singleton.rb @@ -96,6 +96,10 @@ def facl_user_domain ENV['FACL_USER_DOMAIN'] end + def users_from_group + ENV['USERS_FROM_GROUP'] + end + def yaml_file_path Pathname.new(ENV['SHARED_APPS_ROOT']).expand_path().join('mappings.yaml') end diff --git a/config/environments/development.rb b/config/environments/development.rb index 854abbd..7625213 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -10,6 +10,8 @@ # config.eager_load = false config.eager_load = true + config.colorize_logging = false + # Show full error reports and disable caching. config.consider_all_requests_local = true config.action_controller.perform_caching = false