pagure

Clone

#5156 Add "access" field to "project.user.removed" notification.
Opened 4 years ago by zpagure. Modified a year ago
zpagure/pagure forgefed  into  master

Merge
file modified
+4 
@@ -5730,6 +5730,9 @@ 
 

              "User does not have any access on the repo"
 

          )
 

  

+     # Retrieve the access level of the user before it's removed
 

+     access_level = get_obj_access(session, project, user).access
 

+ 

      for u in project.users:
 

          if u.id == user.id:
 

              user = u
 
@@ -5744,6 +5747,7 @@ 
 

          msg=dict(
 

              project=project.to_json(public=True),
 

              removed_user=user.username,
 

+             access=access_level,
 

              agent=agent,
 

          ),
 

      )

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
4 years ago

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 4 years ago 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
4 years ago

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
4 years ago

pretty please pagure-ci rebuild
4 years ago

pretty please pagure-ci rebuild
4 years ago

pretty please pagure-ci rebuild
4 years ago

pretty please pagure-ci rebuild
4 years ago

pretty please pagure-ci rebuild
4 years ago

pretty please pagure-ci rebuild
4 years ago

pretty please pagure-ci rebuild
4 years ago

pretty please pagure-ci rebuild
3 years ago

pretty please pagure-ci rebuild
3 years ago

@zpagure This needs rebasing, please.

rebased onto 51e537427e2747406e21d1afffc541cd5fd3d3e7
a year ago

rebased onto a57c6587ea34bb5ed3af6d672c6530f86e4eb8bb
a year ago

rebased onto 1dd9a0ce7818481150c55562ef593724c70d92a9
a year ago

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 a year ago by wombelix

pretty please pagure-ci rebuild
a year ago

rebased onto 9595687
a year ago
Metadata
None
No Tags
Changes Summary 1
+4 -0
file changed
pagure/lib/query.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.