#4053 Enable token authentication on internal endpoints. Fixes #4000
Merged by pingou. Opened by bkabrda.
bkabrda/pagure internal-endpoints-token-auth  into  master

Download 4053.patch

Some notes:

  • I'm not sure if this is a "valid" usage of admin token, but I think it makes sense to be able to use it this way. Otherwise we'd probably need to create a completely different mechanism.
  • It might make sense to move check_api_acls to pagure.utils and import it from there. I'll do that if it makes sense to you.
  • And yes, once the general approach is figured out and approved, I'll write documentation and tests :)

to access or to get authorized I guess

I think I'm fine with the general approach.

Regarding the second point, we should definitely try to improve on our recursive imports, so the less recursion the better.

doc_and_tests++ :)

rebased onto eebc9b7e65d67f4bf3c5d57cc1afa87c4a9eaef9

Fixed, docs and tests added, ready for re-review.

@bkabrda Can you please rebase the code on top of current master?

rebased onto 29bac50c1e4337ae0dd45fc32008b60f71b582db

@ngompa done.

assertEqual 200 no?

We could just use mock to change the value of the config, solves the issue of resetting it at the end of the test

I've made the following changes and rebased:

diff --git a/ tests/test_pagure_flask_internal.py b/ tests/test_pagure_flask_internal.py
index f40dd61a..6392f69e 100644
--- a/ tests/test_pagure_flask_internal.py      
+++ b/ tests/test_pagure_flask_internal.py      
@@ -47,12 +47,10 @@ class PagureFlaskInternaltests(tests.Modeltests):
         pagure.config.config['GIT_FOLDER'] = os.path.join(
             self.path, 'repos')
+    @patch.dict('pagure.config.config', {'IP_ALLOWED_INTERNAL': []})
     def test_internal_access_only(self):
         output = self.app.post('/pv/ssh/lookupkey/')
-        self.assertNotEqual(output.status_code, 403)
-
-        before = pagure.config.config['IP_ALLOWED_INTERNAL'][:]
-        pagure.config.config['IP_ALLOWED_INTERNAL'] = []
+        self.assertEqual(output.status_code, 403)
         # no internal IP addresses => will fail
         output = self.app.post('/pv/ssh/lookupkey/')
@@ -74,10 +72,7 @@ class PagureFlaskInternaltests(tests.Modeltests):
             '/pv/ssh/lookupkey/',
             headers={"Authorization": "token %s" % token[0].id}
         )
-        self.assertNotEqual(output.status_code, 401)
-
-        pagure.config.config['IP_ALLOWED_INTERNAL'] = before[:]
-
+        self.assertEqual(output.status_code, 400)
     @patch('pagure.lib.notify.send_email')
     def test_pull_request_add_comment(self, send_email):

rebased onto c0a74ccb6426c0eb285e5e1809cea253800fbfb2

I'll look at the failing tests

rebased onto 984d0f132e4727124d51db940637561827600fdb

Looking good, thanks @bkabrda :)

Pull-Request has been merged by pingou

Metadata