Allow authenticated token requirement for tasks
Tasks when defined can now set if they require the user submitting a token to be authenticated. keystone_user is now passed to actions when a token is submitted. This requires all actions to update their submit function, but a suitable fallthrough will exist for a cycle to allow time. Also fixes a minor issue around where error handling for renamed or deprecated tasks is handled that cropped up while testing this patch. Change-Id: I4b51201872cb5a14f299f90e22a8b010d11a71cb
This commit is contained in:
parent
c42463bb29
commit
43e0c625e3
@ -186,8 +186,15 @@ class BaseAction(object):
|
||||
)
|
||||
return self._post_approve()
|
||||
|
||||
def submit(self, token_data):
|
||||
return self._submit(token_data)
|
||||
def submit(self, token_data, keystone_user=None):
|
||||
try:
|
||||
return self._submit(token_data, keystone_user)
|
||||
except TypeError:
|
||||
self.logger.warning(
|
||||
"DEPRECATED: Action '_submit' must accept a second parameter "
|
||||
"'keystone_user=None' along with the required 'token_data'."
|
||||
)
|
||||
return self._submit(token_data)
|
||||
|
||||
def _prepare(self):
|
||||
raise NotImplementedError
|
||||
@ -195,7 +202,7 @@ class BaseAction(object):
|
||||
def _approve(self):
|
||||
raise NotImplementedError
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
raise NotImplementedError
|
||||
|
||||
def __str__(self):
|
||||
|
@ -165,7 +165,7 @@ class SendAdditionalEmailAction(BaseAction):
|
||||
def _approve(self):
|
||||
self.perform_action("approve")
|
||||
|
||||
def _submit(self, data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
self.perform_action("submit")
|
||||
|
||||
def perform_action(self, stage):
|
||||
|
@ -135,7 +135,7 @@ class NewProjectAction(BaseAction, ProjectMixin, UserMixin):
|
||||
% (user.name, project_id, default_roles)
|
||||
)
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
"""
|
||||
Nothing to do here. Everything is done at the approve step.
|
||||
"""
|
||||
@ -406,7 +406,7 @@ class NewProjectWithUserAction(UserNameAction, ProjectMixin, UserMixin):
|
||||
% (self.username, project_id, default_roles)
|
||||
)
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
"""
|
||||
The submit action is performed when a token is submitted.
|
||||
This is done to set a user password only, and so should now only
|
||||
@ -537,5 +537,5 @@ class AddDefaultUsersToProjectAction(BaseAction, ProjectMixin, UserMixin):
|
||||
self.action.save()
|
||||
self.add_note("All users added.")
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
pass
|
||||
|
@ -219,7 +219,7 @@ class NewDefaultNetworkAction(BaseAction, ProjectMixin):
|
||||
if self.setup_network and self.valid:
|
||||
self._create_network()
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
pass
|
||||
|
||||
|
||||
@ -437,7 +437,7 @@ class UpdateProjectQuotasAction(BaseAction, QuotaMixin):
|
||||
|
||||
self.action.save()
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
"""
|
||||
Nothing to do here. Everything is done at approve.
|
||||
"""
|
||||
@ -494,5 +494,5 @@ class SetProjectQuotaAction(UpdateProjectQuotasAction):
|
||||
self.action.state = "completed"
|
||||
self.action.save()
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
pass
|
||||
|
@ -135,7 +135,7 @@ class NewUserAction(UserNameAction, ProjectMixin, UserMixin):
|
||||
def _approve(self):
|
||||
self._validate()
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
self._validate()
|
||||
|
||||
if not self.valid:
|
||||
@ -258,7 +258,7 @@ class ResetUserPasswordAction(UserNameAction, UserMixin):
|
||||
def _approve(self):
|
||||
self._validate()
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
self._validate()
|
||||
|
||||
if not self.valid:
|
||||
@ -366,7 +366,7 @@ class EditUserRolesAction(UserIdAction, ProjectMixin, UserMixin):
|
||||
def _approve(self):
|
||||
self._validate()
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
self._validate()
|
||||
|
||||
if not self.valid:
|
||||
@ -470,7 +470,7 @@ class UpdateUserEmailAction(UserIdAction, UserMixin):
|
||||
self.action.need_token = True
|
||||
self.set_token_fields(["confirm"])
|
||||
|
||||
def _submit(self, token_data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
self._validate()
|
||||
|
||||
if not self.valid:
|
||||
|
@ -27,6 +27,8 @@ from adjutant.api.models import Task, Token, Notification
|
||||
from adjutant.common.tests import fake_clients
|
||||
from adjutant.common.tests.fake_clients import FakeManager, setup_identity_cache
|
||||
from adjutant.config import CONF
|
||||
from adjutant.tasks.v1.users import InviteUser
|
||||
from adjutant.tasks.v1.manager import TaskManager
|
||||
|
||||
|
||||
@mock.patch("adjutant.common.user_store.IdentityManager", FakeManager)
|
||||
@ -218,6 +220,7 @@ class AdminAPITests(APITestCase):
|
||||
"actions": ["ResetUserPasswordAction"],
|
||||
"required_fields": ["password"],
|
||||
"task_type": "reset_user_password",
|
||||
"requires_authentication": False,
|
||||
},
|
||||
)
|
||||
self.assertEqual(1, Token.objects.count())
|
||||
@ -1577,3 +1580,83 @@ class AdminAPITests(APITestCase):
|
||||
},
|
||||
)
|
||||
self.assertEqual(new_notification.task, new_task)
|
||||
|
||||
@mock.patch.object(InviteUser, "token_requires_authentication", True)
|
||||
def test_token_require_authenticated(self):
|
||||
"""
|
||||
test for reissue of tokens
|
||||
"""
|
||||
project = mock.Mock()
|
||||
project.id = "test_project_id"
|
||||
project.name = "test_project"
|
||||
project.domain = "default"
|
||||
project.roles = {}
|
||||
|
||||
user = fake_clients.FakeUser(
|
||||
name="test@example.com", password="123", email="test@example.com"
|
||||
)
|
||||
|
||||
setup_identity_cache(projects=[project], users=[user])
|
||||
|
||||
url = "/v1/actions/InviteUser"
|
||||
headers = {
|
||||
"project_name": project.name,
|
||||
"project_id": project.id,
|
||||
"roles": "project_admin,member,project_mod",
|
||||
"username": "owner@example.com",
|
||||
"user_id": "test_user_id",
|
||||
"authenticated": True,
|
||||
}
|
||||
data = {
|
||||
"email": "test@example.com",
|
||||
"roles": ["member"],
|
||||
"project_id": "test_project_id",
|
||||
}
|
||||
response = self.client.post(url, data, format="json", headers=headers)
|
||||
self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED)
|
||||
|
||||
new_token = Token.objects.all()[0]
|
||||
|
||||
url = "/v1/tokens/" + new_token.token
|
||||
data = {"confirm": True}
|
||||
response = self.client.post(url, data, format="json", headers={})
|
||||
self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
|
||||
self.assertEqual(
|
||||
response.json(),
|
||||
{"errors": ["This token requires authentication to submit."]},
|
||||
)
|
||||
|
||||
headers = {
|
||||
"project_name": project.name,
|
||||
"project_id": project.id,
|
||||
"roles": "project_admin,member,project_mod",
|
||||
"username": "owner@example.com",
|
||||
"user_id": "test_user_id",
|
||||
"authenticated": True,
|
||||
}
|
||||
|
||||
submitted_keystone_user = {}
|
||||
|
||||
def mocked_submit(self, task, token_data, keystone_user):
|
||||
submitted_keystone_user.update(keystone_user)
|
||||
|
||||
with mock.patch.object(TaskManager, "submit", mocked_submit):
|
||||
response = self.client.post(url, data, format="json", headers=headers)
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
self.assertEqual(
|
||||
response.json(),
|
||||
{"notes": ["Token submitted successfully."]},
|
||||
)
|
||||
self.assertEqual(
|
||||
submitted_keystone_user,
|
||||
{
|
||||
"authenticated": True,
|
||||
"project_domain_id": "default",
|
||||
"project_id": "test_project_id",
|
||||
"project_name": "test_project",
|
||||
"roles": ["project_admin", "member", "project_mod"],
|
||||
"user_domain_id": "default",
|
||||
"user_id": "test_user_id",
|
||||
"username": "owner@example.com",
|
||||
},
|
||||
)
|
||||
|
@ -409,6 +409,7 @@ class TokenDetail(APIViewWithLogger):
|
||||
"actions": [str(act) for act in actions],
|
||||
"required_fields": required_fields,
|
||||
"task_type": token.task.task_type,
|
||||
"requires_authentication": token.task.get_task().token_requires_authentication,
|
||||
}
|
||||
)
|
||||
|
||||
@ -428,6 +429,14 @@ class TokenDetail(APIViewWithLogger):
|
||||
{"errors": ["This token does not exist or has expired."]}, status=404
|
||||
)
|
||||
|
||||
self.task_manager.submit(token.task, request.data)
|
||||
task = self.task_manager.get(token.task)
|
||||
if task.token_requires_authentication and not request.keystone_user.get(
|
||||
"authenticated", False
|
||||
):
|
||||
return Response(
|
||||
{"errors": ["This token requires authentication to submit."]}, 401
|
||||
)
|
||||
|
||||
self.task_manager.submit(task, request.data, request.keystone_user)
|
||||
|
||||
return Response({"notes": ["Token submitted successfully."]}, status=200)
|
||||
|
@ -18,6 +18,7 @@ from django.utils import timezone
|
||||
from jsonfield import JSONField
|
||||
|
||||
from adjutant.config import CONF
|
||||
from adjutant import exceptions
|
||||
from adjutant import tasks
|
||||
|
||||
|
||||
@ -76,7 +77,15 @@ class Task(models.Model):
|
||||
|
||||
def get_task(self):
|
||||
"""Returns self as the appropriate task wrapper type."""
|
||||
return tasks.TASK_CLASSES[self.task_type](task_model=self)
|
||||
try:
|
||||
return tasks.TASK_CLASSES[self.task_type](task_model=self)
|
||||
except KeyError:
|
||||
# TODO(adriant): Maybe we should handle this better
|
||||
# for older deprecated tasks:
|
||||
raise exceptions.TaskNotRegistered(
|
||||
"Task type '%s' not registered, "
|
||||
"and used for existing task." % self.task_type
|
||||
)
|
||||
|
||||
@property
|
||||
def config(self):
|
||||
|
@ -129,6 +129,7 @@ class BaseTask(object):
|
||||
deprecated_task_types = None
|
||||
duplicate_policy = "cancel"
|
||||
send_approval_notification = True
|
||||
token_requires_authentication = False
|
||||
|
||||
# config defaults for the task (used to generate default config):
|
||||
allow_auto_approve = True
|
||||
@ -466,7 +467,7 @@ class BaseTask(object):
|
||||
for token in self.task.tokens:
|
||||
token.delete()
|
||||
|
||||
def submit(self, token_data=None):
|
||||
def submit(self, token_data=None, keystone_user=None):
|
||||
|
||||
self.confirm_state(approved=True, completed=False, cancelled=False)
|
||||
|
||||
@ -502,7 +503,7 @@ class BaseTask(object):
|
||||
|
||||
for action in actions:
|
||||
try:
|
||||
action.submit(data)
|
||||
action.submit(data, keystone_user)
|
||||
except Exception as e:
|
||||
handle_task_error(e, self.task, "while submiting task")
|
||||
|
||||
|
@ -66,15 +66,7 @@ class TaskManager(object):
|
||||
"Task not found with uuid of: '%s'" % task
|
||||
)
|
||||
if isinstance(task, Task):
|
||||
try:
|
||||
return tasks.TASK_CLASSES[task.task_type](task)
|
||||
except KeyError:
|
||||
# TODO(adriant): Maybe we should handle this better
|
||||
# for older deprecated tasks:
|
||||
raise exceptions.TaskNotRegistered(
|
||||
"Task type '%s' not registered, "
|
||||
"and used for existing task." % task.task_type
|
||||
)
|
||||
return task.get_task()
|
||||
raise exceptions.TaskNotFound("Task not found for value of: '%s'" % task)
|
||||
|
||||
def update(self, task, action_data):
|
||||
@ -87,9 +79,9 @@ class TaskManager(object):
|
||||
task.approve(approved_by)
|
||||
return task
|
||||
|
||||
def submit(self, task, token_data):
|
||||
def submit(self, task, token_data, keystone_user=None):
|
||||
task = self.get(task)
|
||||
task.submit(token_data)
|
||||
task.submit(token_data, keystone_user)
|
||||
return task
|
||||
|
||||
def cancel(self, task):
|
||||
|
@ -185,7 +185,7 @@ functions::
|
||||
# Do some logic here
|
||||
self.action.task.cache['value'] = self.value1
|
||||
|
||||
def _submit(self, data):
|
||||
def _submit(self, token_data, keystone_user=None):
|
||||
# Do some logic here
|
||||
self.add_note("Submit action performed")
|
||||
|
||||
|
14
releasenotes/notes/authed_token-6d29688676e7ee32.yaml
Normal file
14
releasenotes/notes/authed_token-6d29688676e7ee32.yaml
Normal file
@ -0,0 +1,14 @@
|
||||
---
|
||||
features:
|
||||
- |
|
||||
Tasks can now be configured to required a user to be authenticated when an
|
||||
Adjutant token is submitted for the final phase of a task. Actions will now
|
||||
be passed the ``keystone_user`` who submitted the token to do any processing
|
||||
on that as needed for the final step.
|
||||
deprecations:
|
||||
- |
|
||||
All actions now need to have ``keystone_user`` as a second optional
|
||||
paramater in the ``submit``function. It should have a default of ``None``,
|
||||
set as ``keystone_user=None``. Any existing actions without this will continue
|
||||
to work with a fallback, but that fallback will be removed in the W release
|
||||
cycle.
|
Loading…
Reference in New Issue
Block a user