indigo-dc / flaat

FLAsk with Access Tokens - FLAAT
MIT License
11 stars 6 forks source link

Flask support for factories #60

Closed BorjaEst closed 2 years ago

BorjaEst commented 2 years ago

The following pull request changes and fixes the following:

marcvs commented 2 years ago

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

BorjaEst commented 2 years ago

It might be probably related to other issue, you might have packages not compatible. See https://github.com/indigo-dc/flaat/issues/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: https://github.com/microsoft/pyright/issues/1187

I will try to see how to fix that.

BorjaEst commented 2 years ago

I submitted the pyright issue on pytest-cases, here: https://github.com/smarie/python-pytest-cases/issues/270

marcvs commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@marcvs, firendly reminder for the PR :smiley:

dianagudu commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

:= 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 commented 2 years ago

:= 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 commented 2 years ago

Hm you're right... It comes from here: https://github.com/indigo-dc/flaat/blob/44b4c7c0cdc077014ae8c4741d600aa6a3b84039/flaat/test_env.py#L131-L137

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 commented 2 years ago

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 commented 2 years ago

v1.1.0 released