diff --git a/front/lib/front/models/notification.ex b/front/lib/front/models/notification.ex index 8e9fd33e3..9a1a9b115 100644 --- a/front/lib/front/models/notification.ex +++ b/front/lib/front/models/notification.ex @@ -58,7 +58,7 @@ defmodule Front.Models.Notification do def create(user_id, org_id, notification_data, _metadata \\ nil) do Watchman.benchmark("notifications.create_notification_request.duration", fn -> - notification = construct_notification(notification_data) + notification = construct_notification(notification_data, user_id) {:ok, channel} = GRPC.Stub.connect(api_endpoint()) @@ -90,7 +90,7 @@ defmodule Front.Models.Notification do def update(user_id, org_id, notification_data) do Watchman.benchmark("notifications.update_notification_request.duration", fn -> - notification = construct_notification(notification_data) + notification = construct_notification(notification_data, user_id) {:ok, channel} = GRPC.Stub.connect(api_endpoint()) @@ -110,13 +110,13 @@ defmodule Front.Models.Notification do end) end - def construct_notification(data) do + def construct_notification(data, creator_id) do rules = data.rules |> Enum.map(&construct_rule(&1)) PublicApi.Notification.new( - metadata: PublicApi.Notification.Metadata.new(name: data.name), + metadata: PublicApi.Notification.Metadata.new(name: data.name, creator_id: creator_id), spec: PublicApi.Notification.Spec.new(rules: rules) ) end diff --git a/front/lib/public_api/semaphore/notifications.v1alpha.pb.ex b/front/lib/public_api/semaphore/notifications.v1alpha.pb.ex index cbffbd89c..ffc705a72 100644 --- a/front/lib/public_api/semaphore/notifications.v1alpha.pb.ex +++ b/front/lib/public_api/semaphore/notifications.v1alpha.pb.ex @@ -22,14 +22,16 @@ defmodule Semaphore.Notifications.V1alpha.Notification.Metadata do name: String.t(), id: String.t(), create_time: integer, - update_time: integer + update_time: integer, + creator_id: String.t() } - defstruct [:name, :id, :create_time, :update_time] + defstruct [:name, :id, :create_time, :update_time, :creator_id] field(:name, 1, type: :string) field(:id, 2, type: :string) field(:create_time, 3, type: :int64) field(:update_time, 4, type: :int64) + field(:creator_id, 5, type: :string) end defmodule Semaphore.Notifications.V1alpha.Notification.Spec do diff --git a/notifications/config/config.exs b/notifications/config/config.exs index 2ff7a2acd..40c211fbf 100644 --- a/notifications/config/config.exs +++ b/notifications/config/config.exs @@ -13,9 +13,6 @@ config :logger, :console, import_config "_silent_lager.exs" -config :notifications, rbac_endpoint: "localhost:50052" -config :notifications, secrethub_endpoint: "localhost:50052" # sobelow_skip ["Config.Secrets"] - config :notifications, ecto_repos: [Notifications.Repo] import_config "#{config_env()}.exs" diff --git a/notifications/config/test.exs b/notifications/config/test.exs index e0eef4074..b60952979 100644 --- a/notifications/config/test.exs +++ b/notifications/config/test.exs @@ -8,4 +8,12 @@ config :junit_formatter, include_filename?: true, include_file_line?: true +config :notifications, rbac_endpoint: "localhost:50052" +config :notifications, secrethub_endpoint: "localhost:50052" # sobelow_skip ["Config.Secrets"] +config :notifications, pipeline_endpoint: "localhost:50052" # sobelow_skip ["Config.Secrets"] +config :notifications, projecthub_endpoint: "localhost:50052" # sobelow_skip ["Config.Secrets"] +config :notifications, repo_proxy_endpoint: "localhost:50052" # sobelow_skip ["Config.Secrets"] +config :notifications, workflow_endpoint: "localhost:50052" # sobelow_skip ["Config.Secrets"] +config :notifications, organization_endpoint: "localhost:50052" # sobelow_skip ["Config.Secrets"] + config :notifications, Notifications.Repo, pool: Ecto.Adapters.SQL.Sandbox diff --git a/notifications/lib/notifications/api/internal_api/create.ex b/notifications/lib/notifications/api/internal_api/create.ex index 4bd44e749..b381ed164 100644 --- a/notifications/lib/notifications/api/internal_api/create.ex +++ b/notifications/lib/notifications/api/internal_api/create.ex @@ -9,9 +9,10 @@ defmodule Notifications.Api.InternalApi.Create do def run(req) do org_id = req.metadata.org_id + creator_id = req.metadata.user_id - with {:ok, :valid} <- Validator.validate(req.notification), - {:ok, n} <- create_notification(org_id, req.notification) do + with {:ok, :valid} <- Validator.validate(req.notification, creator_id), + {:ok, n} <- create_notification(org_id, creator_id, req.notification) do %CreateResponse{notification: Serialization.serialize(n)} else {:error, :invalid_argument, message} -> @@ -27,12 +28,13 @@ defmodule Notifications.Api.InternalApi.Create do end end - defp create_notification(org_id, notification) do + defp create_notification(org_id, creator_id, notification) do Repo.transaction(fn -> n = Models.Notification.new( org_id, notification.name, + creator_id, Notifications.Util.Transforms.encode_spec(%{rules: notification.rules}) ) diff --git a/notifications/lib/notifications/api/internal_api/update.ex b/notifications/lib/notifications/api/internal_api/update.ex index 2e6db5322..1995a3714 100644 --- a/notifications/lib/notifications/api/internal_api/update.ex +++ b/notifications/lib/notifications/api/internal_api/update.ex @@ -9,10 +9,11 @@ defmodule Notifications.Api.InternalApi.Update do def run(req) do org_id = req.metadata.org_id + updater_id = req.metadata.user_id - with {:ok, :valid} <- Validator.validate(req.notification), + with {:ok, :valid} <- Validator.validate(req.notification, updater_id), {:ok, n} <- find_by_id_or_name(org_id, req.id, req.name), - {:ok, n} <- update_notification(n, req.notification) do + {:ok, n} <- update_notification(n, updater_id, req.notification) do %UpdateResponse{notification: Serialization.serialize(n)} else {:error, :invalid_argument, message} -> @@ -37,11 +38,12 @@ defmodule Notifications.Api.InternalApi.Update do # notification: existing row from database # apiresource: new notification from the API call that needs to be applied # - defp update_notification(notification, apiresource) do + defp update_notification(notification, updater_id, apiresource) do Repo.transaction(fn -> changes = Model.changeset(notification, %{ name: apiresource.name, + creator_id: updater_id, spec: Notifications.Util.Transforms.encode_spec(%{rules: apiresource.rules}) }) diff --git a/notifications/lib/notifications/api/public_api/create.ex b/notifications/lib/notifications/api/public_api/create.ex index 9e744158f..7915fcd82 100644 --- a/notifications/lib/notifications/api/public_api/create.ex +++ b/notifications/lib/notifications/api/public_api/create.ex @@ -12,8 +12,8 @@ defmodule Notifications.Api.PublicApi.Create do Logger.info("#{inspect(org_id)} #{inspect(user_id)} #{name}") with {:ok, :authorized} <- Auth.can_manage?(user_id, org_id), - {:ok, :valid} <- Validator.validate(notification), - {:ok, n} <- create_notification(org_id, notification) do + {:ok, :valid} <- Validator.validate(notification, user_id), + {:ok, n} <- create_notification(org_id, user_id, notification) do Serialization.serialize(n) else {:error, :permission_denied} -> @@ -34,12 +34,13 @@ defmodule Notifications.Api.PublicApi.Create do end end - def create_notification(org_id, notification) do + def create_notification(org_id, creator_id, notification) do Repo.transaction(fn -> n = Models.Notification.new( org_id, notification.metadata.name, + creator_id, Notifications.Util.Transforms.encode_spec(notification.spec) ) diff --git a/notifications/lib/notifications/api/public_api/update.ex b/notifications/lib/notifications/api/public_api/update.ex index e7dfec0a2..7ea878edc 100644 --- a/notifications/lib/notifications/api/public_api/update.ex +++ b/notifications/lib/notifications/api/public_api/update.ex @@ -12,9 +12,9 @@ defmodule Notifications.Api.PublicApi.Update do Logger.info("#{inspect(org_id)} #{inspect(user_id)} #{name_or_id}") with {:ok, :authorized} <- Auth.can_manage?(user_id, org_id), - {:ok, :valid} <- Validator.validate(req.notification), + {:ok, :valid} <- Validator.validate(req.notification, user_id), {:ok, n} <- Model.find_by_id_or_name(org_id, name_or_id), - {:ok, n} <- update_notification(n, req.notification) do + {:ok, n} <- update_notification(n, user_id, req.notification) do Serialization.serialize(n) else {:error, :permission_denied} -> @@ -44,11 +44,12 @@ defmodule Notifications.Api.PublicApi.Update do # notification: existing row from database # apiresource: new notification from the API call that needs to be applied # - def update_notification(notification, apiresource) do + def update_notification(notification, creator_id, apiresource) do Repo.transaction(fn -> changes = Model.changeset(notification, %{ name: apiresource.metadata.name, + creator_id: creator_id, spec: Notifications.Util.Transforms.encode_spec(apiresource.spec) }) diff --git a/notifications/lib/notifications/auth.ex b/notifications/lib/notifications/auth.ex index 5ec360b03..055e5d082 100644 --- a/notifications/lib/notifications/auth.ex +++ b/notifications/lib/notifications/auth.ex @@ -12,8 +12,14 @@ defmodule Notifications.Auth do authorize(user_id, org_id, "organization.notifications.manage") end - defp authorize(user_id, org_id, permission) do - req = Request.new(user_id: user_id, org_id: org_id) + def can_view_project?(user_id, org_id, project_id) do + authorize(user_id, org_id, project_id, "project.view") + end + + defp authorize(user_id, org_id, permission), do: authorize(user_id, org_id, "", permission) + + defp authorize(user_id, org_id, project_id, permission) do + req = Request.new(user_id: user_id, org_id: org_id, project_id: project_id) endpoint = Application.fetch_env!(:notifications, :rbac_endpoint) {:ok, channel} = GRPC.Stub.connect(endpoint) diff --git a/notifications/lib/notifications/models/notification.ex b/notifications/lib/notifications/models/notification.ex index 48402ee8f..14f6841dd 100644 --- a/notifications/lib/notifications/models/notification.ex +++ b/notifications/lib/notifications/models/notification.ex @@ -12,16 +12,18 @@ defmodule Notifications.Models.Notification do has_many(:rules, Notifications.Models.Rule, on_delete: :delete_all) field(:org_id, :binary_id) + field(:creator_id, :binary_id) field(:name, :string) field(:spec, :map) timestamps() end - def new(org_id, name, spec) do + def new(org_id, name, creator_id, spec) do %__MODULE__{} |> changeset(%{ org_id: org_id, + creator_id: creator_id, name: name, spec: spec }) @@ -78,8 +80,8 @@ defmodule Notifications.Models.Notification do def changeset(notification, params \\ %{}) do notification - |> cast(params, [:org_id, :name, :spec]) - |> validate_required([:org_id, :name, :spec]) + |> cast(params, [:org_id, :creator_id, :name, :spec]) + |> validate_required([:org_id, :creator_id, :name, :spec]) |> valid_name_format(params) |> unique_constraint( :unique_names, diff --git a/notifications/lib/notifications/util/validator.ex b/notifications/lib/notifications/util/validator.ex index 8f31c7dca..8a088e688 100644 --- a/notifications/lib/notifications/util/validator.ex +++ b/notifications/lib/notifications/util/validator.ex @@ -1,16 +1,24 @@ defmodule Notifications.Util.Validator do alias Notifications.Models.Pattern - def validate(notification) do + def validate(notification, user_id) do with {:ok, :valid} <- validate_regex_patterns(notification), {:ok, :valid} <- validate_result_patterns(notification), - {:ok, :valid} <- validate_notification_targets(notification) do + {:ok, :valid} <- validate_notification_targets(notification), + {:ok, :valid} <- validate_user_id(user_id) do {:ok, :valid} else e -> e end end + defp validate_user_id(user_id) do + case Ecto.UUID.cast(user_id) do + {:ok, _} -> {:ok, :valid} + :error -> {:error, :invalid_argument, "Invalid user_id: expected a valid UUID"} + end + end + def validate_notification_targets(notification = %InternalApi.Notifications.Notification{}), do: validate_notification_targets_(notification.rules) diff --git a/notifications/lib/notifications/workers/coordinator.ex b/notifications/lib/notifications/workers/coordinator.ex index 9f370b037..3cdb0edf1 100644 --- a/notifications/lib/notifications/workers/coordinator.ex +++ b/notifications/lib/notifications/workers/coordinator.ex @@ -50,7 +50,9 @@ defmodule Notifications.Workers.Coordinator do organization: organization } - rules |> Enum.each(fn rule -> process(request_id, rule, data) end) + rules + |> Enum.filter(&authorized?(&1.notification.creator_id, &1.org_id, project.metadata.id)) + |> Enum.each(fn rule -> process(request_id, rule, data) end) Logger.info("#{request_id} #{event.pipeline_id}") end) @@ -87,6 +89,15 @@ defmodule Notifications.Workers.Coordinator do Logger.info("#{request_id} [done]") end + defp authorized?(_creator_id = nil, _org_id, _project_id), do: true + + defp authorized?(creator_id, org_id, project_id) do + case Notifications.Auth.can_view_project?(creator_id, org_id, project_id) do + {:ok, :authorized} -> true + _ -> false + end + end + defp map_result_to_string(enum), do: enum |> Atom.to_string() |> String.downcase() end end diff --git a/notifications/lib/notifications/workers/coordinator/api.ex b/notifications/lib/notifications/workers/coordinator/api.ex index 3c6b378db..600dc8d35 100644 --- a/notifications/lib/notifications/workers/coordinator/api.ex +++ b/notifications/lib/notifications/workers/coordinator/api.ex @@ -4,7 +4,6 @@ defmodule Notifications.Workers.Coordinator.Api do alias InternalApi.RepoProxy.DescribeRequest req = DescribeRequest.new(hook_id: hook_id) - endpoint = Application.fetch_env!(:notifications, :repo_proxy_endpoint) {:ok, channel} = GRPC.Stub.connect(endpoint) @@ -39,7 +38,6 @@ defmodule Notifications.Workers.Coordinator.Api do alias InternalApi.Plumber.PipelineService.Stub req = InternalApi.Plumber.DescribeRequest.new(ppl_id: pipeline_id, detailed: true) - endpoint = Application.get_env(:notifications, :pipeline_endpoint) {:ok, channel} = GRPC.Stub.connect(endpoint) diff --git a/notifications/lib/notifications/workers/coordinator/filter.ex b/notifications/lib/notifications/workers/coordinator/filter.ex index 12638455e..2e3ab0187 100644 --- a/notifications/lib/notifications/workers/coordinator/filter.ex +++ b/notifications/lib/notifications/workers/coordinator/filter.ex @@ -10,6 +10,7 @@ defmodule Notifications.Workers.Coordinator.Filter do |> with_pattern([branch, pr_branch], "branch") |> with_pattern(pipeline, "pipeline") |> with_pattern(result, "result") + |> preload(:notification) |> Repo.all() end diff --git a/notifications/lib/public_api/semaphore/notifications.v1alpha.pb.ex b/notifications/lib/public_api/semaphore/notifications.v1alpha.pb.ex index 51046b754..65718dc97 100644 --- a/notifications/lib/public_api/semaphore/notifications.v1alpha.pb.ex +++ b/notifications/lib/public_api/semaphore/notifications.v1alpha.pb.ex @@ -34,15 +34,17 @@ defmodule Semaphore.Notifications.V1alpha.Notification.Metadata do name: String.t(), id: String.t(), create_time: integer, - update_time: integer + update_time: integer, + creator_id: String.t() } - defstruct [:name, :id, :create_time, :update_time] + defstruct [:name, :id, :create_time, :update_time, :creator_id] field(:name, 1, type: :string) field(:id, 2, type: :string) field(:create_time, 3, type: :int64) field(:update_time, 4, type: :int64) + field(:creator_id, 5, type: :string) end defmodule Semaphore.Notifications.V1alpha.Notification.Spec.Rule.Filter do diff --git a/notifications/mix.exs b/notifications/mix.exs index bd9680e14..172b5cecc 100644 --- a/notifications/mix.exs +++ b/notifications/mix.exs @@ -37,6 +37,7 @@ defmodule Notifications.Mixfile do {:ecto_sql, "~> 3.0"}, {:postgrex, ">= 0.0.0"}, {:grpc_mock, github: "renderedtext/grpc-mock", only: [:dev, :test]}, + {:mock, "~> 0.3.0", only: [:dev, :test]}, {:paginator, "~> 1.0"}, {:tackle, github: "renderedtext/ex-tackle"}, {:util, github: "renderedtext/elixir-util", branch: "rw/string_enums"}, diff --git a/notifications/mix.lock b/notifications/mix.lock index 5ebc8c26c..834f01be6 100644 --- a/notifications/mix.lock +++ b/notifications/mix.lock @@ -29,8 +29,10 @@ "jumper": {:hex, :jumper, "1.0.1", "3c00542ef1a83532b72269fab9f0f0c82bf23a35e27d278bfd9ed0865cecabff", [:mix], [], "hexpm", "318c59078ac220e966d27af3646026db9b5a5e6703cb2aa3e26bcfaba65b7433"}, "junit_formatter": {:hex, :junit_formatter, "3.3.1", "c729befb848f1b9571f317d2fefa648e9d4869befc4b2980daca7c1edc468e40", [:mix], [], "hexpm", "761fc5be4b4c15d8ba91a6dafde0b2c2ae6db9da7b8832a55b5a1deb524da72b"}, "lager": {:hex, :lager, "3.9.2", "4cab289120eb24964e3886bd22323cb5fefe4510c076992a23ad18cf85413d8c", [:rebar3], [{:goldrush, "0.1.9", [hex: :goldrush, repo: "hexpm", optional: false]}], "hexpm", "7f904d9e87a8cb7e66156ed31768d1c8e26eba1d54f4bc85b1aa4ac1f6340c28"}, + "meck": {:hex, :meck, "0.9.2", "85ccbab053f1db86c7ca240e9fc718170ee5bda03810a6292b5306bf31bae5f5", [:rebar3], [], "hexpm", "81344f561357dc40a8344afa53767c32669153355b626ea9fcbc8da6b3045826"}, "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"}, "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"}, + "mock": {:hex, :mock, "0.3.9", "10e44ad1f5962480c5c9b9fa779c6c63de9bd31997c8e04a853ec990a9d841af", [:mix], [{:meck, "~> 0.9.2", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm", "9e1b244c4ca2551bb17bb8415eed89e40ee1308e0fbaed0a4fdfe3ec8a4adbd3"}, "paginator": {:hex, :paginator, "1.1.0", "e502a91b08d14a6c8319f86eae027e67b0c699ffca4d76afbcb43eea5f32cd53", [:mix], [{:ecto, "~> 3.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:ecto_sql, "~> 3.0", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.13", [hex: :postgrex, repo: "hexpm", optional: true]}], "hexpm", "b484f533b4a157ff60df7c988c9ef04e9c64e1f96e74a48a700717517a1ba5d9"}, "parse_trans": {:hex, :parse_trans, "3.3.1", "16328ab840cc09919bd10dab29e431da3af9e9e7e7e6f0089dd5a2d2820011d8", [:rebar3], [], "hexpm", "07cd9577885f56362d414e8c4c4e6bdf10d43a8767abb92d24cbe8b24c54888b"}, "plug_crypto": {:hex, :plug_crypto, "1.2.2", "05654514ac717ff3a1843204b424477d9e60c143406aa94daf2274fdd280794d", [:mix], [], "hexpm", "87631c7ad914a5a445f0a3809f99b079113ae4ed4b867348dd9eec288cecb6db"}, diff --git a/notifications/priv/repo/migrations/20250804163141_add_creator_id_to_notifications.exs b/notifications/priv/repo/migrations/20250804163141_add_creator_id_to_notifications.exs new file mode 100644 index 000000000..18036db79 --- /dev/null +++ b/notifications/priv/repo/migrations/20250804163141_add_creator_id_to_notifications.exs @@ -0,0 +1,9 @@ +defmodule Notifications.Repo.Migrations.AddCreatorIdToNotifications do + use Ecto.Migration + + def change do + alter table(:notifications) do + add :creator_id, :binary_id, null: true + end + end +end diff --git a/notifications/test/notifications/api/internal_api/create_test.exs b/notifications/test/notifications/api/internal_api/create_test.exs index c8abab15e..29bea8c11 100644 --- a/notifications/test/notifications/api/internal_api/create_test.exs +++ b/notifications/test/notifications/api/internal_api/create_test.exs @@ -4,6 +4,7 @@ defmodule Notifications.Api.InternalApi.CreateTest do alias InternalApi.Notifications.RequestMeta @org_id Ecto.UUID.generate() + @creator_id Ecto.UUID.generate() describe ".run" do test "it creates DB resources" do @@ -13,7 +14,7 @@ defmodule Notifications.Api.InternalApi.CreateTest do {:ok, channel} = GRPC.Stub.connect("localhost:50051") req = %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: Support.Factories.Notification.internal_api_model("first-notification") } @@ -103,7 +104,7 @@ defmodule Notifications.Api.InternalApi.CreateTest do {:ok, channel} = GRPC.Stub.connect("localhost:50051") req = %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: Support.Factories.Notification.internal_api_model("first-notification") } @@ -150,7 +151,7 @@ defmodule Notifications.Api.InternalApi.CreateTest do ) req = %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: notification } @@ -167,7 +168,7 @@ defmodule Notifications.Api.InternalApi.CreateTest do {:ok, channel} = GRPC.Stub.connect("localhost:50051") req = %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: Support.Factories.Notification.internal_api_model("first-notification") } diff --git a/notifications/test/notifications/api/internal_api/describe_test.exs b/notifications/test/notifications/api/internal_api/describe_test.exs index 62d48f005..db25535ad 100644 --- a/notifications/test/notifications/api/internal_api/describe_test.exs +++ b/notifications/test/notifications/api/internal_api/describe_test.exs @@ -2,6 +2,7 @@ defmodule Notifications.Api.InternalApi.DescribeTest do use Notifications.DataCase @org_id Ecto.UUID.generate() + @creator_id Ecto.UUID.generate() alias InternalApi.Notifications.NotificationsApi.Stub alias InternalApi.Notifications.DescribeRequest @@ -29,7 +30,7 @@ defmodule Notifications.Api.InternalApi.DescribeTest do {:ok, channel} = GRPC.Stub.connect("localhost:50051") req1 = %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: Support.Factories.Notification.internal_api_model("first") } diff --git a/notifications/test/notifications/api/internal_api/destroy_test.exs b/notifications/test/notifications/api/internal_api/destroy_test.exs index c151b89b2..3d4893720 100644 --- a/notifications/test/notifications/api/internal_api/destroy_test.exs +++ b/notifications/test/notifications/api/internal_api/destroy_test.exs @@ -7,6 +7,7 @@ defmodule Notifications.Api.InternalApi.DestroyTest do alias InternalApi.Notifications.RequestMeta @org_id Ecto.UUID.generate() + @creator_id Ecto.UUID.generate() alias Notifications.Models @@ -30,7 +31,7 @@ defmodule Notifications.Api.InternalApi.DestroyTest do {:ok, _} = Stub.create(channel, %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: req }) diff --git a/notifications/test/notifications/api/internal_api/list_test.exs b/notifications/test/notifications/api/internal_api/list_test.exs index 5c03fde84..099c91987 100644 --- a/notifications/test/notifications/api/internal_api/list_test.exs +++ b/notifications/test/notifications/api/internal_api/list_test.exs @@ -2,6 +2,7 @@ defmodule Notifications.Api.InternalApi.ListTest do use Notifications.DataCase @org_id Ecto.UUID.generate() + @creator_id Ecto.UUID.generate() describe ".run" do test "it gets the resources" do @@ -14,7 +15,7 @@ defmodule Notifications.Api.InternalApi.ListTest do Enum.each(1..5, fn index -> req = %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: Support.Factories.Notification.internal_api_model("first-#{index}") } diff --git a/notifications/test/notifications/api/internal_api/update_test.exs b/notifications/test/notifications/api/internal_api/update_test.exs index 15e8c9c00..ec65615f7 100644 --- a/notifications/test/notifications/api/internal_api/update_test.exs +++ b/notifications/test/notifications/api/internal_api/update_test.exs @@ -2,6 +2,7 @@ defmodule Notifications.Api.InternalApi.UpdateTest do use Notifications.DataCase @org_id Ecto.UUID.generate() + @creator_id Ecto.UUID.generate() alias InternalApi.Notifications.NotificationsApi.Stub alias InternalApi.Notifications.UpdateRequest @@ -13,7 +14,7 @@ defmodule Notifications.Api.InternalApi.UpdateTest do {:ok, channel} = GRPC.Stub.connect("localhost:50051") req = %UpdateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, name: "non-exist", notification: Support.Factories.Notification.internal_api_model("first") } @@ -35,12 +36,12 @@ defmodule Notifications.Api.InternalApi.UpdateTest do {:ok, _} = Stub.create(channel, %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: notification }) req = %UpdateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, name: "first", notification: Support.Factories.Notification.internal_api_model("first") } @@ -125,7 +126,7 @@ defmodule Notifications.Api.InternalApi.UpdateTest do end) end - test "it returns serialized notification" do + test "returs error when user_id not present in metadata" do alias Notifications.Models.Notification {:ok, channel} = GRPC.Stub.connect("localhost:50051") @@ -134,7 +135,7 @@ defmodule Notifications.Api.InternalApi.UpdateTest do {:ok, _} = Stub.create(channel, %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: notification }) @@ -144,6 +145,30 @@ defmodule Notifications.Api.InternalApi.UpdateTest do notification: Support.Factories.Notification.internal_api_model("first") } + {:error, result} = Stub.update(channel, req) + + assert match?(%GRPC.RPCError{message: "Invalid user_id: expected a valid UUID"}, result) + end + + test "it returns serialized notification" do + alias Notifications.Models.Notification + + {:ok, channel} = GRPC.Stub.connect("localhost:50051") + + notification = Support.Factories.Notification.internal_api_model("first") + + {:ok, _} = + Stub.create(channel, %CreateRequest{ + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, + notification: notification + }) + + req = %UpdateRequest{ + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, + name: "first", + notification: Support.Factories.Notification.internal_api_model("first") + } + {:ok, %{notification: notification}} = Stub.update(channel, req) assert notification.name == "first" @@ -161,7 +186,7 @@ defmodule Notifications.Api.InternalApi.UpdateTest do {:ok, _} = Stub.create(channel, %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: notification }) @@ -195,7 +220,7 @@ defmodule Notifications.Api.InternalApi.UpdateTest do req = UpdateRequest.new( - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, name: "first", notification: notif ) @@ -216,7 +241,7 @@ defmodule Notifications.Api.InternalApi.UpdateTest do Stub.create( channel, %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: Support.Factories.Notification.internal_api_model("first-notification") } ) @@ -225,7 +250,7 @@ defmodule Notifications.Api.InternalApi.UpdateTest do Stub.create( channel, %CreateRequest{ - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, notification: Support.Factories.Notification.internal_api_model("second-notification") } ) @@ -238,7 +263,7 @@ defmodule Notifications.Api.InternalApi.UpdateTest do req = UpdateRequest.new( - metadata: %RequestMeta{org_id: @org_id}, + metadata: %RequestMeta{org_id: @org_id, user_id: @creator_id}, name: "second-notification", notification: notif ) diff --git a/notifications/test/notifications/api/public_api/create_test.exs b/notifications/test/notifications/api/public_api/create_test.exs index 42d5ffb42..6db196e91 100644 --- a/notifications/test/notifications/api/public_api/create_test.exs +++ b/notifications/test/notifications/api/public_api/create_test.exs @@ -40,6 +40,7 @@ defmodule Notifications.Api.PublicApi.CreateTest do n = Repo.preload(n, :rules) assert n.name == "first-notification" + assert n.creator_id == @user_id assert length(n.rules) == 1 rule = hd(n.rules) diff --git a/notifications/test/notifications/util/validator_test.exs b/notifications/test/notifications/util/validator_test.exs index f9ab7a569..082befbae 100644 --- a/notifications/test/notifications/util/validator_test.exs +++ b/notifications/test/notifications/util/validator_test.exs @@ -6,6 +6,8 @@ defmodule Notifications.Util.ValidatorTest do alias Notification.{Metadata, Spec} alias Spec.Rule + @user_id Ecto.UUID.generate() + describe ".validate" do test "everything is valid" do n = @@ -35,7 +37,39 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == {:ok, :valid} + assert Validator.validate(n, @user_id) == {:ok, :valid} + end + + test "user_id is empty" do + n = + Notification.new( + metadata: Metadata.new(name: "A"), + spec: + Spec.new( + rules: [ + Rule.new( + name: "Example Rule", + filter: + Rule.Filter.new( + projects: [ + "/.*/" + ] + ), + notify: + Rule.Notify.new( + slack: + Rule.Notify.Slack.new( + endpoint: "https://whatever.com", + channels: ["#testing-hq"] + ) + ) + ) + ] + ) + ) + + assert Validator.validate(n, "") == + {:error, :invalid_argument, "Invalid user_id: expected a valid UUID"} end test "reports broken regexes" do @@ -66,7 +100,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == + assert Validator.validate(n, @user_id) == {:error, :invalid_argument, "Pattern /*/ is not a valid regex statement"} end @@ -101,7 +135,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == + assert Validator.validate(n, @user_id) == {:error, :invalid_argument, "Patterns [/*/, /+dasdas/] are not valid regex statements"} end @@ -134,7 +168,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == + assert Validator.validate(n, @user_id) == {:error, :invalid_argument, "Value pass is not a valid result entry. Valid values are: passed, failed, canceled, stopped."} end @@ -170,7 +204,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == {:ok, :valid} + assert Validator.validate(n, @user_id) == {:ok, :valid} end test "regex is valid result entry" do @@ -201,7 +235,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == {:ok, :valid} + assert Validator.validate(n, @user_id) == {:ok, :valid} end test "no notify target specified => not valid" do @@ -225,7 +259,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == + assert Validator.validate(n, @user_id) == {:error, :invalid_argument, "A notification rule must have at least one notification target configured."} end @@ -250,7 +284,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == + assert Validator.validate(n, @user_id) == {:error, :invalid_argument, "A notification rule must have a notify field."} end @@ -282,7 +316,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == {:ok, :valid} + assert Validator.validate(n, @user_id) == {:ok, :valid} end test "only valid webhook target => valid" do @@ -309,7 +343,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == {:ok, :valid} + assert Validator.validate(n, @user_id) == {:ok, :valid} end test "only valid email target => valid" do @@ -334,7 +368,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == {:ok, :valid} + assert Validator.validate(n, @user_id) == {:ok, :valid} end test "valid + invalid rule => fails" do @@ -374,7 +408,7 @@ defmodule Notifications.Util.ValidatorTest do ) ) - assert Validator.validate(n) == + assert Validator.validate(n, @user_id) == {:error, :invalid_argument, "A notification rule must have a notify field."} end end diff --git a/notifications/test/notifications/workers/coordinator_test.exs b/notifications/test/notifications/workers/coordinator_test.exs new file mode 100644 index 000000000..18db4a45d --- /dev/null +++ b/notifications/test/notifications/workers/coordinator_test.exs @@ -0,0 +1,224 @@ +defmodule Notifications.Workers.CoordinatorTest do + use Notifications.DataCase + import Mock + + alias Notifications.Workers.Coordinator.PipelineFinished + alias Notifications.Workers.Slack + alias Notifications.Models.{Rule, Notification, Pattern} + alias Notifications.Repo + + @pipeline_id Ecto.UUID.generate() + @project_id Ecto.UUID.generate() + @org_id Ecto.UUID.generate() + @creator_id Ecto.UUID.generate() + + setup do + mock_grpc_services() + :ok + end + + describe ".handle_message" do + test "when notification creator has access to the project" do + add_notification_with_rule_and_patterns() + authorize_everything() + + with_mock Slack, publish: fn _, _, _, _ -> :ok end do + pipeline_event = InternalApi.Plumber.PipelineEvent.new(pipeline_id: @pipeline_id) + message = InternalApi.Plumber.PipelineEvent.encode(pipeline_event) + + assert PipelineFinished.handle_message(message) + assert_called(Slack.publish(:_, :_, :_, :_)) + end + end + + test "when notification creator doesn't have access to the project" do + add_notification_with_rule_and_patterns() + + with_mock Slack, publish: fn _, _, _, _ -> :error end do + pipeline_event = InternalApi.Plumber.PipelineEvent.new(pipeline_id: @pipeline_id) + message = InternalApi.Plumber.PipelineEvent.encode(pipeline_event) + + assert PipelineFinished.handle_message(message) + assert_not_called(Slack.publish(:_, :_, :_, :_)) + end + end + + test "when notification has empty creator_id" do + setup_notification_with_empty_creator() + + with_mock Slack, publish: fn _, _, _, _ -> :ok end do + pipeline_event = InternalApi.Plumber.PipelineEvent.new(pipeline_id: @pipeline_id) + message = InternalApi.Plumber.PipelineEvent.encode(pipeline_event) + + assert PipelineFinished.handle_message(message) + assert_called(Slack.publish(:_, :_, :_, :_)) + end + end + end + + ### + ### Helper funcs + ### + + defp authorize_everything do + GrpcMock.stub( + RBACMock, + :list_user_permissions, + fn _, _ -> + InternalApi.RBAC.ListUserPermissionsResponse.new( + permissions: ["organization.view", "project.view"] + ) + end + ) + end + + defp mock_grpc_services do + alias InternalApi.{Plumber, Projecthub, RepoProxy, PlumberWF, Organization, ResponseStatus} + + GrpcMock.stub( + PipelinesMock, + :describe, + fn _, _ -> + Plumber.DescribeResponse.new( + response_status: Plumber.ResponseStatus.new(code: :OK), + pipeline: Plumber.Pipeline.new(project_id: @project_id) + ) + end + ) + + GrpcMock.stub( + ProjectServiceMock, + :describe, + fn _, _ -> + project = + Projecthub.Project.new( + metadata: Projecthub.Project.Metadata.new(id: @project_id, org_id: @org_id) + ) + + Projecthub.DescribeResponse.new( + metadata: + Projecthub.ResponseMeta.new( + status: + Projecthub.ResponseMeta.Status.new(code: Projecthub.ResponseMeta.Code.value(:OK)) + ), + project: project + ) + end + ) + + GrpcMock.stub( + RepoProxyMock, + :describe, + fn _, _ -> + RepoProxy.DescribeResponse.new( + status: ResponseStatus.new(code: ResponseStatus.Code.value(:OK)), + hook: RepoProxy.Hook.new() + ) + end + ) + + GrpcMock.stub( + WorkflowMock, + :describe, + fn _, _ -> + PlumberWF.DescribeResponse.new( + status: ResponseStatus.new(code: ResponseStatus.Code.value(:OK)), + workflow: PlumberWF.WorkflowDetails.new() + ) + end + ) + + GrpcMock.stub( + OrganizationMock, + :describe, + fn _, _ -> + Organization.DescribeResponse.new( + status: ResponseStatus.new(code: ResponseStatus.Code.value(:OK)), + organization: Organization.Organization.new() + ) + end + ) + + GrpcMock.stub( + RBACMock, + :list_user_permissions, + fn _, _ -> + InternalApi.RBAC.ListUserPermissionsResponse.new(permissions: []) + end + ) + end + + defp setup_notification_with_empty_creator do + notification = + Repo.insert!(%Notification{ + org_id: @org_id, + creator_id: nil, + name: "Test Notification Empty Creator", + spec: %{"rules" => []} + }) + + create_rule_with_patterns(notification) + end + + defp add_notification_with_rule_and_patterns do + notification = + Repo.insert!(%Notification{ + org_id: @org_id, + creator_id: @creator_id, + name: "Test Notification", + spec: %{"rules" => []} + }) + + create_rule_with_patterns(notification) + end + + defp create_rule_with_patterns(notification) do + rule = + Repo.insert!(%Rule{ + org_id: @org_id, + notification_id: notification.id, + name: "Test Rule", + slack: %{ + "endpoint" => "https://hooks.slack.com/services/TEST", + "channels" => ["#test-channel"] + }, + email: %{}, + webhook: %{} + }) + + # Add patterns to the rule that will match our pipeline + Repo.insert!(%Pattern{ + org_id: @org_id, + rule_id: rule.id, + type: "project", + regex: true, + term: ".*" + }) + + Repo.insert!(%Pattern{ + org_id: @org_id, + rule_id: rule.id, + type: "branch", + regex: true, + term: ".*" + }) + + Repo.insert!(%Pattern{ + org_id: @org_id, + rule_id: rule.id, + type: "pipeline", + regex: true, + term: ".*" + }) + + Repo.insert!(%Pattern{ + org_id: @org_id, + rule_id: rule.id, + type: "result", + regex: true, + term: ".*" + }) + + rule + end +end diff --git a/notifications/test/test_helper.exs b/notifications/test/test_helper.exs index 7a8cbd695..c0b8be8cb 100644 --- a/notifications/test/test_helper.exs +++ b/notifications/test/test_helper.exs @@ -1,8 +1,24 @@ GrpcMock.defmock(SecretMock, for: InternalApi.Secrethub.SecretService.Service) GrpcMock.defmock(RBACMock, for: InternalApi.RBAC.RBAC.Service) +GrpcMock.defmock(PipelinesMock, for: InternalApi.Plumber.PipelineService.Service) +GrpcMock.defmock(ProjectServiceMock, for: InternalApi.Projecthub.ProjectService.Service) +GrpcMock.defmock(RepoProxyMock, for: InternalApi.RepoProxy.RepoProxyService.Service) +GrpcMock.defmock(WorkflowMock, for: InternalApi.PlumberWF.WorkflowService.Service) +GrpcMock.defmock(OrganizationMock, for: InternalApi.Organization.OrganizationService.Service) spawn(fn -> - GRPC.Server.start([SecretMock, RBACMock], 50_052) + GRPC.Server.start( + [ + SecretMock, + RBACMock, + PipelinesMock, + ProjectServiceMock, + RepoProxyMock, + WorkflowMock, + OrganizationMock + ], + 50_052 + ) end) formatters = [ExUnit.CLIFormatter, JUnitFormatter]