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

Parameter normalization and canonicalization are overly complicated #20

Open
AaronAtDuo opened this issue Jun 10, 2024 · 0 comments
Open
Labels

Comments

@AaronAtDuo
Copy link
Contributor

Description

The parameter normalization and canonicalization code was lifted verbatim out of duo_client_python, but has a few issues.

  • Although it appears to expect to handle boolean or integer parameters, it actually cannot (unless they are in lists, but see below about that)
  • There's string/byte manipulation that probably goes back to Python 2 and could be simplified
  • Although we technically accept list type parameters, they won't actually work in GET requests because Duo doesn't fully support duplicate query string parameters
  • Basically, all the tests at https://github.com/duosecurity/duo_hmac_python/blob/main/test/test_hmac_utils.py#L93 seem silly for what the method is intended to do.

It might be possible to dispense with the normalization method entirely?

Expected Behavior

Parameter processing should be easy to understand; it should accept reasonable parameter types like int and bool.
Ideally, the dict should be {string -> int/bool/string} and still produce the same canonical string and url

Actual Behavior

The interaction between the parameter dictionary, the normalization method, and the canonicalization method is hard to understand. Int and bool parameters have to be stringified before being passed in.

Workarounds

N/A

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant