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

test: dpop #35

Merged
merged 8 commits into from
Jul 28, 2023
Merged

test: dpop #35

merged 8 commits into from
Jul 28, 2023

Conversation

salvatorelaiso
Copy link
Collaborator

While testing the property is_valid of DPoPVerifier, I found out it can fail on some invalid inputs.
Specifically, you can find some commented out test cases which causes the aforementioned property to raise different errors on not-valid inputs. Do you prefer to address those cases, for example, by returning False if any of the underlying checks raise an error, or we keep it as it is?

@peppelinux
Copy link
Member

is_valid MUST return a boolean

please specify it if not already defined in the property/function definition

in the test we have to assert true or false, depending by the scope of the test

@peppelinux
Copy link
Member

we must tests situation where the dpop is invalid or with a wrong schema, the tests we need wants to cover also the security aspects and not just the assertion to wellknown parameter, that we have used in the tests

@salvatorelaiso
Copy link
Collaborator Author

As you can see, the test cases for the DPoPVerifier are cases in which we expect the function to return False but instead it raises an error. Do you want this behavior to be fixed in the function? The tests failing are exactly addressing cases in which the parameters are not correct, we can also expand those tests with new cases.

pyeudiw/tests/oauth2/test_dpop.py Outdated Show resolved Hide resolved
pyeudiw/tests/oauth2/test_dpop.py Outdated Show resolved Hide resolved
pyeudiw/tests/oauth2/test_dpop.py Show resolved Hide resolved
@salvatorelaiso salvatorelaiso linked an issue Jul 27, 2023 that may be closed by this pull request
pyeudiw/oauth2/dpop.py Show resolved Hide resolved
pyeudiw/oauth2/dpop.py Outdated Show resolved Hide resolved
@peppelinux peppelinux changed the title Support/dev/tests/dpop test: dpop Jul 28, 2023
pyeudiw/oauth2/dpop.py Outdated Show resolved Hide resolved
pyeudiw/oauth2/dpop.py Show resolved Hide resolved
pyeudiw/oauth2/exceptions.py Outdated Show resolved Hide resolved
@peppelinux peppelinux self-requested a review July 28, 2023 13:26
@peppelinux peppelinux merged commit f542341 into italia:dev Jul 28, 2023
3 checks passed
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.

[TEST] - dpop.py: assertions for unpadded headers and payload
2 participants