From 08cf8665fb0bd8c24587c2cb641a3c4102b1c091 Mon Sep 17 00:00:00 2001 From: Chris Duncan Date: Wed, 26 Apr 2023 17:11:52 -0500 Subject: [PATCH 1/7] Add audit log controller and endpoint --- apps/andi/lib/andi/schemas/audit_events.ex | 2 ++ .../controllers/api/audit_log_controller.ex | 33 +++++++++++++++++++ .../controllers/api/ingestion_controller.ex | 2 +- apps/andi/lib/andi_web/router.ex | 10 ++++-- 4 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex diff --git a/apps/andi/lib/andi/schemas/audit_events.ex b/apps/andi/lib/andi/schemas/audit_events.ex index 7d8689366..c826ac0f1 100644 --- a/apps/andi/lib/andi/schemas/audit_events.ex +++ b/apps/andi/lib/andi/schemas/audit_events.ex @@ -60,6 +60,8 @@ defmodule Andi.Schemas.AuditEvents do Repo.all(query) end + + #TODO figure out what this even means def get_all_by_event_id(event_id) do query = from(event in AuditEvent, diff --git a/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex b/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex new file mode 100644 index 000000000..465748720 --- /dev/null +++ b/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex @@ -0,0 +1,33 @@ +defmodule AndiWeb.API.AuditLogController do + @moduledoc """ + Module returns audit logs from postgres. + """ + use AndiWeb, :controller + alias Andi.Schemas.AuditEvents + + access_levels(get: [:private]) + + @spec get(Plug.Conn.t(), any()) :: Plug.Conn.t() + def get(conn, params) do + audit_events = + case Map.keys(params) do + ["user_id"] -> AuditEvents.get_all_for_user(params["user_id"]) + ["audit_id"] -> AuditEvents.get(params["audit_id"]) + ["type"] -> AuditEvents.get_all_of_type(params["type"]) + ["event_id"] -> AuditEvents.get_all_by_event_id(params["event_id"]) + [] -> AuditEvents.get_all() + end + + response = + for event <- audit_events do + trimmed_event = + Map.from_struct(event) + |> Map.delete(:__meta__) + |> Kernel.inspect() + + "#{trimmed_event}\n" + end + + text(conn, response) + end +end diff --git a/apps/andi/lib/andi_web/controllers/api/ingestion_controller.ex b/apps/andi/lib/andi_web/controllers/api/ingestion_controller.ex index 7cfc54412..3e719d540 100644 --- a/apps/andi/lib/andi_web/controllers/api/ingestion_controller.ex +++ b/apps/andi/lib/andi_web/controllers/api/ingestion_controller.ex @@ -71,7 +71,7 @@ defmodule AndiWeb.API.IngestionController do defp dataset_exists?(id) do case DatasetStore.get(id) do {:ok, nil} -> {:ok, false} - {:ok, dataset} -> {:ok, true} + {:ok, _dataset} -> {:ok, true} {:error, error} -> {:error, error} end end diff --git a/apps/andi/lib/andi_web/router.ex b/apps/andi/lib/andi_web/router.ex index 8e43ad72b..4ef55c415 100644 --- a/apps/andi/lib/andi_web/router.ex +++ b/apps/andi/lib/andi_web/router.ex @@ -83,17 +83,21 @@ defmodule AndiWeb.Router do get "/v1/datasets", DatasetController, :get_all get "/v1/dataset/:dataset_id", DatasetController, :get + put "/v1/dataset", DatasetController, :create + post "/v1/dataset/disable", DatasetController, :disable + post "/v1/dataset/delete", DatasetController, :delete + get "/v1/ingestions", IngestionController, :get_all get "/v1/ingestion/:ingestion_id", IngestionController, :get - put "/v1/dataset", DatasetController, :create put "/v1/ingestion", IngestionController, :create post "/v1/ingestion/publish", IngestionController, :publish - post "/v1/dataset/disable", DatasetController, :disable - post "/v1/dataset/delete", DatasetController, :delete post "/v1/ingestion/delete", IngestionController, :delete + get "/v1/organizations", OrganizationController, :get_all post "/v1/organization/:org_id/users/add", OrganizationController, :add_users_to_organization post "/v1/organization", OrganizationController, :create + + get "/v1/audit", AuditLogController, :get end scope "/auth", AndiWeb do From 8a8feb775a08e8bb9d968825a2192eda24c52d22 Mon Sep 17 00:00:00 2001 From: Chris Duncan Date: Wed, 26 Apr 2023 17:13:12 -0500 Subject: [PATCH 2/7] actually commit unit tests --- apps/andi/lib/andi/schemas/audit_events.ex | 2 - .../api/audit_log_controller_test.exs | 92 +++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs diff --git a/apps/andi/lib/andi/schemas/audit_events.ex b/apps/andi/lib/andi/schemas/audit_events.ex index c826ac0f1..7d8689366 100644 --- a/apps/andi/lib/andi/schemas/audit_events.ex +++ b/apps/andi/lib/andi/schemas/audit_events.ex @@ -60,8 +60,6 @@ defmodule Andi.Schemas.AuditEvents do Repo.all(query) end - - #TODO figure out what this even means def get_all_by_event_id(event_id) do query = from(event in AuditEvent, diff --git a/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs b/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs new file mode 100644 index 000000000..986aa1e49 --- /dev/null +++ b/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs @@ -0,0 +1,92 @@ +defmodule AndiWeb.API.AuditLogControllerTest do + use AndiWeb.Test.AuthConnCase.UnitCase + use Placebo + + alias Andi.Schemas.AuditEvents + alias Andi.Schemas.AuditEvent + @route "/api/v1/audit" + + setup %{} do + logs = [ + struct(AuditEvent, %{ + id: "id", + user_id: "user_id", + event_type: "event_type", + event: %{ + id: "event id", + list: [ + deeply_nested_data: %{ + deep_data: "deep" + }, + list_data: "list" + ] + } + }), + struct(AuditEvent, %{ + id: "id", + user_id: "user_id", + event_type: "event_type", + event: %{ + data: "some data", + other_data: "some other_data" + } + }) + ] + + logs_as_text = """ + %{event: %{id: \"event id\", list: [deeply_nested_data: %{deep_data: \"deep\"}, list_data: \"list\"]}, event_type: \"event_type\", id: \"id\", inserted_at: nil, updated_at: nil, user_id: \"user_id\"} + %{event: %{data: \"some data\", other_data: \"some other_data\"}, event_type: \"event_type\", id: \"id\", inserted_at: nil, updated_at: nil, user_id: \"user_id\"} + """ + + [logs: logs, logs_as_text: logs_as_text] + end + + test "get all audit logs", %{conn: conn, logs: logs, logs_as_text: logs_as_text} do + allow(AuditEvents.get_all(), return: logs) + conn = get(conn, "#{@route}") + + assert_called(AuditEvents.get_all()) + + assert response(conn, 200) =~ logs_as_text + end + + test "get by audit id", %{conn: conn, logs: logs, logs_as_text: logs_as_text} do + audit_id = "12345" + allow(AuditEvents.get(audit_id), return: logs) + conn = get(conn, "#{@route}?audit_id=#{audit_id}") + + assert_called(AuditEvents.get(audit_id)) + + assert response(conn, 200) =~ logs_as_text + end + + test "get by user id", %{conn: conn, logs: logs, logs_as_text: logs_as_text} do + user_id = "user_id" + allow(AuditEvents.get_all_for_user(user_id), return: logs) + conn = get(conn, "#{@route}?user_id=#{user_id}") + + assert_called(AuditEvents.get_all_for_user(user_id)) + + assert response(conn, 200) =~ logs_as_text + end + + test "get by type", %{conn: conn, logs: logs, logs_as_text: logs_as_text} do + type = "some_type" + allow(AuditEvents.get_all_of_type(type), return: logs) + conn = get(conn, "#{@route}?type=#{type}") + + assert_called(AuditEvents.get_all_of_type(type)) + + assert response(conn, 200) =~ logs_as_text + end + + test "get by event id", %{conn: conn, logs: logs, logs_as_text: logs_as_text} do + event_id = "54321" + allow(AuditEvents.get_all_by_event_id(event_id), return: logs) + conn = get(conn, "#{@route}?event_id=#{event_id}") + + assert_called(AuditEvents.get_all_by_event_id(event_id)) + + assert response(conn, 200) =~ logs_as_text + end +end From 376b089051daa11b5fa62229f36214de2b7d4f84 Mon Sep 17 00:00:00 2001 From: Chris Duncan Date: Thu, 27 Apr 2023 09:59:29 -0500 Subject: [PATCH 3/7] Add error handling to audit log controller --- .../controllers/api/audit_log_controller.ex | 50 ++++++++++++------- .../api/audit_log_controller_test.exs | 14 ++++++ 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex b/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex index 465748720..6cac16746 100644 --- a/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex +++ b/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex @@ -9,25 +9,41 @@ defmodule AndiWeb.API.AuditLogController do @spec get(Plug.Conn.t(), any()) :: Plug.Conn.t() def get(conn, params) do - audit_events = - case Map.keys(params) do - ["user_id"] -> AuditEvents.get_all_for_user(params["user_id"]) - ["audit_id"] -> AuditEvents.get(params["audit_id"]) - ["type"] -> AuditEvents.get_all_of_type(params["type"]) - ["event_id"] -> AuditEvents.get_all_by_event_id(params["event_id"]) - [] -> AuditEvents.get_all() - end + with {:ok, audit_events} <- get_audit_events(params) do + text(conn, format_events(audit_events)) + else + _ -> respond_error(conn) + end + end + + defp get_audit_events(params) do + case Map.keys(params) do + ["user_id"] -> {:ok, AuditEvents.get_all_for_user(params["user_id"])} + ["audit_id"] -> {:ok, AuditEvents.get(params["audit_id"])} + ["type"] -> {:ok, AuditEvents.get_all_of_type(params["type"])} + ["event_id"] -> {:ok, AuditEvents.get_all_by_event_id(params["event_id"])} + [] -> {:ok, AuditEvents.get_all()} + _ -> {:error} + end + end - response = - for event <- audit_events do - trimmed_event = - Map.from_struct(event) - |> Map.delete(:__meta__) - |> Kernel.inspect() + defp format_events(audit_events) do + for event <- audit_events do + trimmed_event = + Map.from_struct(event) + |> Map.delete(:__meta__) + |> Kernel.inspect() - "#{trimmed_event}\n" - end + "#{trimmed_event}\n" + end + end - text(conn, response) + defp respond_error(conn) do + conn + |> put_status(:bad_request) + |> text( + "Unsupported request. Only one filter can be used at a time - 'user_id', 'audit_id', 'type', 'event_id.'" <> + "For time, exactly 'start' and 'end' must be used." + ) end end diff --git a/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs b/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs index 986aa1e49..d76deed36 100644 --- a/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs +++ b/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs @@ -5,6 +5,8 @@ defmodule AndiWeb.API.AuditLogControllerTest do alias Andi.Schemas.AuditEvents alias Andi.Schemas.AuditEvent @route "/api/v1/audit" + @error_text "Unsupported request. Only one filter can be used at a time - 'user_id', 'audit_id', 'type', 'event_id.'" <> + "For time, exactly 'start' and 'end' must be used." setup %{} do logs = [ @@ -89,4 +91,16 @@ defmodule AndiWeb.API.AuditLogControllerTest do assert response(conn, 200) =~ logs_as_text end + + test "returns error when given multiple arguments", %{conn: conn, logs: logs} do + conn = get(conn, "#{@route}?event_id=event&type=type") + + assert response(conn, 400) =~ @error_text + end + + test "returns error when given unsupported argument", %{conn: conn, logs: logs} do + conn = get(conn, "#{@route}?foobar=foo") + + assert response(conn, 400) =~ @error_text + end end From a86774e6b2a0fa5f0bfcef44e2561a91cd451ed3 Mon Sep 17 00:00:00 2001 From: Chris Duncan Date: Thu, 27 Apr 2023 13:42:50 -0500 Subject: [PATCH 4/7] add time support --- .../controllers/api/audit_log_controller.ex | 25 +++++++++++++------ .../andi_web/access_level_test.exs | 7 ++++++ .../api/audit_log_controller_test.exs | 17 ++++++++++--- .../api/dataset_controller_test.exs | 2 +- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex b/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex index 6cac16746..659160627 100644 --- a/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex +++ b/apps/andi/lib/andi_web/controllers/api/audit_log_controller.ex @@ -7,12 +7,15 @@ defmodule AndiWeb.API.AuditLogController do access_levels(get: [:private]) + @generic_error_text "Unsupported request. Only one filter can be used at a time - 'user_id', 'audit_id', 'type', 'event_id.'" <> + "For time, exactly 'start_date' and 'end_date' must be used and formatted in ISO-8601. ex. /start_date=2020-12-31&end-date=2021-01-01" + @spec get(Plug.Conn.t(), any()) :: Plug.Conn.t() def get(conn, params) do with {:ok, audit_events} <- get_audit_events(params) do text(conn, format_events(audit_events)) else - _ -> respond_error(conn) + {:error, error} -> respond_error(conn, error) end end @@ -22,8 +25,19 @@ defmodule AndiWeb.API.AuditLogController do ["audit_id"] -> {:ok, AuditEvents.get(params["audit_id"])} ["type"] -> {:ok, AuditEvents.get_all_of_type(params["type"])} ["event_id"] -> {:ok, AuditEvents.get_all_by_event_id(params["event_id"])} + ["start_date", "end_date"] -> get_range(params["start_date"], params["end_date"]) + ["end_date", "start_date"] -> get_range(params["start_date"], params["end_date"]) [] -> {:ok, AuditEvents.get_all()} - _ -> {:error} + _ -> {:error, @generic_error_text} + end + end + + defp get_range(start_date, end_date) do + with {:ok, start_struct} <- Date.from_iso8601(start_date), + {:ok, end_struct} <- Date.from_iso8601(end_date) do + {:ok, AuditEvents.get_all_in_range(start_struct, end_struct)} + else + {:error, _error} -> {:error, "Inproperly formatted dates. Use ISO-8601, ex. 2021-01-01"} end end @@ -38,12 +52,9 @@ defmodule AndiWeb.API.AuditLogController do end end - defp respond_error(conn) do + defp respond_error(conn, error) do conn |> put_status(:bad_request) - |> text( - "Unsupported request. Only one filter can be used at a time - 'user_id', 'audit_id', 'type', 'event_id.'" <> - "For time, exactly 'start' and 'end' must be used." - ) + |> text(error) end end diff --git a/apps/andi/test/integration/andi_web/access_level_test.exs b/apps/andi/test/integration/andi_web/access_level_test.exs index 5dea60927..78315301b 100644 --- a/apps/andi/test/integration/andi_web/access_level_test.exs +++ b/apps/andi/test/integration/andi_web/access_level_test.exs @@ -103,6 +103,13 @@ defmodule AndiWeb.AccessLevelTest do end end + describe "audit logs" do + test "does not allow access to audit logs without curator role", %{curator_conn: conn} do + assert get(conn, "/audit") + |> response(404) + end + end + defp andi_children() do Supervisor.which_children(Andi.Supervisor) |> Enum.map(&elem(&1, 0)) diff --git a/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs b/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs index d76deed36..30cb9acda 100644 --- a/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs +++ b/apps/andi/test/unit/andi_web/controllers/api/audit_log_controller_test.exs @@ -6,7 +6,7 @@ defmodule AndiWeb.API.AuditLogControllerTest do alias Andi.Schemas.AuditEvent @route "/api/v1/audit" @error_text "Unsupported request. Only one filter can be used at a time - 'user_id', 'audit_id', 'type', 'event_id.'" <> - "For time, exactly 'start' and 'end' must be used." + "For time, exactly 'start_date' and 'end_date' must be used and formatted in ISO-8601. ex. /start_date=2020-12-31&end-date=2021-01-01" setup %{} do logs = [ @@ -92,13 +92,24 @@ defmodule AndiWeb.API.AuditLogControllerTest do assert response(conn, 200) =~ logs_as_text end - test "returns error when given multiple arguments", %{conn: conn, logs: logs} do + test "filter by dates", %{conn: conn, logs: logs, logs_as_text: logs_as_text} do + {:ok, start_date_struct} = Date.new(2020, 6, 5) + {:ok, end_date_struct} = Date.new(2020, 10, 9) + allow(AuditEvents.get_all_in_range(start_date_struct, end_date_struct), return: logs) + + conn = get(conn, "#{@route}?start_date=2020-06-05&end_date=2020-10-09") + + assert_called(AuditEvents.get_all_in_range(start_date_struct, end_date_struct)) + assert response(conn, 200) =~ logs_as_text + end + + test "returns error when given multiple arguments", %{conn: conn} do conn = get(conn, "#{@route}?event_id=event&type=type") assert response(conn, 400) =~ @error_text end - test "returns error when given unsupported argument", %{conn: conn, logs: logs} do + test "returns error when given unsupported argument", %{conn: conn} do conn = get(conn, "#{@route}?foobar=foo") assert response(conn, 400) =~ @error_text diff --git a/apps/andi/test/unit/andi_web/controllers/api/dataset_controller_test.exs b/apps/andi/test/unit/andi_web/controllers/api/dataset_controller_test.exs index bc751be7f..9437d577d 100644 --- a/apps/andi/test/unit/andi_web/controllers/api/dataset_controller_test.exs +++ b/apps/andi/test/unit/andi_web/controllers/api/dataset_controller_test.exs @@ -225,7 +225,7 @@ defmodule AndiWeb.API.DatasetControllerTest do end @tag capture_log: true - test "PUT /api/ creating a dataset with a set id returns a 400", %{conn: conn, example_datasets: example_datasets} do + test "PUT /api/ creating a dataset with a set id returns a 400", %{conn: conn} do dataset = TDG.create_dataset(%{}) |> struct_to_map_with_string_keys() allow(Brook.Event.send(@instance_name, any(), any(), any()), return: :ok) From 5ebe3effea885a5f85e81472c093a8c5a43ac193 Mon Sep 17 00:00:00 2001 From: Chris Duncan Date: Thu, 27 Apr 2023 13:45:07 -0500 Subject: [PATCH 5/7] version bump --- apps/andi/mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/andi/mix.exs b/apps/andi/mix.exs index b840a82f3..d91e0b5d8 100644 --- a/apps/andi/mix.exs +++ b/apps/andi/mix.exs @@ -4,7 +4,7 @@ defmodule Andi.MixProject do def project do [ app: :andi, - version: "2.6.64", + version: "2.6.66", build_path: "../../_build", config_path: "../../config/config.exs", deps_path: "../../deps", From 9cbd582ab6ecde168267d6504f81a767993dfe9a Mon Sep 17 00:00:00 2001 From: Chris Duncan Date: Thu, 27 Apr 2023 13:45:33 -0500 Subject: [PATCH 6/7] Remove dead code to reduce noise in unit tests --- .../api/dataset_controller_test.exs | 55 +------------------ 1 file changed, 1 insertion(+), 54 deletions(-) diff --git a/apps/andi/test/unit/andi_web/controllers/api/dataset_controller_test.exs b/apps/andi/test/unit/andi_web/controllers/api/dataset_controller_test.exs index 9437d577d..d97e9c55d 100644 --- a/apps/andi/test/unit/andi_web/controllers/api/dataset_controller_test.exs +++ b/apps/andi/test/unit/andi_web/controllers/api/dataset_controller_test.exs @@ -39,60 +39,7 @@ defmodule AndiWeb.API.DatasetControllerTest do meck_options: [:passthrough] ) - uuid = Faker.UUID.v4() - - request = %{ - "id" => uuid, - "technical" => %{ - "dataName" => "dataset", - "orgId" => "org-123-456", - "orgName" => "org", - "stream" => false, - "sourceUrl" => "https://example.com", - "sourceType" => "stream", - "sourceFormat" => "gtfs", - "cadence" => "9000", - "schema" => [%{name: "billy", type: "writer"}], - "private" => false, - "headers" => %{ - "accepts" => "application/foobar" - }, - "sourceQueryParams" => %{ - "apiKey" => "foobar" - }, - "systemName" => "org__dataset", - "transformations" => [], - "validations" => [] - }, - "business" => %{ - "benefitRating" => 0.5, - "dataTitle" => "dataset title", - "description" => "description", - "modifiedDate" => "", - "orgTitle" => "org title", - "contactName" => "contact name", - "contactEmail" => "contact@email.com", - "license" => "https://www.test.net", - "rights" => "rights information", - "homepage" => "", - "keywords" => [], - "issuedDate" => "2020-01-01T00:00:00Z", - "publishFrequency" => "all day, ey'r day", - "riskRating" => 1.0 - }, - "_metadata" => %{ - "intendedUse" => [], - "expectedBenefit" => [] - } - } - - message = - request - |> SmartCity.Helpers.to_atom_keys() - |> TDG.create_dataset() - |> struct_to_map_with_string_keys() - - {:ok, request: request, message: message, example_datasets: example_datasets} + {:ok, example_datasets: example_datasets} end describe "POST /dataset/disable" do From 7cb047a23c3e4058f118f1a785e1398510b218f9 Mon Sep 17 00:00:00 2001 From: Chris Duncan Date: Thu, 27 Apr 2023 13:59:22 -0500 Subject: [PATCH 7/7] Add integration test around curator role --- .../controllers/audit_log_controller_test.exs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 apps/andi/test/integration/andi_web/controllers/audit_log_controller_test.exs diff --git a/apps/andi/test/integration/andi_web/controllers/audit_log_controller_test.exs b/apps/andi/test/integration/andi_web/controllers/audit_log_controller_test.exs new file mode 100644 index 000000000..61e14d2f5 --- /dev/null +++ b/apps/andi/test/integration/andi_web/controllers/audit_log_controller_test.exs @@ -0,0 +1,30 @@ +defmodule AndiWeb.EditControllerTest do + use ExUnit.Case + use Andi.DataCase + use AndiWeb.Test.AuthConnCase.IntegrationCase + + @moduletag shared_data_connection: true + + alias Andi.Schemas.AuditEvents + alias SmartCity.TestDataGenerator, as: TDG + + @url_path "/api/v1/audit" + + describe "Audit Log controller" do + setup do + AuditEvents.log_audit_event(:api, :event_type, %{data: "data"}) + end + + test "Returns 200 when for users with curator role", %{curator_conn: curator_conn, andi_dataset: andi_dataset} do + conn = get(curator_conn, "#{@url_path}/") + + assert response(conn, 200) + assert redirected_to(conn) =~ "data" + end + + test "displays error for users without a curator role", %{public_conn: public_conn, andi_dataset: andi_dataset} do + conn = get(public_conn, "#{@url_path}/") + assert response(conn, 403) + end + end +end