From 80abfa6ef6238a7d3a06ac92903a6055d25e5d5f Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Wed, 14 Apr 2021 15:04:25 +0300 Subject: [PATCH] Add validations --- lib/plausible/auth/api_key.ex | 3 +- lib/plausible/sites.ex | 18 ++++---- .../api/external_sites_controller.ex | 44 +++++++++++++------ lib/plausible_web/router.ex | 2 +- .../api/external_sites_controller_test.exs | 30 ++++++++----- 5 files changed, 63 insertions(+), 34 deletions(-) diff --git a/lib/plausible/auth/api_key.ex b/lib/plausible/auth/api_key.ex index c07f1ee24..61cbf66f2 100644 --- a/lib/plausible/auth/api_key.ex +++ b/lib/plausible/auth/api_key.ex @@ -3,6 +3,7 @@ defmodule Plausible.Auth.ApiKey do import Ecto.Changeset @required [:user_id, :key, :name] + @optional [:scopes] schema "api_keys" do field :name, :string field :scopes, {:array, :string}, default: ["stats:read:*"] @@ -18,7 +19,7 @@ defmodule Plausible.Auth.ApiKey do def changeset(schema, attrs \\ %{}) do schema - |> cast(attrs, @required) + |> cast(attrs, @required ++ @optional) |> validate_required(@required) |> process_key end diff --git a/lib/plausible/sites.ex b/lib/plausible/sites.ex index 9bc4ca47a..4031388bc 100644 --- a/lib/plausible/sites.ex +++ b/lib/plausible/sites.ex @@ -38,14 +38,16 @@ defmodule Plausible.Sites do base <> domain <> "?auth=" <> link.slug end - def get_for_user!(user_id, domain) do - Repo.one!( - from s in Plausible.Site, - join: sm in Plausible.Site.Membership, - on: sm.site_id == s.id, - where: sm.user_id == ^user_id, - where: s.domain == ^domain, - select: s + def get_for_user!(user_id, domain), do: Repo.one!(get_for_user_q(user_id, domain)) + def get_for_user(user_id, domain), do: Repo.one(get_for_user_q(user_id, domain)) + + def get_for_user_q(user_id, domain) do + from(s in Plausible.Site, + join: sm in Plausible.Site.Membership, + on: sm.site_id == s.id, + where: sm.user_id == ^user_id, + where: s.domain == ^domain, + select: s ) end diff --git a/lib/plausible_web/controllers/api/external_sites_controller.ex b/lib/plausible_web/controllers/api/external_sites_controller.ex index 8111bfa43..30e3e3829 100644 --- a/lib/plausible_web/controllers/api/external_sites_controller.ex +++ b/lib/plausible_web/controllers/api/external_sites_controller.ex @@ -6,9 +6,8 @@ defmodule PlausibleWeb.Api.ExternalSitesController do def create_site(conn, params) do user_id = conn.assigns[:current_user_id] - site_params = Map.get(params, "site", %{}) - case Sites.create(user_id, site_params) do + case Sites.create(user_id, params) do {:ok, %{site: site}} -> json(conn, site) @@ -19,22 +18,39 @@ defmodule PlausibleWeb.Api.ExternalSitesController do end end - def find_or_create_shared_link(conn, %{"domain" => domain, "link_name" => link_name}) do - site = Sites.get_for_user!(conn.assigns[:current_user_id], domain) - shared_link = Repo.get_by(Plausible.Site.SharedLink, site_id: site.id, name: link_name) + def find_or_create_shared_link(conn, %{"link_name" => link_name} = params) do + with {:ok, site_id} <- Map.fetch(params, "site_id"), + site when not is_nil(site) <- Sites.get_for_user(conn.assigns[:current_user_id], site_id) do + shared_link = Repo.get_by(Plausible.Site.SharedLink, site_id: site.id, name: link_name) + + shared_link = + case shared_link do + nil -> Sites.create_shared_link(site, link_name) + link -> {:ok, link} + end - shared_link = case shared_link do - nil -> Sites.create_shared_link(site, link_name) - link -> {:ok, link} + {:ok, link} -> + json(conn, %{ + name: link.name, + url: Sites.shared_link_url(site, link) + }) end + else + nil -> + conn + |> put_status(404) + |> json(%{error: "Site could not be found"}) - case shared_link do - {:ok, link} -> - json(conn, %{ - name: link.name, - url: Sites.shared_link_url(site, link) - }) + :error -> + conn + |> put_status(400) + |> json(%{error: "Query parameter `site_id` is required to create a shared link"}) + + e -> + conn + |> put_status(400) + |> json(%{error: "Something went wrong: #{inspect(e)}"}) end end diff --git a/lib/plausible_web/router.ex b/lib/plausible_web/router.ex index 3ba4ea486..1a325cb7b 100644 --- a/lib/plausible_web/router.ex +++ b/lib/plausible_web/router.ex @@ -84,7 +84,7 @@ defmodule PlausibleWeb.Router do pipe_through [:public_api, PlausibleWeb.AuthorizeSitesApiPlug] post "/", ExternalSitesController, :create_site - put "/:domain/shared-links/:link_name", ExternalSitesController, :find_or_create_shared_link + put "/shared-links/:link_name", ExternalSitesController, :find_or_create_shared_link end scope "/api", PlausibleWeb do diff --git a/test/plausible_web/controllers/api/external_sites_controller_test.exs b/test/plausible_web/controllers/api/external_sites_controller_test.exs index f5cdabf97..fb02170b5 100644 --- a/test/plausible_web/controllers/api/external_sites_controller_test.exs +++ b/test/plausible_web/controllers/api/external_sites_controller_test.exs @@ -13,10 +13,8 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do test "can create a site", %{conn: conn} do conn = post(conn, "/api/v1/sites", %{ - "site" => %{ - "domain" => "some-site.domain", - "timezone" => "Europe/Tallinn" - } + "domain" => "some-site.domain", + "timezone" => "Europe/Tallinn" }) assert json_response(conn, 200) == %{ @@ -28,9 +26,7 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do test "timezone defaults to Etc/Greenwich", %{conn: conn} do conn = post(conn, "/api/v1/sites", %{ - "site" => %{ - "domain" => "some-site.domain" - } + "domain" => "some-site.domain" }) assert json_response(conn, 200) == %{ @@ -66,7 +62,7 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do setup :create_site test "can add a shared link to a site", %{conn: conn, site: site} do - conn = put(conn, "/api/v1/sites/#{site.domain}/shared-links/Wordpress") + conn = put(conn, "/api/v1/sites/shared-links/Wordpress?site_id=#{site.domain}") res = json_response(conn, 200) assert res["name"] == "Wordpress" @@ -74,13 +70,27 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do end test "is idempotent find or create op", %{conn: conn, site: site} do - conn = put(conn, "/api/v1/sites/#{site.domain}/shared-links/Wordpress") + conn = put(conn, "/api/v1/sites/shared-links/Wordpress?site_id=#{site.domain}") %{"url" => url} = json_response(conn, 200) - conn = put(conn, "/api/v1/sites/#{site.domain}/shared-links/Wordpress") + conn = put(conn, "/api/v1/sites/shared-links/Wordpress?site_id=#{site.domain}") assert %{"url" => ^url} = json_response(conn, 200) end + + test "returns 400 when site id missing", %{conn: conn} do + conn = put(conn, "/api/v1/sites/shared-links/Wordpress") + + res = json_response(conn, 400) + assert res["error"] == "Query parameter `site_id` is required to create a shared link" + end + + test "returns 404 when site id is non existent", %{conn: conn} do + conn = put(conn, "/api/v1/sites/shared-links/Wordpress?site_id=bad") + + res = json_response(conn, 404) + assert res["error"] == "Site could not be found" + end end end