openstack/nova
Code Issues Proposed changes

Remove db layer hard-code permission checks for floating_ip_dns

Browse Source 
This patches removes db layer hard-code permission checks for
floating_ip_dns.

Partially implements bp nova-api-policy-final-part

SecurityImpact
UpgradeImpact:
Due to the db layer permission checks are removed, we need to add default
policy rule into policy file. In this patch,
"os_compute_api:os-floating-ip-dns:domain:update" and
"os_compute_api:os-floating-ip-dns:domain:delete" were updated with a default
rule. Admin will be notfied to update their policy configure file to keep
the behavior as before.

Change-Id: Ibddf3529a219cb9a0c1d4cfdb89327b53454c436
This commit is contained in:
ShaoHe Feng 2015-03-02 22:26:12 +08:00
parent 686621e2ad
commit 4d6a50ab3e
7 changed files with 76 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
etc/nova/policy.json
View File

@ -272,6 +272,8 @@
"os_compute_api:os-flavor-manage": "rule:admin_api",
"os_compute_api:os-floating-ip-dns": "",
"os_compute_api:os-floating-ip-dns:discoverable": "",
"os_compute_api:os-floating-ip-dns:domain:update": "rule:admin_api",
"os_compute_api:os-floating-ip-dns:domain:delete": "rule:admin_api",
"os_compute_api:os-floating-ip-pools": "",
"os_compute_api:os-floating-ip-pools:discoverable": "",
"os_compute_api:os-floating-ips": "",

7
nova/api/openstack/compute/contrib/floating_ip_dns.py
View File

@ -19,6 +19,7 @@ import webob
from nova.api.openstack import extensions
from nova.api.openstack import wsgi
from nova import context as nova_context
from nova import exception
from nova.i18n import _
from nova import network
@ -105,6 +106,9 @@ class FloatingIPDNSDomainController(object):
"""Add or modify domain entry."""
context = req.environ['nova.context']
authorize(context)
# NOTE(shaohe-feng): back-compatible with db layer hard-code
# admin permission checks.
nova_context.require_admin_context(context)
fqdomain = _unquote_domain(id)
try:
entry = body['domain_entry']
@ -138,6 +142,9 @@ class FloatingIPDNSDomainController(object):
"""Delete the domain identified by id."""
context = req.environ['nova.context']
authorize(context)
# NOTE(shaohe-feng): back-compatible with db layer hard-code
# admin permission checks.
nova_context.require_admin_context(context)
domain = _unquote_domain(id)
# Delete the whole domain

4
nova/api/openstack/compute/plugins/v3/floating_ip_dns.py
View File

@ -111,7 +111,7 @@ class FloatingIPDNSDomainController(wsgi.Controller):
def update(self, req, id, body):
"""Add or modify domain entry."""
context = req.environ['nova.context']
authorize(context)
authorize(context, action="domain:update")
fqdomain = _unquote_domain(id)
entry = body['domain_entry']
scope = entry['scope']
@ -146,7 +146,7 @@ class FloatingIPDNSDomainController(wsgi.Controller):
def delete(self, req, id):
"""Delete the domain identified by id."""
context = req.environ['nova.context']
authorize(context)
authorize(context, action="domain:delete")
domain = _unquote_domain(id)
# Delete the whole domain

3
nova/db/sqlalchemy/api.py
View File

@ -1072,7 +1072,6 @@ def _dnsdomain_get_or_create(context, session, fqdomain):
return domain_ref
@require_admin_context
def dnsdomain_register_for_zone(context, fqdomain, zone):
session = get_session()
with session.begin():
@ -1082,7 +1081,6 @@ def dnsdomain_register_for_zone(context, fqdomain, zone):
session.add(domain_ref)
@require_admin_context
def dnsdomain_register_for_project(context, fqdomain, project):
session = get_session()
with session.begin():
@ -1092,7 +1090,6 @@ def dnsdomain_register_for_project(context, fqdomain, project):
session.add(domain_ref)
@require_admin_context
def dnsdomain_unregister(context, fqdomain):
model_query(context, models.DNSDomain).\
filter_by(domain=fqdomain).\

74
nova/tests/unit/api/openstack/compute/contrib/test_floating_ip_dns.py
View File

@ -143,6 +143,7 @@ class FloatingIpDNSTestV21(test.TestCase):
self.domain_controller = temp
self.entry_controller = self.floating_ip_dns.\
FloatingIPDNSEntryController()
self.admin_req = fakes.HTTPRequest.blank('', use_admin_context=True)
self.req = fakes.HTTPRequest.blank('')
def tearDown(self):
@ -211,25 +212,28 @@ class FloatingIpDNSTestV21(test.TestCase):
self.assertEqual(entry['dns_entry']['ip'], test_ipv4_address)
def test_create_domain(self):
self._test_create_domain(self.req)
def _test_create_domain(self, req):
body = {'domain_entry':
{'scope': 'private',
'project': 'testproject'}}
self.assertRaises(self._bad_request(),
self.domain_controller.update,
self.req, _quote_domain(domain), body=body)
self.domain_controller.update, req,
_quote_domain(domain), body=body)
body = {'domain_entry':
{'scope': 'public',
'availability_zone': 'zone1'}}
self.assertRaises(self._bad_request(),
self.domain_controller.update,
self.req, _quote_domain(domain), body=body)
self.domain_controller.update, req,
_quote_domain(domain), body=body)
body = {'domain_entry':
{'scope': 'public',
'project': 'testproject'}}
entry = self.domain_controller.update(self.req, _quote_domain(domain),
body=body)
entry = self.domain_controller.update(req,
_quote_domain(domain), body=body)
self.assertEqual(entry['domain_entry']['domain'], domain)
self.assertEqual(entry['domain_entry']['scope'], 'public')
self.assertEqual(entry['domain_entry']['project'], 'testproject')
@ -237,8 +241,8 @@ class FloatingIpDNSTestV21(test.TestCase):
body = {'domain_entry':
{'scope': 'private',
'availability_zone': 'zone1'}}
entry = self.domain_controller.update(self.req, _quote_domain(domain),
body=body)
entry = self.domain_controller.update(req,
_quote_domain(domain), body=body)
self.assertEqual(entry['domain_entry']['domain'], domain)
self.assertEqual(entry['domain_entry']['scope'], 'private')
self.assertEqual(entry['domain_entry']['availability_zone'], 'zone1')
@ -270,6 +274,9 @@ class FloatingIpDNSTestV21(test.TestCase):
name)
def test_delete_domain(self):
self._test_delete_domain(self.req)
def _test_delete_domain(self, req):
calls = []
def network_delete_dns_domain(fakeself, context, fqdomain):
@ -278,20 +285,25 @@ class FloatingIpDNSTestV21(test.TestCase):
self.stubs.Set(network.api.API, "delete_dns_domain",
network_delete_dns_domain)
res = self.domain_controller.delete(self.req, _quote_domain(domain))
res = self.domain_controller.delete(req,
_quote_domain(domain))
self._check_status(202, res, self.domain_controller.delete)
self.assertEqual([domain], calls)
def test_delete_domain_notfound(self):
self._test_delete_domain_notfound(self.req)
def _test_delete_domain_notfound(self, req):
def delete_dns_domain_notfound(fakeself, context, fqdomain):
raise exception.NotFound
self.stubs.Set(network.api.API, "delete_dns_domain",
delete_dns_domain_notfound)
self.assertRaises(webob.exc.HTTPNotFound,
self.domain_controller.delete, self.req, _quote_domain(domain))
self.assertRaises(
webob.exc.HTTPNotFound, self.domain_controller.delete,
req, _quote_domain(domain))
def test_modify(self):
body = {'dns_entry':
@ -329,7 +341,7 @@ class FloatingIpDNSTestV21(test.TestCase):
with mock.patch.object(network.api.API, 'delete_dns_domain',
side_effect=NotImplementedError()):
self.assertRaises(webob.exc.HTTPNotImplemented,
self.domain_controller.delete, self.req,
self.domain_controller.delete, self.admin_req,
_quote_domain(domain))
def test_not_implemented_create_domain(self):
@ -339,8 +351,8 @@ class FloatingIpDNSTestV21(test.TestCase):
with mock.patch.object(network.api.API, 'create_private_dns_domain',
side_effect=NotImplementedError()):
self.assertRaises(webob.exc.HTTPNotImplemented,
self.domain_controller.update,
self.req, _quote_domain(domain), body=body)
self.domain_controller.update, self.admin_req,
_quote_domain(domain), body=body)
def test_not_implemented_dns_domains_list(self):
with mock.patch.object(network.api.API, 'get_dns_domains',
@ -358,6 +370,28 @@ class FloatingIpDNSTestV2(FloatingIpDNSTestV21):
def _bad_request(self):
return webob.exc.HTTPUnprocessableEntity
def test_update_dns_domain_with_non_admin(self):
body = {'domain_entry':
{'scope': 'private',
'project': 'testproject'}}
self.assertRaises(exception.AdminRequired,
self.domain_controller.update,
self.req, _quote_domain(domain), body=body)
def test_delete_dns_domain_with_non_admin(self):
self.assertRaises(exception.AdminRequired,
self.domain_controller.delete,
self.req, _quote_domain(domain))
def test_create_domain(self):
self._test_create_domain(self.admin_req)
def test_delete_domain(self):
self._test_delete_domain(self.admin_req)
def test_delete_domain_notfound(self):
self._test_delete_domain_notfound(self.admin_req)
class FloatingIPDNSDomainPolicyEnforcementV21(test.NoDBTestCase):
@ -369,14 +403,18 @@ class FloatingIPDNSDomainPolicyEnforcementV21(test.NoDBTestCase):
self.req = fakes.HTTPRequest.blank('')
def test_get_floating_ip_dns_policy_failed(self):
rule_name = "os_compute_api:os-floating-ip-dns"
self.policy.set_rules({rule_name: "project:non_fake"})
exc = self.assertRaises(
exception.PolicyNotAuthorized,
self.controller.index, self.req)
self.assertEqual(
"Policy doesn't allow %s to be performed." % self.rule_name,
"Policy doesn't allow %s to be performed." % rule_name,
exc.format_message())
def test_update_floating_ip_dns_policy_failed(self):
rule_name = "os_compute_api:os-floating-ip-dns:domain:update"
self.policy.set_rules({rule_name: "project:non_fake"})
body = {'domain_entry':
{'scope': 'public',
'project': 'testproject'}}
@ -384,15 +422,17 @@ class FloatingIPDNSDomainPolicyEnforcementV21(test.NoDBTestCase):
exception.PolicyNotAuthorized,
self.controller.update, self.req, _quote_domain(domain), body=body)
self.assertEqual(
"Policy doesn't allow %s to be performed." % self.rule_name,
"Policy doesn't allow %s to be performed." % rule_name,
exc.format_message())
def test_delete_floating_ip_dns_policy_failed(self):
rule_name = "os_compute_api:os-floating-ip-dns:domain:delete"
self.policy.set_rules({rule_name: "project:non_fake"})
exc = self.assertRaises(
exception.PolicyNotAuthorized,
self.controller.delete, self.req, _quote_domain(domain))
self.assertEqual(
"Policy doesn't allow %s to be performed." % self.rule_name,
"Policy doesn't allow %s to be performed." % rule_name,
exc.format_message())

2
nova/tests/unit/fake_policy.py
View File

@ -237,6 +237,8 @@ policy_data = """
"os_compute_api:os-flavors:discoverable": "",
"compute_extension:floating_ip_dns": "",
"os_compute_api:os-floating-ip-dns": "",
"os_compute_api:os-floating-ip-dns:domain:update": "",
"os_compute_api:os-floating-ip-dns:domain:delete": "",
"compute_extension:floating_ip_pools": "",
"os_compute_api:os-floating-ip-pools": "",
"compute_extension:floating_ips": "",

32
nova/tests/unit/network/test_manager.py
View File

@ -3188,21 +3188,14 @@ class FloatingIPTestCase(test.TestCase):
name1, zone)
def test_floating_dns_domains_public(self):
zone1 = "testzone"
domain1 = "example.org"
domain2 = "example.com"
address1 = '10.10.10.10'
entryname = 'testentry'
context_admin = context.RequestContext('testuser', 'testproject',
is_admin=True)
self.assertRaises(exception.AdminRequired,
self.network.create_public_dns_domain, self.context,
domain1, zone1)
self.network.create_public_dns_domain(context_admin, domain1,
self.network.create_public_dns_domain(self.context, domain1,
'testproject')
self.network.create_public_dns_domain(context_admin, domain2,
self.network.create_public_dns_domain(self.context, domain2,
'fakeproject')
domains = self.network.get_dns_domains(self.context)
@ -3219,11 +3212,8 @@ class FloatingIPTestCase(test.TestCase):
self.assertEqual(len(entries), 1)
self.assertEqual(entries[0], address1)
self.assertRaises(exception.AdminRequired,
self.network.delete_dns_domain, self.context,
domain1)
self.network.delete_dns_domain(context_admin, domain1)
self.network.delete_dns_domain(context_admin, domain2)
self.network.delete_dns_domain(self.context, domain1)
self.network.delete_dns_domain(self.context, domain2)
# Verify that deleting the domain deleted the associated entry
entries = self.network.get_dns_entries_by_name(self.context,
@ -3436,23 +3426,13 @@ class InstanceDNSTestCase(test.TestCase):
zone1 = 'testzone'
domain1 = 'example.org'
context_admin = context.RequestContext('testuser', 'testproject',
is_admin=True)
self.assertRaises(exception.AdminRequired,
self.network.create_private_dns_domain, self.context,
domain1, zone1)
self.network.create_private_dns_domain(context_admin, domain1, zone1)
self.network.create_private_dns_domain(self.context, domain1, zone1)
domains = self.network.get_dns_domains(self.context)
self.assertEqual(len(domains), 1)
self.assertEqual(domains[0]['domain'], domain1)
self.assertEqual(domains[0]['availability_zone'], zone1)
self.assertRaises(exception.AdminRequired,
self.network.delete_dns_domain, self.context,
domain1)
self.network.delete_dns_domain(context_admin, domain1)
self.network.delete_dns_domain(self.context, domain1)
domain1 = "example.org"