#1159 Allow `auth.get_user()` method to be called multiple times.
Merged 2 months ago by jkaluza. Opened 2 months ago by jkaluza.
jkaluza/fm-orchestrator fix-koji-gc  into  master

file modified
+4 -2

@@ -34,7 +34,7 @@ 

  import openidc_client

  import requests.exceptions

  from six.moves import urllib_parse

- from requests_kerberos import HTTPKerberosAuth

+ import requests_kerberos

  

  

  env_config = {

@@ -160,7 +160,9 @@ 

          if self._auth_mech == AuthMech.OpenIDC:

              headers['Authorization'] = 'Bearer {0}'.format(self._openidc_token)

          elif self._auth_mech == AuthMech.Kerberos:

-             request_data['auth'] = HTTPKerberosAuth()

+             # MBS server does not support mutual auth, so make it optional.

+             request_data['auth'] = requests_kerberos.HTTPKerberosAuth(

+                 mutual_authentication=requests_kerberos.OPTIONAL)

  

          if headers:

              request_data['headers'] = headers

file modified
+8 -6

@@ -30,7 +30,7 @@ 

  

  import requests

  import kerberos

- from flask import Response

+ from flask import Response, g

  # Starting with Flask 0.9, the _app_ctx_stack is the correct one,

  # before that we need to use the _request_ctx_stack.

  try:

@@ -325,8 +325,10 @@ 

          log.debug('Authorization is disabled.')

          return 'anonymous', {'packager'}

  

-     get_user_func_name = 'get_user_{0}'.format(conf.auth_method)

-     get_user_func = globals().get(get_user_func_name)

-     if not get_user_func:

-         raise RuntimeError('The function "{0}" is not implemented'.format(get_user_func_name))

-     return get_user_func(request)

+     if "user" not in g and "groups" not in g:

+         get_user_func_name = 'get_user_{0}'.format(conf.auth_method)

+         get_user_func = globals().get(get_user_func_name)

+         if not get_user_func:

+             raise RuntimeError('The function "{0}" is not implemented'.format(get_user_func_name))

+         g.user, g.groups = get_user_func(request)

+     return g.user, g.groups

file modified
+20 -6

@@ -32,6 +32,7 @@ 

  import module_build_service.auth

  import module_build_service.errors

  import module_build_service.config as mbs_config

+ from module_build_service import app

  

  

  class TestAuthModule:

@@ -44,7 +45,8 @@ 

              request.cookies.return_value = {}

  

              with pytest.raises(module_build_service.errors.Unauthorized) as cm:

-                 module_build_service.auth.get_user(request)

+                 with app.app_context():

+                     module_build_service.auth.get_user(request)

                  assert str(cm.value) == "No 'authorization' header found."

  

      @patch('module_build_service.auth._get_token_info')

@@ -71,7 +73,8 @@ 

              request.headers.__contains__.side_effect = headers.__contains__

  

              with pytest.raises(module_build_service.errors.Unauthorized) as cm:

-                 module_build_service.auth.get_user(request)

+                 with app.app_context():

+                     module_build_service.auth.get_user(request)

                  assert str(cm.value) == "OIDC token invalid or expired."

  

      @pytest.mark.parametrize('allowed_users', (set(), set(['Joey Jo Jo Junior Shabadoo'])))

@@ -100,13 +103,21 @@ 

              request.headers.__setitem__.side_effect = headers.__setitem__

              request.headers.__contains__.side_effect = headers.__contains__

  

-             username, groups = module_build_service.auth.get_user(request)

+             with app.app_context():

+                 username, groups = module_build_service.auth.get_user(request)

+                 username_second_call, groups_second_call = module_build_service.auth.get_user(

+                     request)

              assert username == name

              if allowed_users:

                  assert groups == set()

              else:

                  assert groups == set(get_user_info.return_value["groups"])

  

+             # Test the real auth method has been called just once.

+             get_user_info.assert_called_once()

+             assert username_second_call == username

+             assert groups_second_call == groups

+ 

      @patch.object(mbs_config.Config, 'no_auth', new_callable=PropertyMock, return_value=True)

      def test_disable_authentication(self, conf_no_auth):

          request = mock.MagicMock()

@@ -118,7 +129,8 @@ 

      def test_misconfiguring_oidc_client_secrets_should_be_failed(self):

          request = mock.MagicMock()

          with pytest.raises(module_build_service.errors.Forbidden) as cm:

-             module_build_service.auth.get_user(request)

+             with app.app_context():

+                 module_build_service.auth.get_user(request)

              assert str(cm.value) == "OIDC_CLIENT_SECRETS must be set in server config."

  

      @patch('module_build_service.auth._get_token_info')

@@ -145,7 +157,8 @@ 

              request.headers.__contains__.side_effect = headers.__contains__

  

              with pytest.raises(module_build_service.errors.Unauthorized) as cm:

-                 module_build_service.auth.get_user(request)

+                 with app.app_context():

+                     module_build_service.auth.get_user(request)

                  assert str(cm.value) == ("Required OIDC scope 'mbs-scope' not present: "

                                           "['openid', 'https://id.fedoraproject.org/scope/groups']")

  

@@ -172,7 +185,8 @@ 

              request.headers.__contains__.side_effect = headers.__contains__

  

              with pytest.raises(module_build_service.errors.Forbidden) as cm:

-                 module_build_service.auth.get_user(request)

+                 with app.app_context():

+                     module_build_service.auth.get_user(request)

                  assert str(cm.value) == "OIDC_REQUIRED_SCOPE must be set in server config."

  

  

The ImportModuleAPI calls the auth.get_user() which auths the
user using Kerberos. the ImportModuleAPI later calls SCMHandler
which in its __init__ method calls the auth.get_user() again.
This leads to traceback in GSSAPI, because the user is already
authed.

This commit fixes this by caching the auth results in flask.g,
which is reset after each request based on the Note in
http://flask.pocoo.org/docs/1.0/appcontext/#storing-data.

This commit also marks mutual auth as OPTIONAL in mbs-cli,
because MBS server currently does not do mutual auth.

Commit 1a79f4d fixes this pull-request

Pull-Request has been merged by jkaluza

2 months ago

Pull-Request has been merged by jkaluza

2 months ago