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 a2ff8cd2f..a890bae96 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/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/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 18ed1be58..09d1916f8 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -43,6 +43,7 @@ @import "showcase"; @import "toggle"; @import "workflows"; +@import "groups"; // Vendor @import "cytoscape-panzoom"; diff --git a/app/assets/stylesheets/groups.scss b/app/assets/stylesheets/groups.scss new file mode 100644 index 000000000..a9a2e8253 --- /dev/null +++ b/app/assets/stylesheets/groups.scss @@ -0,0 +1,61 @@ +@import "./themes/mixins/modern_base"; + +.group-member-list { + margin-top: 6px; + + li { + display: flex; + align-items: center; + gap: 10px; + padding: 6px 10px; + border: 1px solid $dark-30-color; + border-radius: 4px; + background: $dark-05-color; + margin-bottom: 4px; + + &.is-owner { + background: lighten($accent-color, 45%); + border-color: lighten($accent-color, 10%); + } + } +} + +.group-member-name { + font-weight: 600; + color: $secondary-color; + flex: 1 1 auto; +} + +.group-member-email { + font-size: 0.85em; + color: $dark-60-color; + flex: 1 1 auto; +} + +.group-member-owner-label { + display: inline-flex; + align-items: center; + gap: 4px; + font-weight: normal; + margin: 0; + white-space: nowrap; + cursor: pointer; + color: $dark-60-color; + + .glyphicon-star { + color: $dark-30-color; + transition: color 0.15s; + + input:checked ~ & { + color: $primary-color; + } + } +} + +.bg-accent { + background-color: $accent-color; +} + +.color-accent { + color: $accent-color; +} diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 505184359..0a7a1cf2d 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 @@ -18,9 +26,9 @@ class ApplicationController < ActionController::Base # User auth should be required in the web interface as well; it's here rather than in routes so that it # doesn't override the token auth, above. + before_action :set_current_user before_action :authenticate_user!, except: [:index, :show, :embed, :calendar, :check_exists, :handle_error, :count, :redirect] before_action :set_current_space - before_action :set_current_user # Should prevent forgery errors for JSON posts. skip_before_action :verify_authenticity_token, :if => Proc.new { |c| c.request.format == 'application/json' } @@ -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] = t('private_space.no_authorized') + raise Pundit::NotAuthorizedError + else + flash[:alert] = t('private_space.needs_sign_in') + raise Pundit::NotAuthorizedError + 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..d99eab3fa 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,19 +25,29 @@ 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, + # wrap the results 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 per_page = per_page_param.blank? ? DEFAULT_PAGE_SIZE : per_page_param.to_i @search_results = @model.search_and_filter(current_user, @search_params, @facet_params, - page: page, per_page: per_page, sort_by: @sort_by) + page: page, per_page: per_page, sort_by: @sort_by, space: Space.current_space) @index_resources = @search_results.results instance_variable_set("@#{controller_name}_results", @search_results) # e.g. @nodes_results else @@ -35,6 +57,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 +70,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 +117,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..38d35dff9 --- /dev/null +++ b/app/controllers/groups_controller.rb @@ -0,0 +1,113 @@ +# 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 ] + before_action :set_breadcrumbs + + # 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 + @memberships = @group.group_memberships.includes(:user) + 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] = permitted[:user_ids] if permitted.key?(:user_ids) + permitted[:owner_ids] = permitted[:owner_ids] if permitted.key?(:owner_ids) + permitted + 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/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/helpers/groups_helper.rb b/app/helpers/groups_helper.rb new file mode 100644 index 000000000..3bb5d1853 --- /dev/null +++ b/app/helpers/groups_helper.rb @@ -0,0 +1,5 @@ +module GroupsHelper + def groups_info + I18n.t('info.groups.description') + end +end diff --git a/app/models/concerns/searchable.rb b/app/models/concerns/searchable.rb index 218ec215a..6073cb1bd 100644 --- a/app/models/concerns/searchable.rb +++ b/app/models/concerns/searchable.rb @@ -19,21 +19,64 @@ def search_and_facet_keys @search_and_facet_keys ||= ([:q] | facet_keys_with_multiple) end - def search_and_filter(user, search_params = '', selected_facets = {}, page: 1, sort_by: nil, per_page: 30) + # Searches and filters resources of the including model using Solr (via Sunspot), + # applying space-based visibility, facets, sorting and pagination in a single query. + # + # Space filtering is applied before any other constraint: if +space+ is a specific + # (non-default) space, only resources belonging to that space are returned; on the + # default space, resources belonging to private spaces that are inaccessible to + # +user+ are excluded via a Solr +without+ clause, leaving default-space resources + # (those with no +space_id+) visible by default. + # + # user:: The currently authenticated user, or +nil+ for anonymous requests. + # Used to determine which private spaces are accessible and to scope + # ownership/collaboration facets. + # search_params:: Full-text search string forwarded to Solr. Defaults to an empty string. + # selected_facets:: Hash of active facet filters (field name → value). Special facets + # (see +Facets.special+) are handled separately from normal ones. + # page:: Current page number for pagination. Defaults to +1+. + # sort_by:: Sort key string (+nil+ or 'default' uses the model-specific + # default ordering; other accepted values: 'early', + # 'late', 'rel', 'mod', 'new', + # 'finished', or any field name accepted by Solr). + # per_page:: Number of results per page. Defaults to +30+. + # space:: The +Space+ to scope results to, or +nil+ / the default space to + # search across all spaces the user can access. + # + # Returns:: A +Sunspot::Search::StandardSearch+ result object whose +#results+ contain + # the matching, access-filtered, paginated model instances. + def search_and_filter(user, search_params = '', selected_facets = {}, page: 1, sort_by: nil, per_page: 30, space: nil) includes = Searchable::EAGER_LOADABLE.select { |a| reflections.key?(a.to_s) } + + has_space = attribute_method?(:space_id) + has_public = attribute_method?(:public) + has_collaborators = attribute_method?(:collaborators) + + accessible_space_ids = nil + inaccessible_space_ids = nil + + if has_space && (space.nil? || space.default?) + inaccessible_space_ids = Space.where(is_private: true).pluck(:id) + end + search(include: includes) do + if has_space + if space && !space.default? + with(:space_id, space.id) + else + if inaccessible_space_ids.present? + without(:space_id, inaccessible_space_ids) + end + end + end + fulltext search_params - # Set the search parameter - # Disjunction clause active_facets = {} - normal_facets = selected_facets.except(*Facets.special) any do - # Set all facets normal_facets.each do |facet_title, facet_value| - any do # Conjunction clause - # Add to array that get executed lower down + any do active_facets[facet_title] ||= [] val = Facets.process(facet_title, facet_value) active_facets[facet_title] << with(facet_title, val) @@ -43,41 +86,24 @@ def search_and_filter(user, search_params = '', selected_facets = {}, page: 1, s if sort_by && sort_by != 'default' case sort_by - when 'early' - # Sort by start date asc - order_by(:start, :asc) - when 'late' - # Sort by start date desc - order_by(:start, :desc) - when 'rel' - # Sort by relevance - when 'mod' - # Sort by last modified - order_by(:updated_at, :desc) - when 'new' - # Sort by newest - order_by(:created_at, :desc) - when 'finished' - # Sort by last finished - order_by(:finished_at, :desc) - else - order_by(:sort_title, sort_by.to_sym) + when 'early' then order_by(:start, :asc) + when 'late' then order_by(:start, :desc) + when 'rel' then nil + when 'mod' then order_by(:updated_at, :desc) + when 'new' then order_by(:created_at, :desc) + when 'finished' then order_by(:finished_at, :desc) + else order_by(:sort_title, sort_by.to_sym) end - # Defaults else case name - when 'Event' - order_by(:start, :asc) - when 'ContentProvider' - order_by(:count, :desc) - when 'Material' - order_by(:created_at, :desc) - else - order_by(:sort_title, :asc) + when 'Event' then order_by(:start, :asc) + when 'ContentProvider' then order_by(:count, :desc) + when 'Material' then order_by(:created_at, :desc) + else order_by(:sort_title, :asc) end end - paginate page: page, per_page: per_page unless page.nil? + paginate page: page, per_page: per_page unless page.nil?; Facets.special.each do |facet_title| if Facets.applicable?(facet_title, self) @@ -87,24 +113,18 @@ def search_and_filter(user, search_params = '', selected_facets = {}, page: 1, s end if name == 'Trainer' || name == 'Profile' - # `public` means "Show in trainer registry" for Trainers/Profiles - any_of do - with(:public, true) - end - elsif attribute_method?(:public) && !user&.is_admin? # Find a better way of checking this + any_of { with(:public, true) } + elsif has_public && !user&.is_admin? any_of do with(:public, true) with(:user_id, user.id) if user - if attribute_method?(:collaborators) - with(:collaborator_ids, user.id) if user - end + with(:collaborator_ids, user.id) if user && has_collaborators end end facet_fields.each do |ff| facet ff, exclude: active_facets[ff] end - end end 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..bdcb6088b 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 :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, dependent: :destroy 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, I18n.t('private_space.needs_groups_in_form')) + 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,42 @@ 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.destroy_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 + 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..d4bcb5ac8 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? || @user.groups.where(id: @space.groups).any? + 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..3c99438b4 --- /dev/null +++ b/app/views/groups/_form.html.erb @@ -0,0 +1,85 @@ +<%= 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: t('group.form.placeholder') %> + <% if group.errors[:title].any? %> + <%= group.errors[:title].first %> + <% end %> +
    + +
    + +

    + + <%= t('group.form.member_explain') %> +

    + + <%# 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", id: "submit-btn" %> + <%= link_to "Cancel", :back, class: "btn btn-default" %> +
      +<% end %> + + 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..b2a425aae --- /dev/null +++ b/app/views/groups/index.html.erb @@ -0,0 +1,61 @@ +<% content_for :title, "Groups" %> + +
      + <%= page_title("Groups") %> +
      + +
      +
      +
      +
      + <% if policy(:group).create? %> +
      + <%= link_to new_group_path, class: 'btn btn-primary' do %> + New group + <% end %> +
      + <% end %> + + <%= info_button("What are groups in #{TeSS::Config.site['title_short']}?", hide_text: true) do %> + <%= render_markdown(groups_info) %> + <% 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..6dab7e13b --- /dev/null +++ b/app/views/groups/show.html.erb @@ -0,0 +1,155 @@ +<% content_for :title, @group.title %> + +
      + <%# SIDEBAR %> + + + <%# MAIN CONTENT %> +
      +
      +
      +
      + <% if policy(@group).update? %> + <%= edit_button(@group) %> + <% end %> + <% if policy(@group).destroy? %> + <%= delete_button(@group) %> + <% end %> +
      +
      + + <%= link_to groups_path, class: 'btn btn-default' do %> + <%= t('group.show.back_to_groups') %> + <% end %> + + <%# Members table %> + + <% if @memberships.any? %> + + + + + <% if @memberships.first.user.respond_to?(:email) %> + + <% end %> + + + + + <% @memberships.each do |membership| %> + + + <% if membership.user.respond_to?(:email) %> + + <% end %> + + + <% end %> + +
      <%= t('group.show.table.name') %><%= t('group.show.table.email') %><%= t('group.show.table.role') %>
      + <%= link_to membership.user.name, membership.user %> + <%= membership.user.email %> + <% if membership.owner? %> + + <%= t('group.show.table.owner') %> + + <% else %> + <%= t('group.show.table.member') %> + <% end %> +
      + <% else %> +
      + +

      + <%= t('group.show.no_members') %> + <%= link_to t('group.show.add_members'), edit_group_path(@group) %>. +

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

      <%= t('group.show.spaces.title') %>

      + + <% if spaces.any? %> + + + + + + + + + <% spaces.each do |space| %> + <% groups = space.groups.all %> + + + + + <% end %> + +
      <%= t('group.show.spaces.space') %><%= t('group.show.spaces.other_groups') %>
      + <%= 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 %> +
      + +

      + <%= t('group.show.spaces.no_access') %> +

      +
      + <% 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..89edda352 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: t('menu.user.view_groups') do %> + <%= t('menu.user.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..bc1449f31 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 %>
      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 7d1a688b3..f97ba3482 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -37,10 +37,44 @@ en: spaces: short: Spaces long: Spaces + groups: + short: Groups + long: Groups pundit: default: 'You are not authorised to perform this action.' material_policy: event_policy: + private_space: + no_authorized: You are not authorized to access this page. + needs_sign_in: You need to sign in to the main application to access this page. + needs_groups_in_form: If the space is private, you must add required groups. + group: + show: + group: Group + members: Members + owner: Owner + no_members: No members yet. + back_to_groups: Back to groups + add_members: Add members + stats: + title: Quick stats + total: Total members + owners: Owners + table: + name: Name + email: Email + role: Role + owner: Owner + member: Member + spaces: + title: Spaces which require this group to have access to + space: Space + other_groups: Other groups which have access + no_access: This group does not give access to any private space + form: + members: Members + placeholder: Group title + member_explain: Search and add members below - check Owner to grant ownership. attributes: url: 'URL' doi: 'DOI' @@ -955,6 +989,7 @@ en: view_stars: 'View stars' administration: 'Administration' view_users: 'View users' + view_groups: 'View groups' view_sources: 'View sources' view_ingestion_sources: 'View ingestion sources' assign_scientific_topics: "Assign scientific topics to %{title} resources" @@ -1151,7 +1186,15 @@ 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. + groups: + title: What are Groups? + description: | + Groups is a feature which for now is used only to manage access of private spaces. + A user being part of a group has access to every private space which requires the group. + Each group has multiple owners. A owner can add and remove people to the group. + orcid: 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..89320428b --- /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 5bec071c7..5374ac950 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -269,6 +269,29 @@ 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 + t.index ["group_id"], name: "index_groups_spaces_on_group_id" + t.index ["space_id"], name: "index_groups_spaces_on_space_id" + end + create_table "learning_path_topic_items", force: :cascade do |t| t.text "comment" t.datetime "created_at", null: false @@ -537,6 +560,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 @@ -689,6 +713,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..5b7cb0481 --- /dev/null +++ b/test/integration/private_space_access_test.rb @@ -0,0 +1,270 @@ +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 + get :show, params: { id: @s1 } + assert_response :forbidden + 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 :forbidden + 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 :forbidden + 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 :forbidden + #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..1397dc9ff 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, I18n.t('private_space.needs_groups_in_form')) + 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..287d0b05d --- /dev/null +++ b/test/system/groups_test.rb @@ -0,0 +1,53 @@ +require "application_system_test_case" + +class GroupsTest < ApplicationSystemTestCase + include Devise::Test::IntegrationHelpers + + setup do + @group = groups(:one) + @admin = users(:admin) + @owner = users(:regular_user) + + @group.group_memberships.find_or_create_by!(user: @owner) { |membership| membership.owner = true } + end + + test "visiting the index" do + sign_in @admin + visit groups_url + assert_selector "h2", text: "Groups" + end + + test "should create group" do + sign_in @admin + visit groups_url + click_on "New group" + + fill_in "Title", with: @group.title + + find("#submit-btn").click + + assert_text "Group was successfully created" + end + + test "should update Group" do + sign_in @owner + visit group_url(@group) + click_on "Edit", match: :first + + fill_in "Title", with: @group.title + find("#submit-btn").click + + assert_text "Group was successfully updated" + end + + test "should destroy Group" do + sign_in @owner + visit group_url(@group) + + accept_confirm do + click_on "Delete", match: :first + end + + assert_text "Group was successfully destroyed" + end +end