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

Prehook user creation #378

Merged
merged 38 commits into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
f7c1a52
base webhook integration
ChaamC Dec 10, 2020
fa0ab13
change webhook feature and add all webhook tests
ChaamC Dec 17, 2020
66fc1c1
updated docs with webhooks info
ChaamC Dec 17, 2020
927ad29
Merge branch 'master' of https://github.com/Ouranosinc/Magpie into pr…
ChaamC Dec 17, 2020
af5b257
fix linter
ChaamC Dec 17, 2020
d393703
fix doc
ChaamC Dec 17, 2020
bc7b78a
move webhook operations
ChaamC Jan 4, 2021
39dcb2e
remove webhook env variable, and fix broken tests if env variable ove…
ChaamC Jan 4, 2021
c704e5d
move webhooks tests
ChaamC Jan 5, 2021
8a59b17
webhook tests refactoring and checking webhook config
ChaamC Jan 7, 2021
18ed180
change user status on error, test for non existent url
ChaamC Jan 12, 2021
a50fbf5
move webhooks code
ChaamC Jan 12, 2021
0099a0d
refactor webhook code
ChaamC Jan 12, 2021
d5d0d1a
updated doc
ChaamC Jan 12, 2021
b1692c3
fix webhook config and complete doc (#343)
ChaamC Jan 13, 2021
b058990
fix lint
ChaamC Jan 13, 2021
dc69fc9
fix docf lint
ChaamC Jan 13, 2021
7fbc039
check if status is ok in webhook tests
ChaamC Jan 14, 2021
36c7897
add more flexible webhook payload and related test
ChaamC Jan 15, 2021
7ff6b6e
fix webhook delete params reading
ChaamC Jan 18, 2021
053539c
fix print
ChaamC Jan 18, 2021
b3e03b5
fix lint
ChaamC Jan 18, 2021
ce02667
simplified dict template replacing
ChaamC Jan 18, 2021
5c8eb69
add url templating and reoganize replace_template function
ChaamC Jan 22, 2021
c31b9ae
optimize template webhook request
ChaamC Jan 25, 2021
9283bfe
Merge branch 'master' of https://github.com/Ouranosinc/Magpie into pr…
ChaamC Jan 25, 2021
2190641
fix fstrings
ChaamC Jan 26, 2021
9445253
implement webhook_error case for the tmp_url
ChaamC Jan 27, 2021
3d0a2b4
change webhook action enum
ChaamC Jan 29, 2021
dd44855
small fixes after pr update
ChaamC Jan 29, 2021
2cd96ec
fix lint
ChaamC Jan 29, 2021
1d801d0
fix webhook action names
Feb 8, 2021
a390dc1
Merge pull request #383 from Ouranosinc/webhook_tmp_url
cwcummings Feb 8, 2021
75462c5
Merge branch 'master' of https://github.com/Ouranosinc/Magpie into pr…
Feb 8, 2021
957718c
Merge branch 'master' of https://github.com/Ouranosinc/Magpie into pr…
cwcummings Feb 9, 2021
f012170
fix test admin user
cwcummings Feb 9, 2021
20c4380
fix for py3.5 test
cwcummings Feb 9, 2021
89209fc
fix lint
cwcummings Feb 9, 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
9 changes: 8 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@ Changes
`Unreleased <https://github.com/Ouranosinc/Magpie/tree/master>`_ (latest)
------------------------------------------------------------------------------------

* Nothing yet.
Features / Changes
~~~~~~~~~~~~~~~~~~~~~
* Add a list of webhook urls, defined in the configuration, that will be called when creating or deleting a user.
(resolves `#343 <https://github.com/Ouranosinc/Magpie/issues/343>`_).

Bug Fixes
~~~~~~~~~~~~~~~~~~~~~
* N/A

`3.4.0 <https://github.com/Ouranosinc/Magpie/tree/3.4.0>`_ (2020-12-09)
------------------------------------------------------------------------------------
Expand Down
11 changes: 11 additions & 0 deletions config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,14 @@ groups:
- name: <groupname> # required if entry provided
description: <some description> # optional
discoverable: True # optional

# Definitions of all the webhooks urls that will be called when creating or deleting a users.
webhooks:
- name: <webhook_name>
action: create_user | remove_user
method: GET | HEAD | POST | PUT | PATCH | DELETE
url: <location>
payload: # add any parameters required with the webhook url here
<param_name> : <param_value>
...

2 changes: 1 addition & 1 deletion config/magpie.ini
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ magpie.max_restart = 5
# This secret should be the same in Twitcher !
magpie.secret =
magpie.push_phoenix = true
magpie.config_path =
# magpie.config_path =
ChaamC marked this conversation as resolved.
Show resolved Hide resolved

# caching settings refer to the Performance section in the documentation
cache.regions = adapter, acl
Expand Down
47 changes: 47 additions & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,15 @@ a combined configuration as follows.
group: my-group # will reference above group
action: create

webhooks:
- name: <webhook_name>
action: create_user | remove_user
method: GET | HEAD | POST | PUT | PATCH | DELETE
url: <location>
payload: # add any parameters required with the webhook url here
<param_name> : <param_value>
...


For backward compatibility reasons, `Magpie` will first look for separate files to load each section individually.
To enforce using a combined file as above, either provide ``MAGPIE_CONFIG_PATH = <path>/config.yml`` or ensure that each
Expand All @@ -160,6 +169,44 @@ creation. Otherwise defaults are assumed and only the specified user or group na
`providers.cfg`_ and `permissions.cfg`_ for further details about specific formatting and behaviour of each available
field.

.. versionadded:: 3.6

A section for webhooks has also been added to the combined configuration file. This section defines a list of
ChaamC marked this conversation as resolved.
Show resolved Hide resolved
urls that should be called after creating or deleting a user. The webhooks urls are responsible for any extra
steps that should be taken on external services after the user creation/deletion. Note that the webhook requests are
asynchronous, so Magpie might execute other requests before the webhooks requests are fully done.

Each webhook should define the following parameters :

- | ``name``

The webhook's name

- | ``action``
| (Values: ``create_user | delete_user``)

The action where the webhook will be executed, either during the user's creation (create_user),
or the user's deletion (delete_user)

- | ``method``

The http method used for the webhook request

- | ``url``

A valid http url that should be called for the webhook request

- | ``payload``

A list of parameters that will be sent with the request. Note that those parameters can contain
template variables using the braces characters {}.
The only permitted template variables for now are ``user_name`` and ``tmp_url``. These variables will get replaced
by the corresponding value at the time of the request.

For example, the parameter "user_name_param" could be defined in the config using a template variable ``user_name``:

``"user_name_param": "{user_name}"``

ChaamC marked this conversation as resolved.
Show resolved Hide resolved
Settings and Constants
----------------------

Expand Down
1 change: 1 addition & 0 deletions env/magpie.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,4 @@ MAGPIE_TEST_STATUS=true
MAGPIE_TEST_UTILS=true
MAGPIE_TEST_REGISTER=true
MAGPIE_TEST_FUNCTIONAL=true
MAGPIE_TEST_WEBHOOKS=true
1 change: 1 addition & 0 deletions magpie/api/management/user/user_formats.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ def fmt_usr(usr, grp_names):
"user_name": str(usr.user_name),
"email": str(usr.email),
"group_names": sorted(list(grp_names) if grp_names else [grp.group_name for grp in user.groups]),
"status": int(user.status)
ChaamC marked this conversation as resolved.
Show resolved Hide resolved
}
if user.user_name != get_constant("MAGPIE_ANONYMOUS_USER"):
user_info["user_id"] = int(user.id)
Expand Down
15 changes: 14 additions & 1 deletion magpie/api/management/user/user_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
HTTPNotFound,
HTTPOk
)
import transaction
from ziggurat_foundations.models.services.group import GroupService
from ziggurat_foundations.models.services.resource import ResourceService
from ziggurat_foundations.models.services.user import UserService
Expand All @@ -21,9 +22,13 @@
from magpie.api.management.resource import resource_utils as ru
from magpie.api.management.service.service_formats import format_service
from magpie.api.management.user import user_formats as uf
from magpie.api.webhooks import process_webhook_requests, WEBHOOK_CREATE_USER_ACTION
from magpie.constants import get_constant
from magpie.permissions import PermissionSet, PermissionType, format_permissions
from magpie.services import service_factory
from magpie.utils import get_logger

LOGGER = get_logger(__name__)

if TYPE_CHECKING:
# pylint: disable=W0611,unused-import
Expand Down Expand Up @@ -106,8 +111,16 @@ def _add_to_group(usr, grp):
_add_to_group(new_user, _get_group(anonym_grp_name))
new_user_groups.append(anonym_grp_name)

user_content = uf.format_user(new_user, new_user_groups)

# Force commit before sending the webhook requests, so that the user's status is editable if a webhook error occurs
transaction.commit()

# Process any webhook requests
process_webhook_requests(WEBHOOK_CREATE_USER_ACTION, {"user_name": user_name, "tmp_url": "tmp_url:80/todo"}, True)
dbyrns marked this conversation as resolved.
Show resolved Hide resolved

return ax.valid_http(http_success=HTTPCreated, detail=s.Users_POST_CreatedResponseSchema.description,
content={"user": uf.format_user(new_user, new_user_groups)})
content={"user": user_content})


def create_user_resource_permission_response(user, resource, permission, db_session, overwrite=False):
Expand Down
6 changes: 6 additions & 0 deletions magpie/api/management/user/user_views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
User Views, both for specific user-name provided as request path variable and special keyword for logged session user.
"""

from pyramid.httpexceptions import HTTPBadRequest, HTTPConflict, HTTPCreated, HTTPForbidden, HTTPNotFound, HTTPOk
from pyramid.settings import asbool
from pyramid.view import view_config
Expand All @@ -15,6 +16,7 @@
from magpie.api.management.service.service_formats import format_service_resources
from magpie.api.management.user import user_formats as uf
from magpie.api.management.user import user_utils as uu
from magpie.api.webhooks import process_webhook_requests, WEBHOOK_DELETE_USER_ACTION
from magpie.constants import MAGPIE_CONTEXT_PERMISSION, MAGPIE_LOGGED_PERMISSION, get_constant
from magpie.permissions import PermissionType, format_permissions
from magpie.services import SERVICE_TYPE_DICT
Expand Down Expand Up @@ -135,6 +137,10 @@ def delete_user_view(request):
http_error=HTTPForbidden, msg_on_fail=s.User_DELETE_ForbiddenResponseSchema.description)
ax.evaluate_call(lambda: request.db.delete(user), fallback=lambda: request.db.rollback(),
http_error=HTTPForbidden, msg_on_fail=s.User_DELETE_ForbiddenResponseSchema.description)

# Process any webhook requests
process_webhook_requests(WEBHOOK_DELETE_USER_ACTION, {"user_name": user.user_name})

ChaamC marked this conversation as resolved.
Show resolved Hide resolved
return ax.valid_http(http_success=HTTPOk, detail=s.User_DELETE_OkResponseSchema.description)


Expand Down
4 changes: 4 additions & 0 deletions magpie/api/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ def service_api_route_info(service_api, **kwargs):
_LOGGED_USER_VALUE = get_constant("MAGPIE_LOGGED_USER")
LoggedUserBase = "/users/{}".format(_LOGGED_USER_VALUE)

# Values for the 'status' field in the Users database
UserOKStatus = 1
UserWebhookErrorStatus = 0

SwaggerGenerator = Service(
path="/json",
name="swagger_schema_json")
Expand Down
122 changes: 122 additions & 0 deletions magpie/api/webhooks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import multiprocessing

from pyramid.threadlocal import get_current_registry
import requests
import transaction

from magpie.api.schemas import UserWebhookErrorStatus
from magpie.constants import MAGPIE_INI_FILE_PATH
from magpie.db import get_db_session_from_config_ini
from magpie import models
from magpie.utils import get_logger, get_settings

# List of keys that should be found for a single webhook item in the config
WEBHOOK_KEYS = {
"name",
"action",
"method",
"url",
"payload"
}

# List of possible actions associated with a webhook
WEBHOOK_CREATE_USER_ACTION = "create_user"
WEBHOOK_DELETE_USER_ACTION = "delete_user"
WEBHOOK_ACTIONS = [
WEBHOOK_CREATE_USER_ACTION,
WEBHOOK_DELETE_USER_ACTION
]

# These are the parameters permitted to use the template form in the webhook payload.
WEBHOOK_TEMPLATE_PARAMS = ["user_name", "tmp_url"]

HTTP_METHODS = ["GET", "HEAD", "POST", "PUT", "PATCH", "DELETE"]

LOGGER = get_logger(__name__)


def process_webhook_requests(action, params, update_user_status_on_error=False):
"""
Checks the config for any webhooks that correspond to the input action, and prepares corresponding requests.

:param action: tag identifying which webhooks to use in the config
:param params: dictionary containing the required parameters for the request, they will replace templates
found in the payload
:param update_user_status_on_error: update the user status or not in case of a webhook error
"""
# Check for webhook requests
webhooks = get_settings(get_current_registry())["webhooks"][action]
if len(webhooks) > 0:
# Execute all webhook requests
pool = multiprocessing.Pool(processes=len(webhooks))
args = [(webhook, params, update_user_status_on_error) for webhook in webhooks]
pool.starmap_async(send_webhook_request, args)


def replace_template(param_name, param_value, payload):
"""
Replace each instances of a template parameter by its corresponding value.

:param param_name: name of a template parameter
:param param_value: value of a template parameter
:param payload: structure containing the data to be processed by the template replacement
:return: structure containing the data with the replaced template parameters
"""
if isinstance(payload, dict):
replace_dict = payload.copy()
dbyrns marked this conversation as resolved.
Show resolved Hide resolved
for key, value in payload.items():
# replace templates in the dictionary value
replace_dict[key] = replace_template(param_name, param_value, value)

# replace templates in the dictionary key
new_key = replace_template(param_name, param_value, key)
if new_key != key:
replace_dict[new_key] = replace_dict[key]
del replace_dict[key]
return replace_dict
if isinstance(payload, list):
return [replace_template(param_name, param_value, value) for value in payload]
if isinstance(payload, str):
return payload.replace("{" + param_name + "}", param_value)
# For any other type, no replacing to do
return payload


def send_webhook_request(webhook_config, params, update_user_status_on_error=False):
"""
Sends a single webhook request using the input config.

:param webhook_config: dictionary containing the config data of a single webhook
:param params: dictionary containing the required parameters for the request, they will replace templates
found in the payload
:param update_user_status_on_error: update the user status or not in case of a webhook error
"""
# Replace each instance of template parameters if a corresponding value was defined in input
try:
for template_param in WEBHOOK_TEMPLATE_PARAMS:
if template_param in params:
webhook_config["payload"] = replace_template(template_param,
dbyrns marked this conversation as resolved.
Show resolved Hide resolved
params[template_param],
webhook_config["payload"])
except Exception as exception:
LOGGER.error("An exception has occured while processing the template parameters in a webhook payload : %s",
str(exception))
try:
resp = requests.request(webhook_config["method"], webhook_config["url"], json=webhook_config["payload"])
dbyrns marked this conversation as resolved.
Show resolved Hide resolved
resp.raise_for_status()
except Exception as exception:
LOGGER.error("An exception has occured with the webhook request : %s", webhook_config["name"])
LOGGER.error(str(exception))
if "user_name" in params.keys() and update_user_status_on_error:
webhook_update_error_status(params["user_name"])


def webhook_update_error_status(user_name):
"""
Updates the user's status to indicate an error occured with the webhook requests.
"""
# find user and change its status to 0 to indicate a webhook error happened
db_session = get_db_session_from_config_ini(MAGPIE_INI_FILE_PATH)
user = db_session.query(models.User).filter(models.User.user_name == user_name) # pylint: disable=E1101,no-member
user.update({"status": UserWebhookErrorStatus})
ChaamC marked this conversation as resolved.
Show resolved Hide resolved
transaction.commit()
32 changes: 31 additions & 1 deletion magpie/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,18 @@
"""
Magpie is a service for AuthN and AuthZ based on Ziggurat-Foundations.
"""
from collections import defaultdict
from urllib.parse import urlparse

from pyramid.settings import asbool
from pyramid_beaker import set_cache_regions_from_settings

from magpie.api.webhooks import HTTP_METHODS, WEBHOOK_ACTIONS, WEBHOOK_KEYS
from magpie.cli.register_defaults import register_defaults
from magpie.constants import get_constant
from magpie.db import get_db_session_from_config_ini, run_database_migration_when_ready, set_sqlalchemy_log_level
from magpie.register import magpie_register_permissions_from_config, magpie_register_services_from_config
from magpie.register import magpie_register_permissions_from_config, magpie_register_services_from_config, \
get_all_configs
from magpie.security import get_auth_config
from magpie.utils import get_logger, patch_magpie_url, print_log

Expand Down Expand Up @@ -74,6 +78,32 @@ def main(global_config=None, **settings): # noqa: F811
raise_missing=False, raise_not_set=False, print_missing=True)
magpie_register_permissions_from_config(perm_cfg, db_session=db_session)

print_log("Register webhook configurations...", LOGGER)
settings["webhooks"] = defaultdict(lambda: [])
if combined_config:
webhook_configs = get_all_configs(combined_config, "webhooks", allow_missing=True)

for cfg in webhook_configs:
for webhook in cfg:
# Validate the webhook config
if webhook.keys() != WEBHOOK_KEYS:
raise ValueError(f"Missing or invalid key found in a webhook config "
f"from the config file {combined_config}")
if webhook["action"] not in WEBHOOK_ACTIONS:
raise ValueError(f"Invalid action {webhook['action']} found in a webhook config "
f"from the config file {combined_config}")
if webhook["method"] not in HTTP_METHODS:
raise ValueError(f"Invalid method {webhook['method']} found in a webhook config "
f"from the config file {combined_config}")
url_parsed = urlparse(webhook["url"])
if not all([url_parsed.scheme, url_parsed.netloc, url_parsed.path]):
raise ValueError(f"Invalid url {webhook['url']} found in a webhook config "
f"from the config file {combined_config}")

# Regroup webhooks by action key
webhook_sub_config = {k: webhook[k] for k in set(list(webhook.keys())) - {"action"}}
settings["webhooks"][webhook["action"]].append(webhook_sub_config)

print_log("Running configurations setup...", LOGGER)
patch_magpie_url(settings)

Expand Down
1 change: 1 addition & 0 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ pyramid-twitcher==0.5.3
pytest
tox>=3.0
webtest
waitress
Copy link
Collaborator

Choose a reason for hiding this comment

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

extra dependency not needed, see other comment

1 change: 1 addition & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ markers =
api: magpie API operations
cli: magpie CLI helper operations
ui: magpie UI operations
webhooks: magpie webhooks operations
utils: magpie utility functions
functional: magpie functional operations
auth_admin: magpie operations that require admin-level access
Expand Down
3 changes: 2 additions & 1 deletion tests/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def filter_test_files(root, filename):
MAGPIE_TEST_RESOURCES = RunOptionDecorator("MAGPIE_TEST_RESOURCES", "magpie resources operations")
MAGPIE_TEST_PERMISSIONS = RunOptionDecorator("MAGPIE_TEST_PERMISSIONS", "magpie permissions operations")
MAGPIE_TEST_GROUPS = RunOptionDecorator("MAGPIE_TEST_GROUPS", "magpie groups operations")
MAGPIE_TEST_USERS = RunOptionDecorator("MAGPIE_TEST_USERS", "magpie groups operations")
MAGPIE_TEST_USERS = RunOptionDecorator("MAGPIE_TEST_USERS", "magpie users operations")
MAGPIE_TEST_LOGGED = RunOptionDecorator("MAGPIE_TEST_LOGGED", "validate 'Logged User' views (i.e.: current)")
MAGPIE_TEST_STATUS = RunOptionDecorator("MAGPIE_TEST_STATUS", "validate views found/displayed as per permissions")
MAGPIE_TEST_PUBLIC = RunOptionDecorator("MAGPIE_TEST_PUBLIC", "validate evaluated permission on 'Public' resources")
Expand All @@ -42,6 +42,7 @@ def filter_test_files(root, filename):
MAGPIE_TEST_API = RunOptionDecorator("MAGPIE_TEST_API", "magpie API operations")
MAGPIE_TEST_CLI = RunOptionDecorator("MAGPIE_TEST_CLI", "magpie CLI operations")
MAGPIE_TEST_UI = RunOptionDecorator("MAGPIE_TEST_UI", "magpie UI operations")
MAGPIE_TEST_WEBHOOKS = RunOptionDecorator("MAGPIE_TEST_WEBHOOKS", "magpie webhooks operations")
MAGPIE_TEST_UTILS = RunOptionDecorator("MAGPIE_TEST_UTILS", "magpie utility functions")
MAGPIE_TEST_FUNCTIONAL = RunOptionDecorator("MAGPIE_TEST_FUNCTIONAL", "functional operations sequence")
MAGPIE_TEST_AUTH_ADMIN = RunOptionDecorator("MAGPIE_TEST_AUTH_ADMIN", "operations that require admin-level access")
Expand Down
Loading