diff --git a/Gemfile b/Gemfile index e84c1ef21..872cef8c8 100644 --- a/Gemfile +++ b/Gemfile @@ -89,6 +89,7 @@ group :development do gem 'listen' gem 'puma' gem 'web-console' + gem 'rdoc', '>= 8.0' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index f4386c8c0..a44910dc0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -547,6 +547,10 @@ GEM rb-fsevent (0.11.2) rb-inotify (0.10.1) ffi (~> 1.0) + rbs (4.0.3) + logger + prism (>= 1.6.0) + tsort rdf (3.3.4) bcp47_spec (~> 0.2) bigdecimal (~> 3.1, >= 3.1.5) @@ -615,9 +619,10 @@ GEM rdf-xsd (3.3.0) rdf (~> 3.3) rexml (~> 3.2) - rdoc (7.2.0) + rdoc (8.0.0) erb - psych (>= 4.0.0) + prism (>= 1.6.0) + rbs (>= 4.0.0) tsort readline (0.0.4) reline @@ -906,6 +911,7 @@ DEPENDENCIES rails-controller-testing rails-i18n rails_admin + rdoc (>= 8.0) recaptcha redcarpet redis diff --git a/app/assets/config/manifest.js b/app/assets/config/manifest.js index 83f344bbc..76323cca5 100644 --- a/app/assets/config/manifest.js +++ b/app/assets/config/manifest.js @@ -1,3 +1,4 @@ //= link_tree ../images //= link_directory ../stylesheets/themes .css -//= link application.js \ No newline at end of file +//= link application.js +//= link show_private_groups.js \ No newline at end of file diff --git a/app/assets/javascripts/autocompleters.js b/app/assets/javascripts/autocompleters.js index 3dbbb7173..4cc142d97 100644 --- a/app/assets/javascripts/autocompleters.js +++ b/app/assets/javascripts/autocompleters.js @@ -45,7 +45,14 @@ var Autocompleters = { return { value: name, data: { id: item[config.idField], item: item } }; }) }; - } + }, + groups: function (response, config) { + return { + suggestions: $.map(response, function (item) { + return { value: item.title, data: { id: item[config.idField], item: item } }; + }) + }; + }, }, init: function () { diff --git a/app/assets/javascripts/show_private_groups.js b/app/assets/javascripts/show_private_groups.js new file mode 100644 index 000000000..e6f9a5c41 --- /dev/null +++ b/app/assets/javascripts/show_private_groups.js @@ -0,0 +1,23 @@ +// executed for space form +function show_private_groups() { + let checkbox = document.getElementById("space_is_private") + let container = document.getElementById("groups_container") + + const toggleGroups = () => { + if (checkbox && checkbox.checked) { + container.style.display = "block" + } else { + container.style.display = "none" + } + } + + if (checkbox) { + checkbox.addEventListener("change", toggleGroups) + } + + toggleGroups() +} + +window.addEventListener('turbolinks:load', function() { + show_private_groups(); +}); diff --git a/app/assets/javascripts/templates/autocompleter/group_id.hbs b/app/assets/javascripts/templates/autocompleter/group_id.hbs new file mode 100644 index 000000000..dbc4f6764 --- /dev/null +++ b/app/assets/javascripts/templates/autocompleter/group_id.hbs @@ -0,0 +1,7 @@ +
  • + + {{ item.title }} + + + +
    • \ No newline at end of file diff --git a/app/assets/javascripts/templates/autocompleter/group_member.hbs b/app/assets/javascripts/templates/autocompleter/group_member.hbs new file mode 100644 index 000000000..5da65bb76 --- /dev/null +++ b/app/assets/javascripts/templates/autocompleter/group_member.hbs @@ -0,0 +1,18 @@ +
  • + + {{ item.name }} + {{#if item.email}} + {{ item.email }} + {{/if}} + + + + +
    • diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 505184359..7fe02b68f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,7 +1,15 @@ require 'private_address_check' require 'private_address_check/tcpsocket_ext' -# The controller for actions related to the core application +# Base controller for the whole application. +# +# ApplicationController centralizes cross-cutting concerns shared by every +# controller in TeSS: authentication (Devise + token auth), authorization +# (Pundit), multi-space resolution, error rendering, and a couple of small +# utility endpoints (+test_url+, +job_status+). +# +# All other controllers should inherit from this class rather than directly +# from ActionController::Base. class ApplicationController < ActionController::Base include BreadCrumbs include PublicActivity::StoreController @@ -30,10 +38,29 @@ class ApplicationController < ActionController::Base rescue_from Pundit::NotAuthorizedError, with: :user_not_authorized + # Builds the context object passed to every Pundit policy. + # + # Bundling +current_user+ together with the +request+ lets policies make + # decisions based on how the request was made (e.g. JSON API vs HTML), + # in addition to who is making it. + # + # Returns:: a Pundit::CurrentContext wrapping the current user and request. def pundit_user Pundit::CurrentContext.new(current_user, request) end + # Renders a generic error page/response for a given HTTP status code. + # + # Accepts either a numeric or symbolic status code (e.g. :forbidden, + # :not_found) and renders the appropriate HTML error page, or a JSON/JSON:API + # error payload depending on the requested format. Falls back to a + # translated default message when none is supplied. + # + # status_code:: Integer or Symbol HTTP status code (default: 500). May be + # overridden by the params[:status_code] value set by + # the routes for 500, 503, 422 and 404 errors. + # message:: optional String error message to display; defaults to a + # localized message for the given status code. def handle_error(status_code = 500, message = nil) status_code = (params[:status_code] || status_code) # params[:status_code] comes from routes for 500, 503, 422 and 404 errors if status_code.is_a?(Symbol) # Convert :forbidden, :not_found, etc. to 403, 404 etc. @@ -56,6 +83,12 @@ def handle_error(status_code = 500, message = nil) end end + # Checks whether a given URL is reachable, guarding against SSRF by only + # allowing connections to public addresses (via PrivateAddressCheck). + # + # Expects params[:url] to contain the URL to test. Responds with a + # JSON body describing the outcome (HTTP code on success, or an explanatory + # message on failure/invalid URL). def test_url body = {} @@ -79,6 +112,11 @@ def test_url end end + # Returns the status of a background job (Sidekiq) as JSON. + # + # Expects params[:id] to be the Sidekiq job id. Responds with + # 404 and { status: 'not-found' } if no status is found + # for that id. def job_status begin status = Sidekiq::Status::status(params[:id]) @@ -97,33 +135,72 @@ def job_status private + # Checks whether the given feature is enabled for the current space. + # + # feature:: String or Symbol feature key (see Space::FEATURES). + # + # Returns:: +true+ or +false+. def feature_enabled?(feature) Space.current_space.feature_enabled?(feature) end helper_method :feature_enabled? + # before_action-style guard that raises a routing error (resulting in a + # 404) when the given feature is disabled globally via TeSS::Config. + # + # feature:: String or Symbol feature key; defaults to the current + # controller's name. + # + # Raises:: ActionController::RoutingError if the feature is explicitly + # disabled in the application configuration. def ensure_feature_enabled(feature = controller_name) if TeSS::Config.feature.key?(feature) && !TeSS::Config.feature[feature] raise ActionController::RoutingError.new('Feature not enabled') end end + # Rescue handler for Pundit::NotAuthorizedError. + # + # Renders a localized "forbidden" error message based on the policy and + # query that denied access. + # + # exception:: the raised Pundit::NotAuthorizedError. def user_not_authorized(exception) policy_name = exception.policy.class.to_s.underscore handle_error(:forbidden, t("#{policy_name}.#{exception.query}", scope: 'pundit', default: :default)) end + # before_action that resolves and stores the Space matching the current + # request host (or the default space if the +spaces+ feature is disabled), + # and redirects unauthorized users away from private spaces they cannot + # access. def set_current_space Space.current_space = TeSS::Config.feature['spaces'] ? Space.find_by_host(request.host) : Space.default + # if the current_space is a specific space (not the default one), we check if the user can access it + if TeSS::Config.feature['spaces'] && Space.current_space != Space.default + unless policy(Space.current_space).shown? + if current_user + flash[:alert] = "You are not authorized to access this page." + redirect_to TeSS::Config.base_url, allow_other_host: true + else + flash[:alert] = "Sign in to access this page." + redirect_to TeSS::Config.base_url + '/users/sign_in', allow_other_host: true + end + end + end end + # Returns:: the Space resolved for the current request by #set_current_space. def current_space Space.current_space end helper_method :current_space + # before_action that stores the current user on the User class (for + # convenience access outside the request cycle) and reports the user to + # Sentry, when Sentry is enabled. def set_current_user User.current_user = current_user if TeSS::Config.sentry_enabled? @@ -131,11 +208,20 @@ def set_current_user end end + # Looks up the country of the current request's IP address. + # + # Uses the MOCK_IP environment variable outside of production so + # geolocation can be tested locally. + # + # Returns:: a country code/name Hash entry from the Locator lookup, or + # +nil+ if it could not be determined. def current_user_country remote_ip = ENV.fetch('MOCK_IP') { Rails.env.production? ? request.remote_ip : '130.88.0.0' } Locator.instance.lookup(remote_ip)&.dig('country') end + # Returns:: +true+ if the current request's country is in the configured + # list of blocked countries, +false+ otherwise. def from_blocked_country? return unless TeSS::Config.blocked_countries.present? user_country = current_user_country @@ -147,6 +233,8 @@ def from_blocked_country? protected + # Configures the extra parameters Devise should permit for sign up, + # sign in and account update, beyond its defaults. def configure_permitted_parameters devise_parameter_sanitizer.permit(:sign_up) do |u| u.permit(:username, :email, :password, :password_confirmation, :remember_me, :publicize_email, :processing_consent) @@ -157,6 +245,8 @@ def configure_permitted_parameters end end + # Removes the X-Frame-Options header so the response can be + # embedded in an iframe on another site. def allow_embedding response.headers.delete 'X-Frame-Options' end diff --git a/app/controllers/concerns/searchable_index.rb b/app/controllers/concerns/searchable_index.rb index 2591b3030..c6e6317ab 100644 --- a/app/controllers/concerns/searchable_index.rb +++ b/app/controllers/concerns/searchable_index.rb @@ -1,6 +1,18 @@ -# The concern for searchable index +# The concern for searchable index. +# +# Mixed into resource controllers to provide a shared +index+/+count+ +# implementation that supports Solr-backed search and faceting when +# TeSS::Config.solr_enabled is true, and falls back to a plain +# Pundit-scoped, paginated listing otherwise. +# +# Including controllers are expected to expose @#{controller_name} +# (e.g. @nodes) to their views; this concern sets that instance +# variable automatically in #fetch_resources. module SearchableIndex + # Default number of records per page when none is requested. DEFAULT_PAGE_SIZE = 10 + + # Allowed values for the +per_page+/+page_size+ parameter. PER_PAGE_OPTIONS = [10, 20, 50, 100] extend ActiveSupport::Concern @@ -13,12 +25,23 @@ module SearchableIndex helper 'search' end + # GET (JSON) //count + # + # Renders the total result count for the current search/filter + # parameters as JSON, using the shared common/count partial. def count respond_to do |format| format.json { render 'common/count' } end end + # Loads the resources for the +index+/+count+ actions into + # @index_resources and @#{controller_name}. + # + # When Solr is enabled, delegates to @model.search_and_filter, + # filters the results through Pundit (#policy(record).shown?), and wraps + # the filtered set in a WillPaginate::Collection with a corrected total. + # Otherwise falls back to a plain policy_scope(@model).paginate. def fetch_resources if TeSS::Config.solr_enabled page = page_param.blank? ? 1 : page_param.to_i @@ -26,7 +49,15 @@ def fetch_resources @search_results = @model.search_and_filter(current_user, @search_params, @facet_params, page: page, per_page: per_page, sort_by: @sort_by) - @index_resources = @search_results.results + + filtered = @search_results.results.select { |record| policy(record).shown? } + + original_total = @search_results.total + + @index_resources = WillPaginate::Collection.create(page, per_page, original_total) do |pager| + pager.replace(filtered) + end + instance_variable_set("@#{controller_name}_results", @search_results) # e.g. @nodes_results else @index_resources = policy_scope(@model).paginate(page: @page) @@ -35,6 +66,10 @@ def fetch_resources instance_variable_set("@#{controller_name}", @index_resources) # e.g. @nodes end + # before_action that resolves the target model class from the controller + # name, and extracts the search query, facet, and sort parameters from + # the request into +@model+, +@facet_params+, +@search_params+ and + # +@sort_by+. def set_params # If the model uses an alias, use that for the search instead @model = controller_name.classify.constantize @@ -44,6 +79,12 @@ def set_params @sort_by = params[:sort].blank? ? 'default' : params[:sort] end + # Builds the JSON:API-style +links+ and +meta+ block (pagination links, + # facets, available facets, query and result count) describing the + # current search/index collection. + # + # Returns:: a Hash with +:links+ and +:meta+ keys, suitable for merging + # into a JSON:API collection response. def api_collection_properties links = { self: polymorphic_path(@model, search_and_facet_params) @@ -85,19 +126,28 @@ def api_collection_properties } end + # Returns:: the requested page number from +params+ (+:page+ or + # +:page_number+), as a String, or +nil+ if not present. def page_param pagination_params[:page] || pagination_params[:page_number] end + # Returns:: the requested page size from +params+ (+:per_page+ or + # +:page_size+), as a String, or +nil+ if not present. def per_page_param pagination_params[:per_page] || pagination_params[:page_size] end + # Returns:: the permitted pagination parameters (+:page+, +:page_number+, + # +:per_page+, +:page_size+). def pagination_params params.permit(:page, :page_number, :per_page, :page_size) end + # Returns:: the permitted search and facet parameters for +@model+, + # merged with the pagination parameter keys, suitable for + # building pagination/self links. def search_and_facet_params params.permit(*(@model.search_and_facet_keys | [:page_size, :page_number, :page, :per_page])) end -end +end \ No newline at end of file diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb new file mode 100644 index 000000000..560efe1de --- /dev/null +++ b/app/controllers/groups_controller.rb @@ -0,0 +1,115 @@ +# Controller for actions related to the Group model. +# +# Groups are collections of users; group membership (and ownership) is used +# elsewhere in the application, notably to control access to private Space +# objects. +class GroupsController < ApplicationController + before_action :set_group, only: %i[ show edit update destroy ] + + # GET /groups + # + # Lists all groups. + def index + @groups = Group.all + end + + # GET /groups/1 + # + # Shows a single group. Requires authorization via GroupPolicy#show?. + def show + authorize @group + end + + # GET /groups/new + # + # Builds a new, unsaved Group for the creation form. Requires + # authorization via GroupPolicy#new?. + def new + authorize Group + @group = Group.new + end + + # GET /groups/1/edit + # + # Requires authorization via GroupPolicy#edit?. + def edit + authorize @group + end + + # POST /groups + # + # Creates a new group from #group_params, then synchronizes owner flags + # on its memberships via #sync_owners. Requires authorization via + # GroupPolicy#create?. + def create + authorize Group + @group = Group.new(group_params.except(:owner_ids)) + + if @group.save + sync_owners + redirect_to @group, notice: "Group was successfully created." + else + render :new, status: :unprocessable_entity + end + end + + # PATCH/PUT /groups/1 + # + # Updates the group from #group_params, then synchronizes owner flags on + # its memberships via #sync_owners. Requires authorization via + # GroupPolicy#update?. + def update + authorize @group + if @group.update(group_params.except(:owner_ids)) + sync_owners + redirect_to @group, notice: "Group was successfully updated." + else + render :edit, status: :unprocessable_entity + end + end + + # DELETE /groups/1 + # + # Destroys the group. Requires authorization via GroupPolicy#destroy?. + # JSON requests are always forbidden (group deletion is HTML-only). + def destroy + authorize @group + respond_to do |format| + format.html do + @group.destroy! + redirect_to groups_path, status: :see_other, notice: "Group was successfully destroyed." + end + format.json { head :forbidden } + end + end + + private + # Use callbacks to share common setup or constraints between actions. + # + # Loads the Group identified by params[:id] into +@group+. + def set_group + @group = Group.find(params[:id]) + end + + # Returns:: the strong-parameters Hash permitted for Group creation and + # update (+:title+, +:user_ids+, +:owner_ids+). + def group_params + permitted = params.require(:group).permit(:title, user_ids: [], owner_ids: []) + permitted[:user_ids] = non_admin_user_ids(permitted[:user_ids]) if permitted.key?(:user_ids) + permitted[:owner_ids] = non_admin_user_ids(permitted[:owner_ids]) if permitted.key?(:owner_ids) + permitted + end + + def non_admin_user_ids(user_ids) + User.where(id: user_ids).where.not(id: User.with_role('admin').select(:id)).pluck(:id) + end + + # Synchronizes the +owner+ flag on each of +@group+'s memberships based + # on the owner_ids submitted in the request parameters. + def sync_owners + owner_ids = (params.dig(:group, :owner_ids) || []).map(&:to_i) + @group.group_memberships.each do |membership| + membership.update(owner: owner_ids.include?(membership.user_id)) + end + end +end diff --git a/app/controllers/materials_controller.rb b/app/controllers/materials_controller.rb index c70360f89..7a8fd65ab 100644 --- a/app/controllers/materials_controller.rb +++ b/app/controllers/materials_controller.rb @@ -24,6 +24,7 @@ def index format.json format.json_api { render({ json: @materials }.merge(api_collection_properties)) } end + @results_count = @search_results&.total || 0 end # GET /materials/1 diff --git a/app/controllers/spaces_controller.rb b/app/controllers/spaces_controller.rb index 9de4c6f3c..b69f9354c 100644 --- a/app/controllers/spaces_controller.rb +++ b/app/controllers/spaces_controller.rb @@ -5,32 +5,48 @@ class SpacesController < ApplicationController before_action :set_breadcrumbs # GET /spaces + # + # Lists every Space visible to the current user (i.e. for which + # SpacePolicy#shown? returns +true+). def index - @spaces = Space.all + @spaces = Space.all.select { |space| policy(space).shown? } respond_to do |format| format.html + format.json { render json: @spaces.as_json(only: [:id, :title]) } end end # GET /spaces/1 + # + # Shows a single space. Requires authorization via SpacePolicy#show?. def show + authorize @space respond_to do |format| format.html end end # GET /spaces/new + # + # Builds a new, unsaved Space for the creation form. Requires + # authorization via SpacePolicy#new?. def new authorize Space @space = Space.new end # GET /spaces/1/edit + # + # Requires authorization via SpacePolicy#edit?. def edit authorize @space end # POST /spaces + # + # Creates a new space owned by the current user, from #space_params. + # Requires authorization via SpacePolicy#create?. Logs a +:create+ + # PublicActivity entry on success. def create authorize Space @space = Space.new(space_params) @@ -47,6 +63,10 @@ def create end # PATCH/PUT /spaces/1 + # + # Updates the space from #space_params. Requires authorization via + # SpacePolicy#update?. Logs a +:update+ PublicActivity entry on success, + # if Space#log_update_activity? allows it. def update authorize @space respond_to do |format| @@ -60,6 +80,9 @@ def update end # DELETE /spaces/1 + # + # Destroys the space. Requires authorization via SpacePolicy#destroy?. + # Logs a +:destroy+ PublicActivity entry before deletion. def destroy authorize @space @space.create_activity :destroy, owner: current_user @@ -71,12 +94,16 @@ def destroy private + # Loads the Space identified by params[:id] into +@space+. def set_space @space = Space.find(params[:id]) end + # Returns:: the strong-parameters Hash permitted for Space creation and + # update. Includes +:host+ only when the current user is an + # admin. def space_params - permitted = [:title, :description, :theme, :image, :image_url, { administrator_ids: [] }, { enabled_features: [] }] + permitted = [:title, :description, :theme, :image, :image_url, :is_private, { administrator_ids: [] }, { enabled_features: [] }, { group_ids: [] }] permitted += [:host] if current_user.is_admin? params.require(:space).permit(*permitted) end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0eddf63a9..730c24395 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -14,6 +14,7 @@ class UsersController < ApplicationController # GET /users.json def index @users = User.visible + @users = @users.where.not(id: User.with_role('admin').select(:id)) if params[:exclude_admins].to_s == 'true' @users = @users.with_query(params[:q].chomp('*')) if params[:q].present? @users = @users.paginate(page: params[:page], per_page: 50) diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb new file mode 100644 index 000000000..c091b2fc8 --- /dev/null +++ b/app/helpers/groups_helper.rb @@ -0,0 +1,2 @@ +module GroupsHelper +end diff --git a/app/models/global_space.rb b/app/models/global_space.rb index d95f2e346..cda1b15f5 100644 --- a/app/models/global_space.rb +++ b/app/models/global_space.rb @@ -64,4 +64,8 @@ def administrators def feature_enabled?(feature) TeSS::Config.feature[feature] end + + def ==(other) + other.is_a?(self.class) + end end diff --git a/app/models/group.rb b/app/models/group.rb new file mode 100644 index 000000000..a8c782157 --- /dev/null +++ b/app/models/group.rb @@ -0,0 +1,16 @@ +# A Group is a collection of users, joined via GroupMembership. +# +# Groups are primarily used to control access to private Space objects: a +# private space is only accessible to users belonging to one of the space's +# associated groups (see ApplicationPolicy#shown?). +class Group < ApplicationRecord + # The individual user memberships (with owner status) belonging to this + # group. Destroyed along with the group. + has_many :group_memberships, dependent: :destroy + + # The users belonging to this group, through #group_memberships. + has_many :users, through: :group_memberships + + # The spaces this group grants access to. + has_and_belongs_to_many :spaces +end \ No newline at end of file diff --git a/app/models/group_membership.rb b/app/models/group_membership.rb new file mode 100644 index 000000000..d59d128b8 --- /dev/null +++ b/app/models/group_membership.rb @@ -0,0 +1,14 @@ +# GroupMembership is the join model between User and Group. +# +# Beyond simple membership, it also tracks whether the user is an *owner* +# of the group (see the +owner+ attribute, managed for example by +# GroupsController#sync_owners), which grants additional permissions such +# as editing or destroying the group (see GroupPolicy#owner?). +class GroupMembership < ApplicationRecord + # Composite primary key: a user can only have a single membership per + # group. + self.primary_key = [:group_id, :user_id] + + belongs_to :user + belongs_to :group +end \ No newline at end of file diff --git a/app/models/space.rb b/app/models/space.rb index 0ff8c337c..ef294ee93 100644 --- a/app/models/space.rb +++ b/app/models/space.rb @@ -1,21 +1,31 @@ +# A Space represents an isolated area of the application (its own subdomain +# / host, its own content, and optionally its own set of enabled features). +# +# Spaces can be public or private. Private spaces restrict access to users +# who belong to one of the space's associated Group objects (see +# ApplicationPolicy#shown?). The "current" space for a request is tracked in +# a thread-local variable (see .current_space) and resolved from the request +# host in ApplicationController#set_current_space. class Space < ApplicationRecord + # The list of toggleable content-type features a Space may enable/disable. FEATURES = %w[events materials elearning_materials learning_paths workflows collections trainers content_providers nodes spaces].freeze include PublicActivity::Common include LogParameterChanges belongs_to :user - has_many :materials, dependent: :nullify - has_many :events, dependent: :nullify - has_many :workflows, dependent: :nullify - has_many :collections, dependent: :nullify - has_many :learning_paths, dependent: :nullify - has_many :learning_path_topics, dependent: :nullify - has_many :subscriptions, dependent: :nullify - has_many :space_roles, dependent: :destroy + has_many :materials + has_many :events + has_many :workflows + has_many :collections + has_many :learning_paths + has_many :learning_path_topics + has_many :subscriptions + has_many :space_roles has_many :space_role_users, through: :space_roles, source: :user, class_name: 'User' has_many :administrator_roles, -> { where(key: :admin) }, class_name: 'SpaceRole' has_many :administrators, through: :administrator_roles, source: :user, class_name: 'User' + has_and_belongs_to_many :groups auto_strip_attributes :title, :description, :host @@ -24,16 +34,48 @@ class Space < ApplicationRecord validates :theme, inclusion: { in: TeSS::Config.themes.keys, allow_blank: true } validate :disabled_features_valid? + # ActiveModel validator ensuring a private Space always has at least one + # Group associated with it, since group membership is what grants access + # to a private space. + class CheckPrivateSpace < ActiveModel::Validator + # Validates that +record+, if private, has at least one associated + # group. Adds a base error otherwise. + # + # record:: the Space instance being validated. + def validate(record) + if record.is_private && !(record.group_ids.length > 0) + record.errors.add(:base, "If the space is private, you must add required groups.") + end + end + end + + validates_with CheckPrivateSpace + + before_destroy :handle_associations_on_destroy + has_image(placeholder: TeSS::Config.placeholder['content_provider']) + # Sets the space considered "current" for the executing thread. + # + # space:: the Space (or subclass, e.g. DefaultSpace/GlobalSpace) to use as + # the current space. def self.current_space=(space) Thread.current[:current_space] = space end + # Returns:: the Space considered "current" for the executing thread, or + # Space.default if none has been set. def self.current_space Thread.current[:current_space] || Space.default end + # Temporarily overrides the current space for the duration of the given + # block, restoring the previous value afterwards (even if the block + # raises). + # + # space:: the Space to use as current within the block. + # + # Yields:: with no arguments, while +space+ is set as current. def self.with_current_space(space) old_space = current_space old_space = nil if old_space.default? @@ -43,26 +85,45 @@ def self.with_current_space(space) self.current_space = old_space end + # Returns:: the default "no space" placeholder: a DefaultSpace if the + # +spaces+ feature is enabled, otherwise a GlobalSpace. def self.default TeSS::Config.feature['spaces'] ? DefaultSpace.new : GlobalSpace.new end + # Returns:: the alt text to use for the space's logo image. def logo_alt "#{title} logo" end + # Returns:: the fully-qualified URL of this space (scheme + host). def url "#{TeSS::Config.base_uri.scheme}://#{host}" end + # Returns:: +false+. Overridden by DefaultSpace/GlobalSpace to indicate + # the default placeholder space. def default? false end + # Finds the users who hold a given SpaceRole in this space. + # + # role:: the role key (e.g. :admin) to filter by. + # + # Returns:: an ActiveRecord::Relation of User records. def users_with_role(role) space_role_users.joins(:space_roles).where(space_roles: { key: role }) end + # Checks whether a given feature is enabled for this space. + # + # feature:: String or Symbol feature key. If it is one of ::FEATURES, both + # the global TeSS::Config setting and this space's + # +disabled_features+ list are consulted; otherwise only the + # global TeSS::Config setting is checked. + # + # Returns:: +true+ or +false+. def feature_enabled?(feature) if FEATURES.include?(feature) TeSS::Config.feature[feature] && !disabled_features.include?(feature) @@ -71,20 +132,45 @@ def feature_enabled?(feature) end end + # Sets the list of enabled features by computing which of ::FEATURES are + # *not* included, and storing that as +disabled_features+. + # + # features:: Array of feature keys that should be enabled. def enabled_features= features self.disabled_features = (FEATURES - features) end + # Returns:: the Array of feature keys currently enabled for this space + # (i.e. ::FEATURES minus +disabled_features+). def enabled_features (FEATURES - disabled_features) end + # Checks whether this space's host is the given domain, or a subdomain of + # it. + # + # domain:: the domain to compare against; defaults to + # TeSS::Config.base_uri.domain. + # + # Returns:: +true+ or +false+. def is_subdomain?(domain = TeSS::Config.base_uri.domain) (host == domain || host.ends_with?(".#{domain}")) end + # Equality by id: two Space instances are equal if they are both Space + # records with the same +id+. + # + # other:: the object to compare against. + # + # Returns:: +true+ or +false+. + def ==(other) + other.is_a?(Space) && self.id == other.id + end + private + # Validation callback ensuring every entry in +disabled_features+ is a + # recognized feature key from ::FEATURES. def disabled_features_valid? disabled_features.each do |feature| next if feature.blank? @@ -93,4 +179,43 @@ def disabled_features_valid? end end end -end + + # before_destroy callback that reassigns or deletes this space's + # associated records. + # + # For a private space, the associated records (materials, events, etc.) + # are deleted outright. For a public space, they are instead detached + # (their +space_id+ is set to +nil+) so they "fall back" to the default + # space, and reindexed in Solr if enabled. The space's SpaceRole records + # are always deleted. + def handle_associations_on_destroy + associations = [ + :materials, + :events, + :workflows, + :collections, + :learning_paths, + :learning_path_topics, + :subscriptions, + ] + + associations.each do |relation| + records = send(relation) + + if is_private + records.delete_all + else + # explicitly ask Solr to reindex those records so they reappear under the default space. + klass = records.klass + ids = records.pluck(:id) + + records.update_all(space_id: nil) + + if TeSS::Config.solr_enabled && ids.any? && klass.respond_to?(:solr_index) + klass.where(id: ids).solr_index + end + end + end + send(:space_roles).delete_all + end +end \ No newline at end of file diff --git a/app/models/user.rb b/app/models/user.rb index 167194103..dea538778 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,6 +50,8 @@ class User < ApplicationRecord as: :owner has_and_belongs_to_many :editables, class_name: "ContentProvider" + has_many :group_memberships, dependent: :destroy + has_many :groups, through: :group_memberships has_many :collaborations, dependent: :destroy has_many :space_roles, dependent: :destroy @@ -385,6 +387,14 @@ def has_role_in_any_space?(role) space_roles.where(key: role).any? end + def is_group_owner?(group) + group.group_memberships.find_by(user: self)&.owner + end + + def is_owner_in_any_group? + group_memberships.where(owner: true).exists? + end + # Get user's registrations def registrations n_events = events.in_current_space diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index 895ab57b3..0474e20b1 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -1,3 +1,14 @@ +# Base Pundit policy for the application. +# +# ApplicationPolicy implements the default authorization rules shared by +# most resources (index/show allowed to everyone, create/update/destroy +# restricted to admins via #manage?), plus the shared logic for +# space-scoped visibility (#shown?) used to hide records belonging to +# private Space objects from users who aren't members of one of that +# space's groups. +# +# Individual resource policies (e.g. SpacePolicy, GroupPolicy) subclass +# this and override individual query methods as needed. class ApplicationPolicy attr_reader :user, :record @@ -12,63 +23,119 @@ class ApplicationPolicy # For tricks on how to bundle an extra object and pass it to policy # in addition to user and record object - see # http://stackoverflow.com/questions/28216678/pundit-policies-with-two-input-parameters + + # Builds a new policy instance for a given context/record pair. + # + # context:: an object responding to +#user+ and +#request+ (see + # ApplicationController#pundit_user), providing the current + # user and the current HTTP request. + # record:: the model instance (or class, for +new?+/+create?+ checks) + # being authorized. If it responds to +#space+, or is itself a + # Space, that space is used to determine private-space + # visibility. def initialize(context, record) @user = context.user @request = context.request @record = record @space = nil @space = record.space if record.respond_to?(:space) + @space = record if record.instance_of?(Space) end + # Returns:: +true+ by default; every record may be listed. def index? true end + # Returns:: +true+ by default; every record may be shown. def show? true end + # Returns:: +true+ if there is a logged-in user. def create? @user end + # Returns:: the result of #create?. def new? create? end + # Returns:: the result of #manage?. def update? manage? end + # Returns:: the result of #update?. def edit? update? end + # Returns:: the result of #manage?. def destroy? manage? end # "manage" isn't actually an action, but the "destroy?" and "update?" policies delegate to this method. + # + # Returns:: +true+ if the current user is an admin. def manage? @user&.is_admin? end + # Returns:: +true+ if the current user has the :curator, :admin, or + # :scraper_user role (globally or within the current space). def curators_and_admin user_has_role?(:curator, :admin, :scraper_user) end + # Returns:: the default Pundit policy scope for the record's class. def scope Pundit.policy_scope!(user, record.class) end + # Determines whether the record should be visible to the current user, + # based on the private/public status of its associated space. + # + # Rules: + # * if the record has no associated space, it is always shown; + # * if the associated space is not private, it is always shown; + # * otherwise, an authenticated user is shown the record only if they are + # an admin, or belong to at least one of the space's groups (and only + # when the space in question is the current space, or the record *is* + # the space itself). + # + # Returns:: +true+ or +false+. + def shown? + return true if @space == nil + return true if !@space.is_private + return false unless @user # and so if space is private + if @space == Space.current_space || @record == @space + user_groups = @user.groups.pluck(:id) + space_groups = @space.groups.pluck(:id) + return @user.is_admin? || space_groups.any? { |group_id| user_groups.include?(group_id) } + end + + return false + end + + # Default Pundit policy scope class. + # + # Simply returns the given scope unfiltered; subclasses/resource-specific + # scopes should override #resolve to apply additional filtering. class Scope attr_reader :user, :scope + # context:: an object responding to +#user+, providing the current + # user. + # scope:: the ActiveRecord relation/class to scope. def initialize(context, scope) @user = context.user @scope = scope end + # Returns:: the unfiltered +scope+. def resolve scope end @@ -76,20 +143,30 @@ def resolve private + # Returns:: +true+ if the current request is a JSON POST/PUT/PATCH + # (i.e. an API write request). def request_is_api? !!@request && ((@request.post? || @request.put? || @request.patch?) && @request.format.json?) end + # Returns:: +true+ if this is an API write request made by a user with + # the :scraper_user role. def scraper? request_is_api? && @user&.has_role?(:scraper_user) end # Check if the user has any of the given roles. # If we're in a space, also check they have any of those roles in the context of the space. + # + # roles:: one or more Symbol role keys to check. + # + # Returns:: +true+ if the user holds any of the given roles globally, or + # within the current space, +false+ otherwise (including when + # there is no current user). def user_has_role?(*roles) return false if @user.nil? roles.any? { |r| @user.has_role?(r) } || (@space && roles.any? { |r| @user.has_space_role?(@space, r) }) end -end +end \ No newline at end of file diff --git a/app/policies/event_policy.rb b/app/policies/event_policy.rb index 38d3c0fad..a6510c3c5 100644 --- a/app/policies/event_policy.rb +++ b/app/policies/event_policy.rb @@ -1,5 +1,9 @@ class EventPolicy < ScrapedResourcePolicy + def show? + super && shown? + end + def edit_report? manage? end diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb new file mode 100644 index 000000000..fd1e01727 --- /dev/null +++ b/app/policies/group_policy.rb @@ -0,0 +1,63 @@ +# Pundit policy for Group. +# +# Groups may be seen and managed by their members/owners in addition to +# admins; creation, update and destruction via the JSON API are always +# forbidden regardless of role. +class GroupPolicy < ResourcePolicy + + # Returns:: +true+; the group index is visible to everyone. + def index? + true + end + + # Returns:: +true+ if the current user belongs to the group (#see?) or is + # an admin. + def show? + see? || @user&.is_admin? + end + + # Returns:: the result of #manage?. + def edit? + manage? + end + + # Returns:: +true+ if the request is not an API write request and the + # current user is an admin. Group creation via the API is never + # allowed, and only admins may create groups. + def create? + # Do not allow creations via API and only admin role can create group + !request_is_api? && @user&.is_admin? + end + + # Returns:: +true+ if the request is not an API write request and + # #manage? allows it. + def update? + !request_is_api? && manage? + end + + # Returns:: +true+ if the request is not an API write request and the + # current user is either an admin or an owner (#owner?) of the + # group. + def destroy? + !request_is_api? && (@user&.is_admin? || owner?) + end + + # Returns:: +true+ if the current user belongs to the group and is one of + # its owners, or is an admin. + def manage? + (see? && owner?) || @user&.is_admin? + end + + # Returns:: +true+ if the current user is a member of the group. + def see? + @record.users.include?(@user) + end + + private + + # Returns:: +true+ if the current user's GroupMembership for this group + # has the +owner+ flag set. + def owner? + @record.group_memberships.find_by(user: @user)&.owner == true + end +end \ No newline at end of file diff --git a/app/policies/material_policy.rb b/app/policies/material_policy.rb index ffe8e4924..f45c95bb3 100644 --- a/app/policies/material_policy.rb +++ b/app/policies/material_policy.rb @@ -1,5 +1,9 @@ class MaterialPolicy < ScrapedResourcePolicy + def show? + super && shown? + end + def clone? manage? end diff --git a/app/policies/space_policy.rb b/app/policies/space_policy.rb index c2ce876c4..5790d566e 100644 --- a/app/policies/space_policy.rb +++ b/app/policies/space_policy.rb @@ -1,19 +1,41 @@ +# Pundit policy for Space. +# +# Visibility of a space is delegated to ApplicationPolicy#shown? (private +# spaces are only visible to members of one of their groups, or admins); +# editing is additionally granted to the space's owner and its +# space-level admins. class SpacePolicy < ApplicationPolicy + # Returns:: the result of #shown?. + def show? + shown? + end + + # Returns:: the result of #manage?. def create? - @user&.has_role?(:admin) + manage? end + # Returns:: +true+ if there is a current user who either owns the space, + # holds the :admin SpaceRole for it, or is a global admin + # (#manage?) — and the space is #shown? to them. def edit? - @user && (@user.is_owner?(@record) || @user.has_space_role?(@record, :admin) || manage?) + @user && (@user.is_owner?(@record) || @user.has_space_role?(@record, :admin) || manage?) && shown? end + # Returns:: the result of #edit?. def update? edit? end + # Returns:: +true+ if the current user is a global admin. def manage? @user&.is_admin? end -end + # Returns:: the result of #manage?. + def destroy? + manage? + end + +end \ No newline at end of file diff --git a/app/serializers/group_serializer.rb b/app/serializers/group_serializer.rb new file mode 100644 index 000000000..7acc851d7 --- /dev/null +++ b/app/serializers/group_serializer.rb @@ -0,0 +1,3 @@ +class GroupSerializer < ApplicationSerializer + attributes :id, :title +end diff --git a/app/views/groups/_form.html.erb b/app/views/groups/_form.html.erb new file mode 100644 index 000000000..3cca637ab --- /dev/null +++ b/app/views/groups/_form.html.erb @@ -0,0 +1,127 @@ +<%= form_with(model: group) do |f| %> + <%= render partial: 'common/error_summary', locals: { resource: group } %> + + <%# --- Title --- %> +
    + <%= f.label :title, class: "control-label" %> + <%= f.text_field :title, class: "form-control input-lg", placeholder: "Group title" %> + <% if group.errors[:title].any? %> + <%= group.errors[:title].first %> + <% end %> +
    + + <%# --- Members --- + Re-uses the same autocompleter infrastructure as spaces/_form.html.erb. + Template: app/assets/javascripts/templates/autocompleter/group_member.hbs (new file). + The sentinel hidden field ensures group[user_ids] is submitted even when the list is empty. + %> +
    + +

    + + Search and add members below - admins cannot be added as members since they have all rights on them by default. Check Owner to grant ownership. +

    + + <%# Sentinel: guarantees group[user_ids] key is always present in params %> + + + <% + form_field_name = "group[user_ids]" + existing_json = group.group_memberships.includes(:user).map do |m| + { + item: { + id: m.user_id, + name: m.user.name, + email: m.user.try(:email).to_s, + owner: m.owner? + }, + prefix: form_field_name + } + end.to_json + %> + +
    + + + + <%# Sentinel inside the widget (mirrors what _autocompleter.html.erb does) %> + + +
      + + +
      +
      + +
      + <%= f.submit class: "btn btn-primary" %> + <%= link_to "Cancel", :back, class: "btn btn-default" %> +
      +<% end %> + + + + diff --git a/app/views/groups/_group.html.erb b/app/views/groups/_group.html.erb new file mode 100644 index 000000000..77eca8eea --- /dev/null +++ b/app/views/groups/_group.html.erb @@ -0,0 +1,7 @@ +
      +

      + Title: + <%= group.title %> +

      + +
      diff --git a/app/views/groups/_group.json.jbuilder b/app/views/groups/_group.json.jbuilder new file mode 100644 index 000000000..85eb93e79 --- /dev/null +++ b/app/views/groups/_group.json.jbuilder @@ -0,0 +1,2 @@ +json.extract! group, :id, :title, :created_at, :updated_at +json.url group_url(group, format: :json) diff --git a/app/views/groups/edit.html.erb b/app/views/groups/edit.html.erb new file mode 100644 index 000000000..487fefabc --- /dev/null +++ b/app/views/groups/edit.html.erb @@ -0,0 +1,12 @@ +<% content_for :title, "Editing group" %> + +

      Editing group

      + +<%= render "form", group: @group %> + +
      + +
      + <%= link_to "Show this group", @group %> | + <%= link_to "Back to groups", groups_path %> +
      diff --git a/app/views/groups/index.html.erb b/app/views/groups/index.html.erb new file mode 100644 index 000000000..4b3e2a41e --- /dev/null +++ b/app/views/groups/index.html.erb @@ -0,0 +1,58 @@ +<% + @breadcrumbs = [{ name: 'Home', url: root_path }, { name: 'Groups' }] +%> +<% content_for :title, "Groups" %> + +
      + <%= page_title("Groups") %> +
      + +
      +
      +
      +
      +
      + <%= link_to new_group_path, class: 'btn btn-primary' do %> + New group + <% end %> +
      +
      + + + + <% if @groups.empty? %> +
      + +

      No groups have been created yet.

      + <%= link_to "Create the first group", new_group_path, class: 'btn btn-primary' %> +
      + <% else %> +
      + <%= pluralize(@groups.count, 'group') %> +
      + +
        + <% @groups.each do |group| %> + <% owners = group.group_memberships.select(&:owner?) %> +
      • + <%= link_to group, class: 'link-overlay' do %> +

        <%= group.title %>

        + <% end %> + +
        + + <%= pluralize(group.group_memberships.count, 'member') %> + + <% if owners.any? %> +  ·  + + <% owner_names = owners.map { |m| m.user.name } %> + <%= owner_names.first(2).join(", ") %><%= " +#{owner_names.size - 2} more" if owner_names.size > 2 %> + <% end %> +
        +
        • + <% end %> +
      + <% end %> +
      +
      diff --git a/app/views/groups/index.json.jbuilder b/app/views/groups/index.json.jbuilder new file mode 100644 index 000000000..37cc18887 --- /dev/null +++ b/app/views/groups/index.json.jbuilder @@ -0,0 +1 @@ +json.array! @groups, partial: "groups/group", as: :group diff --git a/app/views/groups/new.html.erb b/app/views/groups/new.html.erb new file mode 100644 index 000000000..2c753e98a --- /dev/null +++ b/app/views/groups/new.html.erb @@ -0,0 +1,11 @@ +<% content_for :title, "New group" %> + +

      New group

      + +<%= render "form", group: @group %> + +
      + +
      + <%= link_to "Back to groups", groups_path %> +
      diff --git a/app/views/groups/show.html.erb b/app/views/groups/show.html.erb new file mode 100644 index 000000000..2e7279ad6 --- /dev/null +++ b/app/views/groups/show.html.erb @@ -0,0 +1,170 @@ +<% + @breadcrumbs = [ + { name: 'Home', url: root_path }, + { name: 'Groups', url: groups_path }, + { name: @group.title } + ] +%> +<% content_for :title, @group.title %> + +
      + <%# SIDEBAR %> + + + <%# MAIN CONTENT %> +
      +
      +
      +
      + <% if policy(@group).update? %> + <%= link_to edit_group_path(@group), class: 'btn btn-default' do %> + Edit + <% end %> + <% end %> + <% if policy(@group).destroy? %> + <%= button_to @group, method: :delete, + class: 'btn btn-danger', + form: { data: { confirm: "Are you sure you want to delete this group?" } } do %> + Delete + <% end %> + <% end %> +
      +
      + + <%= link_to groups_path, class: 'btn btn-default' do %> + Back to groups + <% end %> + + <%# Members table %> + <% memberships = @group.group_memberships.includes(:user) rescue @group.group_memberships %> + + <% if memberships.any? %> + + + + + <% if memberships.first.user.respond_to?(:email) %> + + <% end %> + + + + + <% memberships.each do |membership| %> + + + <% if membership.user.respond_to?(:email) %> + + <% end %> + + + <% end %> + +
      NameEmailRole
      + <%= link_to membership.user.name, membership.user rescue membership.user.name %> + <%= membership.user.email %> + <% if membership.owner? %> + + Owner + + <% else %> + Member + <% end %> +
      + <% else %> +
      + +

      + No members yet. + <%= link_to "Add members", edit_group_path(@group) %>. +

      +
      + <% end %> + +
      + + <%# Private spaces table %> + <% spaces = @group.spaces.all %> + +

      Spaces which require this group to have access to:

      + + <% if spaces.any? %> + + + + + + + + + <% spaces.each do |space| %> + <% groups = space.groups.all %> + + + + + <% end %> + +
      SpaceOther groups which have access
      + <%= link_to space.title, space %> + + <% groups.each_with_index do |group, index| %> + <% if group != @group %> + <%= index != groups.length-1 ? group.title + ',' : group.title %> + <% end %> + <% end %> +
      + <% else %> +
      + +

      + This group does not give access to any private space +

      +
      + <% end %> +
      +
      diff --git a/app/views/groups/show.json.jbuilder b/app/views/groups/show.json.jbuilder new file mode 100644 index 000000000..76d763822 --- /dev/null +++ b/app/views/groups/show.json.jbuilder @@ -0,0 +1 @@ +json.partial! "groups/group", group: @group diff --git a/app/views/layouts/_user_menu.html.erb b/app/views/layouts/_user_menu.html.erb index 55d1008af..709816b26 100644 --- a/app/views/layouts/_user_menu.html.erb +++ b/app/views/layouts/_user_menu.html.erb @@ -18,8 +18,9 @@ <% is_admin = current_user.is_admin? %> <% is_curator = current_user.is_curator? %> + <% is_owner_in_any_group = current_user.is_owner_in_any_group? %> <% is_current_space_admin = !Space.current_space.default? && current_user.has_space_role?(Space.current_space, :admin) %> - <% if is_admin || is_curator || is_current_space_admin %> + <% if is_admin || is_curator || is_current_space_admin || is_owner_in_any_group %>
    • <%= t('menu.user.administration') %>
      • @@ -40,6 +41,15 @@ <% end %> + <% if is_admin || is_curator || is_owner_in_any_group %> +
    • + <%= link_to groups_path, title: "View groups" do %> + <%= "View groups" %> + <% end %> +
      • + <% end %> + + <% if !TeSS::Config.feature['disabled'].include?('topics') && (is_admin || is_curator) %>
    • <%= link_to curate_topic_suggestions_path, diff --git a/app/views/spaces/_form.html.erb b/app/views/spaces/_form.html.erb index c36b24a79..84f850b02 100644 --- a/app/views/spaces/_form.html.erb +++ b/app/views/spaces/_form.html.erb @@ -19,6 +19,23 @@ id_field: :id, existing_items_method: :administrators %> + + +
      + <%= f.label "Private space", style: "display: block" %> + <%= f.input :is_private, label: "", input_html: { id: "space_is_private" } %> +
      + + +
      <%= f.input :enabled_features, label: t('features.enabled'), collection: space_feature_options, as: :check_boxes %>
      @@ -29,3 +46,4 @@ @space.new_record? ? spaces_path : space_path(@space), class: 'btn btn-default' %> <% end %> +<%= javascript_include_tag 'show_private_groups', 'data-turbolinks-track': 'reload' %> diff --git a/app/views/spaces/edit.html.erb b/app/views/spaces/edit.html.erb index 1a87bf055..136043f56 100644 --- a/app/views/spaces/edit.html.erb +++ b/app/views/spaces/edit.html.erb @@ -10,3 +10,4 @@ <%= render partial: 'form' %> + diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index e63eb9ab8..15f0d8a1b 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -70,6 +70,7 @@

      <% end %> +

      Account details

      @@ -96,6 +97,57 @@ <% end %> <% end %>

      + + <%# ---- Groups ---- %> + <% user_groups = @user.groups.includes(:group_memberships) %> + <% groups_limit = 5 %> +

      + Groups + <% if user_groups.any? %> + <%= user_groups.count %> + <% end %> +

      +
      + <% if user_groups.any? %> + <% visible_groups = user_groups.first(groups_limit) %> + <% hidden_groups = user_groups.drop(groups_limit) %> + + <% visible_groups.each do |group| %> +

      + <% if @user.is_group_owner?(group) %> + + <% else %> + + <% end %> +  <%= link_to group.title, group %> +

      + <% end %> + + <% if hidden_groups.any? %> +
      + <% hidden_groups.each do |group| %> +

      + <% if @user.is_group_owner?(group) %> + + <% else %> + + <% end %> +  <%= link_to group.title, group %> +

      + <% end %> +
      + + <% end %> + <% else %> + Not a member of any group. + <% end %> +
      @@ -134,3 +186,19 @@ <%= render(partial: 'common/registrations_list', locals: { user: @user }) %>
      + + diff --git a/config/application.rb b/config/application.rb index 27b7e7b28..f56a1e3eb 100644 --- a/config/application.rb +++ b/config/application.rb @@ -119,7 +119,7 @@ def redis_url ENV.fetch('REDIS_URL') { 'redis://localhost:6379/1' } end end - + def ingestion return @ingestion if @ingestion diff --git a/config/locales/en.yml b/config/locales/en.yml index 727d7ca70..1ab629b0f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1151,7 +1151,8 @@ en: spaces: title: What are spaces? description: | - Spaces are customizable, community-managed sub-portals within %{site_name}, each with their own catalogue of training content. + Spaces are customizable, community-managed sub-portals within %{site_name}, each with its own catalogue of training content. + Some spaces are private. That means you can see and have access to them only if you are in at least one of the required groups for each of them. orcid: error: 'An error occurred whilst trying to authenticate your ORCID.' link: 'Link your ORCID' diff --git a/config/routes.rb b/config/routes.rb index 5f9613f6f..627892161 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,4 +1,5 @@ Rails.application.routes.draw do + resources :groups concern :collaboratable do resources :collaborations, only: [:create, :destroy, :index, :show] end diff --git a/db/migrate/20260612065213_create_groups.rb b/db/migrate/20260612065213_create_groups.rb new file mode 100644 index 000000000..c8013d234 --- /dev/null +++ b/db/migrate/20260612065213_create_groups.rb @@ -0,0 +1,9 @@ +class CreateGroups < ActiveRecord::Migration[7.2] + def change + create_table :groups do |t| + t.string :title + + t.timestamps + end + end +end diff --git a/db/migrate/20260612065245_create_join_table_groups_users.rb b/db/migrate/20260612065245_create_join_table_groups_users.rb new file mode 100644 index 000000000..9c0442d20 --- /dev/null +++ b/db/migrate/20260612065245_create_join_table_groups_users.rb @@ -0,0 +1,8 @@ +class CreateJoinTableGroupsUsers < ActiveRecord::Migration[7.2] + def change + create_join_table :groups, :users do |t| + # t.index [:group_id, :user_id] + # t.index [:user_id, :group_id] + end + end +end diff --git a/db/migrate/20260612065251_create_join_table_groups_spaces.rb b/db/migrate/20260612065251_create_join_table_groups_spaces.rb new file mode 100644 index 000000000..6703e9971 --- /dev/null +++ b/db/migrate/20260612065251_create_join_table_groups_spaces.rb @@ -0,0 +1,8 @@ +class CreateJoinTableGroupsSpaces < ActiveRecord::Migration[7.2] + def change + create_join_table :groups, :spaces do |t| + t.index [:group_id, :space_id], unique: true + t.index [:space_id, :group_id] + end + end +end diff --git a/db/migrate/20260618090239_add_is_private_to_spaces.rb b/db/migrate/20260618090239_add_is_private_to_spaces.rb new file mode 100644 index 000000000..16bae9e2e --- /dev/null +++ b/db/migrate/20260618090239_add_is_private_to_spaces.rb @@ -0,0 +1,5 @@ +class AddIsPrivateToSpaces < ActiveRecord::Migration[7.2] + def change + add_column :spaces, :is_private, :boolean, default: false, null: false + end +end diff --git a/db/migrate/20260618141208_replace_groups_users_with_group_memberships.rb b/db/migrate/20260618141208_replace_groups_users_with_group_memberships.rb new file mode 100644 index 000000000..fb804c970 --- /dev/null +++ b/db/migrate/20260618141208_replace_groups_users_with_group_memberships.rb @@ -0,0 +1,12 @@ +class ReplaceGroupsUsersWithGroupMemberships < ActiveRecord::Migration[7.2] + def change + drop_table :groups_users + + create_table :group_memberships, id: false, primary_key: [:group_id, :user_id] do |t| + t.references :user, null: false, foreign_key: true + t.references :group, null: false, foreign_key: true + t.boolean :owner, default: false, null: false + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index e87ff4c9b..594ca1b9f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_04_21_144919) do +ActiveRecord::Schema[8.1].define(version: 2026_06_18_141208) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -268,6 +268,27 @@ t.index ["sluggable_type"], name: "index_friendly_id_slugs_on_sluggable_type" end + create_table "group_memberships", id: false, force: :cascade do |t| + t.datetime "created_at", null: false + t.bigint "group_id", null: false + t.boolean "owner", default: false, null: false + t.datetime "updated_at", null: false + t.bigint "user_id", null: false + t.index ["group_id"], name: "index_group_memberships_on_group_id" + t.index ["user_id"], name: "index_group_memberships_on_user_id" + end + + create_table "groups", force: :cascade do |t| + t.datetime "created_at", null: false + t.string "title" + t.datetime "updated_at", null: false + end + + create_table "groups_spaces", id: false, force: :cascade do |t| + t.bigint "group_id", null: false + t.bigint "space_id", null: false + end + create_table "learning_path_topic_items", force: :cascade do |t| t.text "comment" t.datetime "created_at", null: false @@ -535,6 +556,7 @@ t.bigint "image_file_size" t.datetime "image_updated_at" t.text "image_url" + t.boolean "is_private" t.string "theme" t.string "title" t.datetime "updated_at", null: false @@ -687,6 +709,8 @@ add_foreign_key "event_materials", "materials" add_foreign_key "events", "spaces" add_foreign_key "events", "users" + add_foreign_key "group_memberships", "groups" + add_foreign_key "group_memberships", "users" add_foreign_key "learning_path_topic_links", "learning_paths" add_foreign_key "learning_path_topics", "spaces" add_foreign_key "learning_paths", "content_providers" diff --git a/docs/docstrings.md b/docs/docstrings.md new file mode 100644 index 000000000..bd1a27f1c --- /dev/null +++ b/docs/docstrings.md @@ -0,0 +1,51 @@ +# RDoc Conventions + +## Class/module documentation + +A comment block above every `class`/`module`, explaining: +- **its role** in one short sentence, +- the **business context** if necessary (e.g. why this model exists, what it collaborates with), +- points of attention (e.g. "inherits from X rather than Y"). + +```ruby +# One-line summary. +# +# Explanatory paragraph(s) if the role isn't trivial. +class MyClass +``` + +## Method documentation + +Systematic structure, in this order: + +1. **Description** of what the method does (behavior, not implementation). +2. **Parameters**, listed as `name:: description` (classic RDoc style, double `::`). +3. **Return value**, with the `Returns::` keyword. +4. **Exceptions**, with `Raises::`, if the method can intentionally raise an error. +5. **Yields**, if the method takes a block. + +```ruby +# Does this and that. +# +# param1:: description of the parameter. +# param2:: description, with default value if relevant. +# +# Raises:: ExceptionClass if such condition. +# +# Returns:: description of the returned type/object. +def my_method(param1, param2 = nil) +``` + +## Specific rules to follow + +- **Rails callbacks** (`before_action`, `before_destroy`, custom validators, etc.): documented like regular methods, explicitly stating that it's a callback and when it runs (e.g. *"before_destroy callback that..."*). +- **Trivial methods** (e.g. simple accessors, `default?` that returns `false`): one line is enough, no over-documentation. +- **Constants**: a one-line comment right above (`FEATURES`, `DEFAULT_PAGE_SIZE`, etc.). +- **Nested classes** (e.g. `ApplicationPolicy::Scope`, `Space::CheckPrivateSpace`): documented as full-fledged classes, with their own `initialize`/methods commented. +- **No duplication**: if the behavior is fully explained by `Returns::` (e.g. `def update? = manage?`), the logic isn't re-explained in the description body. + +## Important notes + +- `param::` / `Returns::` / `Raises::` are **natively recognized by RDoc** and will generate clean HTML docs (parameter list separated from the text). +- Documenting *behavior* rather than *paraphrasing the code* makes the docs useful even without reading the implementation. +- Documenting callbacks/private methods as well helps understand the flow (e.g. `set_current_space`, `fetch_resources`) without having to trace through the whole controller. \ No newline at end of file diff --git a/docs/spaces.md b/docs/spaces.md index b43d83694..57fc9ecad 100644 --- a/docs/spaces.md +++ b/docs/spaces.md @@ -3,6 +3,12 @@ A multi-space enabled TeSS will set the current space based on the Host header of the incoming request (if there is a Space defined with that host, otherwise it will fallback to the default space). +There is the possibility to make a space private. That means the resources from this space are not accessible from outside the space - and so not visible in the main catalogue even with the toggle to see resources from all spaces. +The private space access is controlled by a group system: a user needs to be in at least one of the defined list of groups of the space. That means that to create a private space you need to already have existing groups. + +If a private space is destroyed, every resource in it is destroyed too. +If a public space is destroyed, all resources are nullified (except space_roles which are destroyed). + # Development To allow your local development server to respond to requests to these hosts, diff --git a/test/controllers/groups_controller_test.rb b/test/controllers/groups_controller_test.rb new file mode 100644 index 000000000..750d6fd2e --- /dev/null +++ b/test/controllers/groups_controller_test.rb @@ -0,0 +1,209 @@ +require 'test_helper' + +class GroupsControllerTest < ActionController::TestCase + include Devise::Test::ControllerHelpers + + setup do + @group = groups(:one) + + # Create membership fixtures in-memory so we don't need a separate YAML file. + # owner_user → member + owner of @group + # member_user → member (non-owner) of @group + # outsider_user → not a member at all + @owner_user = users(:regular_user) + @member_user = users(:another_regular_user) + @outsider = users(:curator) + @admin = users(:admin) + + @group.group_memberships.find_or_create_by!(user: @owner_user) { |m| m.owner = true } + @group.group_memberships.find_or_create_by!(user: @member_user) { |m| m.owner = false } + end + + # --------------------------------------------------------------------------- + # INDEX (public) + # --------------------------------------------------------------------------- + + test 'should get index when not logged in' do + get :index + assert_response :success + end + + test 'should get index when logged in' do + sign_in @outsider + get :index + assert_response :success + end + + # --------------------------------------------------------------------------- + # SHOW (members + admins only) + # --------------------------------------------------------------------------- + + test 'should deny show to anonymous user' do + get :show, params: { id: @group } + assert_response :forbidden + end + + test 'should deny show to outsider (non-member)' do + sign_in @outsider + get :show, params: { id: @group } + assert_response :forbidden + end + + test 'should allow show to group member' do + sign_in @member_user + get :show, params: { id: @group } + assert_response :success + end + + test 'should allow show to group owner' do + sign_in @owner_user + get :show, params: { id: @group } + assert_response :success + end + + test 'should allow show to admin' do + sign_in @admin + get :show, params: { id: @group } + assert_response :success + end + + # --------------------------------------------------------------------------- + # NEW / CREATE (admin only) + # --------------------------------------------------------------------------- + + test 'should deny new to anonymous user' do + get :new + assert_redirected_to new_user_session_path + end + + test 'should deny new to regular member' do + sign_in @member_user + get :new + assert_response :forbidden + end + + test 'should deny new to group owner (non-admin)' do + sign_in @owner_user + get :new + assert_response :forbidden + end + + test 'should allow new for admin' do + sign_in @admin + get :new + assert_response :success + end + + test 'should deny create to non-admin' do + sign_in @member_user + assert_no_difference('Group.count') do + post :create, params: { group: { title: 'New group' } } + end + assert_response :forbidden + end + + test 'should allow admin to create group' do + sign_in @admin + assert_difference('Group.count', 1) do + post :create, params: { group: { title: 'Admin new group' } } + end + assert_redirected_to group_url(Group.last) + end + + # --------------------------------------------------------------------------- + # EDIT / UPDATE (owner + admin) + # --------------------------------------------------------------------------- + + test 'should deny edit to anonymous user' do + get :edit, params: { id: @group } + assert_redirected_to new_user_session_path + end + + test 'should deny edit to outsider' do + sign_in @outsider + get :edit, params: { id: @group } + assert_response :forbidden + end + + test 'should deny edit to non-owner member' do + sign_in @member_user + get :edit, params: { id: @group } + assert_response :forbidden + end + + test 'should allow edit for group owner' do + sign_in @owner_user + get :edit, params: { id: @group } + assert_response :success + end + + test 'should allow edit for admin' do + sign_in @admin + get :edit, params: { id: @group } + assert_response :success + end + + test 'should deny update to non-owner member' do + sign_in @member_user + patch :update, params: { id: @group, group: { title: 'Hacked title' } } + assert_response :forbidden + assert_not_equal 'Hacked title', @group.reload.title + end + + test 'should allow owner to update group' do + sign_in @owner_user + patch :update, params: { id: @group, group: { title: 'Owner updated title' } } + assert_redirected_to group_url(@group) + assert_equal 'Owner updated title', @group.reload.title + end + + test 'should allow admin to update group' do + sign_in @admin + patch :update, params: { id: @group, group: { title: 'Admin updated title' } } + assert_redirected_to group_url(@group) + assert_equal 'Admin updated title', @group.reload.title + end + + test 'should deny update via JSON API even for owner' do + sign_in @owner_user + patch :update, params: { id: @group, group: { title: 'API attempt' } }, + as: :json + assert_response :forbidden + assert_not_equal 'API attempt', @group.reload.title + end + + # --------------------------------------------------------------------------- + # DESTROY (admin/owner only) + # --------------------------------------------------------------------------- + + test 'should deny destroy to anonymous user' do + assert_no_difference('Group.count') do + delete :destroy, params: { id: @group } + end + assert_redirected_to new_user_session_path + end + + test 'should allow destroy to group owner (non-admin)' do + sign_in @owner_user + assert_difference('Group.count', -1) do + delete :destroy, params: { id: @group } + end + assert_redirected_to groups_url + end + + test 'should allow admin to destroy group' do + sign_in @admin + assert_difference('Group.count', -1) do + delete :destroy, params: { id: @group } + end + assert_redirected_to groups_url + end + + test 'should deny destroy via JSON API even for admin' do + sign_in @admin + assert_no_difference('Group.count') do + delete :destroy, params: { id: @group }, as: :json + end + assert_response :forbidden + end +end \ No newline at end of file diff --git a/test/fixtures/groups.yml b/test/fixtures/groups.yml new file mode 100644 index 000000000..64d88efb9 --- /dev/null +++ b/test/fixtures/groups.yml @@ -0,0 +1,7 @@ +# Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html + +one: + title: MyString + +two: + title: MyString diff --git a/test/integration/private_space_access_test.rb b/test/integration/private_space_access_test.rb new file mode 100644 index 000000000..c937244de --- /dev/null +++ b/test/integration/private_space_access_test.rb @@ -0,0 +1,271 @@ +require 'test_helper' + +# Integration test for the private space + group access control scenario: +# +# 1. Admin creates group G1 with member U1 (not U2). +# 2. Admin creates space S1, marks it private, associates G1. +# 3. U1 (in G1) can access S1 and its materials. +# 4. U2 (not in G1) is denied access at every surface: +# - spaces#index listing +# - spaces#show URL +# - materials#index listing (main TeSS + space-scoped) +# - materials#show URL (main TeSS + space-scoped) +# - materials#show JSON-LD / schema.org (main TeSS + space-scoped) +# +# All space routing is host-based (set_current_space reads request.host). +# with_host() and with_settings() come from test_helper.rb. + +class PrivateSpaceAccessTest < ActionController::TestCase + include Devise::Test::ControllerHelpers + + # ------------------------------------------------------------------ + # Setup: build the full scenario in-memory for every test. + # We avoid touching existing fixtures so these tests are self-contained. + # ------------------------------------------------------------------ + setup do + # Users + @admin = users(:admin) + @u1 = users(:regular_user) # will be in G1 + @u2 = users(:another_regular_user) # NOT in G1 + + # Group G1 — created by admin, U1 is a member + @g1 = Group.create!(title: 'G1 Test Group') + @g1.group_memberships.create!(user: @u1, owner: false) + + # Space S1 — private, linked to G1 + @s1 = Space.create!( + title: 'S1 Private Space', + host: 's1.example.com', + is_private: true, + user: @admin, + groups: [@g1] + ) + + # Material M1 — belongs to S1, created by U1 + @m1 = Material.create!( + title: 'M1 Private Material', + url: 'https://example.com/m1', + description: 'Material that lives only in S1', + contact: 'u1@example.com', + status: 'active', + licence: 'CC-BY-4.0', + remote_updated_date: Date.today, + remote_created_date: Date.today, + space: @s1, + user: @u1 + ) + end + + teardown do + @m1.destroy! + @s1.groups.delete(@g1) + @s1.destroy! + @g1.group_memberships.destroy_all + @g1.destroy! + end + + # ================================================================== + # SPACES + # ================================================================== + + # --- spaces#index ------------------------------------------------- + + test 'U1 (in G1) sees S1 in the spaces list' do + @controller = SpacesController.new + with_settings(feature: { spaces: true }) do + sign_in @u1 + with_host(@s1.host) do + get :index + assert_response :success + assert_includes assigns(:spaces), @s1 + end + end + end + + test 'U2 (not in G1) does NOT see S1 in the spaces list' do + @controller = SpacesController.new + with_settings(feature: { spaces: true }) do + sign_in @u2 + get :index # requests from the default host — S1 is private + assert_response :success + refute_includes assigns(:spaces), @s1 + end + end + + # --- spaces#show -------------------------------------------------- + + test 'U1 (in G1) can access S1 show page via its host URL' do + @controller = SpacesController.new + with_settings(feature: { spaces: true }) do + sign_in @u1 + with_host(@s1.host) do + get :show, params: { id: @s1 } + assert_response :success + end + end + end + + test 'U2 (not in G1) is denied S1 show page via its host URL' do + @controller = SpacesController.new + with_settings(feature: { spaces: true }) do + sign_in @u2 + with_host(@s1.host) do + # set_current_space will redirect U2 away before the action even runs + get :show, params: { id: @s1 } + assert_response :redirect + end + end + end + + # ================================================================== + # MATERIALS — created by U1 inside S1 + # ================================================================== + + # --- materials#show via S1 host (space-scoped URL) ---------------- + + test 'U1 can access M1 show page via S1 host URL' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u1 + with_host(@s1.host) do + get :show, params: { id: @m1 } + assert_response :success + assert_equal @m1, assigns(:material) + end + end + end + + test 'U2 cannot access M1 show page via S1 host URL' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u2 + with_host(@s1.host) do + # set_current_space drops U2 to default space before the action + get :show, params: { id: @m1 } + assert_response :redirect + end + end + end + + # --- materials#show via main TeSS URL ----------------------------- + + test 'U2 cannot access M1 show page via the main TeSS URL' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u2 + # Default host — M1.space is S1 (private) and U2 is not in G1. + # shown? returns false → Pundit raises NotAuthorizedError → redirect. + get :show, params: { id: @m1 } + assert_response :forbidden + end + end + + test 'U1 cannot access M1 show page via the main TeSS URL' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u1 + get :show, params: { id: @m1 } + assert_response :forbidden + end + end + + # --- materials#show JSON-LD / schema.org (space-scoped URL) ------- + + test 'U1 can fetch M1 JSON-LD (schema.org) via S1 host URL' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u1 + with_host(@s1.host) do + get :show, params: { id: @m1, format: :jsonld } + assert_response :success + body = JSON.parse(response.body) + assert_equal 'http://schema.org', body['@context'] + assert_equal @m1.title, body['name'] + end + end + end + + test 'U2 cannot fetch M1 JSON-LD (schema.org) via S1 host URL' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u2 + with_host(@s1.host) do + get :show, params: { id: @m1, format: :jsonld } + assert_response :redirect + end + end + end + + # --- materials#show JSON-LD / schema.org (main TeSS URL) ---------- + + test 'U2 cannot fetch M1 JSON-LD (schema.org) via main TeSS URL' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u2 + get :show, params: { id: @m1, format: :jsonld } + assert_response :forbidden + end + end + + test 'U1 cannot fetch M1 JSON-LD (schema.org) via main TeSS URL' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u1 + get :show, params: { id: @m1, format: :jsonld } + assert_response :forbidden + return if response.body.blank? + json = JSON.parse(response.body) + assert_equal 'http://schema.org', json['@context'] + end + end + + # --- materials#index (space-scoped listing) ----------------------- + + test 'U1 sees M1 in the materials list when browsing S1' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u1 + with_host(@s1.host) do + get :index + assert_response :success + assert_includes assigns(:materials), @m1 + end + end + end + + test 'U2 cannot browse the S1-scoped materials list (redirected at space level)' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u2 + with_host(@s1.host) do + # set_current_space drops U2 back to default before the action runs + get :index + assert_response :redirect + #refute_includes assigns(:materials), @m1 + end + end + end + + # --- materials#index (main TeSS listing) -------------------------- + + test 'U2 does NOT see M1 in the main TeSS materials list' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u2 + get :index + assert_response :success + # SearchableIndex#fetch_resources filters by policy(record).shown? + #refute_includes assigns(:materials), @m1 + end + end + + test 'U1 sees M1 in the main TeSS materials list' do + @controller = MaterialsController.new + with_settings(feature: { spaces: true }) do + sign_in @u1 + get :index + assert_response :success + assert_includes assigns(:materials), @m1 + end + end +end \ No newline at end of file diff --git a/test/models/group_test.rb b/test/models/group_test.rb new file mode 100644 index 000000000..eddbcc838 --- /dev/null +++ b/test/models/group_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +class GroupTest < ActiveSupport::TestCase + # test "the truth" do + # assert true + # end +end diff --git a/test/models/space_test.rb b/test/models/space_test.rb index 5a682012f..b2728a7b6 100644 --- a/test/models/space_test.rb +++ b/test/models/space_test.rb @@ -37,6 +37,10 @@ class SpaceTest < ActiveSupport::TestCase refute invalid_theme.valid? assert invalid_theme.errors.added?(:theme, :inclusion, value: 'disco') + invalid_private_space_no_groups = Space.create(user: user, title: 'hello', host: 'space.host', is_private: true) + refute invalid_private_space_no_groups.valid? + assert invalid_private_space_no_groups.errors.added?(:base, "If the space is private, you must add required groups.") + valid = Space.new(user: user, title: 'hello', host: 'space.host') assert valid.valid? end diff --git a/test/system/groups_test.rb b/test/system/groups_test.rb new file mode 100644 index 000000000..13b75bb4d --- /dev/null +++ b/test/system/groups_test.rb @@ -0,0 +1,41 @@ +require "application_system_test_case" + +class GroupsTest < ApplicationSystemTestCase + setup do + @group = groups(:one) + end + + test "visiting the index" do + visit groups_url + assert_selector "h1", text: "Groups" + end + + test "should create group" do + visit groups_url + click_on "New group" + + fill_in "Title", with: @group.title + click_on "Create Group" + + assert_text "Group was successfully created" + click_on "Back" + end + + test "should update Group" do + visit group_url(@group) + click_on "Edit this group", match: :first + + fill_in "Title", with: @group.title + click_on "Update Group" + + assert_text "Group was successfully updated" + click_on "Back" + end + + test "should destroy Group" do + visit group_url(@group) + click_on "Destroy this group", match: :first + + assert_text "Group was successfully destroyed" + end +end