Permalink
Browse files

Merge #321

321: Add a second, less-permissive mode for users to be in r=notriddle a=notriddle

Part of #251 

Mere "members," as opposed to "reviewers," have permission to run "bors try" and will have permission to run "bors retry" once that's also implemented.
  • Loading branch information...
bors[bot] committed Nov 6, 2017
2 parents 1d99459 + 25b09a2 commit 8f5974b9302fef7018fda2b7a34c468864509771
@@ -25,6 +25,7 @@ defmodule BorsNG.Database.Context do
alias BorsNG.Database.Installation
alias BorsNG.Database.LinkPatchBatch
alias BorsNG.Database.LinkUserProject
alias BorsNG.Database.LinkMemberProject
alias BorsNG.Database.Patch
alias BorsNG.Database.Project
alias BorsNG.Database.Repo
@@ -8,12 +8,34 @@ defmodule BorsNG.Database.Context.Permission do
use BorsNG.Database.Context
def permission_to_approve_patch?(user, patch) do
def list_users_for_project(:member, project_id) do
Repo.all(from u in User,
join: l in LinkMemberProject,
where: l.project_id == ^project_id,
where: u.id == l.user_id)
end
def list_users_for_project(:reviewer, project_id) do
Repo.all(from u in User,
join: l in LinkUserProject,
where: l.project_id == ^project_id,
where: u.id == l.user_id)
end
def permission?(:member, user, patch) do
%User{id: user_id} = user
%Patch{project_id: project_id} = patch
project_member?(user_id, project_id) or
permission?(:reviewer, user, patch)
end
def permission?(:reviewer, user, patch) do
%User{id: user_id} = user
%Patch{id: patch_id, project_id: project_id} = patch
project_reviewer?(user_id, project_id) or
patch_delegated_reviewer?(user_id, patch_id)
end
def permission?(:none, _, _) do
true
end
defp project_reviewer?(user_id, project_id) do
LinkUserProject
@@ -24,6 +46,15 @@ defmodule BorsNG.Database.Context.Permission do
|> Kernel.not()
end
defp project_member?(user_id, project_id) do
LinkMemberProject
|> where([l], l.user_id == ^user_id and l.project_id == ^project_id)
|> Repo.one()
|> is_nil()
# elixirc squawks about unary operators if the module is left off.
|> Kernel.not()
end
defp patch_delegated_reviewer?(user_id, patch_id) do
UserPatchDelegation
|> where([d], d.user_id == ^user_id and d.patch_id == ^patch_id)
@@ -0,0 +1,27 @@
defmodule BorsNG.Database.LinkMemberProject do
@moduledoc """
The connection between a project and its reviewers.
People with this link can bring up the dashboard page and settings
for a project, and can r+ a commit. Otherwise, they can't.
"""
use BorsNG.Database.Model
schema "link_member_project" do
belongs_to :user, User
belongs_to :project, Project
end
@doc """
Builds a changeset based on the `struct` and `params`.
"""
def changeset(struct, params \\ %{}) do
struct
|> cast(params, [:user_id, :project_id])
|> validate_required([:user_id, :project_id])
|> unique_constraint(
:user_id,
name: :link_member_project_user_id_project_id_index)
end
end
@@ -17,7 +17,7 @@ defmodule BorsNG.Database.Project do
ping!(to_string(project_id))
end
def ping!(project_id) do
BorsNG.Endpoint.broadcast!("project_ping:1", "new_msg", %{})
BorsNG.Endpoint.broadcast!("project_ping:#{project_id}", "new_msg", %{})
end
schema "projects" do
@@ -17,13 +17,6 @@ defmodule BorsNG.Database.User do
timestamps()
end
def by_project(project_id) do
from u in User,
join: l in LinkUserProject,
where: l.project_id == ^project_id,
where: u.id == l.user_id
end
def has_perm(_repo, %User{is_admin: true}, _project_id) do
true
end
@@ -0,0 +1,12 @@
defmodule BorsNG.Database.Repo.Migrations.LinkMemberProject do
use Ecto.Migration
def change do
create table(:link_member_project) do
add :project_id, references(:projects, on_delete: :delete_all)
add :user_id, references(:users, on_delete: :delete_all)
end
create index(:link_member_project, [:user_id, :project_id],
name: :link_member_project_user_id_project_id_index, unique: true)
end
end
@@ -4,6 +4,7 @@ defmodule BorsNG.Database.Context.PermissionTest do
alias BorsNG.Database.Context.Permission
alias BorsNG.Database.Installation
alias BorsNG.Database.LinkUserProject
alias BorsNG.Database.LinkMemberProject
alias BorsNG.Database.Patch
alias BorsNG.Database.Project
alias BorsNG.Database.User
@@ -29,18 +30,28 @@ defmodule BorsNG.Database.Context.PermissionTest do
test "user does not have permission by default", params do
%{patch: patch, user: user} = params
refute Permission.permission_to_approve_patch?(user, patch)
refute Permission.permission?(:reviewer, user, patch)
refute Permission.permission?(:member, user, patch)
end
test "reviewers have permission", params do
%{project: project, patch: patch, user: user} = params
Repo.insert!(%LinkUserProject{user: user, project: project})
assert Permission.permission_to_approve_patch?(user, patch)
assert Permission.permission?(:reviewer, user, patch)
assert Permission.permission?(:member, user, patch)
end
test "delegated users have permission", params do
%{patch: patch, user: user} = params
Repo.insert!(%UserPatchDelegation{user: user, patch: patch})
assert Permission.permission_to_approve_patch?(user, patch)
assert Permission.permission?(:reviewer, user, patch)
assert Permission.permission?(:member, user, patch)
end
test "members have partial permission", params do
%{project: project, patch: patch, user: user} = params
Repo.insert!(%LinkMemberProject{user: user, project: project})
refute Permission.permission?(:reviewer, user, patch)
assert Permission.permission?(:member, user, patch)
end
end
@@ -101,6 +101,17 @@ defmodule BorsNG.CommandTest do
assert [] == Command.parse("Xbors tryZ")
end
test "command permissions" do
assert :none == Command.required_permission_level([])
assert :none == Command.required_permission_level([:ping])
assert :member == Command.required_permission_level([{:try, ""}])
assert :member == Command.required_permission_level([{:try, ""}, :ping])
assert :reviewer ==
Command.required_permission_level([:approve, {:try, ""}])
assert :reviewer ==
Command.required_permission_level([{:try, ""}, :approve])
end
test "running ping command should post comment", %{proj: proj} do
GitHub.ServerMock.put_state(%{
{{:installation, 91}, 14} => %{
@@ -284,18 +284,43 @@ defmodule BorsNG.Command do
|> fetch_patch()
cmd_list = parse(c.comment)
cond do
cmd_list == [] ->
:ok
cmd_list == [:ping] ->
run(c, :ping)
Permission.permission_to_approve_patch?(c.commenter, c.patch) ->
Enum.each(cmd_list, &run(c, &1))
true ->
permission_denied(c)
cmd_list
|> required_permission_level()
|> Permission.permission?(c.commenter, c.patch)
|> if do
Enum.each(cmd_list, &run(c, &1))
else
permission_denied(c)
end
end
@spec run(t, cmd) :: :ok
def required_permission_level_cmd(:ping) do
:none
end
def required_permission_level_cmd({:autocorrect, _}) do
:none
end
def required_permission_level_cmd({:try, _}) do
:member
end
def required_permission_level_cmd(_) do
:reviewer
end
def required_permission_level(cmd_list) do
cmd_list
|> Enum.reduce(:none, fn cmd, perm ->
new_perm = cmd |> required_permission_level_cmd()
case {perm, new_perm} do
{:none, new_perm} -> new_perm
{perm, :none} -> perm
{_, :reviewer} -> :reviewer
{:reviewer, _} -> :reviewer
{p, p} -> p
end
end)
end
def permission_denied(c) do
login = c.commenter.login
url = project_url(
@@ -313,6 +338,8 @@ defmodule BorsNG.Command do
Existing reviewers: [click here to make #{login} a reviewer](#{url})
""")
end
@spec run(t, cmd) :: :ok
def run(c, :activate) do
run(c, {:activate_by, c.commenter.login})
end
@@ -363,6 +390,7 @@ defmodule BorsNG.Command do
end
delegate_to(c, delegatee)
end
def delegate_to(c, delegatee) do
Permission.delegate(delegatee, c.patch)
c.project.repo_xref
@@ -12,7 +12,9 @@ defmodule BorsNG.ProjectController do
use BorsNG.Web, :controller
alias BorsNG.Worker.Batcher
alias BorsNG.Database.Context.Permission
alias BorsNG.Database.Repo
alias BorsNG.Database.LinkMemberProject
alias BorsNG.Database.LinkUserProject
alias BorsNG.Database.Project
alias BorsNG.Database.Batch
@@ -82,10 +84,12 @@ defmodule BorsNG.ProjectController do
def settings(_, :ro, _, _), do: raise BorsNG.PermissionDeniedError
def settings(conn, :rw, project, _params) do
reviewers = Repo.all(User.by_project(project.id))
reviewers = Permission.list_users_for_project(:reviewer, project.id)
members = Permission.list_users_for_project(:member, project.id)
render conn, "settings.html",
project: project,
reviewers: reviewers,
members: members,
current_user_id: conn.assigns.user.id,
update_branches: Project.changeset_branches(project)
end
@@ -130,17 +134,20 @@ defmodule BorsNG.ProjectController do
|> put_flash(:ok, "Successfully updated branches")
|> redirect(to: project_path(conn, :settings, project))
{:error, changeset} ->
reviewers = Repo.all(User.by_project(project.id))
reviewers = Permission.list_users_for_project(:reviewer, project.id)
members = Permission.list_users_for_project(:member, project.id)
conn
|> put_flash(:error, "Cannot update branches")
|> render("settings.html",
project: project,
reviewers: reviewers,
members: members,
current_user_id: conn.assigns.user.id,
update_branches: changeset)
end
end
def add_reviewer(_, :ro, _, _), do: raise BorsNG.PermissionDeniedError
def add_reviewer(conn, :rw, project, %{"reviewer" => %{"login" => ""}}) do
conn
@@ -188,6 +195,53 @@ defmodule BorsNG.ProjectController do
|> redirect(to: project_path(conn, :settings, project))
end
def add_member(_, :ro, _, _), do: raise BorsNG.PermissionDeniedError
def add_member(conn, :rw, project, %{"member" => %{"login" => ""}}) do
conn
|> put_flash(:error, "Please enter a GitHub user's nickname")
|> redirect(to: project_path(conn, :settings, project))
end
def add_member(conn, :rw, project, %{"member" => %{"login" => login}}) do
user = case Repo.get_by(User, login: login) do
nil ->
{:installation, project.installation.installation_xref}
|> GitHub.get_user_by_login!(login)
|> case do
nil -> nil
gh_user ->
case Repo.get_by(User, user_xref: gh_user.id) do
nil ->
User.changeset(%User{}, %{
user_xref: gh_user.id,
login: gh_user.login
})
|> Repo.insert!()
user -> user
end
end
user -> user
end
{state, msg} = case user do
nil ->
{:error, "GitHub user not found; maybe you typo-ed?"}
user ->
%LinkMemberProject{}
|> LinkMemberProject.changeset(%{
user_id: user.id,
project_id: project.id})
|> Repo.insert()
|> case do
{:error, _} ->
{:error, "This user is already a member"}
{:ok, _login} ->
{:ok, "Successfully added #{user.login} as a member"}
end
end
conn
|> put_flash(state, msg)
|> redirect(to: project_path(conn, :settings, project))
end
def confirm_add_reviewer(_, :ro, _, _) do
raise BorsNG.PermissionDeniedError
end
@@ -210,6 +264,18 @@ defmodule BorsNG.ProjectController do
|> redirect(to: project_path(conn, :settings, project))
end
def remove_member(_, :ro, _, _), do: raise BorsNG.PermissionDeniedError
def remove_member(conn, :rw, project, %{"user_id" => user_id}) do
link = Repo.get_by!(
LinkMemberProject,
project_id: project.id,
user_id: user_id)
Repo.delete!(link)
conn
|> put_flash(:ok, "Removed member")
|> redirect(to: project_path(conn, :settings, project))
end
def synchronize(_, :ro, _, _), do: raise BorsNG.PermissionDeniedError
def synchronize(conn, :rw, project, _params) do
Syncer.start_synchronize_project(project.id)
@@ -64,10 +64,12 @@ defmodule BorsNG.Router do
put "/:id/settings/branches", ProjectController, :update_branches
delete "/:id/batches/incomplete", ProjectController, :cancel_all
post "/:id/reviewer", ProjectController, :add_reviewer
post "/:id/member", ProjectController, :add_member
get "/:id/add-reviewer/:login", ProjectController, :confirm_add_reviewer
put "/:id/synchronize", ProjectController, :synchronize
get "/:id/log", ProjectController, :log
delete "/:id/reviewer/:user_id", ProjectController, :remove_reviewer
delete "/:id/member/:user_id", ProjectController, :remove_member
end
scope @wobserver_url, Wobserver do
Oops, something went wrong.

0 comments on commit 8f5974b

Please sign in to comment.