feat: Minimal infrastructure for http, httpApiKey, and oauth2 security schemes

[alex-zywicki]

This PR does not yet include examples or tests and is intended to be a first draft attempt at #110 and will require further development pending review of initial changes.

[codecov-commenter]

Codecov Report

Merging #120 (edf7647) into main (1646805) will increase coverage by 0.47%.
The diff coverage is 97.92%.

@@            Coverage Diff             @@
##             main     #120      +/-   ##
==========================================
+ Coverage   96.58%   97.06%   +0.47%     
==========================================
  Files           9       11       +2     
  Lines         527      885     +358     
==========================================
+ Hits          509      859     +350     
- Misses         18       26       +8     

Flag	Coverage Δ
unittests	97.06% <97.92%> (+0.47%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
asynction/security.py	96.26% <96.26%> (ø)
asynction/__init__.py	77.77% <100.00%> (+2.77%)	⬆️
asynction/exceptions.py	100.00% <100.00%> (ø)
asynction/mock_server.py	93.33% <100.00%> (+0.28%)	⬆️
asynction/server.py	100.00% <100.00%> (ø)
asynction/types.py	100.00% <100.00%> (ø)
asynction/utils.py	100.00% <100.00%> (ø)
asynction/validation.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1646805...edf7647. Read the comment docs.

[dedoussis]

Very much appreciate the time and effort you've put into this PR! Thank you! Don't let the number of comments/suggestions mislead you -- it generally looks impressively sensible.

The logic and functionality looks good. Most of my comments are revolving the package/module structure. Apologies if I seem too opinionated, but I'm keen to maintain the existing style of the code as well as the structure, at least in the short term. Happy to discuss the restructuring of the code/project, but I'd like this to be part of another PR/issue. Let's keep this PR free of refactoring noise, just focusing on adding the security feature.

TL;DR of my comments:

1. Let's add the new security types under the existing top-lvl types module.
2. No need for any smart scheme resolution. We can just pass the components section of the spec to the security handler factory.
3. Validation of the YAML data using the __post_init__ dataclass method.
4. No need for a new security package within the asynction package. We can just have a security.py top-lvl module which includes the validation stuff.
5. Finally, the security wrapper you implemented should also be added in the mock server class.

[alex-zywicki]

At this point I could probably use some recommendations from you on what kind of tests you'd like to see in the unit tests in order to get the code coverage up to where it needs to be. I will think on it as well, but some recommendations would be very helpful.

[dedoussis]

Thanks for addressing a large amount of comments from the previous review!

Added some more, so that we can take the implementation part of this PR to a solid state.

Have not yet reviewed the testing part of this PR thoroughly. The tests you've already added look sensible. However, I'd prefer to focus on testing on the next round of review, once we have got the implementation part agreed.

When addressing the comments of this round of reviews, I would suggest to start with the comments under types.py. These include suggestions with regards to:

1. Completely aligning the dataclass types with the AsyncAPI YAML data structure
2. Removing redundant validation
3. Adding missing validation in the correct dataclasses

With regards to the deserialisation validation of types.py:

• The AsyncAPISpec class should validate the the servers.securiy_requirements adhere to the components.security_schemes definitions.
• The SecurityScheme class should validate that all the necessary dataclass fields are initialised based on the scheme type.
• The OAuthFlows class should validate that each of the flows has all the fields required. For example the internal flow should include an authorization_url field etc.

Once you address the types.py comments, you would have the foundation needed to address the rest of the comments.

[alex-zywicki]

I also noticed that someone has already proposed adding more granular level security asyncapi/spec#584 (review)

[dedoussis]

Apologies for the late review. Implementation looks good! Added some extra comments with regards to typing annotations.

With respect to testing:

• The integration tests you've added look very good.
• An e2e test would also be needed,
• There are plenty of unit tests missing. For example, I'd like to see tests in tests_types.py covering the __post_init__ validators, as well as making sure that the security schemes that interest us can be deserialised (only the basic http scheme is checked atm). Within test_server.py, you will also need to add tests that make sure that the correct security checkers wrap the registered handlers. The same logic as the tests of the validation wrappers can be used. You may look at unit tests like this, and follow this pattern of registering a handler with security, and then calling this registered handler to see if it raises a SecurityException. Both cases of the registered handler passing and failing the security checks should be tested (as done for the validation wrappers).
• Would also be useful to check the codecov links to see what parts of your PR have been left uncovered.

[dedoussis]

A few more minor comments. This is probably the final batch. 🤞

[alex-zywicki]

Looks like the python 3.10 e2e is failing after I added the from typing_extensions import TypedDict to types.py but I'm not sure why that is

[dedoussis]

@alex-zywicki Adding typing-extensions==4.0.0 in requirements.txt should fix the test error.

[alex-zywicki]

Once this work is in, I will probably follow up with a new issue and PR for per namespace security. I don't want to complicate the core security any more. EDIT: #129

[alex-zywicki]

@dedoussis Once you're satisfied with the changes I can squash down the commits into a more useful commit message so the git history is less cluttered by this.

[dedoussis]

@alex-zywicki I just left a couple of final comments.

No need to squash the commits on your branch. Github does it automatically upon merging.

Once you resolve the above comments, I will (finally 😅 ) proceed with merging.

[alex-zywicki]

I have one small commit coming. Using the SecurityInfo in the test handlers