From 058ea1b6d7bea0c16e6cd78f810c7be443682b82 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Fri, 4 Jan 2019 12:39:45 -0800 Subject: [PATCH 01/18] add field for is_moderator to user model --- warehouse/accounts/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/warehouse/accounts/models.py b/warehouse/accounts/models.py index e68cf2e9f1cd..1ef3f6845ab8 100644 --- a/warehouse/accounts/models.py +++ b/warehouse/accounts/models.py @@ -72,6 +72,7 @@ class User(SitemapMixin, db.Model): password_date = Column(DateTime, nullable=True, server_default=sql.func.now()) is_active = Column(Boolean, nullable=False) is_superuser = Column(Boolean, nullable=False) + is_moderator = Column(Boolean, nullable=False) date_joined = Column(DateTime, server_default=sql.func.now()) last_login = Column(DateTime, nullable=False, server_default=sql.func.now()) disabled_for = Column( From a6787880215e0207b931ebea06cafd71dcff3bc7 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Fri, 4 Jan 2019 13:31:26 -0800 Subject: [PATCH 02/18] migration after adding field --- ...dd_field_on_user_model_for_is_moderator.py | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py diff --git a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py new file mode 100644 index 000000000000..145bd3aabf1c --- /dev/null +++ b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py @@ -0,0 +1,45 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Add field on User model for is_moderator + +Revision ID: 3db69c05dd11 +Revises: 2d6390eebe90 +Create Date: 2019-01-04 21:29:45.455607 +""" + +from alembic import op +import sqlalchemy as sa + + +revision = '3db69c05dd11' +down_revision = '2d6390eebe90' + +# Note: It is VERY important to ensure that a migration does not lock for a +# long period of time and to ensure that each individual migration does +# not break compatibility with the *previous* version of the code base. +# This is because the migrations will be ran automatically as part of the +# deployment process, but while the previous version of the code is still +# up and running. Thus backwards incompatible changes must be broken up +# over multiple migrations inside of multiple pull requests in order to +# phase them in over multiple deploys. + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('users', sa.Column('is_moderator', sa.Boolean(), nullable=False)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('users', 'is_moderator') + # ### end Alembic commands ### From 830607f6affbf25c5790050c13f025c607cd8645 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Fri, 4 Jan 2019 13:40:41 -0800 Subject: [PATCH 03/18] plumb moderator through constructor and principal setting --- tests/common/db/accounts.py | 1 + warehouse/accounts/__init__.py | 2 ++ warehouse/accounts/interfaces.py | 2 +- warehouse/accounts/services.py | 3 ++- warehouse/admin/templates/admin/users/detail.html | 1 + warehouse/admin/templates/admin/users/list.html | 2 ++ warehouse/admin/views/users.py | 1 + 7 files changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/common/db/accounts.py b/tests/common/db/accounts.py index 39449ea9c62c..a86f52a714ef 100644 --- a/tests/common/db/accounts.py +++ b/tests/common/db/accounts.py @@ -29,6 +29,7 @@ class Meta: password = "!" is_active = True is_superuser = False + is_moderator = False date_joined = factory.fuzzy.FuzzyNaiveDateTime( datetime.datetime(2005, 1, 1), datetime.datetime(2010, 1, 1) ) diff --git a/warehouse/accounts/__init__.py b/warehouse/accounts/__init__.py index 0d747d07757e..1f3a68a1e20a 100644 --- a/warehouse/accounts/__init__.py +++ b/warehouse/accounts/__init__.py @@ -98,6 +98,8 @@ def _authenticate(userid, request): if user.is_superuser: principals.append("group:admins") + if user.is_moderator or user.is_superuser: + principals.append("group:moderators") return principals diff --git a/warehouse/accounts/interfaces.py b/warehouse/accounts/interfaces.py index 9671b435c5e7..83a7c8f0196f 100644 --- a/warehouse/accounts/interfaces.py +++ b/warehouse/accounts/interfaces.py @@ -66,7 +66,7 @@ def check_password(user_id, password, *, tags=None): checking the password. """ - def create_user(username, name, password, is_active=False, is_superuser=False): + def create_user(username, name, password, is_active=False, is_superuser=False, is_moderator=False): """ Accepts a user object, and attempts to create a user with those attributes. diff --git a/warehouse/accounts/services.py b/warehouse/accounts/services.py index 0874196cf51e..7d0db3d9ed2a 100644 --- a/warehouse/accounts/services.py +++ b/warehouse/accounts/services.py @@ -172,7 +172,7 @@ def check_password(self, userid, password, *, tags=None): return False def create_user( - self, username, name, password, is_active=False, is_superuser=False + self, username, name, password, is_active=False, is_superuser=False, is_moderator=True, ): user = User( @@ -181,6 +181,7 @@ def create_user( password=self.hasher.hash(password), is_active=is_active, is_superuser=is_superuser, + is_moderator=is_moderator, ) self.db.add(user) self.db.flush() # flush the db now so user.id is available diff --git a/warehouse/admin/templates/admin/users/detail.html b/warehouse/admin/templates/admin/users/detail.html index b3f1d33ae6af..bbe34f512aca 100644 --- a/warehouse/admin/templates/admin/users/detail.html +++ b/warehouse/admin/templates/admin/users/detail.html @@ -143,6 +143,7 @@

Permissions

{{ render_field("Active", form.is_active, "is-active") }} {{ render_field("Superuser", form.is_superuser, "is-superuser")}} + {{ render_field("Moderator", form.is_moderator, "is-moderator")}}
diff --git a/warehouse/admin/templates/admin/users/list.html b/warehouse/admin/templates/admin/users/list.html index a181ed7f79a2..68753d98d7d3 100644 --- a/warehouse/admin/templates/admin/users/list.html +++ b/warehouse/admin/templates/admin/users/list.html @@ -44,6 +44,7 @@ Name Email Admin + Moderator Active @@ -56,6 +57,7 @@ {{ user.name }} {{ user.email }} {% if user.is_superuser %}{% endif %} + {% if user.is_moderator %}{% endif %} {% if user.is_active %}{% endif %} {% endfor %} diff --git a/warehouse/admin/views/users.py b/warehouse/admin/views/users.py index 641d549c278a..42136b1d7803 100644 --- a/warehouse/admin/views/users.py +++ b/warehouse/admin/views/users.py @@ -84,6 +84,7 @@ class UserForm(forms.Form): is_active = wtforms.fields.BooleanField() is_superuser = wtforms.fields.BooleanField() + is_moderator = wtforms.fields.BooleanField() emails = wtforms.fields.FieldList(wtforms.fields.FormField(EmailField)) From 47a1f8cea787c2118e72d06d4fbd6aa4b241e21a Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Fri, 4 Jan 2019 14:14:57 -0800 Subject: [PATCH 04/18] update tests, format, lint --- tests/unit/accounts/test_core.py | 11 ++++++++--- warehouse/accounts/interfaces.py | 9 ++++++++- warehouse/accounts/services.py | 8 +++++++- ...05dd11_add_field_on_user_model_for_is_moderator.py | 11 ++++++----- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/tests/unit/accounts/test_core.py b/tests/unit/accounts/test_core.py index d144707bd5a1..1406c344eda5 100644 --- a/tests/unit/accounts/test_core.py +++ b/tests/unit/accounts/test_core.py @@ -207,10 +207,15 @@ def test_via_basic_auth_compromised( class TestAuthenticate: @pytest.mark.parametrize( - ("is_superuser", "expected"), [(False, []), (True, ["group:admins"])] + ("is_superuser", "is_moderator", "expected"), + [ + (False, False, []), + (True, False, ["group:admins", "group:moderators"]), + (False, True, ["group:moderators"]), + ], ) - def test_with_user(self, is_superuser, expected): - user = pretend.stub(is_superuser=is_superuser) + def test_with_user(self, is_superuser, is_moderator, expected): + user = pretend.stub(is_superuser=is_superuser, is_moderator=is_moderator) service = pretend.stub(get_user=pretend.call_recorder(lambda userid: user)) request = pretend.stub(find_service=lambda iface, context: service) diff --git a/warehouse/accounts/interfaces.py b/warehouse/accounts/interfaces.py index 83a7c8f0196f..67b3da4dba2a 100644 --- a/warehouse/accounts/interfaces.py +++ b/warehouse/accounts/interfaces.py @@ -66,7 +66,14 @@ def check_password(user_id, password, *, tags=None): checking the password. """ - def create_user(username, name, password, is_active=False, is_superuser=False, is_moderator=False): + def create_user( + username, + name, + password, + is_active=False, + is_superuser=False, + is_moderator=False, + ): """ Accepts a user object, and attempts to create a user with those attributes. diff --git a/warehouse/accounts/services.py b/warehouse/accounts/services.py index 7d0db3d9ed2a..a64a7d1eea86 100644 --- a/warehouse/accounts/services.py +++ b/warehouse/accounts/services.py @@ -172,7 +172,13 @@ def check_password(self, userid, password, *, tags=None): return False def create_user( - self, username, name, password, is_active=False, is_superuser=False, is_moderator=True, + self, + username, + name, + password, + is_active=False, + is_superuser=False, + is_moderator=True, ): user = User( diff --git a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py index 145bd3aabf1c..817ff3a1ea90 100644 --- a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py +++ b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py @@ -17,12 +17,12 @@ Create Date: 2019-01-04 21:29:45.455607 """ -from alembic import op import sqlalchemy as sa +from alembic import op -revision = '3db69c05dd11' -down_revision = '2d6390eebe90' +revision = "3db69c05dd11" +down_revision = "2d6390eebe90" # Note: It is VERY important to ensure that a migration does not lock for a # long period of time and to ensure that each individual migration does @@ -33,13 +33,14 @@ # over multiple migrations inside of multiple pull requests in order to # phase them in over multiple deploys. + def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.add_column('users', sa.Column('is_moderator', sa.Boolean(), nullable=False)) + op.add_column("users", sa.Column("is_moderator", sa.Boolean(), nullable=False)) # ### end Alembic commands ### def downgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.drop_column('users', 'is_moderator') + op.drop_column("users", "is_moderator") # ### end Alembic commands ### From d5b5a71a37cdafe025845eae4dbcb42c2a73f7d3 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Fri, 4 Jan 2019 14:30:58 -0800 Subject: [PATCH 05/18] edit generated migration --- .../3db69c05dd11_add_field_on_user_model_for_is_moderator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py index 817ff3a1ea90..71489ccafea4 100644 --- a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py +++ b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py @@ -36,7 +36,7 @@ def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.add_column("users", sa.Column("is_moderator", sa.Boolean(), nullable=False)) + op.add_column("users", sa.Column("is_moderator", sa.Boolean(), nullable=False, server_default='False')) # ### end Alembic commands ### From d87adcfce46d41d7d5aab6c9ca366df1c75090e2 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Fri, 4 Jan 2019 14:43:45 -0800 Subject: [PATCH 06/18] fix default value for creating users --- warehouse/accounts/services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/warehouse/accounts/services.py b/warehouse/accounts/services.py index a64a7d1eea86..1670f95bc42f 100644 --- a/warehouse/accounts/services.py +++ b/warehouse/accounts/services.py @@ -178,7 +178,7 @@ def create_user( password, is_active=False, is_superuser=False, - is_moderator=True, + is_moderator=False, ): user = User( From 8a0144aafce5fdc3ff2f84544f3ff5c746fdee5c Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Fri, 4 Jan 2019 15:22:29 -0800 Subject: [PATCH 07/18] reformat migration --- .../3db69c05dd11_add_field_on_user_model_for_is_moderator.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py index 71489ccafea4..135a8d987810 100644 --- a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py +++ b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py @@ -36,7 +36,10 @@ def upgrade(): # ### commands auto generated by Alembic - please adjust! ### - op.add_column("users", sa.Column("is_moderator", sa.Boolean(), nullable=False, server_default='False')) + op.add_column( + "users", + sa.Column("is_moderator", sa.Boolean(), nullable=False, server_default="False"), + ) # ### end Alembic commands ### From 19ba7fbd41aedf9c74038f07a1f7607224900c4d Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Fri, 4 Jan 2019 15:04:35 -0800 Subject: [PATCH 08/18] add acl permission moderator --- docs/application.rst | 3 +++ warehouse/config.py | 2 +- warehouse/packaging/models.py | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/application.rst b/docs/application.rst index 4bd2d6b95b3c..4e971553818f 100644 --- a/docs/application.rst +++ b/docs/application.rst @@ -56,6 +56,9 @@ Warehouse serves three main classes of users: Dustin Ingram, and Donald Stufft, who add classifiers, ban spam/malware projects, help users with account recovery, and so on. There are under ten such admins. +4. *PyPI application moderators*. These users have a subset of the + permissions of *PyPI application administrators* to assist in some + routine administration tasks. Since reads are *much* more common than writes (much more goes out than goes in), we try to cache as much as possible. This is a big reason diff --git a/warehouse/config.py b/warehouse/config.py index 26292a500d27..4550ac0401bb 100644 --- a/warehouse/config.py +++ b/warehouse/config.py @@ -55,7 +55,7 @@ class RootFactory: __parent__ = None __name__ = None - __acl__ = [(Allow, "group:admins", "admin"), (Allow, Authenticated, "manage:user")] + __acl__ = [(Allow, "group:admins", "admin"), (Allow, "group:moderators", "moderator"), (Allow, Authenticated, "manage:user")] def __init__(self, request): pass diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 1d1de7ced6bc..747b54f87b4b 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -165,7 +165,7 @@ def __getitem__(self, version): def __acl__(self): session = orm.object_session(self) - acls = [(Allow, "group:admins", "admin")] + acls = [(Allow, "group:admins", "admin"), (Allow, "group:moderators", "moderator")] # Get all of the users for this project. query = session.query(Role).filter(Role.project == self) From 16ceea9e32553d7a6b4d9cefdb692defae3bdda3 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Mon, 7 Jan 2019 10:32:50 -0800 Subject: [PATCH 09/18] Leverage moderator acl to view and edit certain admin dashboard features --- warehouse/admin/views/blacklist.py | 5 +++-- warehouse/admin/views/classifiers.py | 5 +++-- warehouse/admin/views/core.py | 2 +- warehouse/admin/views/emails.py | 6 ++++-- warehouse/admin/views/flags.py | 3 ++- warehouse/admin/views/journals.py | 2 +- warehouse/admin/views/projects.py | 25 ++++++++++++++++++++----- warehouse/admin/views/squats.py | 3 ++- warehouse/admin/views/users.py | 12 +++++++++++- 9 files changed, 47 insertions(+), 16 deletions(-) diff --git a/warehouse/admin/views/blacklist.py b/warehouse/admin/views/blacklist.py index ed415966a8d8..f142d4462e48 100644 --- a/warehouse/admin/views/blacklist.py +++ b/warehouse/admin/views/blacklist.py @@ -29,7 +29,8 @@ @view_config( route_name="admin.blacklist.list", renderer="admin/blacklist/list.html", - permission="admin", + permission="moderator", + request_method="GET", uses_session=True, ) def blacklist(request): @@ -68,7 +69,7 @@ def blacklist(request): @view_config( route_name="admin.blacklist.add", renderer="admin/blacklist/confirm.html", - permission="admin", + permission="moderator", request_method="GET", uses_session=True, ) diff --git a/warehouse/admin/views/classifiers.py b/warehouse/admin/views/classifiers.py index 1d58cc71b969..75dcb3643919 100644 --- a/warehouse/admin/views/classifiers.py +++ b/warehouse/admin/views/classifiers.py @@ -19,7 +19,8 @@ @view_config( route_name="admin.classifiers", renderer="admin/classifiers/index.html", - permission="admin", + permission="moderator", + request_method="GET", uses_session=True, ) def get_classifiers(request): @@ -30,7 +31,7 @@ def get_classifiers(request): @view_defaults( route_name="admin.classifiers.add", - permission="admin", + permission="moderator", request_method="POST", uses_session=True, require_methods=False, diff --git a/warehouse/admin/views/core.py b/warehouse/admin/views/core.py index 9e92d1ee8220..a24768cd397c 100644 --- a/warehouse/admin/views/core.py +++ b/warehouse/admin/views/core.py @@ -23,7 +23,7 @@ def forbidden(exc, request): @view_config( route_name="admin.dashboard", renderer="admin/dashboard.html", - permission="admin", + permission="moderator", uses_session=True, ) def dashboard(request): diff --git a/warehouse/admin/views/emails.py b/warehouse/admin/views/emails.py index 5001215fa16a..dd1c576d5bfe 100644 --- a/warehouse/admin/views/emails.py +++ b/warehouse/admin/views/emails.py @@ -25,7 +25,8 @@ @view_config( route_name="admin.emails.list", renderer="admin/emails/list.html", - permission="admin", + permission="moderator", + request_method="GET", uses_session=True, ) def email_list(request): @@ -62,7 +63,8 @@ def email_list(request): @view_config( route_name="admin.emails.detail", renderer="admin/emails/detail.html", - permission="admin", + permission="moderator", + request_method="GET", uses_session=True, ) def email_detail(request): diff --git a/warehouse/admin/views/flags.py b/warehouse/admin/views/flags.py index 02b0fc4e73e1..0e17d9b19f82 100644 --- a/warehouse/admin/views/flags.py +++ b/warehouse/admin/views/flags.py @@ -19,7 +19,8 @@ @view_config( route_name="admin.flags", renderer="admin/flags/index.html", - permission="admin", + permission="moderator", + request_method="GET", uses_session=True, ) def get_flags(request): diff --git a/warehouse/admin/views/journals.py b/warehouse/admin/views/journals.py index 21bc033c8ebe..e1b2347d7c2b 100644 --- a/warehouse/admin/views/journals.py +++ b/warehouse/admin/views/journals.py @@ -25,7 +25,7 @@ @view_config( route_name="admin.journals.list", renderer="admin/journals/list.html", - permission="admin", + permission="moderator", uses_session=True, ) def journals_list(request): diff --git a/warehouse/admin/views/projects.py b/warehouse/admin/views/projects.py index 4226fff7dea2..f25327847eb9 100644 --- a/warehouse/admin/views/projects.py +++ b/warehouse/admin/views/projects.py @@ -31,7 +31,8 @@ @view_config( route_name="admin.project.list", renderer="admin/projects/list.html", - permission="admin", + permission="moderator", + request_method="GET", uses_session=True, ) def project_list(request): @@ -63,10 +64,21 @@ def project_list(request): return {"projects": projects, "query": q} + +@view_config( + route_name="admin.project.detail", + renderer="admin/projects/detail.html", + permission="moderator", + request_method="GET", + uses_session=True, + require_csrf=True, + require_methods=False, +) @view_config( route_name="admin.project.detail", renderer="admin/projects/detail.html", permission="admin", + request_method="POST", uses_session=True, require_csrf=True, require_methods=False, @@ -138,7 +150,8 @@ def project_detail(project, request): @view_config( route_name="admin.project.releases", renderer="admin/projects/releases_list.html", - permission="admin", + permission="moderator", + request_method="GET", uses_session=True, ) def releases_list(project, request): @@ -186,7 +199,8 @@ def releases_list(project, request): @view_config( route_name="admin.project.release", renderer="admin/projects/release_detail.html", - permission="admin", + permission="moderator", + request_method="GET", uses_session=True, ) def release_detail(release, request): @@ -204,7 +218,8 @@ def release_detail(release, request): @view_config( route_name="admin.project.journals", renderer="admin/projects/journals_list.html", - permission="admin", + permission="moderator", + request_method="GET", uses_session=True, ) def journals_list(project, request): @@ -252,7 +267,7 @@ def journals_list(project, request): @view_config( route_name="admin.project.set_upload_limit", - permission="admin", + permission="moderator", request_method="POST", uses_session=True, require_methods=False, diff --git a/warehouse/admin/views/squats.py b/warehouse/admin/views/squats.py index 62b9c2d0258b..e41fad12d76f 100644 --- a/warehouse/admin/views/squats.py +++ b/warehouse/admin/views/squats.py @@ -19,7 +19,8 @@ @view_config( route_name="admin.squats", renderer="admin/squats/index.html", - permission="admin", + permission="moderator", + request_method="GET", uses_session=True, ) def get_squats(request): diff --git a/warehouse/admin/views/users.py b/warehouse/admin/views/users.py index 42136b1d7803..51cb2053a12d 100644 --- a/warehouse/admin/views/users.py +++ b/warehouse/admin/views/users.py @@ -30,7 +30,7 @@ @view_config( route_name="admin.user.list", renderer="admin/users/list.html", - permission="admin", + permission="moderator", uses_session=True, ) def user_list(request): @@ -89,10 +89,20 @@ class UserForm(forms.Form): emails = wtforms.fields.FieldList(wtforms.fields.FormField(EmailField)) +@view_config( + route_name="admin.user.detail", + renderer="admin/users/detail.html", + permission="moderator", + request_method='GET', + uses_session=True, + require_csrf=True, + require_methods=False, +) @view_config( route_name="admin.user.detail", renderer="admin/users/detail.html", permission="admin", + request_method='POST', uses_session=True, require_csrf=True, require_methods=False, From 6f38fc168242ec535ecb883671b6183e205356b3 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Mon, 7 Jan 2019 10:37:58 -0800 Subject: [PATCH 10/18] allow moderators to deprecate classifier --- warehouse/admin/views/classifiers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/warehouse/admin/views/classifiers.py b/warehouse/admin/views/classifiers.py index 75dcb3643919..dc16910b0fad 100644 --- a/warehouse/admin/views/classifiers.py +++ b/warehouse/admin/views/classifiers.py @@ -88,7 +88,7 @@ def add_child_classifier(self): @view_config( route_name="admin.classifiers.deprecate", - permission="admin", + permission="moderator", request_method="POST", uses_session=True, require_methods=False, From 252d1a0e3e8b179b0778b2d96e25a0fee7960637 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Mon, 7 Jan 2019 15:37:38 -0800 Subject: [PATCH 11/18] Update admin pages to disable buttons when a moderator visits the pages --- warehouse/admin/templates/admin/blacklist/list.html | 8 ++++---- warehouse/admin/templates/admin/flags/index.html | 6 +++--- warehouse/admin/templates/admin/projects/delete.html | 4 ++-- warehouse/admin/templates/admin/projects/detail.html | 12 ++++++------ warehouse/admin/templates/admin/squats/index.html | 2 +- warehouse/admin/templates/admin/users/detail.html | 6 +++--- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/warehouse/admin/templates/admin/blacklist/list.html b/warehouse/admin/templates/admin/blacklist/list.html index ed958c975759..3d6165238139 100644 --- a/warehouse/admin/templates/admin/blacklist/list.html +++ b/warehouse/admin/templates/admin/blacklist/list.html @@ -63,7 +63,7 @@ - @@ -96,19 +96,19 @@

Blacklist project

- +
- +
- +
diff --git a/warehouse/admin/templates/admin/flags/index.html b/warehouse/admin/templates/admin/flags/index.html index 1fa3657a4407..a7c6f1270926 100644 --- a/warehouse/admin/templates/admin/flags/index.html +++ b/warehouse/admin/templates/admin/flags/index.html @@ -43,10 +43,10 @@

Edit Flags

{{ flag.id }} - + {{ flag.notify }} - - + + {% endfor %} diff --git a/warehouse/admin/templates/admin/projects/delete.html b/warehouse/admin/templates/admin/projects/delete.html index 17ade1e46d9e..f650f2fc32d9 100644 --- a/warehouse/admin/templates/admin/projects/delete.html +++ b/warehouse/admin/templates/admin/projects/delete.html @@ -35,13 +35,13 @@

Delete Project

- +
- +
diff --git a/warehouse/admin/templates/admin/projects/detail.html b/warehouse/admin/templates/admin/projects/detail.html index 1087d789d5c9..fdf5ce618eb5 100644 --- a/warehouse/admin/templates/admin/projects/detail.html +++ b/warehouse/admin/templates/admin/projects/detail.html @@ -77,7 +77,7 @@

Maintainers:

{{ role.user.username }} {{ role.role_name }} -
- +
From e6dea69272cb28448c9e5b905812881ed57e38a6 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Mon, 7 Jan 2019 15:42:34 -0800 Subject: [PATCH 12/18] update readme to better describe the role of moderators --- docs/application.rst | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/application.rst b/docs/application.rst index 4e971553818f..8ca77cade7d5 100644 --- a/docs/application.rst +++ b/docs/application.rst @@ -42,7 +42,7 @@ PyPI. People and groups who want to run their own package indexes usually use other tools, like `devpi `_. -Warehouse serves three main classes of users: +Warehouse serves four main classes of users: 1. *People who are not logged in.* This accounts for the majority of browser traffic and all API download traffic. @@ -52,13 +52,14 @@ Warehouse serves three main classes of users: available to a logged-in user other than to manage projects they own/maintain. As of March 2018, PyPI had about 270,000 users, and Test PyPI had about 30,000 users. -3. *PyPI application administrators*, e.g., Ernest W. Durbin III, - Dustin Ingram, and Donald Stufft, who add classifiers, ban +3. *PyPI application moderators*. These users have a subset of the + permissions of *PyPI application administrators* to assist in some + routine administration tasks such as adding new trove classifiers and + adjusting upload limits for distribution packages. +4. *PyPI application administrators*, e.g., Ernest W. Durbin III, + Dustin Ingram, and Donald Stufft, who can ban spam/malware projects, help users with account recovery, and so on. There are under ten such admins. -4. *PyPI application moderators*. These users have a subset of the - permissions of *PyPI application administrators* to assist in some - routine administration tasks. Since reads are *much* more common than writes (much more goes out than goes in), we try to cache as much as possible. This is a big reason From 42e9ac69f47201f93f6012141fe02f9b203ccb10 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Mon, 7 Jan 2019 16:06:11 -0800 Subject: [PATCH 13/18] lint --- warehouse/admin/views/projects.py | 1 - warehouse/admin/views/users.py | 4 ++-- warehouse/config.py | 6 +++++- warehouse/packaging/models.py | 5 ++++- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/warehouse/admin/views/projects.py b/warehouse/admin/views/projects.py index f25327847eb9..8b794d352598 100644 --- a/warehouse/admin/views/projects.py +++ b/warehouse/admin/views/projects.py @@ -64,7 +64,6 @@ def project_list(request): return {"projects": projects, "query": q} - @view_config( route_name="admin.project.detail", renderer="admin/projects/detail.html", diff --git a/warehouse/admin/views/users.py b/warehouse/admin/views/users.py index 51cb2053a12d..39fbb4e7883e 100644 --- a/warehouse/admin/views/users.py +++ b/warehouse/admin/views/users.py @@ -93,7 +93,7 @@ class UserForm(forms.Form): route_name="admin.user.detail", renderer="admin/users/detail.html", permission="moderator", - request_method='GET', + request_method="GET", uses_session=True, require_csrf=True, require_methods=False, @@ -102,7 +102,7 @@ class UserForm(forms.Form): route_name="admin.user.detail", renderer="admin/users/detail.html", permission="admin", - request_method='POST', + request_method="POST", uses_session=True, require_csrf=True, require_methods=False, diff --git a/warehouse/config.py b/warehouse/config.py index 4550ac0401bb..5e72a1b59be0 100644 --- a/warehouse/config.py +++ b/warehouse/config.py @@ -55,7 +55,11 @@ class RootFactory: __parent__ = None __name__ = None - __acl__ = [(Allow, "group:admins", "admin"), (Allow, "group:moderators", "moderator"), (Allow, Authenticated, "manage:user")] + __acl__ = [ + (Allow, "group:admins", "admin"), + (Allow, "group:moderators", "moderator"), + (Allow, Authenticated, "manage:user"), + ] def __init__(self, request): pass diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 747b54f87b4b..e428c3a0aef4 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -165,7 +165,10 @@ def __getitem__(self, version): def __acl__(self): session = orm.object_session(self) - acls = [(Allow, "group:admins", "admin"), (Allow, "group:moderators", "moderator")] + acls = [ + (Allow, "group:admins", "admin"), + (Allow, "group:moderators", "moderator"), + ] # Get all of the users for this project. query = session.query(Role).filter(Role.project == self) From 43feab4656e26fd8ce5f089bf1244c7beb530a71 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Wed, 9 Jan 2019 09:29:48 -0800 Subject: [PATCH 14/18] incorporate feedback from di --- tests/unit/accounts/test_core.py | 1 + tests/unit/packaging/test_models.py | 2 ++ warehouse/accounts/interfaces.py | 2 -- warehouse/accounts/models.py | 4 ++-- warehouse/accounts/services.py | 6 ++---- ...dd_field_on_user_model_for_is_moderator.py | 19 +++++-------------- 6 files changed, 12 insertions(+), 22 deletions(-) diff --git a/tests/unit/accounts/test_core.py b/tests/unit/accounts/test_core.py index 1406c344eda5..ad78ac431a54 100644 --- a/tests/unit/accounts/test_core.py +++ b/tests/unit/accounts/test_core.py @@ -212,6 +212,7 @@ class TestAuthenticate: (False, False, []), (True, False, ["group:admins", "group:moderators"]), (False, True, ["group:moderators"]), + (True, True, ["group:admins", "group:moderators"]), ], ) def test_with_user(self, is_superuser, is_moderator, expected): diff --git a/tests/unit/packaging/test_models.py b/tests/unit/packaging/test_models.py index 5b213926193f..0e25e723690c 100644 --- a/tests/unit/packaging/test_models.py +++ b/tests/unit/packaging/test_models.py @@ -123,6 +123,7 @@ def test_acl(self, db_session): assert acls == [ (Allow, "group:admins", "admin"), + (Allow, "group:moderators", "moderator"), (Allow, str(owner1.user.id), ["manage:project", "upload"]), (Allow, str(owner2.user.id), ["manage:project", "upload"]), (Allow, str(maintainer1.user.id), ["upload"]), @@ -291,6 +292,7 @@ def test_acl(self, db_session): assert acls == [ (Allow, "group:admins", "admin"), + (Allow, "group:moderators", "moderator"), (Allow, str(owner1.user.id), ["manage:project", "upload"]), (Allow, str(owner2.user.id), ["manage:project", "upload"]), (Allow, str(maintainer1.user.id), ["upload"]), diff --git a/warehouse/accounts/interfaces.py b/warehouse/accounts/interfaces.py index 67b3da4dba2a..5b2d60babed5 100644 --- a/warehouse/accounts/interfaces.py +++ b/warehouse/accounts/interfaces.py @@ -71,8 +71,6 @@ def create_user( name, password, is_active=False, - is_superuser=False, - is_moderator=False, ): """ Accepts a user object, and attempts to create a user with those diff --git a/warehouse/accounts/models.py b/warehouse/accounts/models.py index 1ef3f6845ab8..27e3bd0fdec2 100644 --- a/warehouse/accounts/models.py +++ b/warehouse/accounts/models.py @@ -71,8 +71,8 @@ class User(SitemapMixin, db.Model): password = Column(String(length=128), nullable=False) password_date = Column(DateTime, nullable=True, server_default=sql.func.now()) is_active = Column(Boolean, nullable=False) - is_superuser = Column(Boolean, nullable=False) - is_moderator = Column(Boolean, nullable=False) + is_superuser = Column(Boolean, nullable=False, server_default=sql.expression.false()) + is_moderator = Column(Boolean, nullable=False, server_default=sql.expression.false()) date_joined = Column(DateTime, server_default=sql.func.now()) last_login = Column(DateTime, nullable=False, server_default=sql.func.now()) disabled_for = Column( diff --git a/warehouse/accounts/services.py b/warehouse/accounts/services.py index 1670f95bc42f..7923bfd57470 100644 --- a/warehouse/accounts/services.py +++ b/warehouse/accounts/services.py @@ -177,8 +177,6 @@ def create_user( name, password, is_active=False, - is_superuser=False, - is_moderator=False, ): user = User( @@ -186,8 +184,8 @@ def create_user( name=name, password=self.hasher.hash(password), is_active=is_active, - is_superuser=is_superuser, - is_moderator=is_moderator, + is_superuser=False, + is_moderator=False, ) self.db.add(user) self.db.flush() # flush the db now so user.id is available diff --git a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py index 135a8d987810..651c7ad3945b 100644 --- a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py +++ b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py @@ -24,26 +24,17 @@ revision = "3db69c05dd11" down_revision = "2d6390eebe90" -# Note: It is VERY important to ensure that a migration does not lock for a -# long period of time and to ensure that each individual migration does -# not break compatibility with the *previous* version of the code base. -# This is because the migrations will be ran automatically as part of the -# deployment process, but while the previous version of the code is still -# up and running. Thus backwards incompatible changes must be broken up -# over multiple migrations inside of multiple pull requests in order to -# phase them in over multiple deploys. - def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### op.add_column( "users", - sa.Column("is_moderator", sa.Boolean(), nullable=False, server_default="False"), + sa.Column("is_moderator", sa.Boolean(), nullable=False, server_default=sa.sql.expression.false()), + ) + op.alter_column( + "users", + sa.Column("is_superuser", sa.Boolean(), nullable=False, server_default=sa.sql.expression.false()), ) - # ### end Alembic commands ### def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### op.drop_column("users", "is_moderator") - # ### end Alembic commands ### From 010ae8d8f48b271e67e2d8c613cbf2a2e763a811 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Wed, 9 Jan 2019 09:35:03 -0800 Subject: [PATCH 15/18] reformat --- warehouse/accounts/interfaces.py | 7 +------ warehouse/accounts/models.py | 8 ++++++-- warehouse/accounts/services.py | 8 +------- ...d11_add_field_on_user_model_for_is_moderator.py | 14 ++++++++++++-- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/warehouse/accounts/interfaces.py b/warehouse/accounts/interfaces.py index 5b2d60babed5..fd6bf30a13cb 100644 --- a/warehouse/accounts/interfaces.py +++ b/warehouse/accounts/interfaces.py @@ -66,12 +66,7 @@ def check_password(user_id, password, *, tags=None): checking the password. """ - def create_user( - username, - name, - password, - is_active=False, - ): + def create_user(username, name, password, is_active=False): """ Accepts a user object, and attempts to create a user with those attributes. diff --git a/warehouse/accounts/models.py b/warehouse/accounts/models.py index 27e3bd0fdec2..d28bde3b572e 100644 --- a/warehouse/accounts/models.py +++ b/warehouse/accounts/models.py @@ -71,8 +71,12 @@ class User(SitemapMixin, db.Model): password = Column(String(length=128), nullable=False) password_date = Column(DateTime, nullable=True, server_default=sql.func.now()) is_active = Column(Boolean, nullable=False) - is_superuser = Column(Boolean, nullable=False, server_default=sql.expression.false()) - is_moderator = Column(Boolean, nullable=False, server_default=sql.expression.false()) + is_superuser = Column( + Boolean, nullable=False, server_default=sql.expression.false() + ) + is_moderator = Column( + Boolean, nullable=False, server_default=sql.expression.false() + ) date_joined = Column(DateTime, server_default=sql.func.now()) last_login = Column(DateTime, nullable=False, server_default=sql.func.now()) disabled_for = Column( diff --git a/warehouse/accounts/services.py b/warehouse/accounts/services.py index 7923bfd57470..8f7a5c510f51 100644 --- a/warehouse/accounts/services.py +++ b/warehouse/accounts/services.py @@ -171,13 +171,7 @@ def check_password(self, userid, password, *, tags=None): return False - def create_user( - self, - username, - name, - password, - is_active=False, - ): + def create_user(self, username, name, password, is_active=False): user = User( username=username, diff --git a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py index 651c7ad3945b..9a59d67036ce 100644 --- a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py +++ b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py @@ -28,11 +28,21 @@ def upgrade(): op.add_column( "users", - sa.Column("is_moderator", sa.Boolean(), nullable=False, server_default=sa.sql.expression.false()), + sa.Column( + "is_moderator", + sa.Boolean(), + nullable=False, + server_default=sa.sql.expression.false(), + ), ) op.alter_column( "users", - sa.Column("is_superuser", sa.Boolean(), nullable=False, server_default=sa.sql.expression.false()), + sa.Column( + "is_superuser", + sa.Boolean(), + nullable=False, + server_default=sa.sql.expression.false(), + ), ) From 7e65656ae561f140b33c0bf3c93e6faf28bae536 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Thu, 10 Jan 2019 12:30:27 -0800 Subject: [PATCH 16/18] modify migration post merge --- warehouse/accounts/models.py | 1 + ...11_add_field_on_user_model_for_is_moderator.py | 15 +++------------ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/warehouse/accounts/models.py b/warehouse/accounts/models.py index a2d7dc28e6e8..81daf0adfb09 100644 --- a/warehouse/accounts/models.py +++ b/warehouse/accounts/models.py @@ -72,6 +72,7 @@ class User(SitemapMixin, db.Model): password_date = Column(DateTime, nullable=True, server_default=sql.func.now()) is_active = Column(Boolean, nullable=False, server_default=sql.false()) is_superuser = Column(Boolean, nullable=False, server_default=sql.false()) + is_moderator = Column(Boolean, nullable=False, server_default=sql.false()) date_joined = Column(DateTime, server_default=sql.func.now()) last_login = Column(DateTime, nullable=False, server_default=sql.func.now()) disabled_for = Column( diff --git a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py index 9a59d67036ce..013b0cb20e47 100644 --- a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py +++ b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py @@ -13,7 +13,7 @@ Add field on User model for is_moderator Revision ID: 3db69c05dd11 -Revises: 2d6390eebe90 +Revises: 67f52a64a389 Create Date: 2019-01-04 21:29:45.455607 """ @@ -22,7 +22,7 @@ from alembic import op revision = "3db69c05dd11" -down_revision = "2d6390eebe90" +down_revision = "67f52a64a389" def upgrade(): @@ -32,16 +32,7 @@ def upgrade(): "is_moderator", sa.Boolean(), nullable=False, - server_default=sa.sql.expression.false(), - ), - ) - op.alter_column( - "users", - sa.Column( - "is_superuser", - sa.Boolean(), - nullable=False, - server_default=sa.sql.expression.false(), + server_default=sa.sql.false(), ), ) From e25faa3f9af07f38b43fde3d30ed23eebbe58efd Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Thu, 10 Jan 2019 12:48:46 -0800 Subject: [PATCH 17/18] lint --- .../3db69c05dd11_add_field_on_user_model_for_is_moderator.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py index 013b0cb20e47..55cf318c5ea8 100644 --- a/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py +++ b/warehouse/migrations/versions/3db69c05dd11_add_field_on_user_model_for_is_moderator.py @@ -29,10 +29,7 @@ def upgrade(): op.add_column( "users", sa.Column( - "is_moderator", - sa.Boolean(), - nullable=False, - server_default=sa.sql.false(), + "is_moderator", sa.Boolean(), nullable=False, server_default=sa.sql.false() ), ) From f31791df5ac6ba3263ed17b02f173e03f76de1d2 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Tue, 15 Jan 2019 16:32:46 -0800 Subject: [PATCH 18/18] disable nuke user and add admin to user dropdown for moderators --- warehouse/admin/templates/admin/users/detail.html | 2 +- warehouse/templates/includes/current-user-indicator.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/warehouse/admin/templates/admin/users/detail.html b/warehouse/admin/templates/admin/users/detail.html index c76db78b7f1f..e146851f26fd 100644 --- a/warehouse/admin/templates/admin/users/detail.html +++ b/warehouse/admin/templates/admin/users/detail.html @@ -68,7 +68,7 @@

Actions

  • -