#247 Use werkzeug.exceptions.Unauthorized instead
Closed 4 months ago by lucarval. Opened 5 months ago by cqi.

@@ -26,12 +26,12 @@ 

  from flask import Flask, jsonify

  from flask_login import LoginManager

  from flask_sqlalchemy import SQLAlchemy

- from werkzeug.exceptions import BadRequest

+ from werkzeug.exceptions import BadRequest, Unauthorized

  

  from odcs.server.logger import init_logging

  from odcs.server.config import init_config

  from odcs.server.proxy import ReverseProxy

- from odcs.server.errors import NotFound, Unauthorized, Forbidden

+ from odcs.server.errors import NotFound, Forbidden

  

  import pkg_resources

  

@@ -77,7 +77,7 @@ 

  @app.errorhandler(Unauthorized)

  def unauthorized_error(e):

      """Flask error handler for Unauthorized exceptions"""

-     return json_error(401, 'Unauthorized', e.args[0])

+     return json_error(401, 'Unauthorized', e.description)

  

  

  @app.errorhandler(Forbidden)

file modified
+2 -1

@@ -32,8 +32,9 @@ 

  from flask import g

  from flask_login import login_required as _login_required

  

+ from werkzeug.exceptions import Unauthorized

  from odcs.server import conf, log

- from odcs.server.errors import Unauthorized, Forbidden

+ from odcs.server.errors import Forbidden

  from odcs.server.models import User

  from odcs.server.models import commit_on_success

  

@@ -30,9 +30,5 @@ 

      pass

  

  

- class Unauthorized(ValueError):

-     pass

- 

- 

  class Forbidden(ValueError):

      pass

file modified
+9 -5

@@ -29,6 +29,7 @@ 

  

  import odcs.server.auth

  

+ from werkzeug.exceptions import Unauthorized

  from odcs.server.auth import init_auth

  from odcs.server.auth import load_krb_user_from_request

  from odcs.server.auth import load_openidc_user

@@ -36,7 +37,6 @@ 

  from odcs.server.auth import require_scopes

  from odcs.server.auth import load_krb_or_ssl_user_from_request

  from odcs.server.auth import load_ssl_user_from_request

- from odcs.server.errors import Unauthorized

  from odcs.server.errors import Forbidden

  from odcs.server import app, conf, db

  from odcs.server.models import User

@@ -94,7 +94,8 @@ 

          with app.test_request_context(environ_base=environ_base):

              with self.assertRaises(Unauthorized) as ctx:

                  load_ssl_user_from_request(flask.request)

-             self.assertTrue('Cannot verify client: GENEROUS' in ctx.exception.args)

+             self.assertIn('Cannot verify client: GENEROUS',

+                           ctx.exception.description)

  

      def test_401_if_cn_not_set(self):

          environ_base = {

@@ -104,7 +105,8 @@ 

          with app.test_request_context(environ_base=environ_base):

              with self.assertRaises(Unauthorized) as ctx:

                  load_ssl_user_from_request(flask.request)

-             self.assertTrue('Unable to get user information (DN) from client certificate' in ctx.exception.args)

+             self.assertIn('Unable to get user information (DN) from client certificate',

+                           ctx.exception.description)

  

  

  class TestLoadKrbOrSSLUserFromRequest(unittest.TestCase):

@@ -192,7 +194,8 @@ 

          with app.test_request_context():

              with self.assertRaises(Unauthorized) as ctx:

                  load_krb_user_from_request(flask.request)

-             self.assertTrue('REMOTE_USER is not present in request.' in ctx.exception.args)

+             self.assertIn('REMOTE_USER is not present in request.',

+                           ctx.exception.description)

  

  

  class TestLoadOpenIDCUserFromRequest(ModelsBaseTest):

@@ -325,7 +328,8 @@ 

              with app.test_request_context(environ_base=environ_base):

                  with self.assertRaises(Unauthorized) as ctx:

                      load_openidc_user(flask.request)

-                 self.assertTrue('Required OIDC scope new-compose not present.' in ctx.exception.args)

+                 self.assertIn('Required OIDC scope new-compose not present.',

+                               ctx.exception.description)

  

  

  class TestQueryLdapGroups(unittest.TestCase):

file modified
+3 -1

@@ -10,7 +10,9 @@ 

  # using sitepackages is not a good idea, but Koji... :(

  sitepackages = True

  install_command = pip install --force-reinstall --ignore-installed {packages}

- deps = -r{toxinidir}/test-requirements.txt

+ deps =

A more generic approach is to declare this dependency in requirements.txt directly:

-flufl.lock
+flufl.lock<3.0;python_version < '3.0'
+flufl.lock; python_version >= '3.0'

+     -r{toxinidir}/test-requirements.txt

+     py27: flufl.lock<3.0

  commands =

      pip install .[server]

      py.test \

The issue this patch fixes is same as freshmaker#0c11b0e. Please refer to that commit message to see detailed info. Signed-off-by: Chenxiong Qi <cqi@redhat.com>

1 new commit added

  • Install flufl.lock<3.0 for running tests on py27
5 months ago

Pretty please pagure-ci rebuild

@cqi, looks like a unit test failure in py27

@lucarval psutil should not be a direct requirement of odcs. I'll check it later.

A more generic approach is to declare this dependency in requirements.txt directly:

-flufl.lock
+flufl.lock<3.0;python_version < '3.0'
+flufl.lock; python_version >= '3.0'

A more generic approach is to declare this dependency in requirements.txt directly:
-flufl.lock
+flufl.lock<3.0;python_version < '3.0'
+flufl.lock; python_version >= '3.0'

Actually the version of setuptools in rhel 7 doesn't support this syntax...

@cqi, the psutil failure is because tox is actually using system site packages. The Jenkins worker likely does not have that library installed for python 2. I think if we define psutil in requirements.txt it will solve the problem.

I created #250 to build on this PR and fix psutil requirement.

PR 250 takes care of these commits. Thank you for fixing this.

Pull-Request has been closed by lucarval

4 months ago