From c296381ed632f43042e617d5123e00229cb38f58 Mon Sep 17 00:00:00 2001
From: Thomas Citharel <tcit@tcit.fr>
Date: Fri, 9 Oct 2020 18:12:35 +0200
Subject: [PATCH] [Security] Fix events being editable by other users that
organizers
Closes #385
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
---
js/src/types/event.model.ts | 2 +-
js/src/views/Event/Edit.vue | 22 ++++++---
lib/graphql/resolvers/event.ex | 15 ++++--
test/graphql/resolvers/event_test.exs | 68 ++++++++++++++++++++++++++-
4 files changed, 94 insertions(+), 13 deletions(-)
diff --git a/js/src/types/event.model.ts b/js/src/types/event.model.ts
index d357c5d3a..2d34844c5 100644
--- a/js/src/types/event.model.ts
+++ b/js/src/types/event.model.ts
@@ -275,7 +275,7 @@ export class EventModel implements IEvent {
this.title = hash.title;
this.slug = hash.slug;
- this.description = hash.description;
+ this.description = hash.description || "";
this.beginsOn = new Date(hash.beginsOn);
if (hash.endsOn) this.endsOn = new Date(hash.endsOn);
diff --git a/js/src/views/Event/Edit.vue b/js/src/views/Event/Edit.vue
index 2ad2d6f80..06e38968a 100644
--- a/js/src/views/Event/Edit.vue
+++ b/js/src/views/Event/Edit.vue
@@ -1,6 +1,6 @@
<template>
<section>
- <div class="container">
+ <div class="container" v-if="isCurrentActorOrganizer">
<h1 class="title" v-if="isUpdate === true">
{{ $t("Update event {name}", { name: event.title }) }}
</h1>
@@ -255,6 +255,7 @@
aria-label="main navigation"
class="navbar"
:class="{ 'is-fixed-bottom': showFixedNavbar }"
+ v-if="isCurrentActorOrganizer"
>
<div class="container">
<div class="navbar-menu">
@@ -511,6 +512,8 @@ export default class EditEvent extends Vue {
this.limitedPlaces = this.event.options.maximumAttendeeCapacity > 0;
if (!(this.isUpdate || this.isDuplicate)) {
this.initializeEvent();
+ } else {
+ this.event.description = this.event.description || "";
}
}
@@ -533,11 +536,6 @@ export default class EditEvent extends Vue {
}
}
- @Watch("currentActor")
- setCurrentActor(): void {
- this.event.organizerActor = this.currentActor;
- }
-
@Watch("event")
setInitialData(): void {
if (this.isUpdate && this.unmodifiedEvent === undefined && this.event && this.event.uuid) {
@@ -620,6 +618,14 @@ export default class EditEvent extends Vue {
}
}
+ get isCurrentActorOrganizer(): boolean {
+ return !(
+ this.eventId &&
+ this.event.organizerActor &&
+ this.currentActor.id !== this.event.organizerActor.id
+ ) as boolean;
+ }
+
get updateEventMessage(): string {
if (this.unmodifiedEvent.draft && !this.event.draft)
return this.$i18n.t("The event has been updated and published") as string;
@@ -720,6 +726,10 @@ export default class EditEvent extends Vue {
* Build variables for Event GraphQL creation query
*/
private async buildVariables() {
+ this.event.organizerActor =
+ this.event.organizerActor && this.event.organizerActor.id
+ ? this.event.organizerActor
+ : this.currentActor;
let res = this.event.toEditJSON();
if (this.event.organizerActor) {
res = Object.assign(res, {
diff --git a/lib/graphql/resolvers/event.ex b/lib/graphql/resolvers/event.ex
index 16e5c3479..3be1cfdf5 100644
--- a/lib/graphql/resolvers/event.ex
+++ b/lib/graphql/resolvers/event.ex
@@ -224,9 +224,11 @@ defmodule Mobilizon.GraphQL.Resolvers.Event do
# See https://github.com/absinthe-graphql/absinthe/issues/490
with args <- Map.put(args, :options, args[:options] || %{}),
{:ok, %Event{} = event} <- Events.get_event_with_preload(event_id),
- organizer_actor_id <- args |> Map.get(:organizer_actor_id, event.organizer_actor_id),
- {:is_owned, %Actor{} = organizer_actor} <-
- User.owns_actor(user, organizer_actor_id),
+ {:old_actor, {:is_owned, %Actor{}}} <-
+ {:old_actor, User.owns_actor(user, event.organizer_actor_id)},
+ new_organizer_actor_id <- args |> Map.get(:organizer_actor_id, event.organizer_actor_id),
+ {:new_actor, {:is_owned, %Actor{} = organizer_actor}} <-
+ {:new_actor, User.owns_actor(user, new_organizer_actor_id)},
args <- Map.put(args, :organizer_actor, organizer_actor),
{:ok, %Activity{data: %{"object" => %{"type" => "Event"}}}, %Event{} = event} <-
API.Events.update_event(args, event) do
@@ -235,8 +237,11 @@ defmodule Mobilizon.GraphQL.Resolvers.Event do
{:error, :event_not_found} ->
{:error, dgettext("errors", "Event not found")}
- {:is_owned, nil} ->
- {:error, dgettext("errors", "User doesn't own profile")}
+ {:old_actor, _} ->
+ {:error, dgettext("errors", "You can't edit this event.")}
+
+ {:new_actor, _} ->
+ {:error, dgettext("errors", "You can't attribute this new event to this profile.")}
{:error, _, %Ecto.Changeset{} = error, _} ->
{:error, error}
diff --git a/test/graphql/resolvers/event_test.exs b/test/graphql/resolvers/event_test.exs
index 6f75e70d5..3cd7bf25b 100644
--- a/test/graphql/resolvers/event_test.exs
+++ b/test/graphql/resolvers/event_test.exs
@@ -666,6 +666,40 @@ defmodule Mobilizon.Web.Resolvers.EventTest do
assert hd(json_response(res, 200)["errors"])["message"] == "Event not found"
end
+ test "update_event/3 should check the user can change the organizer", %{
+ conn: conn,
+ actor: actor,
+ user: user
+ } do
+ event = insert(:event, organizer_actor: actor)
+ actor2 = insert(:actor)
+
+ mutation = """
+ mutation {
+ updateEvent(
+ title: "my event updated",
+ event_id: #{event.id}
+ organizer_actor_id: #{actor2.id}
+ ) {
+ title,
+ uuid,
+ tags {
+ title,
+ slug
+ }
+ }
+ }
+ """
+
+ res =
+ conn
+ |> auth_conn(user)
+ |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
+
+ assert hd(json_response(res, 200)["errors"])["message"] ==
+ "You can't attribute this new event to this profile."
+ end
+
test "update_event/3 should check the user is the organizer", %{
conn: conn,
actor: _actor,
@@ -694,7 +728,39 @@ defmodule Mobilizon.Web.Resolvers.EventTest do
|> auth_conn(user)
|> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
- assert hd(json_response(res, 200)["errors"])["message"] == "User doesn't own profile"
+ assert hd(json_response(res, 200)["errors"])["message"] == "You can't edit this event."
+ end
+
+ test "update_event/3 should check the user is the organizer also when it's changed", %{
+ conn: conn,
+ actor: actor,
+ user: user
+ } do
+ event = insert(:event)
+
+ mutation = """
+ mutation {
+ updateEvent(
+ title: "my event updated",
+ event_id: #{event.id},
+ organizer_actor_id: #{actor.id}
+ ) {
+ title,
+ uuid,
+ tags {
+ title,
+ slug
+ }
+ }
+ }
+ """
+
+ res =
+ conn
+ |> auth_conn(user)
+ |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
+
+ assert hd(json_response(res, 200)["errors"])["message"] == "You can't edit this event."
end
test "update_event/3 should check end time is after the beginning time", %{