openstack/glance
Code Issues Proposed changes

Fix glance service policy rule

Browse Source 
Glance service APIs are default to 'service_roles: service'

- 6c33a667a9/glance/policies/base.py (L116)

The issue here is the service token, which is sent from the
service for the user token expiry case but glance uses that
service token (keystonemiddleware sets the service token roles
in Requestcontext in 'service_roles' field) for RBAC, which
is not correct.

The OpenStack services communicate with each other by
passing the user token and service token wrapped in
keystoneauth's ServiceTokenAuthWrapper. The only purpose
of passing the service token is for long-running
operations and in case the user token gets expired.

For RBAC, we need to check if a user token has the 'service'
role or not. Service needs to load the configured user auth
plugin (where the user should have the 'service' role) from
keystoneauth and pass that to the other services (for example,
cinder change depends-on) and glance will use that user role
to verify the policy permission. To fix that, we need to make
the service APIs default to ``role:service`` and not
`service_role`:`service`.

This commit does one more change. Cinder does not have the
way to configure the glance service user, we are adding the
new config in this release. For backward compatibility,
we need to allow admin access in service policy rule. In
future release (after one SLURP release), we cna remove
the admin access.

Closes-Bug: #2121622

Co-Authored-By: : Sean Mooney <work@seanmooney.info>

Change-Id: I50909e6bdb3227ca99b7eba642546da791f9552a
Signed-off-by: Sean Mooney <work@seanmooney.info>
Signed-off-by: Ghanshyam Maan <gmaan@ghanshyammann.com>
This commit is contained in:
Ghanshyam Maan
2025-08-28 03:19:27 +00:00
committed by Rajat Dhasmana
parent 6c33a667a9
commit 301d8f580f
3 changed files with 79 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
glance/policies/base.py
View File

@@ -113,7 +113,12 @@ rules = [
policy.RuleDefault(name='context_is_admin', check_str='role:admin',
description='Defines the rule for the is_admin:True '
'check.'),
policy.RuleDefault(name='service_api', check_str='service_roles:service',
# TODO(gmaan): The admin role access is added for backward compatibility
# because some services does not send the service role token to glance.
# Remove the admin role access from service rule in 2026.2 or later(after
# 2026.1 SLURP release).
policy.RuleDefault(name='service_api',
check_str='role:service or role:admin',
description='Default rule for the service-to-service '
'API.'),
]

81
glance/tests/functional/v2/test_images.py
View File

@@ -2465,7 +2465,7 @@ class TestImagesSingleStore(functional.SynchronousAPIBase):
)
# Get locations of `queued` image
headers = self._headers({'X-Service-Roles': 'service'})
headers = self._headers({'X-Roles': 'service'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(200, response.status_code, response.text)
@@ -2493,14 +2493,28 @@ class TestImagesSingleStore(functional.SynchronousAPIBase):
delay_sec=0.2,
start_delay_sec=1)
# Get Locations not allowed for any other user
headers = self._headers({'X-Roles': 'admin,member'})
self.set_policy_rules({
'get_image': 'role:member',
'fetch_image_location': 'role:service or role:admin',
})
# Get Locations not allowed for any non-admin or non-service user
headers = self._headers({'X-Roles': 'member'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(http.FORBIDDEN, response.status_code, response.text)
# Get Locations allowed only for service user
headers = self._headers({'X-Service-Roles': 'service'})
# TODO(gmaan): For backward compatibility, we are allowing admin
# user to access service only rules, but once we remove that
# access, we need to assert here that the admin cannot access the
# service only rules.
headers = self._headers({'X-Roles': 'admin'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(200, response.status_code, response.text)
# Get Locations allowed for service user
headers = self._headers({'X-Roles': 'service'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(200, response.status_code, response.text)
@@ -2515,15 +2529,30 @@ class TestImagesSingleStore(functional.SynchronousAPIBase):
# Upload some image data
image_data = b'ZZZZZ'
self.api_methods.upload_and_verify(image_id, image_data)
self.set_policy_rules({
'get_image': 'role:member',
'fetch_image_location': 'role:service or role:admin',
})
# Get Locations not allowed for any other user
headers = self._headers({'X-Roles': 'admin,member'})
# Get Locations not allowed for any non-admin or non-service user
headers = self._headers({'X-Roles': 'member'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(http.FORBIDDEN, response.status_code, response.text)
# TODO(gmaan): For backward compatibility, we are allowing admin
# user to access service only rules, but once we remove that
# access, we need to assert here that the admin cannot access the
# service only rules.
headers = self._headers({'X-Roles': 'admin'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(200, response.status_code, response.text)
output = jsonutils.loads(response.text)
self.assertTrue(output[0]['url'])
# Get Locations allowed only for service user
headers = self._headers({'X-Service-Roles': 'service'})
headers = self._headers({'X-Roles': 'service'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(200, response.status_code, response.text)
@@ -4588,13 +4617,20 @@ class TestStoreWeight(functional.SynchronousAPIBase):
class TestMultipleBackendsLocationApi(functional.SynchronousAPIBase):
def setUp(self):
super(TestMultipleBackendsLocationApi, self).setUp()
self.start_server()
for i in range(3):
ret = test_utils.start_http_server("foo_image_id%d" % i,
"foo_image%d" % i)
setattr(self, 'http_server%d' % i, ret[1])
setattr(self, 'http_port%d' % i, ret[2])
self.policy = policy.Enforcer(suppress_deprecation_warnings=True)
self.start_server()
def start_server(self):
with mock.patch.object(policy, 'Enforcer') as mock_enf:
mock_enf.return_value = self.policy
super(TestMultipleBackendsLocationApi, self).start_server()
def setup_stores(self):
pass
@@ -4918,7 +4954,7 @@ class TestMultipleBackendsLocationApi(functional.SynchronousAPIBase):
self.assertEqual('queued', image['status'])
# Get location of `queued` image
headers = self._headers({'X-Service-Roles': 'service'})
headers = self._headers({'X-Roles': 'service'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(200, response.status_code, response.text)
@@ -4946,14 +4982,31 @@ class TestMultipleBackendsLocationApi(functional.SynchronousAPIBase):
delay_sec=0.2,
start_delay_sec=1, multistore=True)
# Get Locations not allowed for any other user
headers = self._headers({'X-Roles': 'admin,member'})
self.policy.set_rules(
oslo_policy.policy.Rules.from_dict({
'get_image': 'role:member',
'fetch_image_location': 'role:service or role:admin'}),
overwrite=True)
# Get Locations not allowed for any non-admin or non-service user
headers = self._headers({'X-Roles': 'member'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(http.FORBIDDEN, response.status_code, response.text)
# Get Locations allowed only for service user
headers = self._headers({'X-Service-Roles': 'service'})
# TODO(gmaan): For backward compatibility, we are allowing admin
# user to access service only rules, but once we remove that
# access, we need to assert here that the admin cannot access the
# service only rules.
headers = self._headers({'X-Roles': 'admin'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(200, response.status_code, response.text)
output = jsonutils.loads(response.text)
self.assertTrue(output[0]['url'])
# Get Locations allowed for service user
headers = self._headers({'X-Roles': 'service'})
path = '/v2/images/%s/locations' % image_id
response = self.api_get(path, headers=headers)
self.assertEqual(200, response.status_code, response.text)

6
releasenotes/notes/service-api-default-eae660940ed97010.yaml Normal file
View File

@@ -0,0 +1,6 @@
---
deprecations:
- |
The ``add_image_location`` and ``fetch_image_location`` APIs policy rule
default value ``role:admin or role:service`` is deprecated and will be
changed to ``role:service`` in future release.