Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flask support for factories #60

Merged
merged 20 commits into from Jun 24, 2022
Merged

Flask support for factories #60

merged 20 commits into from Jun 24, 2022

Conversation

BorjaEst
Copy link
Contributor

@BorjaEst BorjaEst commented Apr 14, 2022

The following pull request changes and fixes the following:

  • Allows the configuration of Flaat by settings object.
  • Solves the issue RuntimeError: Working outside of application context. when using flask factories.
  • Update example_flask.py to use the new flaat interface.
  • Replace Flask tests to use the example application for validation.
  • Update documentation with the new flask example.

@marcvs
Copy link
Collaborator

marcvs commented Apr 14, 2022

I'm not sure, but I didn't manage to run the tests successfully on your PR. Did you succeed?

@BorjaEst
Copy link
Contributor Author

It might be probably related to other issue, you might have packages not compatible.
See #56

You should purge your environment and install again.

The other option is to use tox $ tox which I see is failing on pyright:

SKIPPED:  py37: InterpreterNotFound: python3.7
  py38: commands succeeded
SKIPPED:  py39: InterpreterNotFound: python3.9
SKIPPED:  py310: InterpreterNotFound: python3.10
  pylint: commands succeeded
ERROR:   pyright: commands failed
  black: commands succeeded
  docs: commands succeeded

Which is related to this other issue:
microsoft/pyright#1187

I will try to see how to fix that.

@BorjaEst
Copy link
Contributor Author

I submitted the pyright issue on pytest-cases, here:
smarie/python-pytest-cases#270

@marcvs
Copy link
Collaborator

marcvs commented Apr 14, 2022

I can't reproduce a working tox run for py38:

========================================= ERRORS =========================================
_____________________________ ERROR collecting test session ______________________________
/usr/lib64/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1014: in _gcd_import
    ???
<frozen importlib._bootstrap>:991: in _find_and_load
    ???
<frozen importlib._bootstrap>:975: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:671: in _load_unlocked
    ???
.tox/py38/lib/python3.8/site-packages/_pytest/assertion/rewrite.py:168: in exec_module
    exec(co, module.__dict__)
flaat/flask/conftest.py:4: in <module>
    from pytest_cases import fixture
E   ModuleNotFoundError: No module named 'pytest_cases'
================================ short test summary info =================================
ERROR  - ModuleNotFoundError: No module named 'pytest_cases'
!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!!!!!!
==================================== 1 error in 0.26s ====================================
ERROR: InvocationError for command /home/marcus/projects/flaat/.tox/py38/bin/pytest (exited with code 2)
________________________________________ summary _________________________________________
ERROR:   py38: commands failed

It's the same error as on py39 and py310.

@BorjaEst
Copy link
Contributor Author

It is due to the addition of pytest_cases to the dependencies, your cached tox envrioment installed the old test-requirements.txt, see:

`E   ModuleNotFoundError: No module named 'pytest_cases'`

The solution is to ask tox to create new environments using the modified test-requirements. You have two options:

  1. Ask tox to clean the cached env: tox --recreate
  2. Clean it yourself: rm -r .tox

See how-to-clean-a-tox-environment-after-running.

@BorjaEst
Copy link
Contributor Author

The linked issue is solved. So if you just run tox --recreate all should be ok.
I do not see any constrain now to merge.

@BorjaEst
Copy link
Contributor Author

BorjaEst commented Jun 7, 2022

@marcvs, firendly reminder for the PR 😃

@dianagudu
Copy link
Contributor

Some tests fail when using the .env-template:

$ cp .env-template .env
$ tox
...
flaat/flask/flask_test.py::test_authorized[ProductionConfig-ValidToken-/authenticated] FAILED                                        [ 43%]
flaat/flask/flask_test.py::test_authorized[ProductionConfig-ValidToken-/info_no_strict] PASSED                                       [ 44%]
flaat/flask/flask_test.py::test_authorized[ProductionConfig-ValidToken-/info] FAILED                                                 [ 45%]
flaat/flask/flask_test.py::test_authorized[ProductionConfig-ValidToken-/authorized_vo] FAILED                                        [ 46%]
flaat/flask/flask_test.py::test_authorized[ProductionConfig-ValidToken-/authorized_claim] FAILED                                     [ 47%]
flaat/flask/flask_test.py::test_authorized[ProductionConfig-ValidToken-/authenticated_callback] FAILED                               [ 48%]
...

With the following log:

____________________________________________ test_authorized[ProductionConfig-ValidToken-/info] ____________________________________________

client = <FlaskClient <Flask 'examples.example_flask'>>
test_authorized_path_headers = ('/info', {'Authorization': 'Bearer mock_non_jwt_at'})

>   ???

<makefun-gen-13>:2: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.tox/py310/lib/python3.10/site-packages/pytest_cases/fixture_parametrize_plus.py:1072: in wrapped_test_func
    return test_func(*args, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

client = <FlaskClient <Flask 'examples.example_flask'>>, path = '/info', headers = {'Authorization': 'Bearer mock_non_jwt_at'}

    @parametrize_with_cases("path, headers", cases=cases.Authorized)
    def test_authorized(client, path, headers):
        response = client.get(path, headers=headers)
>       assert response.status_code == 200
E       assert 401 == 200
E        +  where 401 = <WrapperTestResponse streamed [401 UNAUTHORIZED]>.status_code

flaat/flask/flask_test.py:8: AssertionError
------------------------------------------------------------ Captured log call -------------------------------------------------------------
DEBUG    flaat:__init__.py:440 Mapping exception: User identity could not be determined

@BorjaEst
Copy link
Contributor Author

Thanks for the ping, so I did not expect to run tests using an .env file.
Basically the issue is here:
https://github.com/BorjaEst/flaat/blob/57a8d38a0ebf99390ed11f6fef8bec2ff27a72ef/flaat/flask/conftest.py#L34

I shoud use rather flaat.test_env.FLAAT_AT to use the access token collected by oidc-agent and not the mockup. However, that introduces a limitation, the example uses some "mock" emails and calims which of course might change depending on the user's token.

To do so I had to mock user_info on some specific tests, it is not that simple but I have given a try. I was forced to modify as well one line in the generic conftest.py file, as tests are passing, it should not be a problem.

If errors persist let me know.

@dianagudu
Copy link
Contributor

:= is not a valid syntax in Python 3.7, the tests fail there...

You are right, using an .env file is not standard. I guess not everything can be mocked, you need a token from a "real" issuer to test some of the issuer-related code.

@BorjaEst
Copy link
Contributor Author

BorjaEst commented Jun 24, 2022

:= is not a valid syntax in Python 3.7, the tests fail there...

right... my bad

BTW, I get flaat.exceptions.FlaatException: FLAAT_CLAIM_GROUP must point to list of at least two groups when using .env with something like export FLAAT_CLAIM_GROUP="eduperson_scoped_affiliation". Any idea why?

@dianagudu
Copy link
Contributor

Hm you're right... It comes from here:

flaat/flaat/test_env.py

Lines 131 to 137 in 44b4c7c

self.groups = self.user_infos.user_info.get(self.claim_groups, None)
if (
not isinstance(self.groups, list) or len(self.groups) < 2
): # pragma: no cover
raise FlaatException(
"FLAAT_CLAIM_GROUP must point to list of at least two groups"
)

The eduperson_scoped_affiliation is a multivalued attribute (see spec).
In my experience, if the claim contains only one value, then it might not be represented as a list. But I don't see why it should not be a valid claim.

@BorjaEst
Copy link
Contributor Author

Aaaaa I see, my token did not had "eduperson_scoped_affiliation". Replacing that by for example "eduperson_assurance" (or any other available field) solves the error.

With that solved I was able to test with.env and tox for all version. I think that was all, thanks for the help.

@dianagudu dianagudu merged commit 0247f7e into indigo-dc:master Jun 24, 2022
@dianagudu
Copy link
Contributor

v1.1.0 released

@BorjaEst BorjaEst deleted the flask branch June 28, 2022 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants