From 6ab0b8eab4285ff09f5250ffcd7cd1f50decf5ab Mon Sep 17 00:00:00 2001 From: Leo Wilkin Date: Wed, 9 Sep 2026 23:46:37 +0100 Subject: [PATCH 1/2] Create vote.hackclub.com galleries from the series page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A vote event is per Attend event, but choosing which of a series' events get a voting gallery is a decision about the series — and vote could only be reached from each event's own integrations page, so making that choice meant walking every satellite. Series owners now get an Integrations page listing every event with its gallery or a button to create one, disabled with the reason when an event has no logo and banner yet. The create path was inline in the dashboard controller, and the order of its checks is the whole substance of it: already linked, server configured, adopt an existing vote event with this slug, insist on artwork, create, and adopt the winner if it loses the race. That moves to Vote::EventLinker so both pages share one path rather than a copy each, and gets the spec coverage it never had. The event's own page behaves exactly as before. Owner-only, the same bar as issuing a series API key: creating a gallery publishes the event's name and artwork to another service. --- app/controllers/admin/dashboard_controller.rb | 66 +------- .../admin/series_integrations_controller.rb | 49 ++++++ app/policies/event_series_policy.rb | 6 + app/services/vote/event_linker.rb | 98 ++++++++++++ app/views/admin/event_series/show.html.erb | 6 + .../admin/series_integrations/show.html.erb | 107 +++++++++++++ config/routes.rb | 5 + .../admin/series_integrations_spec.rb | 151 ++++++++++++++++++ spec/services/vote/event_linker_spec.rb | 129 +++++++++++++++ 9 files changed, 553 insertions(+), 64 deletions(-) create mode 100644 app/controllers/admin/series_integrations_controller.rb create mode 100644 app/services/vote/event_linker.rb create mode 100644 app/views/admin/series_integrations/show.html.erb create mode 100644 spec/requests/admin/series_integrations_spec.rb create mode 100644 spec/services/vote/event_linker_spec.rb diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index aaa2cb1..fac3614 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -75,56 +75,9 @@ def create_vote_event # hides it from every non-global admin's audit log. set_current_event(@event) - client = Vote::Client.new - - if @event.vote_event_linked? - redirect_to admin_event_integrations_path(@event), - notice: "This event is already linked to a vote.hackclub.com event." - return - end - - unless client.configured? - redirect_to admin_event_integrations_path(@event), - alert: "The vote.hackclub.com API key is not configured on the server." - return - end - - # Backfill: if a vote event already exists for this slug, link it instead of - # creating a duplicate. - if (existing = client.find_event(@event.slug)) - @event.link_vote_event!(existing) - redirect_to admin_event_integrations_path(@event), - notice: "Linked to the existing vote.hackclub.com event for this slug." - return - end - - unless @event.logo.attached? && @event.banner.attached? - redirect_to admin_event_integrations_path(@event), - alert: "This event needs both a logo and a banner before a vote.hackclub.com event can be created." - return - end - - result = client.create_event( - name: @event.name, - slug: @event.slug, - logo_url: public_attachment_url(@event.logo), - background_url: public_attachment_url(@event.banner), - admins: vote_admin_emails - ) - @event.link_vote_event!(result) - + result = Vote::EventLinker.new(@event).call redirect_to admin_event_integrations_path(@event), - notice: "Created vote.hackclub.com event." - rescue Vote::Error => e - # Lost a race (or slug taken): try to link the now-existing event. - if e.status == 409 && (existing = client.find_event(@event.slug)) - @event.link_vote_event!(existing) - redirect_to admin_event_integrations_path(@event), - notice: "Linked to the existing vote.hackclub.com event for this slug." - else - redirect_to admin_event_integrations_path(@event), - alert: "vote.hackclub.com: #{e.message.presence || 'Failed to create the vote event.'}" - end + (result.linked? ? :notice : :alert) => result.message end def update_integrations @@ -169,21 +122,6 @@ def airtable_settings_saved? AIRTABLE_SETTINGS.any? { |setting| @event.public_send("saved_change_to_#{setting}?") } end - # Emails granted event-admin access on the vote.hackclub.com event. Only - # Attend event admins qualify — ops, safeguarding, and read-only roles don't - # imply control over voting. - def vote_admin_emails - @event.event_role_assignments.event_admin.includes(:user).map { |a| a.user.email } - end - - def public_attachment_url(attachment) - Rails.application.routes.url_helpers.rails_storage_proxy_url( - attachment, - host: ENV.fetch("APP_HOST", "attend.hackclub.com"), - protocol: "https" - ) - end - def load_airtable_sync_status @airtable_sync_configured = current_event.airtable_sync_configured? @airtable_synced_at = current_event.airtable_synced_at diff --git a/app/controllers/admin/series_integrations_controller.rb b/app/controllers/admin/series_integrations_controller.rb new file mode 100644 index 0000000..fcc3479 --- /dev/null +++ b/app/controllers/admin/series_integrations_controller.rb @@ -0,0 +1,49 @@ +module Admin + # Series-level view of the vote.hackclub.com integration. + # + # A vote event is per Attend event, but deciding which of a series' events + # get a voting gallery is a decision about the series — so this lists them + # all with a button each, instead of making somebody walk every satellite's + # own integrations page. + class SeriesIntegrationsController < BaseController + skip_before_action :set_current_event_from_session + + before_action :set_series + before_action :require_series_owner_access + + def show + @vote_client = Vote::Client.new + @events = @series.events + .includes(logo_attachment: :blob, banner_attachment: :blob) + .order(Arel.sql("starts_at ASC NULLS LAST")) + end + + def create_vote_event + event = @series.events.find_by(slug: params[:event_id]) || + @series.events.find_by(id: params[:event_id]) + return redirect_to admin_series_integrations_path(@series), alert: "That event is not in this series." if event.nil? + + authorize event, :update? + + # Same reason as the event's own page: without a current event the audit + # row lands with a null event_id and hides from non-global admins. + set_current_event(event) + + result = Vote::EventLinker.new(event).call + redirect_to admin_series_integrations_path(@series), + (result.linked? ? :notice : :alert) => "#{event.name}: #{result.message}" + end + + private + + def set_series + @series = EventSeries.find_by!(slug: params[:series_slug]) + end + + def require_series_owner_access + return if policy(@series).manage_integrations? + + redirect_to admin_series_path(@series), alert: "Only series owners can manage integrations." + end + end +end diff --git a/app/policies/event_series_policy.rb b/app/policies/event_series_policy.rb index 23bc882..6e39b39 100644 --- a/app/policies/event_series_policy.rb +++ b/app/policies/event_series_policy.rb @@ -28,6 +28,12 @@ def manage_api_tokens? user.series_owner_for?(record) end + # Creating a vote.hackclub.com event publishes the event's name and artwork + # to another service, so the same bar as issuing an API key: owner-only. + def manage_integrations? + user.series_owner_for?(record) + end + def destroy? user.global_admin? end diff --git a/app/services/vote/event_linker.rb b/app/services/vote/event_linker.rb new file mode 100644 index 0000000..1a05f66 --- /dev/null +++ b/app/services/vote/event_linker.rb @@ -0,0 +1,98 @@ +module Vote + # Creates (or adopts) the vote.hackclub.com event for one Attend event, and + # records the linkage. + # + # Two pages drive this — an event's own integrations page and its series' + # integrations page — and the order of the checks is the whole substance of + # it, so it lives here rather than in either controller: already linked, then + # server configured, then adopt an existing event with this slug, then insist + # on artwork, then create. Losing the create race means adopting instead. + class EventLinker + Result = Struct.new(:status, :message, keyword_init: true) do + # Everything the caller shows as a notice rather than an alert: the event + # is linked afterwards, whether this call is what linked it. + def linked? + %i[created linked_existing already_linked].include?(status) + end + end + + def initialize(event, client: nil) + @event = event + @client = client || Vote::Client.new + end + + def call + if @event.vote_event_linked? + return result(:already_linked, "This event is already linked to a vote.hackclub.com event.") + end + + unless @client.configured? + return result(:not_configured, "The vote.hackclub.com API key is not configured on the server.") + end + + # Backfill: if a vote event already exists for this slug, link it instead + # of creating a duplicate. + if (existing = @client.find_event(@event.slug)) + return adopt(existing) + end + + unless artwork_ready? + return result( + :missing_artwork, + "This event needs both a logo and a banner before a vote.hackclub.com event can be created." + ) + end + + @event.link_vote_event!( + @client.create_event( + name: @event.name, + slug: @event.slug, + logo_url: public_attachment_url(@event.logo), + background_url: public_attachment_url(@event.banner), + admins: admin_emails + ) + ) + result(:created, "Created vote.hackclub.com event.") + rescue Vote::Error => e + # Lost a race (or slug taken): try to link the now-existing event. + if e.status == 409 && (existing = @client.find_event(@event.slug)) + adopt(existing) + else + result(:failed, "vote.hackclub.com: #{e.message.presence || 'Failed to create the vote event.'}") + end + end + + # Both pages disable the button without artwork, and say why. + def artwork_ready? + @event.logo.attached? && @event.banner.attached? + end + + private + + def adopt(existing) + @event.link_vote_event!(existing) + result(:linked_existing, "Linked to the existing vote.hackclub.com event for this slug.") + end + + # Emails granted event-admin access on the vote.hackclub.com event. Only + # Attend event admins qualify — ops, safeguarding, and read-only roles + # don't imply control over voting. + def admin_emails + @event.event_role_assignments.event_admin.includes(:user).map { |a| a.user.email } + end + + # vote.hackclub.com fetches these images itself, so they have to be public + # and absolute against the deployed host — not the host of this request. + def public_attachment_url(attachment) + Rails.application.routes.url_helpers.rails_storage_proxy_url( + attachment, + host: ENV.fetch("APP_HOST", "attend.hackclub.com"), + protocol: "https" + ) + end + + def result(status, message) + Result.new(status: status, message: message) + end + end +end diff --git a/app/views/admin/event_series/show.html.erb b/app/views/admin/event_series/show.html.erb index 083f6f0..daeb848 100644 --- a/app/views/admin/event_series/show.html.erb +++ b/app/views/admin/event_series/show.html.erb @@ -65,6 +65,12 @@ API Keys <% end %> <% end %> + <% if policy(@series).manage_integrations? %> + <%= link_to admin_series_integrations_path(@series), class: "inline-flex items-center gap-2 cursor-pointer border border-(--border-strong) bg-(--bg-elev) text-(--text) hover:bg-(--bg-elev-3) hover:text-(--text-strong) text-sm font-medium py-2 px-4 rounded-md transition-colors" do %> + + Integrations + <% end %> + <% end %> <% if policy(@series).update? %> <%= link_to edit_admin_series_path(@series), class: "inline-flex items-center gap-2 cursor-pointer border border-(--border-strong) bg-(--bg-elev) text-(--text) hover:bg-(--bg-elev-3) hover:text-(--text-strong) text-sm font-medium py-2 px-4 rounded-md transition-colors" do %> diff --git a/app/views/admin/series_integrations/show.html.erb b/app/views/admin/series_integrations/show.html.erb new file mode 100644 index 0000000..021715d --- /dev/null +++ b/app/views/admin/series_integrations/show.html.erb @@ -0,0 +1,107 @@ +<% content_for :title, "#{@series.name} Integrations – Attend" %> + +
+
+
+ <%= render "admin/events/avatar", event: @series, size: 32, rounded: "rounded-lg" %> +

<%= @series.name %> Integrations

+
+ <%= link_to "API Keys", admin_series_api_tokens_path(@series), class: "w-full sm:w-auto text-center px-4 py-2 border border-gray-300 text-gray-700 rounded-md hover:bg-gray-50 transition-colors" %> +
+ +

+ Where this series' events get their projects judged. A gallery is per event, so a series + running satellites gets one each — create them here rather than event by event. +

+ +
+
+
+ +
+
+

vote.hackclub.com

+

A peer-voting gallery for one event at a time

+
+
+ +
+

+ Creates a DRAFT vote event from an Attend event's name, slug, logo and banner, + with its event admins as admins over there — you finish setting it up on vote.hackclub.com. + An event that already exists for the same slug is linked rather than duplicated. +

+ + <% unless @vote_client.configured? %> +
+ ⚠️ No vote.hackclub.com API key is configured on this server, so nothing can be created. + Set VOTE_API_KEY (or the vote.api_key credential) first. +
+ <% end %> +
+ + <% if @events.any? %> +
+ + + + + + + + + + <% @events.each do |event| %> + <% artwork_ready = event.logo.attached? && event.banner.attached? %> + + + + + + <% end %> + +
EventGalleryActions
+
<%= event.name %>
+
<%= event.slug %>
+ 		+ <% if event.vote_event_linked? %> +
+ Linked + <% if event.vote_event_gallery_url.present? %> + <%= link_to "View gallery", event.vote_event_gallery_url, target: "_blank", rel: "noopener", + class: "text-[#ec3750] hover:underline font-medium" %> + <% end %> +
+ <% elsif artwork_ready %> + Not created yet + <% else %> + <%# Same bar as the event's own page: vote.hackclub.com fetches both + images itself, so it cannot be created without them. %> + Needs a logo and a banner first + <% end %> + 		+
+ <% if event.vote_event_linked? %> + <% if event.vote_event_admin_url.present? %> + <%= link_to "Manage", event.vote_event_admin_url, target: "_blank", rel: "noopener", + class: "text-[#ec3750] hover:text-[#d42f46]" %> + <% end %> + <% else %> + <%= button_to "Create gallery", + create_vote_event_admin_series_integrations_path(@series, event_id: event.slug), + method: :post, + disabled: !artwork_ready || !@vote_client.configured?, + data: { turbo_confirm: "Create a DRAFT vote.hackclub.com event for \"#{event.name}\"?" }, + class: "text-[#ec3750] hover:text-[#d42f46] disabled:text-gray-400 disabled:cursor-not-allowed" %> + <% end %> + <%= link_to "Event page", admin_event_integrations_path(event), class: "text-gray-500 hover:text-gray-700" %> +
+
+
+ <% else %> +
+

This series has no events yet.

+
+ <% end %> +
+
diff --git a/config/routes.rb b/config/routes.rb index 7dd99a7..18c2455 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -332,6 +332,11 @@ end resources :series, controller: "event_series", param: :slug, except: [ :destroy ] do + resource :integrations, only: [ :show ], controller: "series_integrations" do + # Vote events are per event, so this one names which of the series' + # events it is for. + post "vote_event/:event_id", action: :create_vote_event, as: :create_vote_event + end resources :members, only: [ :index, :new, :create, :destroy ], controller: "series_members" resources :api_tokens, only: [ :index, :create, :destroy ], controller: "series_api_tokens" do member do diff --git a/spec/requests/admin/series_integrations_spec.rb b/spec/requests/admin/series_integrations_spec.rb new file mode 100644 index 0000000..08d6117 --- /dev/null +++ b/spec/requests/admin/series_integrations_spec.rb @@ -0,0 +1,151 @@ +require "rails_helper" + +RSpec.describe "Admin::SeriesIntegrations", type: :request do + include Devise::Test::IntegrationHelpers + include ActiveJob::TestHelper + + let(:series) { create(:event_series, name: "Sunbeam", slug: "sunbeam-integrations") } + let!(:sub_event) { create(:event, event_series: series, slug: "sunbeam-one") } + + let(:owner) do + User.create!(email: "owner-integrations@example.com", name: "Owner").tap do |user| + SeriesRoleAssignment.create!(user: user, event_series: series, role: "owner") + end + end + let(:organizer) do + User.create!(email: "organizer-integrations@example.com", name: "Organizer").tap do |user| + SeriesRoleAssignment.create!(user: user, event_series: series, role: "organizer") + end + end + + # Whether the server holds a vote.hackclub.com key. Without one the page + # still has to render — it is where an organizer goes to find that out. + let(:configured) { false } + + before do + allow_any_instance_of(Vote::Client).to receive(:configured?).and_return(configured) + end + + describe "GET" do + it "tells an owner when no vote.hackclub.com key is configured on the server" do + sign_in owner + + get admin_series_integrations_path(series) + + expect(response).to have_http_status(:ok) + expect(response.body).to include("No vote.hackclub.com API key is configured") + end + + context "with a key configured" do + let(:configured) { true } + + it "lists the series' events with a button each" do + sign_in owner + + get admin_series_integrations_path(series) + + expect(response.body).to include("vote.hackclub.com") + expect(response.body).to include(sub_event.name) + expect(response.body).to include("Create gallery") + end + + it "shows an existing gallery instead of offering to create one" do + sub_event.link_vote_event!( + "id" => "vote-evt-1", + "slug" => sub_event.slug, + "adminUrl" => "https://vote.hackclub.com/admin/vote-evt-1", + "galleryUrl" => "https://vote.hackclub.com/vote-evt-1" + ) + sign_in owner + + get admin_series_integrations_path(series) + + expect(response.body).to include("https://vote.hackclub.com/vote-evt-1") + expect(response.body).to include("Linked") + expect(response.body).not_to include("Create gallery") + end + end + + it "keeps a series organizer out — creating a gallery publishes the event elsewhere" do + sign_in organizer + + get admin_series_integrations_path(series) + + expect(response).to redirect_to(admin_series_path(series)) + expect(flash[:alert]).to match(/Only series owners/) + end + end + + describe "POST create_vote_event" do + let(:vote_client) { instance_double(Vote::Client, configured?: true) } + + let(:vote_body) do + { + "id" => "vote-evt-1", + "slug" => sub_event.slug, + "adminUrl" => "https://vote.hackclub.com/admin/vote-evt-1", + "galleryUrl" => "https://vote.hackclub.com/vote-evt-1" + } + end + + before { allow(Vote::Client).to receive(:new).and_return(vote_client) } + + it "creates the gallery for one of the series' events and comes back here" do + allow(vote_client).to receive(:find_event).with(sub_event.slug).and_return(vote_body) + sign_in owner + + post create_vote_event_admin_series_integrations_path(series, event_id: sub_event.slug) + + expect(response).to redirect_to(admin_series_integrations_path(series)) + expect(flash[:notice]).to include(sub_event.name) + expect(sub_event.reload.vote_event_id).to eq("vote-evt-1") + end + + # The event is the audited record here, and it needs to be the current one + # or the row lands with a null event_id and hides from non-global admins. + it "audit-logs against the event, not the series" do + allow(vote_client).to receive(:find_event).and_return(vote_body) + sign_in owner + get admin_series_integrations_path(series) + + post create_vote_event_admin_series_integrations_path(series, event_id: sub_event.slug) + + log = AuditLog.find_by!(action: "create_vote_event") + expect(log.record).to eq(sub_event) + expect(log.event).to eq(sub_event) + expect(log.actor).to eq(owner) + end + + it "explains itself when the event has no artwork yet" do + allow(vote_client).to receive(:find_event).and_return(nil) + allow(vote_client).to receive(:create_event) + sign_in owner + + post create_vote_event_admin_series_integrations_path(series, event_id: sub_event.slug) + + expect(flash[:alert]).to include("needs both a logo and a banner") + expect(vote_client).not_to have_received(:create_event) + end + + it "refuses an event in another series" do + elsewhere = create(:event, slug: "not-in-this-series") + allow(vote_client).to receive(:find_event) + sign_in owner + + post create_vote_event_admin_series_integrations_path(series, event_id: elsewhere.slug) + + expect(flash[:alert]).to eq("That event is not in this series.") + expect(elsewhere.reload.vote_event_id).to be_nil + end + + it "keeps a series organizer out" do + allow(vote_client).to receive(:find_event) + sign_in organizer + + post create_vote_event_admin_series_integrations_path(series, event_id: sub_event.slug) + + expect(response).to redirect_to(admin_series_path(series)) + expect(sub_event.reload.vote_event_id).to be_nil + end + end +end diff --git a/spec/services/vote/event_linker_spec.rb b/spec/services/vote/event_linker_spec.rb new file mode 100644 index 0000000..b443708 --- /dev/null +++ b/spec/services/vote/event_linker_spec.rb @@ -0,0 +1,129 @@ +require "rails_helper" + +RSpec.describe Vote::EventLinker do + subject(:linker) { described_class.new(event, client: client) } + + let(:event) { create(:event, slug: "sunbeam-vote") } + let(:client) { instance_double(Vote::Client, configured?: true) } + + let(:created_body) do + { + "id" => "vote-evt-1", + "slug" => "sunbeam-vote", + "adminUrl" => "https://vote.hackclub.com/admin/vote-evt-1", + "galleryUrl" => "https://vote.hackclub.com/vote-evt-1" + } + end + + def attach_artwork + png = file_fixture("headshot.png").binread + event.logo.attach(io: StringIO.new(png), filename: "logo.png", content_type: "image/png") + event.banner.attach(io: StringIO.new(png), filename: "banner.png", content_type: "image/png") + end + + it "creates the vote event and records the linkage" do + attach_artwork + admin = User.create!(email: "vote-admin@example.com", name: "Vote Admin") + EventRoleAssignment.create!(user: admin, event: event, role: "event_admin") + allow(client).to receive(:find_event).with("sunbeam-vote").and_return(nil) + allow(client).to receive(:create_event).and_return(created_body) + + result = linker.call + + expect(result.status).to eq(:created) + expect(result).to be_linked + expect(event.reload.vote_event_id).to eq("vote-evt-1") + expect(event.vote_event_gallery_url).to eq("https://vote.hackclub.com/vote-evt-1") + expect(client).to have_received(:create_event).with( + hash_including( + name: event.name, + slug: "sunbeam-vote", + # vote.hackclub.com fetches the images itself, so they point at the + # deployed host rather than whatever host served this request. + logo_url: a_string_starting_with("https://"), + background_url: a_string_starting_with("https://"), + admins: [ "vote-admin@example.com" ] + ) + ) + end + + it "only offers Attend event admins as vote admins" do + attach_artwork + ops = User.create!(email: "vote-ops@example.com", name: "Ops") + EventRoleAssignment.create!(user: ops, event: event, role: "ops") + allow(client).to receive(:find_event).and_return(nil) + allow(client).to receive(:create_event).and_return(created_body) + + linker.call + + expect(client).to have_received(:create_event).with(hash_including(admins: [])) + end + + it "adopts an existing vote event with the same slug instead of duplicating it" do + attach_artwork + allow(client).to receive(:find_event).with("sunbeam-vote").and_return(created_body) + allow(client).to receive(:create_event) + + result = linker.call + + expect(result.status).to eq(:linked_existing) + expect(result).to be_linked + expect(event.reload.vote_event_id).to eq("vote-evt-1") + expect(client).not_to have_received(:create_event) + end + + it "adopts the winner when it loses the create race" do + attach_artwork + allow(client).to receive(:find_event).with("sunbeam-vote").and_return(nil, created_body) + allow(client).to receive(:create_event).and_raise(Vote::Error.new("Slug taken", status: 409)) + + result = linker.call + + expect(result.status).to eq(:linked_existing) + expect(event.reload.vote_event_id).to eq("vote-evt-1") + end + + it "refuses without both a logo and a banner" do + allow(client).to receive(:find_event).and_return(nil) + allow(client).to receive(:create_event) + + result = linker.call + + expect(result.status).to eq(:missing_artwork) + expect(result).not_to be_linked + expect(client).not_to have_received(:create_event) + end + + it "says so when the event is already linked, without calling out at all" do + event.link_vote_event!(created_body) + allow(client).to receive(:find_event) + + result = linker.call + + expect(result.status).to eq(:already_linked) + expect(result).to be_linked + expect(client).not_to have_received(:find_event) + end + + it "says so when the server has no vote.hackclub.com key" do + allow(client).to receive(:configured?).and_return(false) + allow(client).to receive(:find_event) + + result = linker.call + + expect(result.status).to eq(:not_configured) + expect(client).not_to have_received(:find_event) + end + + it "surfaces any other failure with the API's own message" do + attach_artwork + allow(client).to receive(:find_event).and_return(nil) + allow(client).to receive(:create_event).and_raise(Vote::Error.new("Name is required", status: 422)) + + result = linker.call + + expect(result.status).to eq(:failed) + expect(result.message).to eq("vote.hackclub.com: Name is required") + expect(event.reload.vote_event_id).to be_nil + end +end From fa334849d881a3e27f258897187715c805514d0b Mon Sep 17 00:00:00 2001 From: Leo Wilkin Date: Wed, 9 Sep 2026 23:56:31 +0100 Subject: [PATCH 2/2] Only link a vote URL that is actually http(s) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brakeman's LinkToHref check flagged both links in the events table: vote decides its own admin and gallery URLs, we store whatever it sends, and by the time a view renders them they are external input — a stored `javascript:` URL would become a clickable script. `external_http_url` returns the URL only when it parses as http(s), and nil otherwise so the link is simply dropped, which is a real fix rather than an ignore entry. The event's own integrations page renders the same two URLs and Brakeman doesn't resolve them there, so it stays as it was. --- app/helpers/admin/series_helper.rb | 13 +++++++++++++ .../admin/series_integrations/show.html.erb | 11 +++++++---- .../admin/series_integrations_spec.rb | 19 +++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/app/helpers/admin/series_helper.rb b/app/helpers/admin/series_helper.rb index c08a12f..4030b18 100644 --- a/app/helpers/admin/series_helper.rb +++ b/app/helpers/admin/series_helper.rb @@ -55,6 +55,19 @@ def series_map_markers(rows) end end + # vote.hackclub.com decides its own admin and gallery URLs and we store + # whatever it sends, so they are external input by the time a view renders + # them: anything but http(s) — a `javascript:` URL above all — must never + # reach an href. Returns nil when there is nothing safe to link to, so the + # caller can drop the link entirely. + def external_http_url(url) + return nil if url.blank? + + URI.parse(url).is_a?(URI::HTTP) ? url : nil + rescue URI::InvalidURIError + nil + end + # The participant list for one event, filtered to the people stuck at a stage. # Passing the event slug in the path is what switches the admin event picker # (Admin::BaseController#switch_event_if_needed), so these links land on a diff --git a/app/views/admin/series_integrations/show.html.erb b/app/views/admin/series_integrations/show.html.erb index 021715d..b833f63 100644 --- a/app/views/admin/series_integrations/show.html.erb +++ b/app/views/admin/series_integrations/show.html.erb @@ -53,6 +53,9 @@ <% @events.each do |event| %> <% artwork_ready = event.logo.attached? && event.banner.attached? %> + <%# vote.hackclub.com sends these; only an http(s) one gets linked. %> + <% gallery_url = external_http_url(event.vote_event_gallery_url) %> + <% admin_url = external_http_url(event.vote_event_admin_url) %>
<%= event.name %>
@@ -62,8 +65,8 @@ <% if event.vote_event_linked? %>
Linked - <% if event.vote_event_gallery_url.present? %> - <%= link_to "View gallery", event.vote_event_gallery_url, target: "_blank", rel: "noopener", + <% if gallery_url %> + <%= link_to "View gallery", gallery_url, target: "_blank", rel: "noopener", class: "text-[#ec3750] hover:underline font-medium" %> <% end %>
@@ -78,8 +81,8 @@
<% if event.vote_event_linked? %> - <% if event.vote_event_admin_url.present? %> - <%= link_to "Manage", event.vote_event_admin_url, target: "_blank", rel: "noopener", + <% if admin_url %> + <%= link_to "Manage", admin_url, target: "_blank", rel: "noopener", class: "text-[#ec3750] hover:text-[#d42f46]" %> <% end %> <% else %> diff --git a/spec/requests/admin/series_integrations_spec.rb b/spec/requests/admin/series_integrations_spec.rb index 08d6117..88f39d0 100644 --- a/spec/requests/admin/series_integrations_spec.rb +++ b/spec/requests/admin/series_integrations_spec.rb @@ -64,6 +64,25 @@ expect(response.body).to include("Linked") expect(response.body).not_to include("Create gallery") end + + # The URLs come back from vote.hackclub.com and we store what it sends, so + # a `javascript:` one must never end up in an href. + it "refuses to link a stored URL that is not http(s)" do + sub_event.link_vote_event!( + "id" => "vote-evt-1", + "slug" => sub_event.slug, + "adminUrl" => "javascript:alert(document.cookie)", + "galleryUrl" => "javascript:alert(1)" + ) + sign_in owner + + get admin_series_integrations_path(series) + + expect(response.body).to include("Linked") + expect(response.body).not_to include("javascript:") + expect(response.body).not_to include("View gallery") + expect(response.body).not_to include(">Manage<") + end end it "keeps a series organizer out — creating a gallery publishes the event elsewhere" do