From 2154da4767143f1cfb06b869d734e7a7d2af9642 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 23 Mar 2018 08:36:49 -0500 Subject: [PATCH 1/9] Add read-only AdminFlag --- tests/unit/test_db.py | 47 ++++++++++++++++++++++++++++++++++++++++++- warehouse/db.py | 6 ++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index e4b9eb7aad7e..56b3b0efed77 100644 --- a/tests/unit/test_db.py +++ b/tests/unit/test_db.py @@ -147,7 +147,10 @@ def test_creates_engine(monkeypatch): ], ) def test_create_session(monkeypatch, read_only, tx_status): - session_obj = pretend.stub(close=pretend.call_recorder(lambda: None)) + session_obj = pretend.stub( + close=pretend.call_recorder(lambda: None), + query=lambda *a: pretend.stub(get=lambda *a: None), + ) session_cls = pretend.call_recorder(lambda bind: session_obj) monkeypatch.setattr(db, "Session", session_cls) @@ -200,6 +203,48 @@ def test_create_session(monkeypatch, read_only, tx_status): connection.connection.rollback.calls == [pretend.call()] +@pytest.mark.parametrize("admin_flag, doom_calls", [ + (None, []), + (pretend.stub(enabled=False), []), + ( + pretend.stub(enabled=True, description='flag description'), + [pretend.call()], + ), +]) +def test_create_session_read_only_mode(admin_flag, doom_calls, monkeypatch): + get = pretend.call_recorder(lambda *a: admin_flag) + session_obj = pretend.stub( + close=lambda: None, + query=lambda *a: pretend.stub(get=get), + ) + session_cls = pretend.call_recorder(lambda bind: session_obj) + monkeypatch.setattr(db, "Session", session_cls) + + register = pretend.call_recorder(lambda session, transaction_manager: None) + monkeypatch.setattr(zope.sqlalchemy, "register", register) + + connection = pretend.stub( + connection=pretend.stub( + get_transaction_status=lambda: pretend.stub(), + set_session=lambda **kw: None, + rollback=lambda: None, + ), + info={}, + close=lambda: None, + ) + engine = pretend.stub(connect=pretend.call_recorder(lambda: connection)) + request = pretend.stub( + registry={"sqlalchemy.engine": engine}, + tm=pretend.stub(doom=pretend.call_recorder(lambda: None)), + read_only=False, + add_finished_callback=lambda callback: None, + ) + + assert _create_session(request) is session_obj + assert get.calls == [pretend.call('read-only')] + assert request.tm.doom.calls == doom_calls + + @pytest.mark.parametrize( ("predicates", "expected"), [ diff --git a/warehouse/db.py b/warehouse/db.py index 77879105dca1..7a1ce0f289f1 100644 --- a/warehouse/db.py +++ b/warehouse/db.py @@ -162,6 +162,12 @@ def cleanup(request): session.close() connection.close() + # Check if we're in read-only mode + from warehouse.utils.admin_flags import AdminFlag + flag = session.query(AdminFlag).get('read-only') + if flag and flag.enabled: + request.tm.doom() + # Return our session now that it's created and registered return session From ed1667b31b0f5c8634024e5f1cb63a099724e58e Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 23 Mar 2018 09:09:07 -0500 Subject: [PATCH 2/9] Move admin_flags from /utils to /admin --- tests/common/db/{utils.py => admin.py} | 2 +- tests/unit/accounts/test_views.py | 2 +- tests/unit/{utils/test_admin_flags.py => admin/test_flags.py} | 4 ++-- tests/unit/forklift/test_legacy.py | 2 +- warehouse/accounts/views.py | 2 +- warehouse/admin/__init__.py | 4 ++-- warehouse/{utils/admin_flags.py => admin/flags.py} | 0 warehouse/db.py | 2 +- warehouse/forklift/legacy.py | 2 +- 9 files changed, 10 insertions(+), 10 deletions(-) rename tests/common/db/{utils.py => admin.py} (94%) rename tests/unit/{utils/test_admin_flags.py => admin/test_flags.py} (88%) rename warehouse/{utils/admin_flags.py => admin/flags.py} (100%) diff --git a/tests/common/db/utils.py b/tests/common/db/admin.py similarity index 94% rename from tests/common/db/utils.py rename to tests/common/db/admin.py index 71e6ce1dc7a9..ab2a79da7385 100644 --- a/tests/common/db/utils.py +++ b/tests/common/db/admin.py @@ -12,7 +12,7 @@ import factory.fuzzy -from warehouse.utils.admin_flags import AdminFlag +from warehouse.admin.flags import AdminFlag from .base import WarehouseFactory diff --git a/tests/unit/accounts/test_views.py b/tests/unit/accounts/test_views.py index 84174b9eb676..2cb4df4aadc7 100644 --- a/tests/unit/accounts/test_views.py +++ b/tests/unit/accounts/test_views.py @@ -25,7 +25,7 @@ IUserService, ITokenService, TokenExpired, TokenInvalid, TokenMissing, TooManyFailedLogins ) -from warehouse.utils.admin_flags import AdminFlag +from warehouse.admin.flags import AdminFlag from ...common.db.accounts import EmailFactory, UserFactory diff --git a/tests/unit/utils/test_admin_flags.py b/tests/unit/admin/test_flags.py similarity index 88% rename from tests/unit/utils/test_admin_flags.py rename to tests/unit/admin/test_flags.py index 5bcf635b07fa..555b27961f54 100644 --- a/tests/unit/utils/test_admin_flags.py +++ b/tests/unit/admin/test_flags.py @@ -10,9 +10,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -from warehouse.utils.admin_flags import AdminFlag +from warehouse.admin.flags import AdminFlag -from ...common.db.utils import AdminFlagFactory as DBAdminFlagFactory +from ...common.db.admin import AdminFlagFactory as DBAdminFlagFactory class TestAdminFlag: diff --git a/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index 6558a78d9050..1d8b492763d4 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -36,7 +36,7 @@ File, Filename, Dependency, DependencyKind, Release, Project, Role, JournalEntry, ) -from warehouse.utils.admin_flags import AdminFlag +from warehouse.admin.flags import AdminFlag from ...common.db.accounts import UserFactory, EmailFactory from ...common.db.packaging import ( diff --git a/warehouse/accounts/views.py b/warehouse/accounts/views.py index d8d111513bb7..8b0ae4315873 100644 --- a/warehouse/accounts/views.py +++ b/warehouse/accounts/views.py @@ -35,7 +35,7 @@ send_password_reset_email, send_email_verification_email, ) from warehouse.packaging.models import Project, Release -from warehouse.utils.admin_flags import AdminFlag +from warehouse.admin.flags import AdminFlag from warehouse.utils.http import is_safe_url diff --git a/warehouse/admin/__init__.py b/warehouse/admin/__init__.py index 7a0a0edbf49f..4569fe05e570 100644 --- a/warehouse/admin/__init__.py +++ b/warehouse/admin/__init__.py @@ -10,10 +10,10 @@ # See the License for the specific language governing permissions and # limitations under the License. -from warehouse.accounts.views import login, logout - def includeme(config): + from warehouse.accounts.views import login, logout + # Setup Jinja2 Rendering for the Admin application config.add_jinja2_search_path("templates", name=".html") diff --git a/warehouse/utils/admin_flags.py b/warehouse/admin/flags.py similarity index 100% rename from warehouse/utils/admin_flags.py rename to warehouse/admin/flags.py diff --git a/warehouse/db.py b/warehouse/db.py index 7a1ce0f289f1..e8a30f5728a2 100644 --- a/warehouse/db.py +++ b/warehouse/db.py @@ -163,7 +163,7 @@ def cleanup(request): connection.close() # Check if we're in read-only mode - from warehouse.utils.admin_flags import AdminFlag + from warehouse.admin.flags import AdminFlag flag = session.query(AdminFlag).get('read-only') if flag and flag.enabled: request.tm.doom() diff --git a/warehouse/forklift/legacy.py b/warehouse/forklift/legacy.py index bd24e88c2682..1194539fa87c 100644 --- a/warehouse/forklift/legacy.py +++ b/warehouse/forklift/legacy.py @@ -45,7 +45,7 @@ Project, Release, Dependency, DependencyKind, Role, File, Filename, JournalEntry, BlacklistedProject, ) -from warehouse.utils.admin_flags import AdminFlag +from warehouse.admin.flags import AdminFlag from warehouse.utils import http From 136e55d69916593c72f292ddf312087b0ec0eddf Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 23 Mar 2018 09:15:48 -0500 Subject: [PATCH 3/9] Add request methods for admin flags --- tests/conftest.py | 2 ++ tests/unit/accounts/test_views.py | 12 +++++++----- tests/unit/admin/test_core.py | 5 ++++- tests/unit/admin/test_flags.py | 15 +++++++-------- warehouse/accounts/views.py | 3 +-- warehouse/admin/__init__.py | 3 +++ warehouse/admin/flags.py | 29 ++++++++++++++++++++--------- warehouse/forklift/legacy.py | 5 +---- 8 files changed, 45 insertions(+), 29 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 42da9d157536..9f191941f60b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,6 +27,7 @@ from warehouse.config import configure from warehouse.accounts import services +from warehouse.admin.flags import Flags from .common.db import Session @@ -215,6 +216,7 @@ def query_recorder(app_config): def db_request(pyramid_request, db_session, datadog): pyramid_request.registry.datadog = datadog pyramid_request.db = db_session + pyramid_request.flags = Flags(pyramid_request) return pyramid_request diff --git a/tests/unit/accounts/test_views.py b/tests/unit/accounts/test_views.py index 2cb4df4aadc7..437b72d78c00 100644 --- a/tests/unit/accounts/test_views.py +++ b/tests/unit/accounts/test_views.py @@ -394,11 +394,13 @@ def test_register_redirect(self, db_request, monkeypatch): assert send_email.calls == [pretend.call(db_request, email)] def test_register_fails_with_admin_flag_set(self, db_request): - admin_flag = (db_request.db.query(AdminFlag) - .filter( - AdminFlag.id == 'disallow-new-user-registration') - .first()) - admin_flag.enabled = True + # This flag was already set via migration, just need to enable it + flag = ( + db_request.db.query(AdminFlag) + .get('disallow-new-user-registration') + ) + flag.enabled = True + db_request.method = "POST" db_request.POST.update({ diff --git a/tests/unit/admin/test_core.py b/tests/unit/admin/test_core.py index 83a2f85bc82e..abae5f6cd42b 100644 --- a/tests/unit/admin/test_core.py +++ b/tests/unit/admin/test_core.py @@ -34,7 +34,10 @@ def test_includeme(): assert config.add_static_view.calls == [ pretend.call("admin/static", "static", cache_max_age=0), ] - assert config.include.calls == [pretend.call(".routes")] + assert config.include.calls == [ + pretend.call(".routes"), + pretend.call(".flags"), + ] assert config.add_view.calls == [ pretend.call( accounts_views.login, diff --git a/tests/unit/admin/test_flags.py b/tests/unit/admin/test_flags.py index 555b27961f54..cf750826bd30 100644 --- a/tests/unit/admin/test_flags.py +++ b/tests/unit/admin/test_flags.py @@ -10,16 +10,15 @@ # See the License for the specific language governing permissions and # limitations under the License. -from warehouse.admin.flags import AdminFlag - -from ...common.db.admin import AdminFlagFactory as DBAdminFlagFactory +from ...common.db.admin import AdminFlagFactory class TestAdminFlag: - def test_default(self, db_session): - assert not AdminFlag.is_enabled(db_session, 'not-a-real-flag') + def test_default(self, db_request): + assert not db_request.flags.enabled('not-a-real-flag') + + def test_enabled(self, db_request): + AdminFlagFactory(id='this-flag-is-enabled') - def test_enabled(self, db_session): - DBAdminFlagFactory.create(id='this-flag-is-enabled', enabled=True) - assert AdminFlag.is_enabled(db_session, 'this-flag-is-enabled') + assert db_request.flags.enabled('this-flag-is-enabled') diff --git a/warehouse/accounts/views.py b/warehouse/accounts/views.py index 8b0ae4315873..f9eb04afd043 100644 --- a/warehouse/accounts/views.py +++ b/warehouse/accounts/views.py @@ -35,7 +35,6 @@ send_password_reset_email, send_email_verification_email, ) from warehouse.packaging.models import Project, Release -from warehouse.admin.flags import AdminFlag from warehouse.utils.http import is_safe_url @@ -233,7 +232,7 @@ def register(request, _form_class=RegistrationForm): if request.method == "POST" and request.POST.get('confirm_form'): return HTTPSeeOther(request.route_path("index")) - if AdminFlag.is_enabled(request.db, 'disallow-new-user-registration'): + if request.flags.enabled('disallow-new-user-registration'): request.session.flash( ("New User Registration Temporarily Disabled " "See https://pypi.org/help#admin-intervention for details"), diff --git a/warehouse/admin/__init__.py b/warehouse/admin/__init__.py index 4569fe05e570..4c733a7a8402 100644 --- a/warehouse/admin/__init__.py +++ b/warehouse/admin/__init__.py @@ -23,6 +23,9 @@ def includeme(config): # Add our routes config.include(".routes") + # Add our flags + config.include(".flags") + config.add_view( login, route_name="admin.login", diff --git a/warehouse/admin/flags.py b/warehouse/admin/flags.py index 706be618834d..1fd09a5931e4 100644 --- a/warehouse/admin/flags.py +++ b/warehouse/admin/flags.py @@ -15,7 +15,7 @@ from warehouse import db -class AdminFlag(db.Model): +class AdminFlag(db.ModelBase): __tablename__ = "warehouse_admin_flag" @@ -23,11 +23,22 @@ class AdminFlag(db.Model): description = Column(Text, nullable=False) enabled = Column(Boolean, nullable=False) - @classmethod - def is_enabled(cls, session, flag_name): - flag = (session.query(cls) - .filter(cls.id == flag_name) - .first()) - if flag is None: - return False - return flag.enabled + +class Flags: + def __init__(self, request): + self.request = request + + def all(self): + return ( + self.request.db.query(AdminFlag) + .filter(AdminFlag.enabled.is_(True)) + .all() + ) + + def enabled(self, flag_name): + flag = self.request.db.query(AdminFlag).get(flag_name) + return flag.enabled if flag else False + + +def includeme(config): + config.add_request_method(Flags, name='flags', reify=True) diff --git a/warehouse/forklift/legacy.py b/warehouse/forklift/legacy.py index 1194539fa87c..32e7731fbbc9 100644 --- a/warehouse/forklift/legacy.py +++ b/warehouse/forklift/legacy.py @@ -45,7 +45,6 @@ Project, Release, Dependency, DependencyKind, Role, File, Filename, JournalEntry, BlacklistedProject, ) -from warehouse.admin.flags import AdminFlag from warehouse.utils import http @@ -789,9 +788,7 @@ def file_upload(request): # Check for AdminFlag set by a PyPI Administrator disabling new project # registration, reasons for this include Spammers, security # vulnerabilities, or just wanting to be lazy and not worry ;) - if AdminFlag.is_enabled( - request.db, - 'disallow-new-project-registration'): + if request.flags.enabled('disallow-new-project-registration'): raise _exc_with_message( HTTPForbidden, ("New Project Registration Temporarily Disabled " From 67b927beb85db35b7b1b9a0c8ba1bb4680d85763 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 23 Mar 2018 10:16:27 -0500 Subject: [PATCH 4/9] Show notification for enabled flags --- warehouse/templates/base.html | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/warehouse/templates/base.html b/warehouse/templates/base.html index d669a458ef1a..629ade1b55ff 100644 --- a/warehouse/templates/base.html +++ b/warehouse/templates/base.html @@ -109,6 +109,12 @@ {% endcsi %} {% endblock %} + {% for flag in request.flags.all() %} +
+ {{ flag.description }} +
+ {% endfor %} +
Help us improve Python packaging - Donate today!
From 8de5b2ef6504873c4c4ef6d8addc4dff1b05398a Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 23 Mar 2018 13:52:39 -0500 Subject: [PATCH 5/9] Add ability to edit flags from admin --- tests/unit/admin/test_routes.py | 10 ++ tests/unit/admin/views/test_flags.py | 129 ++++++++++++++++++ warehouse/admin/routes.py | 12 ++ warehouse/admin/templates/admin/base.html | 5 + .../admin/templates/admin/flags/index.html | 54 ++++++++ warehouse/admin/views/flags.py | 55 ++++++++ 6 files changed, 265 insertions(+) create mode 100644 tests/unit/admin/views/test_flags.py create mode 100644 warehouse/admin/templates/admin/flags/index.html create mode 100644 warehouse/admin/views/flags.py diff --git a/tests/unit/admin/test_routes.py b/tests/unit/admin/test_routes.py index 76aabe9a4990..21717778004b 100644 --- a/tests/unit/admin/test_routes.py +++ b/tests/unit/admin/test_routes.py @@ -116,4 +116,14 @@ def test_includeme(): "/admin/blacklist/remove/", domain=warehouse, ), + pretend.call( + "admin.flags", + "/admin/flags/", + domain=warehouse, + ), + pretend.call( + "admin.flags.edit", + "/admin/flags/edit/", + domain=warehouse, + ), ] diff --git a/tests/unit/admin/views/test_flags.py b/tests/unit/admin/views/test_flags.py new file mode 100644 index 000000000000..3a36327ee3bd --- /dev/null +++ b/tests/unit/admin/views/test_flags.py @@ -0,0 +1,129 @@ +# 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. + +import pretend +import pytest + +from warehouse.admin.flags import AdminFlag +from warehouse.admin.views import flags as views + +from ....common.db.admin import AdminFlagFactory + + +class TestGetFlags: + + def test_get_classifiers(self, db_request): + # Clear out any existing flags added from migrations + db_request.db.query(AdminFlag).delete() + + flag_a = AdminFlagFactory(id='flag-a') + flag_b = AdminFlagFactory(id='flag-b') + + assert views.get_flags(db_request) == { + 'flags': [flag_a, flag_b], + } + + +class TestEditFlag: + + @pytest.mark.parametrize( + "description, enabled, post, expected_description, expected_enabled", + [ + ( + # Nothing changed when enabled + 'old', True, + {'id': 'foo-bar', 'description': 'old', 'enabled': 'on'}, + 'old', True, + ), + ( + # Nothing changed when disabled + 'old', False, + {'id': 'foo-bar', 'description': 'old'}, + 'old', False, + ), + ( + # Enable flag + 'old', False, + {'id': 'foo-bar', 'description': 'old', 'enabled': 'on'}, + 'old', True, + ), + ( + # Disable flag + 'old', True, + {'id': 'foo-bar', 'description': 'old'}, + 'old', False, + ), + ( + # Change description when enabled + 'old', True, + {'id': 'foo-bar', 'description': 'new', 'enabled': 'on'}, + 'new', True, + ), + ( + # Change description when disabled + 'old', False, + {'id': 'foo-bar', 'description': 'new'}, + 'new', False, + ), + ] + ) + def test_edit_flag( + self, db_request, description, enabled, post, expected_description, + expected_enabled): + + # Clear out any existing flags added from migrations + db_request.db.query(AdminFlag).delete() + + flag = AdminFlagFactory( + id='foo-bar', + description=description, + enabled=enabled, + ) + + db_request.POST = post + db_request.route_path = lambda *a: '/the/redirect' + db_request.flash = lambda *a: None + + views.edit_flag(db_request) + + db_request.db.flush() + + assert flag.enabled == expected_enabled + assert flag.description == expected_description + + def test_edit_read_only_flag(self, db_request, monkeypatch): + # Clear out any existing flags added from migrations + db_request.db.query(AdminFlag).delete() + + active = pretend.stub() + doomed = pretend.stub() + txn = pretend.stub(status=doomed) + monkeypatch.setattr( + views.transaction._transaction.Status, 'ACTIVE', active + ) + + db_request.tm = pretend.stub( + isDoomed=lambda: True, + get=lambda: txn, + ) + db_request.POST = { + 'id': 'read-only', + 'description': 'some new description', + } + db_request.route_path = lambda *a: '/the/redirect' + db_request.flash = lambda *a: None + + AdminFlagFactory(id='read-only', enabled=True) + + views.edit_flag(db_request) + + assert txn.status == active diff --git a/warehouse/admin/routes.py b/warehouse/admin/routes.py index 7ede6a70f5a0..931480a08765 100644 --- a/warehouse/admin/routes.py +++ b/warehouse/admin/routes.py @@ -119,3 +119,15 @@ def includeme(config): "/admin/blacklist/remove/", domain=warehouse, ) + + # Flags + config.add_route( + "admin.flags", + "/admin/flags/", + domain=warehouse, + ) + config.add_route( + "admin.flags.edit", + "/admin/flags/edit/", + domain=warehouse, + ) diff --git a/warehouse/admin/templates/admin/base.html b/warehouse/admin/templates/admin/base.html index b0f8ad148a95..1612d4cc5db2 100644 --- a/warehouse/admin/templates/admin/base.html +++ b/warehouse/admin/templates/admin/base.html @@ -112,6 +112,11 @@ Blacklist +
  • + + Flags + +
    • diff --git a/warehouse/admin/templates/admin/flags/index.html b/warehouse/admin/templates/admin/flags/index.html new file mode 100644 index 000000000000..07e8c7c0d515 --- /dev/null +++ b/warehouse/admin/templates/admin/flags/index.html @@ -0,0 +1,54 @@ +{# + # 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. +-#} +{% extends "admin/base.html" %} + +{% import "admin/utils/pagination.html" as pagination %} + +{% block title %}Flags{% endblock %} + +{% block breadcrumb %} +
  • Flags
    • +{% endblock %} + +{% set csrf_token = request.session.get_csrf_token() %} + +{% block content %} +
    +
    +

    Edit Flags

    +
    +
    + + + + + + + + {% for flag in flags %} + + + + + + + + + + + {% endfor %} +
    Flag IDDescriptionEnabled?
    {{ flag.id }}
    +
    +
    +{% endblock content %} diff --git a/warehouse/admin/views/flags.py b/warehouse/admin/views/flags.py new file mode 100644 index 000000000000..2784d108ead8 --- /dev/null +++ b/warehouse/admin/views/flags.py @@ -0,0 +1,55 @@ +# 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. + +from pyramid.httpexceptions import HTTPSeeOther +from pyramid.view import view_config +import transaction + +from warehouse.admin.flags import AdminFlag + + +@view_config( + route_name='admin.flags', + renderer='admin/flags/index.html', + permission='admin', + uses_session=True, +) +def get_flags(request): + return { + 'flags': request.db.query(AdminFlag).order_by(AdminFlag.id).all(), + } + + +@view_config( + route_name='admin.flags.edit', + permission='admin', + request_method='POST', + uses_session=True, + require_methods=False, + require_csrf=True, +) +def edit_flag(request): + flag = request.db.query(AdminFlag).get(request.POST['id']) + flag.description = request.POST['description'] + flag.enabled = bool(request.POST.get('enabled')) + + if flag.id == 'read-only' and request.tm.isDoomed(): + # We are trying to disable the `read-only` flag, but the flag has + # alreeady doomed the transaction, so we need to manually un-doom it + request.tm.get().status = transaction._transaction.Status.ACTIVE + + request.session.flash( + f'Successfully edited flag {flag.id!r}', + queue='success', + ) + + return HTTPSeeOther(request.route_path('admin.flags')) From 68dea55f2f6861e25b2c9a8f90488d1bf07dc59e Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 23 Mar 2018 16:25:46 -0500 Subject: [PATCH 6/9] Add AdminFlag.notify, only show notifiable flags --- warehouse/admin/flags.py | 7 ++-- .../admin/templates/admin/flags/index.html | 2 + ...e785eed9_add_notify_column_to_adminflag.py | 40 +++++++++++++++++++ warehouse/templates/base.html | 2 +- 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 warehouse/migrations/versions/bf73e785eed9_add_notify_column_to_adminflag.py diff --git a/warehouse/admin/flags.py b/warehouse/admin/flags.py index 1fd09a5931e4..00ffa860900d 100644 --- a/warehouse/admin/flags.py +++ b/warehouse/admin/flags.py @@ -10,7 +10,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from sqlalchemy import Column, Boolean, Text +from sqlalchemy import Column, Boolean, Text, sql from warehouse import db @@ -22,16 +22,17 @@ class AdminFlag(db.ModelBase): id = Column(Text, primary_key=True, nullable=False) description = Column(Text, nullable=False) enabled = Column(Boolean, nullable=False) + notify = Column(Boolean, nullable=False, server_default=sql.false()) class Flags: def __init__(self, request): self.request = request - def all(self): + def notifications(self): return ( self.request.db.query(AdminFlag) - .filter(AdminFlag.enabled.is_(True)) + .filter(AdminFlag.enabled.is_(True), AdminFlag.notify.is_(True)) .all() ) diff --git a/warehouse/admin/templates/admin/flags/index.html b/warehouse/admin/templates/admin/flags/index.html index 07e8c7c0d515..2647225b6458 100644 --- a/warehouse/admin/templates/admin/flags/index.html +++ b/warehouse/admin/templates/admin/flags/index.html @@ -33,6 +33,7 @@

    Edit Flags

    Flag ID Description + Notify? Enabled? @@ -43,6 +44,7 @@

    Edit Flags

    {{ flag.id }} + {{ flag.notify }} diff --git a/warehouse/migrations/versions/bf73e785eed9_add_notify_column_to_adminflag.py b/warehouse/migrations/versions/bf73e785eed9_add_notify_column_to_adminflag.py new file mode 100644 index 000000000000..5de34425b280 --- /dev/null +++ b/warehouse/migrations/versions/bf73e785eed9_add_notify_column_to_adminflag.py @@ -0,0 +1,40 @@ +# 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 notify column to AdminFlag + +Revision ID: bf73e785eed9 +Revises: 5dda74213989 +Create Date: 2018-03-23 21:20:05.834821 +""" + +from alembic import op +import sqlalchemy as sa + +revision = 'bf73e785eed9' +down_revision = '5dda74213989' + + +def upgrade(): + op.add_column( + 'warehouse_admin_flag', + sa.Column( + 'notify', + sa.Boolean(), + server_default=sa.text('false'), + nullable=False, + ), + ) + + +def downgrade(): + op.drop_column('warehouse_admin_flag', 'notify') diff --git a/warehouse/templates/base.html b/warehouse/templates/base.html index 629ade1b55ff..0de2b4120330 100644 --- a/warehouse/templates/base.html +++ b/warehouse/templates/base.html @@ -109,7 +109,7 @@ {% endcsi %} {% endblock %} - {% for flag in request.flags.all() %} + {% for flag in request.flags.notifications() %}
    {{ flag.description }}
    From 8b269c2bc129a4d079d0f2ad3c8b7a453aefbb91 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 23 Mar 2018 16:29:05 -0500 Subject: [PATCH 7/9] Migration to add read-only AdminFlag --- .../6418f7d86a4b_add_read_only_adminflag.py | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 warehouse/migrations/versions/6418f7d86a4b_add_read_only_adminflag.py diff --git a/warehouse/migrations/versions/6418f7d86a4b_add_read_only_adminflag.py b/warehouse/migrations/versions/6418f7d86a4b_add_read_only_adminflag.py new file mode 100644 index 000000000000..7319818d7af3 --- /dev/null +++ b/warehouse/migrations/versions/6418f7d86a4b_add_read_only_adminflag.py @@ -0,0 +1,40 @@ +# 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 read-only AdminFlag + +Revision ID: 6418f7d86a4b +Revises: 5dda74213989 +Create Date: 2018-03-23 20:51:31.558587 +""" + +from alembic import op + + +revision = '6418f7d86a4b' +down_revision = 'bf73e785eed9' + + +def upgrade(): + op.execute(""" + INSERT INTO warehouse_admin_flag(id, description, enabled, notify) + VALUES ( + 'read-only', + 'Read Only Mode: Any write operations will have no effect', + FALSE, + TRUE + ) + """) + + +def downgrade(): + op.execute("DELETE FROM warehouse_admin_flag WHERE id = 'read-only'") From 80b1e105d5119cb1f94db4e388980d4115d251c5 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 23 Mar 2018 17:04:00 -0500 Subject: [PATCH 8/9] Allow admins to still write --- tests/unit/admin/views/test_flags.py | 29 ---------------------------- tests/unit/test_db.py | 27 +++++++++++++++++--------- warehouse/admin/views/flags.py | 6 ------ warehouse/db.py | 2 +- 4 files changed, 19 insertions(+), 45 deletions(-) diff --git a/tests/unit/admin/views/test_flags.py b/tests/unit/admin/views/test_flags.py index 3a36327ee3bd..5ded4700ba64 100644 --- a/tests/unit/admin/views/test_flags.py +++ b/tests/unit/admin/views/test_flags.py @@ -10,7 +10,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import pretend import pytest from warehouse.admin.flags import AdminFlag @@ -99,31 +98,3 @@ def test_edit_flag( assert flag.enabled == expected_enabled assert flag.description == expected_description - - def test_edit_read_only_flag(self, db_request, monkeypatch): - # Clear out any existing flags added from migrations - db_request.db.query(AdminFlag).delete() - - active = pretend.stub() - doomed = pretend.stub() - txn = pretend.stub(status=doomed) - monkeypatch.setattr( - views.transaction._transaction.Status, 'ACTIVE', active - ) - - db_request.tm = pretend.stub( - isDoomed=lambda: True, - get=lambda: txn, - ) - db_request.POST = { - 'id': 'read-only', - 'description': 'some new description', - } - db_request.route_path = lambda *a: '/the/redirect' - db_request.flash = lambda *a: None - - AdminFlagFactory(id='read-only', enabled=True) - - views.edit_flag(db_request) - - assert txn.status == active diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index 56b3b0efed77..fb20b50e4ba5 100644 --- a/tests/unit/test_db.py +++ b/tests/unit/test_db.py @@ -203,15 +203,23 @@ def test_create_session(monkeypatch, read_only, tx_status): connection.connection.rollback.calls == [pretend.call()] -@pytest.mark.parametrize("admin_flag, doom_calls", [ - (None, []), - (pretend.stub(enabled=False), []), - ( - pretend.stub(enabled=True, description='flag description'), - [pretend.call()], - ), -]) -def test_create_session_read_only_mode(admin_flag, doom_calls, monkeypatch): +@pytest.mark.parametrize( + "admin_flag, is_superuser, doom_calls", + [ + (None, True, []), + (None, False, []), + (pretend.stub(enabled=False), True, []), + (pretend.stub(enabled=False), False, []), + (pretend.stub(enabled=True, description='flag description'), True, []), + ( + pretend.stub(enabled=True, description='flag description'), + False, + [pretend.call()], + ), + ], +) +def test_create_session_read_only_mode( + admin_flag, is_superuser, doom_calls, monkeypatch): get = pretend.call_recorder(lambda *a: admin_flag) session_obj = pretend.stub( close=lambda: None, @@ -238,6 +246,7 @@ def test_create_session_read_only_mode(admin_flag, doom_calls, monkeypatch): tm=pretend.stub(doom=pretend.call_recorder(lambda: None)), read_only=False, add_finished_callback=lambda callback: None, + user=pretend.stub(is_superuser=is_superuser), ) assert _create_session(request) is session_obj diff --git a/warehouse/admin/views/flags.py b/warehouse/admin/views/flags.py index 2784d108ead8..2d7e7740656e 100644 --- a/warehouse/admin/views/flags.py +++ b/warehouse/admin/views/flags.py @@ -12,7 +12,6 @@ from pyramid.httpexceptions import HTTPSeeOther from pyramid.view import view_config -import transaction from warehouse.admin.flags import AdminFlag @@ -42,11 +41,6 @@ def edit_flag(request): flag.description = request.POST['description'] flag.enabled = bool(request.POST.get('enabled')) - if flag.id == 'read-only' and request.tm.isDoomed(): - # We are trying to disable the `read-only` flag, but the flag has - # alreeady doomed the transaction, so we need to manually un-doom it - request.tm.get().status = transaction._transaction.Status.ACTIVE - request.session.flash( f'Successfully edited flag {flag.id!r}', queue='success', diff --git a/warehouse/db.py b/warehouse/db.py index e8a30f5728a2..46fe00495abb 100644 --- a/warehouse/db.py +++ b/warehouse/db.py @@ -165,7 +165,7 @@ def cleanup(request): # Check if we're in read-only mode from warehouse.admin.flags import AdminFlag flag = session.query(AdminFlag).get('read-only') - if flag and flag.enabled: + if flag and flag.enabled and not request.user.is_superuser: request.tm.doom() # Return our session now that it's created and registered From 35bdb659864a05683508adb45756d73449072c66 Mon Sep 17 00:00:00 2001 From: Dustin Ingram Date: Fri, 23 Mar 2018 17:19:12 -0500 Subject: [PATCH 9/9] Give a error message on upload in read-only mode --- tests/unit/forklift/test_legacy.py | 15 +++++++++++++++ warehouse/forklift/legacy.py | 7 +++++++ 2 files changed, 22 insertions(+) diff --git a/tests/unit/forklift/test_legacy.py b/tests/unit/forklift/test_legacy.py index 1d8b492763d4..225f9f599172 100644 --- a/tests/unit/forklift/test_legacy.py +++ b/tests/unit/forklift/test_legacy.py @@ -738,6 +738,7 @@ def test_fails_invalid_version(self, pyramid_config, pyramid_request, version): pyramid_config.testing_securitypolicy(userid=1) pyramid_request.POST["protocol_version"] = version + pyramid_request.flags = pretend.stub(enabled=lambda *a: False) with pytest.raises(HTTPBadRequest) as excinfo: legacy.file_upload(pyramid_request) @@ -2797,7 +2798,21 @@ def test_upload_purges_legacy(self, pyramid_config, db_request, ), ] + def test_fails_in_read_only_mode(self, pyramid_request): + pyramid_request.flags = pretend.stub(enabled=lambda *a: True) + + with pytest.raises(HTTPForbidden) as excinfo: + legacy.file_upload(pyramid_request) + + resp = excinfo.value + + assert resp.status_code == 403 + assert resp.status == ( + '403 Read Only Mode: Uploads are temporarily disabled' + ) + def test_fails_without_user(self, pyramid_config, pyramid_request): + pyramid_request.flags = pretend.stub(enabled=lambda *a: False) pyramid_config.testing_securitypolicy(userid=None) with pytest.raises(HTTPForbidden) as excinfo: diff --git a/warehouse/forklift/legacy.py b/warehouse/forklift/legacy.py index 32e7731fbbc9..dfff2228fa9c 100644 --- a/warehouse/forklift/legacy.py +++ b/warehouse/forklift/legacy.py @@ -695,6 +695,13 @@ def _is_duplicate_file(db_session, filename, hashes): require_methods=["POST"], ) def file_upload(request): + # If we're in read-only mode, let upload clients know + if request.flags.enabled('read-only'): + raise _exc_with_message( + HTTPForbidden, + 'Read Only Mode: Uploads are temporarily disabled', + ) + # Before we do anything, if there isn't an authenticated user with this # request, then we'll go ahead and bomb out. if request.authenticated_userid is None: