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

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

Merged
merged 27 commits into from
Nov 30, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
5e44d44
Adds minimal infrastructure for http, httpApiKey, and oauth2 security…
alex-zywicki Nov 10, 2021
d02b1a7
Adding basic integration tests
alex-zywicki Nov 10, 2021
7fef7a7
Handling more test issues
alex-zywicki Nov 11, 2021
70cd039
Apply suggestions from code review
alex-zywicki Nov 13, 2021
7f31070
Resolve review items.
alex-zywicki Nov 15, 2021
434ad27
Add some security unit tests
alex-zywicki Nov 15, 2021
9f7bc36
Add scope validate func loading and usage
alex-zywicki Nov 15, 2021
0d8f327
Rename scope_verify -> scope_validate. Actually load x-scopeValidateFunc
alex-zywicki Nov 15, 2021
eca58da
Adress review items
alex-zywicki Nov 18, 2021
4ce814f
Add validation for oauth2 flows
alex-zywicki Nov 18, 2021
34974e6
Add validataion for HTTP API Key security scheme
alex-zywicki Nov 18, 2021
acc2b78
Fix line length lint issue for line that can't really be shortened
alex-zywicki Nov 22, 2021
256cfff
Address review items
alex-zywicki Nov 23, 2021
a8e4e58
Add unit tests for types validation
alex-zywicki Nov 23, 2021
24ac1ac
Add lots more security unit tests
alex-zywicki Nov 23, 2021
7b29e59
Resolve most of the outstanding review items
alex-zywicki Nov 24, 2021
7e460ff
Refactor security check error handling scheme
alex-zywicki Nov 24, 2021
811f628
Remove scope validation from security checks for schemes that do not …
alex-zywicki Nov 24, 2021
becef94
Consolidate auth header format validation logic.
alex-zywicki Nov 24, 2021
5f67ace
Add a few more tests and refactor loading logic
alex-zywicki Nov 24, 2021
6a11f0f
Resolve more review items
alex-zywicki Nov 29, 2021
6ade4e2
Fix mock server test. Pass correct args to connect handler to match c…
alex-zywicki Nov 29, 2021
4b3b132
Add more secutiy tests to raise coverage level
alex-zywicki Nov 29, 2021
fece4c6
Add SecurityInfo type
alex-zywicki Nov 29, 2021
c72c818
Move Security info to security.py. Add to __init__.py. Add typing_ext…
alex-zywicki Nov 29, 2021
512cef3
Apply suggestions from code review
alex-zywicki Nov 30, 2021
edf7647
Use SecurityInfo in test handlers
alex-zywicki Nov 30, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions asynction/common_types.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from typing import Any, Mapping

JSONMappingValue = Any
JSONMapping = Mapping[str, JSONMappingValue]
JSONSchema = JSONMapping
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could stay in the top level types module. See the comments below with regards to keeping everything under a single types module/package.

7 changes: 7 additions & 0 deletions asynction/default_handlers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
def default_on_connect_handler(*args, **kwargs):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to use a separate module for this.

"""Injected into api when security is specified by no connect handler is provided"""

pass


DEFAULT_ON_CONNECT_HANDLER = "asynction.default_handlers.default_on_connect_handler"
9 changes: 9 additions & 0 deletions asynction/loader.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from importlib import import_module
from typing import Callable


def load_handler(handler_id: str) -> Callable:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would prefer this to remain under server.py, at least for now where the existing modules aren't that big.

*module_path_elements, object_name = handler_id.split(".")
module = import_module(".".join(module_path_elements))

return getattr(module, object_name)
5 changes: 4 additions & 1 deletion asynction/mock_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from asynction.types import JSONMapping
from asynction.types import JSONSchema
from asynction.types import Message
from asynction.types import SecurityRequirement
from asynction.validation import bindings_validator_factory
from asynction.validation import publish_message_validator_factory

Expand Down Expand Up @@ -210,7 +211,9 @@ def from_spec(
)

def _register_handlers(
self, default_error_handler: Optional[ErrorHandler] = None
self,
default_error_handler: Optional[ErrorHandler] = None,
server_security: Optional[Sequence[SecurityRequirement]] = None
alex-zywicki marked this conversation as resolved.
Show resolved Hide resolved
) -> None:
for namespace, channel in self.spec.channels.items():
if channel.publish is not None:
Expand Down
56 changes: 56 additions & 0 deletions asynction/security/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
from typing import Sequence

from asynction.common_types import JSONMapping

from .exceptions import UnregisteredSecurityScheme
from .exceptions import UnsupportedSecurityScheme
from .types import SecurityRequirement
from .types import SecurityScheme
from .types import SecuritySchemesType
from .validation import security_handler_factory


def _resolve_security_scheme(
security: Sequence[JSONMapping], schemes: JSONMapping
) -> Sequence[JSONMapping]:
new_security = []
for item in security:
for scheme_name, scopes in item.items():
if scheme_name not in schemes:
raise UnregisteredSecurityScheme
scheme = schemes[scheme_name]
new_security.append(dict(name=scheme_name, scopes=scopes, scheme=scheme))

return new_security


def _resolve_server_security_schemes(
raw_spec: JSONMapping, schemes: JSONMapping
) -> JSONMapping:
for name, server in raw_spec.get("servers", {}).items():
if "security" in server:
server["security"] = (
_resolve_security_scheme(server["security"], schemes) or None
)

return raw_spec


def resolve_security_schemes(raw_spec: JSONMapping) -> JSONMapping:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can get away with not having this resolver function. Since the AsyncAPI specification does not use JSONSchema references for these objects, I think it would be more intuitive to not move data structures from one place of the spec to another. Let's deserialise the complete map of security scheme objects under the components filed of the AsyncAPIObject as suggested in the other comments below.

schemes = raw_spec.get("components", {}).get("securitySchemes", {})
if not schemes:
return raw_spec
raw_spec = _resolve_server_security_schemes(raw_spec, schemes)

return raw_spec


__all__ = [
"SecurityRequirement",
"SecurityScheme",
"SecuritySchemesType",
"security_handler_factory",
"resolve_security_schemes",
"UnregisteredSecurityScheme",
"UnsupportedSecurityScheme",
]
23 changes: 23 additions & 0 deletions asynction/security/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
from asynction.exceptions import AsynctionException


class SecurityException(AsynctionException):
"""
Base Security Exception type.
"""
pass


class UnregisteredSecurityScheme(SecurityException):
"""
Raised when a security scheme not listed in the securitySchemes section of the
spec is used in a ``security`` or ``x-security`` specification
"""
pass


class UnsupportedSecurityScheme(SecurityException):
"""
Raised when a specified security scheme is not supported by asynction
"""
pass
190 changes: 190 additions & 0 deletions asynction/security/types.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
from dataclasses import dataclass
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the top level types module is not that large, I'd add these new types there, rather than having a module just for the security types. And we can always split it up once it gets too overcrowded.

I wouldn't want to spread types across multiple places. For me the top level types module should be treated as a stand-alone entity that can serialise/deserialise AsyncAPI data. This top lvl types module could have very well been its own package, on top of which, other AsyncAPI python tools are built. It's something that I plan to do in the near future.

For now let's concentrate all the AsyncAPI relevant types under the top level types module. If it starts getting too big we can make this module a types/ package.

from enum import Enum
from typing import Mapping, Optional, Sequence, Type

from svarog import register_forge
from svarog.types import Forge

from asynction.common_types import JSONMapping
from .exceptions import UnsupportedSecurityScheme


class HTTPSecuritySchemeType(Enum):
BASIC = "basic"
DIGEST = "digest"
BEARER = "bearer"
alex-zywicki marked this conversation as resolved.
Show resolved Hide resolved


class OAuth2FlowType(Enum):
"""
https://www.asyncapi.com/docs/specifications/v2.2.0#oauthFlowsObject
"""
IMPLICIT = "implicit"
PASSWORD = "password"
CLIENT_CREDENTIALS = "clientCredentials "
alex-zywicki marked this conversation as resolved.
Show resolved Hide resolved
AUTHORIZATION_CODE = "authorizationCode"


@dataclass
class OAuth2Flow:
"""
https://www.asyncapi.com/docs/specifications/v2.2.0#oauthFlowObject
"""
scopes: Sequence[str]
alex-zywicki marked this conversation as resolved.
Show resolved Hide resolved
authorization_url: Optional[str] = None
token_url: Optional[str] = None
refresh_url: Optional[str] = None

@staticmethod
def forge(
type_: Type["OAuth2Flow"],
data: JSONMapping,
forge: Forge
) -> "OAuth2Flow":
return type_(
scopes=forge(
type_.__annotations__["scopes"],
data.get("scopes")
),
authorization_url=forge(
type_.__annotations__["authorization_url"],
data.get("authorizationUrl")
),
token_url=forge(
type_.__annotations__["token_url"],
data.get("tokenUrl")
),
refresh_url=forge(
type_.__annotations__["refresh_url"],
data.get("refreshUrl")
)
)


register_forge(OAuth2Flow, OAuth2Flow.forge)


class SecuritySchemesType(Enum):
"""
https://www.asyncapi.com/docs/specifications/v2.2.0#securitySchemeObject
alex-zywicki marked this conversation as resolved.
Show resolved Hide resolved
"""
USER_PASSWORD = "userPassword"
API_KEY = "apiKey"
X509 = "X509"
SYMMETRIC_ENCRYPTION = "symmetricEncryption"
ASYMMETRIC_ENCRYPTION = "asymmetricEncryption"
HTTP_API_KEY = "httpApiKey"
HTTP = "http"
OAUTH2 = "oauth2"
OPENID_CONNECT = "openIdConnect"
PLAIN = "plain"
SCRAM_SHA256 = "scramSha256"
SCRAM_SHA512 = "scramSha512"
GSSAPI = "gssapi"


@dataclass
class SecurityScheme:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dataclass should also have a __post_init__ method to perform validation.

Validation required:

  1. flows is required if type == oauth2
  2. name is required if type == http
  3. scheme is required if type == http
  4. ...and more (see the SecuritySchemeObject fields table for the complete set of validation rules)

"""
https://www.asyncapi.com/docs/specifications/v2.2.0#securitySchemeObject
"""
type: SecuritySchemesType
alex-zywicki marked this conversation as resolved.
Show resolved Hide resolved
description: Optional[str] = None
name: Optional[str] = None # Required for httpApiKey
in_: Optional[str] = None # Required for httpApiKey | apiKey
scheme: Optional[HTTPSecuritySchemeType] = None # Required for http
alex-zywicki marked this conversation as resolved.
Show resolved Hide resolved
bearer_format: Optional[str] = None # Optional for http ("bearer")
flows: Optional[Mapping[OAuth2FlowType, OAuth2Flow]] = None # Required for oauth2
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do see the reason behind choosing a Mapping type of enum keys for the OAuth2Flows object, however I feel that a dataclass would fit better as it allows the use of __post_init__ validation.

For example:

@dataclass
class OAuthFlows:
    """https://www.asyncapi.com/docs/specifications/v2.2.0#oauthFlowsObject"""

    internal: Optional[OAuthFlow] = None
    password: Optional[OAuthFlow] = None
    client_credentials: Optional[OAuthFlow] = None
    authorization_code: Optional[OAuthFlow] = None

    def __post_init__(self):
        if self.internal is not None:
            if self.internal.authorization_url is None:
                raise ValueError("Internal OAuth flow is missing Authorization URL")
        # and similar rules for the rest of the flow kinds, as described in the AsyncAPI docs

open_id_connect_url: Optional[str] = None # Required for openIdConnect

x_basic_info_func: Optional[str] = None # Required for http(basic)
x_token_info_func: Optional[str] = None # Required for oauth2
x_api_key_info_func: Optional[str] = None # Required for apiKey
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also add x_bearer_info_func (as connexion does)?


@staticmethod
def forge(
type_: Type["SecurityScheme"],
data: JSONMapping,
forge: Forge
) -> "SecurityScheme":

scheme_type_raw = data.get("type")
if not scheme_type_raw:
raise UnsupportedSecurityScheme
try:
SecuritySchemesType(scheme_type_raw)
except ValueError:
raise UnsupportedSecurityScheme(scheme_type_raw)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed. Svarog and dataclasses will take care of this validation.


return type_(
type=forge(
type_.__annotations__["type"],
data.get("type")
),
description=forge(
type_.__annotations__["description"],
data.get("description")
),
name=forge(
type_.__annotations__["name"],
data.get("name")
),
in_=forge(
type_.__annotations__["in_"],
data.get("in")
),
scheme=forge(
type_.__annotations__["scheme"],
data.get("scheme")
),
bearer_format=forge(
type_.__annotations__["bearer_format"],
data.get("bearerFormat")
),
flows=forge(
type_.__annotations__["flows"],
data.get("flows")
),
open_id_connect_url=forge(
type_.__annotations__["open_id_connect_url"],
data.get("openIdConnectUrl")
),
x_basic_info_func=forge(
type_.__annotations__["x_basic_info_func"],
data.get("x-basicInfoFunc")
),
x_token_info_func=forge(
type_.__annotations__["x_token_info_func"],
data.get("x-tokenInfoFunc")
),
x_api_key_info_func=forge(
type_.__annotations__["x_api_key_info_func"],
data.get("x-apiKeyInfoFunc")
)
)


register_forge(SecurityScheme, SecurityScheme.forge)


@dataclass
class SecurityRequirement:
alex-zywicki marked this conversation as resolved.
Show resolved Hide resolved
# https://www.asyncapi.com/docs/specifications/v2.2.0#securityRequirementObject
name: str
scopes: Sequence[str]
scheme: SecurityScheme

@staticmethod
def forge(
type_: Type["SecurityRequirement"],
data: JSONMapping,
forge: Forge
) -> "SecurityRequirement":
return type_(
name=forge(type_.__annotations__["name"], data.get("name")),
scopes=forge(type_.__annotations__["scopes"], data.get("scopes")),
scheme=forge(type_.__annotations__["scheme"], data.get("scheme"))
)


register_forge(SecurityRequirement, SecurityRequirement.forge)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An explicit forge method is only needed when there is some sort of field transformation in the deserialiser (for example camerCase to snake_case, or kebab-case to snake_case). In this case there is no transformation, hence the deserialisation can be handled by svarog's generic dataclass deserialiser, ie we don't need an explicit forge.

Loading