[Security] Fix events being editable by other users that organizers

Closes #385

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
This commit is contained in:
Thomas Citharel 2020-10-09 18:12:35 +02:00
parent 24238da393
commit c296381ed6
No known key found for this signature in database
GPG Key ID: A061B9DDE0CA0773
4 changed files with 94 additions and 13 deletions

View File

@ -275,7 +275,7 @@ export class EventModel implements IEvent {
this.title = hash.title; this.title = hash.title;
this.slug = hash.slug; this.slug = hash.slug;
this.description = hash.description; this.description = hash.description || "";
this.beginsOn = new Date(hash.beginsOn); this.beginsOn = new Date(hash.beginsOn);
if (hash.endsOn) this.endsOn = new Date(hash.endsOn); if (hash.endsOn) this.endsOn = new Date(hash.endsOn);

View File

@ -1,6 +1,6 @@
<template> <template>
<section> <section>
<div class="container"> <div class="container" v-if="isCurrentActorOrganizer">
<h1 class="title" v-if="isUpdate === true"> <h1 class="title" v-if="isUpdate === true">
{{ $t("Update event {name}", { name: event.title }) }} {{ $t("Update event {name}", { name: event.title }) }}
</h1> </h1>
@ -255,6 +255,7 @@
aria-label="main navigation" aria-label="main navigation"
class="navbar" class="navbar"
:class="{ 'is-fixed-bottom': showFixedNavbar }" :class="{ 'is-fixed-bottom': showFixedNavbar }"
v-if="isCurrentActorOrganizer"
> >
<div class="container"> <div class="container">
<div class="navbar-menu"> <div class="navbar-menu">
@ -511,6 +512,8 @@ export default class EditEvent extends Vue {
this.limitedPlaces = this.event.options.maximumAttendeeCapacity > 0; this.limitedPlaces = this.event.options.maximumAttendeeCapacity > 0;
if (!(this.isUpdate || this.isDuplicate)) { if (!(this.isUpdate || this.isDuplicate)) {
this.initializeEvent(); 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") @Watch("event")
setInitialData(): void { setInitialData(): void {
if (this.isUpdate && this.unmodifiedEvent === undefined && this.event && this.event.uuid) { 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 { get updateEventMessage(): string {
if (this.unmodifiedEvent.draft && !this.event.draft) if (this.unmodifiedEvent.draft && !this.event.draft)
return this.$i18n.t("The event has been updated and published") as string; 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 * Build variables for Event GraphQL creation query
*/ */
private async buildVariables() { private async buildVariables() {
this.event.organizerActor =
this.event.organizerActor && this.event.organizerActor.id
? this.event.organizerActor
: this.currentActor;
let res = this.event.toEditJSON(); let res = this.event.toEditJSON();
if (this.event.organizerActor) { if (this.event.organizerActor) {
res = Object.assign(res, { res = Object.assign(res, {

View File

@ -224,9 +224,11 @@ defmodule Mobilizon.GraphQL.Resolvers.Event do
# See https://github.com/absinthe-graphql/absinthe/issues/490 # See https://github.com/absinthe-graphql/absinthe/issues/490
with args <- Map.put(args, :options, args[:options] || %{}), with args <- Map.put(args, :options, args[:options] || %{}),
{:ok, %Event{} = event} <- Events.get_event_with_preload(event_id), {:ok, %Event{} = event} <- Events.get_event_with_preload(event_id),
organizer_actor_id <- args |> Map.get(:organizer_actor_id, event.organizer_actor_id), {:old_actor, {:is_owned, %Actor{}}} <-
{:is_owned, %Actor{} = organizer_actor} <- {:old_actor, User.owns_actor(user, event.organizer_actor_id)},
User.owns_actor(user, 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), args <- Map.put(args, :organizer_actor, organizer_actor),
{:ok, %Activity{data: %{"object" => %{"type" => "Event"}}}, %Event{} = event} <- {:ok, %Activity{data: %{"object" => %{"type" => "Event"}}}, %Event{} = event} <-
API.Events.update_event(args, event) do API.Events.update_event(args, event) do
@ -235,8 +237,11 @@ defmodule Mobilizon.GraphQL.Resolvers.Event do
{:error, :event_not_found} -> {:error, :event_not_found} ->
{:error, dgettext("errors", "Event not found")} {:error, dgettext("errors", "Event not found")}
{:is_owned, nil} -> {:old_actor, _} ->
{:error, dgettext("errors", "User doesn't own profile")} {: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, _, %Ecto.Changeset{} = error, _} ->
{:error, error} {:error, error}

View File

@ -666,6 +666,40 @@ defmodule Mobilizon.Web.Resolvers.EventTest do
assert hd(json_response(res, 200)["errors"])["message"] == "Event not found" assert hd(json_response(res, 200)["errors"])["message"] == "Event not found"
end 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", %{ test "update_event/3 should check the user is the organizer", %{
conn: conn, conn: conn,
actor: _actor, actor: _actor,
@ -694,7 +728,39 @@ defmodule Mobilizon.Web.Resolvers.EventTest do
|> auth_conn(user) |> auth_conn(user)
|> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) |> 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 end
test "update_event/3 should check end time is after the beginning time", %{ test "update_event/3 should check end time is after the beginning time", %{