diff --git a/app/controllers/urls_controller.rb b/app/controllers/urls_controller.rb index 425f95dc..a8cad791 100644 --- a/app/controllers/urls_controller.rb +++ b/app/controllers/urls_controller.rb @@ -1,24 +1,31 @@ # controllers/url_controller.rb class UrlsController < ApplicationController + before_action :ensure_signed_in before_action :set_url, only: %i[edit update destroy] before_action :set_url_friendly, only: [:show] - before_action :ensure_signed_in + before_action :set_group, only: %i[index create] # GET /urls # GET /urls.json def index - @group = Group.find(current_user.context_group_id) + group_ids_to_get = if @group == current_user.context_group + current_user.groups.pluck(:id) + else + [@group.id] + end + + # NOTE: the table is actually populated by datatables, + # so this doesn't really have any visible effect + @urls = Url.created_by_ids(group_ids_to_get) + .not_in_pending_transfer_request - @urls = - Url.created_by_id(current_user.context_group_id) - .not_in_pending_transfer_request @pending_transfer_requests_to = - TransferRequest.pending.where(to_group_id: current_user.context_group_id) + TransferRequest.pending.where(to_group_id: @group.id) @pending_transfer_requests_from = TransferRequest.pending - .where(from_group_id: current_user.context_group_id) - @url = Url.new + .where(from_group_id: @group.id) + @url = Url.new(group_id: @group.id) end # GET /urls/1 @@ -73,8 +80,10 @@ def edit def create @url_identifier = params[:new_identifier] @url = Url.new(url_params) - @url.group = current_user.context_group - @empty_url = Url.new + @url.group = @group + + # reset the form using the same group + @empty_url = Url.new(group: @group) respond_to do |format| if @url.save @@ -154,4 +163,16 @@ def set_url_friendly def url_params params.require(:url).permit(:url, :keyword, :group_id, :modified_by) end + + def set_group + # use collection query string if present, + # or the url's group_id if present. + # otherwise use current_user's context_group_id + group_id = params[:collection].presence || params.dig(:url, :group_id) || current_user.context_group_id + @group = Group.find(group_id) + + authorize @group, :update? + rescue Pundit::NotAuthorizedError + redirect_to urls_path, alert: "You do not have permission to access this collection. Redirected to your Z-Links." + end end diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 2bf660d6..1ff2f3d5 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -19,8 +19,6 @@ def destroy? user_has_access? end - private - def user_has_access? return true if user.admin? return true if record.is_a?(Group) && record.users.exists?(user.id) diff --git a/app/views/urls/_sleek_form.html.erb b/app/views/urls/_sleek_form.html.erb index 044f1ce3..f3d8a441 100644 --- a/app/views/urls/_sleek_form.html.erb +++ b/app/views/urls/_sleek_form.html.erb @@ -19,6 +19,7 @@ <%= f.text_field :keyword, class: 'js-new-url-keyword tw-border-none without-focus-ring tw-w-full tw-bg-transparent tw-text-umn-neutral-900 tw-h-full tw-py-4 tw-pl-1', placeholder: t('views.urls.sleek_form.placeholders.keyword'), 'aria-label': 'keyword' %> + <%= f.hidden_field :group_id, value: @group.id if @group %> diff --git a/config/environments/test.rb b/config/environments/test.rb index 3e3b5341..0d87e1ee 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -8,8 +8,7 @@ Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb. - # While tests run files are not watched, reloading is not necessary. - config.enable_reloading = false + config.enable_reloading = true # Eager loading loads your entire application. When running a single test locally, # this is usually not necessary, and can slow down your test suite. However, it's diff --git a/cypress/e2e/createZlinkInCollection.cy.ts b/cypress/e2e/createZlinkInCollection.cy.ts new file mode 100644 index 00000000..1c7cf423 --- /dev/null +++ b/cypress/e2e/createZlinkInCollection.cy.ts @@ -0,0 +1,54 @@ +import user1 from "../fixtures/users/user1.json"; +import user2 from "../fixtures/users/user2.json"; +import { RailsModel } from "../types"; + +describe("create zlink in collection", () => { + let user1Collection: RailsModel.Group | null = null; + let user2Collection: RailsModel.Group | null = null; + + beforeEach(() => { + cy.app("clean"); + cy.createUser(user1.umndid, { internet_id_loaded: user1.internet_id }); + cy.createUser(user2.umndid, { internet_id_loaded: user2.internet_id }); + + cy.login(user1.umndid); + + cy.createGroup("user1collection") + .then((grp) => { + user1Collection = grp; + cy.addUserToGroup(user1.umndid, grp); + }) + .then(() => cy.createGroup("user2collection")) + .then((grp) => { + user2Collection = grp; + cy.addUserToGroup(user2.umndid, grp); + }); + }); + it("creates a zlink within the collection when on a collection page", () => { + cy.visit(`/shortener/urls?collection=${user1Collection.id}`); + + // create a zlink on this collection page + cy.get("#url_url").type("https://example.com"); + cy.contains("Z-Link It").click(); + + // expect to see the zlink within the collection + cy.get("#urls-table tbody tr:first-child").within(() => { + cy.contains("https://example.com").should("exist"); + }); + }); + + it("does not permit creating a zlink in a collection you do not have access to", () => { + cy.visit(`/shortener/urls?collection=${user2Collection.id}`); + cy.get("#url_url").type("https://madeByUser1.com"); + cy.contains("Z-Link It").click(); + + // expect to see the url in User 1's default collection + cy.visit("/shortener/urls"); + cy.contains("https://madeByUser1.com").should("exist"); + + // then check the user 2 collection to verify the url is not there + cy.login(user2.umndid); + cy.visit(`/shortener/urls?collection=${user2Collection.id}`); + cy.contains("https://madeByUser1.com").should("not.exist"); + }); +}); diff --git a/cypress/e2e/urlsPageListZlinks.cy.ts b/cypress/e2e/urlsPageListZlinks.cy.ts index 51f91c57..dd4b84ad 100644 --- a/cypress/e2e/urlsPageListZlinks.cy.ts +++ b/cypress/e2e/urlsPageListZlinks.cy.ts @@ -101,13 +101,17 @@ describe("urlsPageListZlinks - /shortener/urls", () => { // Sometimes cypress doesn't complete typing the full string // into the input field. Using `force: true` and manually triggering // `input` event to work around this issue. - cy.get("#group_name").type("testcollection", { force: true }).trigger('input'); + cy.get("#group_name") + .type("testcollection", { force: true }) + .trigger("input"); - cy.get("#group_description").type("test description", { force: true }).trigger('input'); + cy.get("#group_description") + .type("test description", { force: true }) + .trigger("input"); // submit - cy.contains('Submit').click(); - cy.contains('Confirm').click(); + cy.contains("Submit").click(); + cy.contains("Confirm").click(); // verify that the url was added to the collection cy.get("@claRow") @@ -148,6 +152,22 @@ describe("urlsPageListZlinks - /shortener/urls", () => { ); }); + it("redirects to the user's main urls page if params include a collection that the user does not own", () => { + cy.createGroup("testcollection").then((group) => { + cy.visit(`/shortener/urls?collection=${group.id}`, { + failOnStatusCode: false, + }); + + // should be redirected to the main urls page without + // the collection param + cy.location("pathname").should("eq", "/shortener/urls"); + cy.location("search").should("eq", ""); + + // expect a flash message + cy.contains("You do not have permission to access this collection"); + }); + }); + describe("filter and search the list of z-links", () => { beforeEach(() => { // create a test collection and add the cla url to it