pagure

#5156 Add "access" field to "project.user.removed" notification.
Closed by ryanlerch. Opened by zpagure.
zpagure/pagure forgefed  into  master

Download 5156.patch

The "project.user.removed" does not send out the "access" level that was revoked, unlike "project.user.added"

"The following steps that have been detected may have insecure interpolation of sensitive variables"

Is this a bug in Jenkins?

pretty please pagure-ci rebuild

Pretty sure there will be a few tests that will be impacted by this change. Did you run the tests locally?

I have all the pagure/requirements* installed but I get this error when running pytest tests. How can I fix this?

$ pytest  tests/
=================================================== test session starts ===================================================
platform linux -- Python 3.7.3, pytest-6.2.3, py-1.10.0, pluggy-0.13.1
rootdir: /home/user/Desktop/pagure/pagure
plugins: xdist-2.2.1, cov-2.11.1, forked-1.3.0
collected 1699 items / 1 error / 1 skipped / 1697 selected                                                                
========================================================= ERRORS ==========================================================
____________________________________ ERROR collecting tests/test_pagure_flask_docs.py _____________________________________
tests/test_pagure_flask_docs.py:27: in <module>
    import pagure.docs_server
pagure/docs_server.py:44: in <module>
    SESSION = pagure.lib.model_base.create_session(APP.config["DB_URL"])
pagure/lib/model_base.py:51: in create_session
    db_url and db_url != ("%s" % SESSIONMAKER.kw["bind"].engine.url)
E   TypeError: not all arguments converted during string formatting
==================================================== warnings summary =====================================================
tests/__init__.py:13
  /home/user/Desktop/pagure/pagure/tests/__init__.py:13: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
    import imp
../pagure_env/lib/python3.7/site-packages/kombu/utils/compat.py:93
  /home/user/Desktop/pagure/pagure_env/lib/python3.7/site-packages/kombu/utils/compat.py:93: DeprecationWarning: SelectableGroups dict interface is deprecated. Use select.
    for ep in importlib_metadata.entry_points().get(namespace, [])
../pagure_env/lib/python3.7/site-packages/markdown/util.py:87
  /home/user/Desktop/pagure/pagure_env/lib/python3.7/site-packages/markdown/util.py:87: DeprecationWarning: SelectableGroups dict interface is deprecated. Use select.
    INSTALLED_EXTENSIONS = metadata.entry_points().get('markdown.extensions', ())
pagure/forms.py:338
  /home/user/Desktop/pagure/pagure/tests/../pagure/forms.py:338: DeprecationWarning: Required is going away in WTForms 3.0, use DataRequired
    [wtforms.validators.Required()],
pagure/forms.py:967
  /home/user/Desktop/pagure/pagure/tests/../pagure/forms.py:967: DeprecationWarning: Required is going away in WTForms 3.0, use DataRequired
    "comment", [wtforms.validators.Required()], choices=[]
tests/test_style.py:90
  /home/user/Desktop/pagure/pagure/tests/test_style.py:90: DeprecationWarning: invalid escape sequence \.
    '"/(\.eggs|\.git|\.hg|\.mypy_cache|\.nox|\.tox|\.venv|_build|buck-out|build|dist)/"',
-- Docs: https://docs.pytest.org/en/stable/warnings.html
================================================= short test summary info =================================================
ERROR tests/test_pagure_flask_docs.py - TypeError: not all arguments converted during string formatting
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
========================================= 1 skipped, 6 warnings, 1 error in 6.88s =========================================

EDIT: does this test require to run on an existing installation of Pagure? I tried to run it just after "git clone".
Edited by zpagure

Anyway, running all the tests locally is a problem for me because there are 1700 of them and it takes days to finish. I rely on Jenkins because it can return an answer in a few hours and then I can check locally only the ones that broke. But this time it wasn't very useful.

By the way, is "alembic-3" a Fedora thing? When I install alembic from Debian or PIP, all I see is "alembic" in PATH. Symlinking alembic-3 to alembic fixes some errors, but I still get the same error as above TypeError: not all arguments converted during string formatting in many other tests.

pretty please pagure-ci rebuild

By the way, is "alembic-3" a Fedora thing? When I install alembic from Debian or PIP, all I see is "alembic" in PATH. Symlinking alembic-3 to alembic fixes some errors, but I still get the same error as above TypeError: not all arguments converted during string formatting in many other tests.

Debian uses alternatives to manage python2 vs python3 variants, Fedora renames the binaries and has the preferred one statically symlinked.

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

@zpagure This needs rebasing, please.

rebased onto 51e537427e2747406e21d1afffc541cd5fd3d3e7

rebased onto a57c6587ea34bb5ed3af6d672c6530f86e4eb8bb

rebased onto 1dd9a0ce7818481150c55562ef593724c70d92a9

This is missing after https://pagure.io/pagure/blob/master/f/tests/test_pagure_flask_ui_repo.py#_1084 to make the test pass:

{'access': 'admin'}

One test is failing that's related to this PR.

13:34:47  FAILED tests/test_pagure_flask_ui_repo.py::PagureFlaskRepotests::test_remove_user - AssertionError: assert {'access': 'a..._user': 'foo'} == {'agent': 'pi..._u...

13:34:47  ____________________ PagureFlaskRepotests.test_remove_user _____________________
13:34:47  [gw5] linux -- Python 3.9.19 /pagure/.tox/py39/bin/python
13:34:47  
13:34:47  self = <tests.test_pagure_flask_ui_repo.PagureFlaskRepotests testMethod=test_remove_user>
13:34:47  ast = <MagicMock name='admin_session_timedout' id='139969835603616'>
13:34:47  
13:34:47      @patch.dict(
13:34:47          "pagure.config.config", {"FEDORA_MESSAGING_NOTIFICATIONS": True}
13:34:47      )
13:34:47      @patch("pagure.decorators.admin_session_timedout")
13:34:47      def test_remove_user(self, ast):
13:34:47          """Test the remove_user endpoint."""
13:34:47          ast.return_value = False
13:34:47      
13:34:47          # Git repo not found
13:34:47          output = self.app.post("/foo/dropuser/1")
13:34:47          self.assertEqual(output.status_code, 404)
13:34:47      
13:34:47          user = tests.FakeUser()
13:34:47          with tests.user_set(self.app.application, user):
13:34:47              output = self.app.post("/foo/dropuser/1")
13:34:47              self.assertEqual(output.status_code, 404)
13:34:47      
13:34:47              tests.create_projects(self.session)
13:34:47              tests.create_projects_git(os.path.join(self.path, "repos"))
13:34:47      
13:34:47              output = self.app.post("/test/dropuser/1")
13:34:47              self.assertEqual(output.status_code, 403)
13:34:47      
13:34:47              ast.return_value = True
13:34:47              output = self.app.post("/test/dropuser/1")
13:34:47              self.assertEqual(output.status_code, 302)
13:34:47              ast.return_value = False
13:34:47      
13:34:47          # User not logged in
13:34:47          output = self.app.post("/test/dropuser/1")
13:34:47          self.assertEqual(output.status_code, 302)
13:34:47      
13:34:47          user.username = "pingou"
13:34:47          with tests.user_set(self.app.application, user):
13:34:47              output = self.app.post("/test/settings")
13:34:47              output_text = output.get_data(as_text=True)
13:34:47      
13:34:47              csrf_token = output_text.split(
13:34:47                  'name="csrf_token" type="hidden" value="'
13:34:47              )[1].split('">')[0]
13:34:47      
13:34:47              data = {"csrf_token": csrf_token}
13:34:47      
13:34:47              output = self.app.post(
13:34:47                  "/test/dropuser/2", data=data, follow_redirects=True
13:34:47              )
13:34:47              self.assertEqual(output.status_code, 200)
13:34:47              output_text = output.get_data(as_text=True)
13:34:47              self.assertIn(
13:34:47                  "<title>Settings - test - Pagure</title>", output_text
13:34:47              )
13:34:47              self.assertIn(
13:34:47                  '<h5 class="pl-2 font-weight-bold text-muted">Project Settings</h5>',
13:34:47                  output_text,
13:34:47              )
13:34:47              self.assertIn(
13:34:47                  "User does not have any " "access on the repo", output_text
13:34:47              )
13:34:47      
13:34:47          # Add an user to a project
13:34:47          repo = pagure.lib.query.get_authorized_project(self.session, "test")
13:34:47          self.assertEqual(len(repo.users), 0)
13:34:47          msg = pagure.lib.query.add_user_to_project(
13:34:47              session=self.session, project=repo, new_user="foo", user="pingou"
13:34:47          )
13:34:47          self.session.commit()
13:34:47          self.assertEqual(msg, "User added")
13:34:47          self.assertEqual(len(repo.users), 1)
13:34:47      
13:34:47          with tests.user_set(self.app.application, user):
13:34:47              output = self.app.post("/test/dropuser/2", follow_redirects=True)
13:34:47              self.assertEqual(output.status_code, 200)
13:34:47              output_text = output.get_data(as_text=True)
13:34:47              self.assertIn(
13:34:47                  "<title>Settings - test - Pagure</title>", output_text
13:34:47              )
13:34:47              self.assertIn(
13:34:47                  '<h5 class="pl-2 font-weight-bold text-muted">Project Settings</h5>',
13:34:47                  output_text,
13:34:47              )
13:34:47              self.assertNotIn("User removed", output_text)
13:34:47              self.assertIn('action="/test/dropuser/2">', output_text)
13:34:47              repo = pagure.lib.query.get_authorized_project(
13:34:47                  self.session, "test"
13:34:47              )
13:34:47              self.assertEqual(len(repo.users), 1)
13:34:47      
13:34:47              data = {"csrf_token": csrf_token}
13:34:47      
13:34:47              with testing.mock_sends(
13:34:47                  pagure_messages.ProjectUserRemovedV1(
13:34:47                      topic="pagure.project.user.removed",
13:34:47                      body={
13:34:47                          "project": {
13:34:47                              "id": 1,
13:34:47                              "name": "test",
13:34:47                              "fullname": "test",
13:34:47                              "url_path": "test",
13:34:47                              "full_url": "http://localhost.localdomain/test",
13:34:47                              "description": "test project #1",
13:34:47                              "namespace": None,
13:34:47                              "parent": None,
13:34:47                              "date_created": ANY,
13:34:47                              "date_modified": ANY,
13:34:47                              "user": {
13:34:47                                  "name": "pingou",
13:34:47                                  "fullname": "PY C",
13:34:47                                  "url_path": "user/pingou",
13:34:47                                  "full_url": "http://localhost.localdomain/user/pingou",
13:34:47                              },
13:34:47                              "access_users": {
13:34:47                                  "owner": ["pingou"],
13:34:47                                  "admin": [],
13:34:47                                  "commit": [],
13:34:47                                  "collaborator": [],
13:34:47                                  "ticket": [],
13:34:47                              },
13:34:47                              "access_groups": {
13:34:47                                  "admin": [],
13:34:47                                  "commit": [],
13:34:47                                  "collaborator": [],
13:34:47                                  "ticket": [],
13:34:47                              },
13:34:47                              "tags": [],
13:34:47                              "priorities": {},
13:34:47                              "custom_keys": [],
13:34:47                              "close_status": [
13:34:47                                  "Invalid",
13:34:47                                  "Insufficient data",
13:34:47                                  "Fixed",
13:34:47                                  "Duplicate",
13:34:47                              ],
13:34:47                              "milestones": {},
13:34:47                          },
13:34:47                          "removed_user": "foo",
13:34:47                          "agent": "pingou",
13:34:47                      },
13:34:47                  )
13:34:47              ):
13:34:47  >               output = self.app.post(
13:34:47                      "/test/dropuser/2", data=data, follow_redirects=True
13:34:47                  )
13:34:47  
13:34:47  tests/test_pagure_flask_ui_repo.py:1089: 
13:34:47  _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
13:34:47  /usr/lib64/python3.9/contextlib.py:126: in __exit__
13:34:47      next(self.gen)
13:34:47  _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
13:34:47  
13:34:47  expected_messages = (ProjectUserRemovedV1(id='58a74adb-a0c9-4143-a820-313f62caa81e', topic='pagure.project.user.removed', body={'project':...['Invalid', 'Insufficient data', 'Fixed', 'Duplicate'], 'milestones': {}}, 'removed_user': 'foo', 'agent': 'pingou'}),)
13:34:47  sent = [ProjectUserRemovedV1(id='fa7c5bc1-c324-41bd-aa74-4bc9b975de69', topic='pagure.project.user.removed', body={'project':...ficient data', 'Fixed', 'Duplicate'], 'milestones': {}}, 'removed_user': 'foo', 'access': 'admin', 'agent': 'pingou'})]
13:34:47  mock_pub = <MagicMock name='_twisted_publish' id='139969864176496'>
13:34:47  messages = [ProjectUserRemovedV1(id='fa7c5bc1-c324-41bd-aa74-4bc9b975de69', topic='pagure.project.user.removed', body={'project':...ficient data', 'Fixed', 'Duplicate'], 'milestones': {}}, 'removed_user': 'foo', 'access': 'admin', 'agent': 'pingou'})]
13:34:47  
13:34:47      @contextmanager
13:34:47      def mock_sends(*expected_messages):
13:34:47          """
13:34:47          Assert a block of code results in the provided messages being sent without
13:34:47          actually sending them.
13:34:47      
13:34:47          This is intended for unit tests. The call to publish is mocked out and messages
13:34:47          are captured and checked at the end of the ``with``.
13:34:47      
13:34:47          For example:
13:34:47      
13:34:47              >>> from fedora_messaging import api, testing
13:34:47              >>> def publishes():
13:34:47              ...     api.publish(api.Message(body={"Hello": "world"}))
13:34:47              ...
13:34:47              >>> with testing.mock_sends(api.Message, api.Message(body={"Hello": "world"})):
13:34:47              ...     publishes()
13:34:47              ...     publishes()
13:34:47              ...
13:34:47              >>> with testing.mock_sends(api.Message(body={"Goodbye": "everybody"})):
13:34:47              ...     publishes()
13:34:47              ...
13:34:47              AssertionError
13:34:47      
13:34:47          Args:
13:34:47              *expected_messages: The messages you expect to be sent. These can be classes
13:34:47                  instances of classes derived from :class:`fedora_messaging.message.Message`.
13:34:47                  If the class is provided, the message is checked to make sure it is an
13:34:47                  instance of that class and that it passes schema validation. If an instance
13:34:47                  is provided, it is checked for equality with the sent message.
13:34:47      
13:34:47          Raises:
13:34:47              AssertionError: If the messages published don't match the messages asserted.
13:34:47          """
13:34:47      
13:34:47          sent = []
13:34:47          with mock.patch("fedora_messaging.api.crochet"):
13:34:47              with mock.patch("fedora_messaging.api._twisted_publish") as mock_pub:
13:34:47                  yield sent
13:34:47      
13:34:47          messages = [call[0][0] for call in mock_pub.call_args_list]
13:34:47          sent.extend(messages)
13:34:47          if len(expected_messages) != len(messages):
13:34:47              raise AssertionError(
13:34:47                  "Expected {} messages to be sent, but {} were sent".format(
13:34:47                      len(expected_messages), len(messages)
13:34:47                  )
13:34:47              )
13:34:47          for msg, expected in zip(messages, expected_messages):
13:34:47              if inspect.isclass(expected):
13:34:47                  if not isinstance(msg, expected):
13:34:47                      raise AssertionError(
13:34:47                          "Expected message of type {}, but {} was sent".format(
13:34:47                              expected, msg.__class__
13:34:47                          )
13:34:47                      )
13:34:47              else:
13:34:47                  assert msg.topic == expected.topic
13:34:47  >               assert msg.body == expected.body
13:34:47  E               AssertionError: assert {'access': 'a..._user': 'foo'} == {'agent': 'pi..._user': 'foo'}
13:34:47  E                 
13:34:47  E                 Omitting 3 identical items, use -vv to show
13:34:47  E                 Left contains 1 more item:
13:34:47  E                 {'access': 'admin'}
13:34:47  E                 Use -v to get more diff
13:34:47  
13:34:47  .tox/py39/lib/python3.9/site-packages/fedora_messaging/testing.py:89: AssertionError
Edited by wombelix

pretty please pagure-ci rebuild

rebased onto 9595687196cb244df5d78882498820594423a619

This project has moved to https://forge.fedoraproject.org/apps/pagure. As part of the migration, all open pull requests on pagure.io have been closed. If you'd like to continue working on this, please fork the repository on the new forge and re-submit your PR there.

Pull-Request has been closed by ryanlerch
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.