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

Alert - Support for authentication when calling receiver endpoint. #560

Open
wants to merge 27 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c39273f
added support for authentication in the alert (both basic and bearer).
andreee94 Feb 24, 2022
1fa0693
added unit tests for alert authentication
andreee94 Feb 24, 2022
bd88ef3
updated documentation for alert authentication
andreee94 Feb 24, 2022
1c3d61e
fixed yq command in makefile
andreee94 Feb 24, 2022
3de5ef9
added support for authentication in the alert (both basic and bearer).
andreee94 Feb 24, 2022
51ea8a4
added unit tests for alert authentication
andreee94 Feb 24, 2022
9a10f8f
updated documentation for alert authentication
andreee94 Feb 24, 2022
d771c5e
Merge remote-tracking branch 'refs/remotes/origin/feat/alert-authenti…
andreee94 Feb 28, 2022
7613ba3
Improved alert class accordingly to the pull request
andreee94 Feb 28, 2022
3d383a5
Updated alert documentation
andreee94 Feb 28, 2022
7957567
Renamed authorization_prefix to authentication_scheme in the alert_sc…
andreee94 Feb 28, 2022
f96b907
Testing authentication_scheme validation in alert.
andreee94 Feb 28, 2022
be8c718
added support for authentication in the alert (both basic and bearer).
andreee94 Feb 24, 2022
cec1a0f
added unit tests for alert authentication
andreee94 Feb 24, 2022
8050102
updated documentation for alert authentication
andreee94 Feb 24, 2022
c04c902
Improved alert class accordingly to the pull request
andreee94 Feb 28, 2022
d98c6bd
Updated alert documentation
andreee94 Feb 28, 2022
a34506d
Renamed authorization_prefix to authentication_scheme in the alert_sc…
andreee94 Feb 28, 2022
14f58cd
Testing authentication_scheme validation in alert.
andreee94 Feb 28, 2022
dffbd88
Merge remote-tracking branch 'refs/remotes/origin/feat/alert-authenti…
andreee94 Mar 1, 2022
b3a54c0
Code updated to fix pylint suggestions, and other minor changes.
andreee94 Mar 1, 2022
0f26f6d
Embracing the EAFP paradigm.
andreee94 Mar 2, 2022
0a8b73b
Merge branch 'develop' into feat/alert-authentication
andreee94 Mar 2, 2022
b207360
Merge branch 'develop' into feat/alert-authentication
andreee94 Mar 3, 2022
826c78e
Moving the alert webhook authentication documentation in the addition…
andreee94 Mar 5, 2022
f4d02bb
Improved the helm chart to reference existing secrets and config maps…
andreee94 Mar 5, 2022
6aa67ab
Merge remote-tracking branch 'refs/remotes/origin/feat/alert-authenti…
andreee94 Mar 5, 2022
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
171 changes: 169 additions & 2 deletions connaisseur/alert.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,166 @@ def alerting_required(self, event_category: str) -> bool:
return bool(self.config.get(event_category))


class AlertReceiverAuthentication:
"""
Class to store authentication information for securely sending events to the alert receiver.
"""

class AlertReceiverBasicAuthentication:
"""
Class to store authentication information for basic authentication type with username and password.
"""
username: str
password: str
authentication_type: str
authorization_prefix: str = "Basic"

def __init__(self, alert_receiver_config: dict):
basic_authentication_config = alert_receiver_config.get(
"receiver_authentication_basic", None
Copy link
Member

Choose a reason for hiding this comment

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

None is the default for dict.get. Same for other occurrences.

)

if (
basic_authentication_config is None
): # TODO maybe remove this check since it is included in the json validation?
Copy link
Member

Choose a reason for hiding this comment

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

the check is fine

raise ConfigurationError("No basic authentication configuration found.")

username_env = basic_authentication_config.get("username_env", None)
password_env = basic_authentication_config.get("password_env", None)

if (
username_env is None or password_env is None
): # TODO maybe remove this check since it is included in the json validation?
raise ConfigurationError(
"No username_env or password_env configuration found."
)

self.username = os.environ.get(username_env, None)
self.password = os.environ.get(password_env, None)

if self.username is None or self.password is None:
raise ConfigurationError(
f"No username or password found from environmental variables {username_env} and {password_env}."
)

self.authorization_prefix = basic_authentication_config.get(
"authorization_prefix", "Basic"
Copy link
Member

Choose a reason for hiding this comment

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

Should the default be self.authorization_prefix, as the value is hardcoded at the class?

)
# TODO maybe validate authorization prefix
Copy link
Member

Choose a reason for hiding this comment

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

Sounds like a good idea

Copy link
Author

Choose a reason for hiding this comment

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

I cannot find an actual validation rule.
The only thing I can found is: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization.
I have assumed that the authentication scheme (previously called authorization prefix) is not null or empty and does not contain any space.


def get_header(self) -> dict:
return {
"Authorization": f"{self.authorization_prefix} {self.username}:{self.password}"
}

class AlertReceiverBearerAuthentication:
"""
Class to store authentication information for bearer authentication type which uses a token.
"""
token: str
authorization_prefix: str = "Bearer" # default is bearer

def __init__(self, alert_receiver_config: dict):
bearer_authentication_config = alert_receiver_config.get(
"receiver_authentication_bearer", None
)

if (
bearer_authentication_config is None
): # TODO maybe remove this check since it is included in the json validation?
raise ConfigurationError(
"No bearer authentication configuration found."
)
Copy link
Member

Choose a reason for hiding this comment

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

This is boilerplate (together with assigning self.authentication_prefix at the end of the method).

Suggestion: make a base class, introduce a variable for the lookup key (e.g. "receiver_authentication_bearer"), and run all this via super().__init__(*args, **kwargs).


token_env = bearer_authentication_config.get("token_env", None)
token_file = bearer_authentication_config.get("token_file", None)

if (
token_env is None and token_file is None
): # TODO maybe remove this check since it is included in the json validation?
raise ConfigurationError(
"No token_env and token_file configuration found."
)

if (
token_env is not None and token_file is not None
): # TODO maybe remove this check since it is included in the json validation?
raise ConfigurationError(
"Both token_env and token_file configuration found. Only one is required."
Copy link
Member

Choose a reason for hiding this comment

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

Only one can be given.

)

if token_env is not None:
self.token = os.environ.get(token_env, None)

if self.token is None:
raise ConfigurationError(
f"No token found from environmental variable {token_env}."
)
else:
try:
with open(token_file, "r") as token_file:
self.token = token_file.read()
except FileNotFoundError:
raise ConfigurationError(f"No token file found at {token_file}.")
except Exception as err:
raise ConfigurationError(
f"An error occurred while loading the token file {token_file}: {str(err)}"
)

self.authorization_prefix = bearer_authentication_config.get(
"authorization_prefix", "Bearer"
)
# TODO maybe validate authorization prefix

def get_header(self) -> dict:
return {"Authorization": f"{self.authorization_prefix} {self.token}"}

def __init__(self, alert_receiver_config: dict):
self.authentication_type = alert_receiver_config.get(
"receiver_authentication_type", "none"
)

if self.is_basic():
self.__init_basic_authentication(alert_receiver_config)
elif self.is_bearer():
self.__init_bearer_authentication(alert_receiver_config)
Copy link
Member

Choose a reason for hiding this comment

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

init_map = {
    "basic": AlertReceiverAuthentication.AlertReceiverBasicAuthentication,
    "bearer": AlertReceiverAuthentication.AlertReceiverBearerAuthentication,
}
try:
    self.auth_instance = init_map[self.authentication_type](alert_receiver_config)
except AttributeError:
    ....

Then, it get_auth_header(), you don't need to do the case handling, but can just return {} if self.auth_instance is None self.auth_instance.get_header().


def is_basic(self):
return self.authentication_type == "basic"

def is_bearer(self):
return self.authentication_type == "bearer"

def is_none(self):
return self.authentication_type == "none"

def __init_bearer_authentication(self, alert_receiver_config: dict):
self.bearer_authentication = (
AlertReceiverAuthentication.AlertReceiverBearerAuthentication(
alert_receiver_config
)
)

def __init_basic_authentication(self, alert_receiver_config: dict):
self.basic_authentication = (
AlertReceiverAuthentication.AlertReceiverBasicAuthentication(
alert_receiver_config
)
)

def get_auth_header(self) -> dict:
if self.is_basic():
return self.basic_authentication.get_header()
elif self.is_bearer():
return self.bearer_authentication.get_header()
elif self.is_none():
return {}
else:
raise ConfigurationError(
"No authentication type found."
) # hopefully this never happens


class Alert:
"""
Class to store image information about an alert as attributes and a sending
Expand All @@ -59,6 +219,7 @@ class Alert:

template: str
receiver_url: str
receiver_authentication: AlertReceiverAuthentication
payload: str
headers: dict

Expand Down Expand Up @@ -101,12 +262,13 @@ def __init__(
"images": images,
}
self.receiver_url = receiver_config["receiver_url"]
self.receiver_authentication = AlertReceiverAuthentication(receiver_config)
self.template = receiver_config["template"]
self.throw_if_alert_sending_fails = receiver_config.get(
"fail_if_alert_sending_fails", False
)
self.payload = self.__construct_payload(receiver_config)
self.headers = self.__get_headers(receiver_config)
self.headers = self.__get_headers(receiver_config, self.receiver_authentication)

def __construct_payload(self, receiver_config: dict) -> str:
try:
Expand Down Expand Up @@ -153,13 +315,18 @@ def send_alert(self) -> Optional[requests.Response]:
return response

@staticmethod
def __get_headers(receiver_config):
def __get_headers(
receiver_config: dict, receiver_authentication: AlertReceiverAuthentication
) -> dict:
headers = {"Content-Type": "application/json"}
additional_headers = receiver_config.get("custom_headers")
if additional_headers is not None:
for header in additional_headers:
key, value = header.split(":", 1)
headers.update({key.strip(): value.strip()})
auth_header = receiver_authentication.get_auth_header()
if auth_header: # not None and not empty
headers.update(auth_header)
return headers


Expand Down
148 changes: 147 additions & 1 deletion connaisseur/res/alertconfig_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,50 @@
"receiver_url": {
"type": "string"
},
"receiver_authentication_type": {
"type": "string",
"enum": [
"none",
"basic",
"bearer"
]
},
"receiver_authentication_basic": {
"type": "object",
"properties": {
"username_env": {
"type": "string"
},
"password_env": {
"type": "string"
},
"authorization_prefix": {
"type": "string"
}
},
"required": [
"username_env",
"password_env"
]
},
"receiver_authentication_bearer": {
"type": "object",
"oneOf": [
{"required": ["token_file"]},
{"required": ["token_env"]}
],
"properties": {
"authorization_prefix": {
"type": "string"
},
"token_file": {
"type": "string"
},
"token_env": {
"type": "string"
}
}
},
"priority": {
"type": "integer"
},
Expand All @@ -35,6 +79,35 @@
"type": "boolean"
}
},
"anyOf": [
{
"properties": {
"receiver_authentication_type": {
"const": "basic"
}
},
"required": [
"receiver_authentication_basic"
]
},
{
"properties": {
"receiver_authentication_type": {
"const": "bearer"
}
},
"required": [
"receiver_authentication_bearer"
]
},
{
"properties": {
"receiver_authentication_type": {
"const": "none"
}
}
}
],
"required": [
"template",
"receiver_url"
Expand All @@ -60,6 +133,50 @@
"receiver_url": {
"type": "string"
},
"receiver_authentication_type": {
"type": "string",
"enum": [
"none",
"basic",
"bearer"
]
},
"receiver_authentication_basic": {
"type": "object",
"properties": {
"username_env": {
"type": "string"
},
"password_env": {
"type": "string"
},
"authorization_prefix": {
"type": "string"
}
},
"required": [
"username_env",
"password_env"
]
},
"receiver_authentication_bearer": {
"type": "object",
"oneOf": [
{"required": ["token_file"]},
{"required": ["token_env"]}
],
"properties": {
"authorization_prefix": {
"type": "string"
},
"token_file": {
"type": "string"
},
"token_env": {
"type": "string"
}
}
},
"priority": {
"type": "integer"
},
Expand All @@ -77,6 +194,35 @@
"type": "boolean"
}
},
"anyOf": [
{
"properties": {
"receiver_authentication_type": {
"const": "basic"
}
},
"required": [
"receiver_authentication_basic"
]
},
{
"properties": {
"receiver_authentication_type": {
"const": "bearer"
}
},
"required": [
"receiver_authentication_bearer"
]
},
{
"properties": {
"receiver_authentication_type": {
"const": "none"
}
}
}
],
"required": [
"template",
"receiver_url"
Expand All @@ -89,4 +235,4 @@
]
}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

missing newline