diff --git a/lib/cog/commands/pipeline/info.ex b/lib/cog/commands/pipeline/info.ex index 63d2718c..da3b5780 100644 --- a/lib/cog/commands/pipeline/info.ex +++ b/lib/cog/commands/pipeline/info.ex @@ -6,27 +6,53 @@ defmodule Cog.Commands.Pipeline.Info do alias Cog.Commands.Pipeline.Util alias Cog.Repository.PipelineHistory, as: HistoryRepo + alias Cog.Repository.Users, as: UserRepo + alias Cog.Models.User + alias Cog.Repository.Permissions - @description "Display command pipeline details" + @description """ + Display command pipeline details. Note: You can only view the details \ + of other users pipelines if you have the operable:manage_user_pipeline \ + permission. + """ @arguments "id ..." # Allow any user to run info rule "when command is #{Cog.Util.Misc.embedded_bundle}:pipeline-info allow" + # We need to lock down who can manage a user's pipelines to the owning + # user and admins. We don't have a way to express that via rules currently + # so we have to handle it within the command. + permission "manage_user_pipeline" + def handle_message(%{args: ids} = req, state) do infos = ids - |> Enum.reduce([], &pipeline_info/2) + |> Enum.reduce([], pipeline_info_fn(req)) |> Enum.map(&Util.entry_to_map/1) {:reply, req.reply_to, "pipeline-info", infos, state} end - defp pipeline_info(id, accum) do - case HistoryRepo.by_short_id(id) do - nil -> - accum - entry -> - [entry|accum] + defp pipeline_info_fn(req) do + # Wrapping the permission checking bits up in a closure + # so we only make the request for the user and perm once. + {:ok, req_user} = UserRepo.by_username(req.user["username"]) + perm = Permissions.by_name("operable:manage_user_pipeline") + has_perm? = User.has_permission(req_user, perm) + + fn(id, accum) -> + case HistoryRepo.by_short_id(id) do + nil -> + accum + entry -> + # If the user has the manage_user_pipeline perm or owns + # the pipeline, return it. Otherwise just ignore it. + if has_perm? || entry.user.id == req_user.id do + [entry|accum] + else + accum + end + end end end diff --git a/lib/cog/commands/pipeline/kill.ex b/lib/cog/commands/pipeline/kill.ex index 54347138..34222616 100644 --- a/lib/cog/commands/pipeline/kill.ex +++ b/lib/cog/commands/pipeline/kill.ex @@ -6,16 +6,27 @@ defmodule Cog.Commands.Pipeline.Kill do alias Cog.Commands.Pipeline.Util alias Cog.Pipeline alias Cog.Repository.PipelineHistory, as: HistoryRepo + alias Cog.Repository.Users, as: UserRepo + alias Cog.Models.User + alias Cog.Repository.Permissions - @description "Abort a running pipeline" + @description """ + Abort a running pipeline. Note: You can only abort other users pipelines \ + if you have the operable:manage_user_pipeline permission. + """ @arguments "id ..." # Allow any user to run ps rule "when command is #{Cog.Util.Misc.embedded_bundle}:pipeline-kill allow" + # We need to lock down who can manage a user's pipelines to the owning + # user and admins. We don't have a way to express that via rules currently + # so we have to handle it within the command. + permission "manage_user_pipeline" + def handle_message(%{args: ids} = req, state) do - killed = Enum.reduce(ids, [], &kill_pipeline/2) + killed = Enum.reduce(ids, [], kill_pipeline_fn(req)) killed_text = case killed do [] -> "none" @@ -27,18 +38,29 @@ defmodule Cog.Commands.Pipeline.Kill do {:reply, req.reply_to, "pipeline-kill", results, state} end - defp kill_pipeline(id, killed) do - case HistoryRepo.by_short_id(id, "finished") do - nil -> - killed - entry -> - if Process.alive?(entry.pid) do - Pipeline.teardown(entry.pid) - [Util.short_id(entry.id)|killed] - else + defp kill_pipeline_fn(req) do + # Wrapping the permission checking bits up in a closure + # so we only make the request for the user and perm once. + {:ok, req_user} = UserRepo.by_username(req.user["username"]) + perm = Permissions.by_name("operable:manage_user_pipeline") + has_perm? = User.has_permission(req_user, perm) + + can_kill? = fn(entry) -> + Process.alive?(entry.pid) && (has_perm? || entry.user.id == req_user.id) + end + + fn(id, killed) -> + case HistoryRepo.by_short_id(id, "finished") do + nil -> killed - end + entry -> + if can_kill?.(entry) do + Pipeline.teardown(entry.pid) + [Util.short_id(entry.id)|killed] + else + killed + end + end end end - end diff --git a/lib/cog/commands/pipeline/list.ex b/lib/cog/commands/pipeline/list.ex index ee2116e1..7849f853 100644 --- a/lib/cog/commands/pipeline/list.ex +++ b/lib/cog/commands/pipeline/list.ex @@ -7,15 +7,29 @@ defmodule Cog.Commands.Pipeline.List do alias Cog.Models.PipelineHistory alias Cog.Repository.PipelineHistory, as: HistoryRepo alias Cog.Repository.Users, as: UserRepo + alias Cog.Repository.Permissions + alias Cog.Models.User - @description "Display command pipeline statistics" + @description """ + Display command pipeline statistics. Note: You can only list your own \ + pipelines unless you have the operable:manage_user_pipeline permission. + """ @valid_state_names ["R","W","F","running","waiting","finished"] # Allow any user to run ps rule "when command is #{Cog.Util.Misc.embedded_bundle}:pipeline-list allow" + # We need to lock down who can manage a user's pipelines to the owning + # user and admins. We don't have a way to express that via rules currently + # so we have to handle it within the command. + permission "manage_user_pipeline" + option "user", short: "u", type: "string", required: false, - description: "View pipelines for specified user. Use 'all' to view pipeline history for all users." + description: """ + View pipelines for specified user. Use 'all' to view pipeline history \ + for all users. Note: You can only view pipelines for users other than \ + yourself if you have the operable:manage_user_pipeline permission. + """ option "last", short: "l", type: "int", required: false, description: "View pipelines" @@ -68,17 +82,36 @@ defmodule Cog.Commands.Pipeline.List do nil -> {:ok, HistoryRepo.pipelines_for_user(req.user["id"], Map.get(opts, "last", 20))} "all" -> - {:ok, HistoryRepo.all_pipelines(Map.get(opts, "last", 20))} - user -> - case UserRepo.by_username(user) do - {:ok, app_user} -> - {:ok, HistoryRepo.pipelines_for_user(app_user.id, Map.get(opts, "last", 20))} + # Check that the user has permission to view all pipelines + if has_perm?(req.user["username"]) do + {:ok, HistoryRepo.all_pipelines(Map.get(opts, "last", 20))} + else + # Return an error if the user doesn't have the proper perms + {:error, "You must have the operable:manage_user_pipeline permission to view pipeline history for all users."} + end + username -> + case UserRepo.by_username(username) do + {:ok, user} -> + # If a username is specified it must be the name of the requesting + # user, or the requesting user must have the manage_user_pipeline + # permission. + if has_perm?(req.user["username"]) || user.id == req.user["id"] do + {:ok, HistoryRepo.pipelines_for_user(user.id, Map.get(opts, "last", 20))} + else + {:error, "You must have the operable:manage_user_pipeline permission to view pipeline history for other users."} + end {:error, :not_found} -> - {:error, "User '#{user}' not found"} + {:error, "User '#{username}' not found"} end end end + defp has_perm?(username) do + perm = Permissions.by_name("operable:manage_user_pipeline") + {:ok, user} = UserRepo.by_username(username) + User.has_permission(user, perm) + end + defp valid_state_option?(opts) do case Map.get(opts, "state") do nil -> diff --git a/test/commands/permission/list_test.exs b/test/commands/permission/list_test.exs index 135aed39..56e90b6a 100644 --- a/test/commands/permission/list_test.exs +++ b/test/commands/permission/list_test.exs @@ -15,6 +15,7 @@ defmodule Cog.Test.Commands.Permission.ListTest do %{bundle: "operable", name: "manage_relays"}, %{bundle: "operable", name: "manage_roles"}, %{bundle: "operable", name: "manage_triggers"}, + %{bundle: "operable", name: "manage_user_pipeline"}, %{bundle: "operable", name: "manage_users"}, %{bundle: "operable", name: "st-echo"}, %{bundle: "operable", name: "st-thorn"}] = payload diff --git a/test/commands/pipeline/info_test.exs b/test/commands/pipeline/info_test.exs new file mode 100644 index 00000000..13794352 --- /dev/null +++ b/test/commands/pipeline/info_test.exs @@ -0,0 +1,67 @@ +defmodule Cog.Test.Pipeline.InfoTest do + use Cog.CommandCase, command_module: Cog.Commands.Pipeline.Info, + command_tag: "pipeline_info" + + alias Cog.Repository.PipelineHistory + import Cog.Support.ModelUtilities, only: [user: 1, + with_permission: 2] + + setup do + user = user("pipelineuser") + other_user = user("otherpipelineuser") + admin_user = user("adminpipelineuser") + |> with_permission("operable:manage_user_pipeline") + + pipeline = PipelineHistory.new(%{id: "fakeid", + text: "Some text", + room_name: "FakeRoom", + room_id: "fakeroomid", + provider: "test", + count: 1, + state: "running", + user_id: user.id}) + + {:ok, %{user: user, + other_user: other_user, + admin_user: admin_user, + pipeline: pipeline}} + end + + test "pipeline info", %{user: user, pipeline: pipeline} do + response = new_req(args: [pipeline.id], user: user) + |> send_req() + |> unwrap() + + assert([%{id: "fakeid", + room: "FakeRoom", + state: "running", + text: "Some text", + user: "pipelineuser"}] = response) + end + + test "other user can't see pipeline info without permission", + %{other_user: other_user, + pipeline: pipeline} do + + response = new_req(args: [pipeline.id], user: other_user) + |> send_req() + |> unwrap() + + assert([] = response) + end + + test "can see pipeline info for other's pipelines with permission", + %{admin_user: admin_user, + pipeline: pipeline} do + + response = new_req(args: [pipeline.id], user: admin_user) + |> send_req() + |> unwrap() + + assert([%{id: "fakeid", + room: "FakeRoom", + state: "running", + text: "Some text", + user: "pipelineuser"}] = response) + end +end diff --git a/test/commands/pipeline/kill_test.exs b/test/commands/pipeline/kill_test.exs new file mode 100644 index 00000000..24e4da96 --- /dev/null +++ b/test/commands/pipeline/kill_test.exs @@ -0,0 +1,65 @@ +defmodule Cog.Test.Pipeline.KillTest do + use Cog.CommandCase, command_module: Cog.Commands.Pipeline.Kill, + command_tag: "pipeline_kill" + + alias Cog.Repository.PipelineHistory + import Cog.Support.ModelUtilities, only: [user: 1, + with_permission: 2] + + setup do + user = user("pipelineuser") + other_user = user("otherpipelineuser") + admin_user = user("adminpipelineuser") + |> with_permission("operable:manage_user_pipeline") + + {:ok, task} = Task.start_link(fn -> Process.sleep(:infinity) end) + + pipeline = PipelineHistory.new(%{id: "fakeid", + text: "Some text", + room_name: "FakeRoom", + room_id: "fakeroomid", + provider: "test", + count: 1, + pid: task, + state: "running", + user_id: user.id}) + + {:ok, %{user: user, + other_user: other_user, + admin_user: admin_user, + pipeline: pipeline}} + end + + test "a user can kill their own pipeline", + %{user: user, pipeline: pipeline} do + + response = new_req(args: [pipeline.id], user: user) + |> send_req() + |> unwrap() + + assert(%{killed: ["fakeid"], + killed_text: "fakeid"} = response) + end + + test "a user cannot kill other user's pipelines", + %{other_user: other_user, pipeline: pipeline} do + + response = new_req(args: [pipeline.id], user: other_user) + |> send_req() + |> unwrap() + + assert(%{killed: [], killed_text: "none"} = response) + end + + test "a user can kill othe user's pipelines with the correct permission", + %{admin_user: admin_user, pipeline: pipeline} do + + response = new_req(args: [pipeline.id], user: admin_user) + |> send_req() + |> unwrap() + + assert(%{killed: ["fakeid"], + killed_text: "fakeid"} = response) + end + +end diff --git a/test/commands/pipeline/list_test.exs b/test/commands/pipeline/list_test.exs new file mode 100644 index 00000000..8f0b377a --- /dev/null +++ b/test/commands/pipeline/list_test.exs @@ -0,0 +1,64 @@ +defmodule Cog.Test.Pipeline.ListTest do + use Cog.CommandCase, command_module: Cog.Commands.Pipeline.List, + command_tag: "pipeline_list" + + alias Cog.Repository.PipelineHistory + import Cog.Support.ModelUtilities, only: [user: 1, + with_permission: 2] + + setup do + user = user("pipelineuser") + other_user = user("otherpipelineuser") + admin_user = user("adminpipelineuser") + |> with_permission("operable:manage_user_pipeline") + + for n <- 1..5 do + PipelineHistory.new(%{id: "fakeid#{n}", + text: "Some text", + room_name: "FakeRoom", + room_id: "fakeroomid", + provider: "test", + count: n, + state: "running", + user_id: user.id}) + end + + {:ok, %{user: user, other_user: other_user, admin_user: admin_user}} + end + + test "listing pipelines", %{user: user} do + response = new_req(user: user) + |> send_req() + |> unwrap() + + assert(%{pipeline_count: 5, + pipelines: [%{id: "fakeid5"}, + %{id: "fakeid4"}, + %{id: "fakeid3"}, + %{id: "fakeid2"}, + %{id: "fakeid1"}]} = response) + end + + test "Can't list other user's pipelines without permission", + %{user: user, other_user: other_user} do + + response = new_req(options: %{"user" => user.username}, user: other_user) + |> send_req() + |> unwrap_error() + + assert("You must have the operable:manage_user_pipeline permission to view pipeline history for other users." = response) + end + + test "Can list other user's pipelines with permission", %{user: user, admin_user: admin_user} do + response = new_req(options: %{"user" => user.username}, user: admin_user) + |> send_req() + |> unwrap() + + assert(%{pipeline_count: 5, + pipelines: [%{id: "fakeid5"}, + %{id: "fakeid4"}, + %{id: "fakeid3"}, + %{id: "fakeid2"}, + %{id: "fakeid1"}]} = response) + end +end diff --git a/test/controllers/v1/permission_controller_test.exs b/test/controllers/v1/permission_controller_test.exs index 38984411..aa58ee1d 100644 --- a/test/controllers/v1/permission_controller_test.exs +++ b/test/controllers/v1/permission_controller_test.exs @@ -41,6 +41,7 @@ defmodule Cog.V1.PermissionControllerTest do "manage_relays", "manage_roles", "manage_triggers", + "manage_user_pipeline", "manage_users", "st-echo", "st-thorn"] diff --git a/test/support/command_case.ex b/test/support/command_case.ex index 675e20b7..b35066c7 100644 --- a/test/support/command_case.ex +++ b/test/support/command_case.ex @@ -84,7 +84,9 @@ defmodule Cog.CommandCase do room: Keyword.get(opts, :room, %Cog.Chat.Room{}), service_token: Keyword.get(opts, :service_token, service_token()), services_root: Keyword.get(opts, :services_root, services_root()), - user: Keyword.get(opts, :user, %{})} + # Doing the poison encode dance here because thats what happens when the + # message is sent over the bus. + user: Poison.encode!(Keyword.get(opts, :user, %{})) |> Poison.decode!()} end def send_req(%Cog.Messages.Command{}=req, module) do