From ee6fa9a2454332dfd9db16064144720c77d84662 Mon Sep 17 00:00:00 2001 From: Ivan Kolodyazhny Date: Fri, 10 Apr 2020 17:15:39 +0300 Subject: [PATCH] Remove Keystone v2 related code Kyestone V2 support was removed in Train, so it's safe to do such cleanup. * Functions which just return horizon settings are dropped and the settings are referred directly now. * The service catalog in the sample test data is updated to match the format of the keystone API v3. * Related to the above change of the sample service catalog, openstack_dashboard.test.unit.api.test_keystone.ServiceAPITests is updated to specify the region name explicitly because 'RegionTwo' endpoint is no longer the second entry of the endpoint list in the keystone API v3. Co-Authored-By: Akihiro Motoki Change-Id: Ib60f360c96341fa5c618595f4a9bfdfe7ec5ae83 --- openstack_auth/forms.py | 2 +- openstack_auth/tests/unit/test_auth.py | 1 - openstack_auth/urls.py | 3 +- openstack_auth/user.py | 3 +- openstack_auth/utils.py | 27 --- openstack_auth/views.py | 20 +- openstack_dashboard/api/base.py | 34 +-- openstack_dashboard/api/keystone.py | 14 +- .../identity/application_credentials/panel.py | 4 - .../dashboards/identity/domains/panel.py | 6 - .../dashboards/identity/groups/panel.py | 9 +- .../dashboards/identity/groups/tests.py | 26 +-- .../identity/identity_providers/panel.py | 6 +- .../dashboards/identity/mappings/panel.py | 6 +- .../dashboards/identity/projects/tables.py | 20 +- .../dashboards/identity/projects/tabs.py | 34 ++- .../dashboards/identity/projects/tests.py | 211 ++++++------------ .../dashboards/identity/projects/views.py | 44 ++-- .../dashboards/identity/projects/workflows.py | 71 +++--- .../dashboards/identity/roles/panel.py | 9 +- .../dashboards/identity/users/forms.py | 60 ++--- .../dashboards/identity/users/panel.py | 5 +- .../dashboards/identity/users/tables.py | 19 +- .../dashboards/identity/users/tabs.py | 34 ++- .../dashboards/identity/users/tests.py | 89 ++------ .../dashboards/identity/users/views.py | 35 ++- .../templatetags/context_selection.py | 7 +- .../test/test_data/keystone_data.py | 134 ++++++++--- .../test/unit/api/test_keystone.py | 14 +- 29 files changed, 377 insertions(+), 570 deletions(-) diff --git a/openstack_auth/forms.py b/openstack_auth/forms.py index ba9cccfe6a..0603bed57e 100644 --- a/openstack_auth/forms.py +++ b/openstack_auth/forms.py @@ -100,7 +100,7 @@ class Login(django_auth_forms.AuthenticationForm): # if websso is enabled and keystone version supported # prepend the websso_choices select input to the form - if utils.is_websso_enabled(): + if settings.WEBSSO_ENABLED: initial = settings.WEBSSO_INITIAL_CHOICE self.fields['auth_type'] = forms.ChoiceField( label=_("Authenticate using"), diff --git a/openstack_auth/tests/unit/test_auth.py b/openstack_auth/tests/unit/test_auth.py index bb663042ec..6246b09532 100644 --- a/openstack_auth/tests/unit/test_auth.py +++ b/openstack_auth/tests/unit/test_auth.py @@ -967,7 +967,6 @@ class OpenStackAuthTestsWebSSO(OpenStackAuthTestsMixin, test.TestCase): (settings.OPENSTACK_KEYSTONE_URL, protocol, origin)) url = reverse('login') - # POST to the page and redirect to keystone. response = self.client.get(url) self.assertRedirects(response, redirect_url, status_code=302, diff --git a/openstack_auth/urls.py b/openstack_auth/urls.py index f1ac47c1aa..dd59f7745e 100644 --- a/openstack_auth/urls.py +++ b/openstack_auth/urls.py @@ -11,6 +11,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from django.conf import settings from django.conf.urls import url from django.views import generic @@ -37,7 +38,7 @@ if utils.allow_expired_passowrd_change(): name='password') ) -if utils.is_websso_enabled(): +if settings.WEBSSO_ENABLED: urlpatterns += [ url(r"^websso/$", views.websso, name='websso'), url(r"^error/$", diff --git a/openstack_auth/user.py b/openstack_auth/user.py index 4971a2adfe..9eeb84dab3 100644 --- a/openstack_auth/user.py +++ b/openstack_auth/user.py @@ -187,8 +187,7 @@ class User(models.AbstractBaseUser, models.AnonymousUser): .. attribute:: password_expires_at - Password expiration date. This attribute could be None when using - keystone version < 3.0 or if the feature is not enabled in keystone. + Password expiration date. """ diff --git a/openstack_auth/utils.py b/openstack_auth/utils.py index 8f5ad177eb..511748e094 100644 --- a/openstack_auth/utils.py +++ b/openstack_auth/utils.py @@ -136,33 +136,6 @@ def allow_expired_passowrd_change(): return getattr(settings, 'ALLOW_USERS_CHANGE_EXPIRED_PASSWORD', True) -def is_websso_enabled(): - """Websso is supported in Keystone version 3.""" - return settings.WEBSSO_ENABLED - - -def is_websso_default_redirect(): - """Checks if the websso default redirect is available. - - As with websso, this is only supported in Keystone version 3. - """ - websso_default_redirect = settings.WEBSSO_DEFAULT_REDIRECT - keystonev3_plus = (get_keystone_version() >= 3) - return websso_default_redirect and keystonev3_plus - - -def get_websso_default_redirect_protocol(): - return settings.WEBSSO_DEFAULT_REDIRECT_PROTOCOL - - -def get_websso_default_redirect_region(): - return settings.WEBSSO_DEFAULT_REDIRECT_REGION - - -def get_websso_default_redirect_logout(): - return settings.WEBSSO_DEFAULT_REDIRECT_LOGOUT - - def build_absolute_uri(request, relative_url): """Ensure absolute_uri are relative to WEBROOT.""" webroot = settings.WEBROOT diff --git a/openstack_auth/views.py b/openstack_auth/views.py index 54f0ed881f..8766fb4f58 100644 --- a/openstack_auth/views.py +++ b/openstack_auth/views.py @@ -56,10 +56,10 @@ def login(request): # If the user enabled websso and the default redirect # redirect to the default websso url - if (request.method == 'GET' and utils.is_websso_enabled and - utils.is_websso_default_redirect()): - protocol = utils.get_websso_default_redirect_protocol() - region = utils.get_websso_default_redirect_region() + if (request.method == 'GET' and settings.WEBSSO_ENABLED and + settings.WEBSSO_DEFAULT_REDIRECT): + protocol = settings.WEBSSO_DEFAULT_REDIRECT_PROTOCOL + region = settings.WEBSSO_DEFAULT_REDIRECT_REGION origin = utils.build_absolute_uri(request, '/auth/websso/') url = ('%s/auth/OS-FEDERATION/websso/%s?origin=%s' % (region, protocol, origin)) @@ -70,7 +70,7 @@ def login(request): if request.method == 'POST': auth_type = request.POST.get('auth_type', 'credentials') request.session['auth_type'] = auth_type - if utils.is_websso_enabled() and auth_type != 'credentials': + if settings.WEBSSO_ENABLED and auth_type != 'credentials': region_id = request.POST.get('region') auth_url = getattr(settings, 'WEBSSO_KEYSTONE_URL', None) if auth_url is None: @@ -105,7 +105,7 @@ def login(request): extra_context = { 'redirect_field_name': auth.REDIRECT_FIELD_NAME, 'csrf_failure': request.GET.get('csrf_failure'), - 'show_sso_opts': utils.is_websso_enabled() and len(choices) > 1, + 'show_sso_opts': settings.WEBSSO_ENABLED and len(choices) > 1, } if request.is_ajax(): @@ -171,7 +171,7 @@ def websso(request): request.user = auth.authenticate(request, auth_url=auth_url, token=token) except exceptions.KeystoneAuthException as exc: - if utils.is_websso_default_redirect(): + if settings.WEBSSO_DEFAULT_REDIRECT: res = django_http.HttpResponseRedirect(settings.LOGIN_ERROR) else: msg = 'Login failed: %s' % exc @@ -202,11 +202,11 @@ def logout(request, login_url=None, **kwargs): LOG.info(msg) """ Securely logs a user out. """ - if (utils.is_websso_enabled and utils.is_websso_default_redirect() and - utils.get_websso_default_redirect_logout()): + if (settings.WEBSSO_ENABLED and settings.WEBSSO_DEFAULT_REDIRECT and + settings.WEBSSO_DEFAULT_REDIRECT_LOGOUT): auth_user.unset_session_user_variables(request) return django_http.HttpResponseRedirect( - utils.get_websso_default_redirect_logout()) + settings.WEBSSO_DEFAULT_REDIRECT_LOGOUT) else: return django_auth_views.logout_then_login(request, login_url=login_url, diff --git a/openstack_dashboard/api/base.py b/openstack_dashboard/api/base.py index 9c2ec3b940..b7b00ce8e1 100644 --- a/openstack_dashboard/api/base.py +++ b/openstack_dashboard/api/base.py @@ -293,17 +293,9 @@ def get_service_from_catalog(catalog, service_type): return None -def get_version_from_service(service): - if service and service.get('endpoints'): - endpoint = service['endpoints'][0] - if 'interface' in endpoint: - return 3 - else: - return 2.0 - return 2.0 - - # Mapping of V2 Catalog Endpoint_type to V3 Catalog Interfaces +# TODO(e0ne): remove this mapping once OPENSTACK_ENDPOINT_TYPE config option +# will be removed. ENDPOINT_TYPE_TO_INTERFACE = { 'publicURL': 'public', 'internalURL': 'internal', @@ -315,31 +307,25 @@ def get_url_for_service(service, region, endpoint_type): if 'type' not in service: return None - identity_version = get_version_from_service(service) service_endpoints = service.get('endpoints', []) available_endpoints = [endpoint for endpoint in service_endpoints if region == _get_endpoint_region(endpoint)] - """if we are dealing with the identity service and there is no endpoint - in the current region, it is okay to use the first endpoint for any - identity service endpoints and we can assume that it is global - """ + # if we are dealing with the identity service and there is no endpoint + # in the current region, it is okay to use the first endpoint for any + # identity service endpoints and we can assume that it is global if service['type'] == 'identity' and not available_endpoints: available_endpoints = [endpoint for endpoint in service_endpoints] for endpoint in available_endpoints: try: - if identity_version < 3: - return endpoint.get(endpoint_type) - else: - interface = \ - ENDPOINT_TYPE_TO_INTERFACE.get(endpoint_type, '') - if endpoint.get('interface') == interface: - return endpoint.get('url') - except (IndexError, KeyError): + interface = \ + ENDPOINT_TYPE_TO_INTERFACE.get(endpoint_type, '') + if endpoint['interface'] == interface: + return endpoint['url'] + except KeyError: # it could be that the current endpoint just doesn't match the # type, continue trying the next one pass - return None def url_for(request, service_type, endpoint_type=None, region=None): diff --git a/openstack_dashboard/api/keystone.py b/openstack_dashboard/api/keystone.py index 96d6dd455c..76268ebc44 100644 --- a/openstack_dashboard/api/keystone.py +++ b/openstack_dashboard/api/keystone.py @@ -146,7 +146,7 @@ def keystoneclient(request, admin=False): user = request.user token_id = user.token.id - if is_multi_domain_enabled(): + if settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT: is_domain_context_specified = bool( request.session.get("domain_context")) @@ -274,8 +274,7 @@ def get_default_domain(request, get_name=True): """ domain_id = request.session.get("domain_context", None) domain_name = request.session.get("domain_context_name", None) - # if running in Keystone V3 or later - if VERSIONS.active >= 3 and domain_id is None: + if domain_id is None: # if no domain context set, default to user's domain domain_id = request.user.user_domain_id domain_name = request.user.user_domain_name @@ -861,15 +860,6 @@ def get_version(): return VERSIONS.active -def is_multi_domain_enabled(): - return (VERSIONS.active >= 3 and - settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT) - - -def is_federation_management_enabled(): - return settings.OPENSTACK_KEYSTONE_FEDERATION_MANAGEMENT - - def identity_provider_create(request, idp_id, description=None, enabled=False, remote_ids=None): manager = keystoneclient(request, admin=True).federation.identity_providers diff --git a/openstack_dashboard/dashboards/identity/application_credentials/panel.py b/openstack_dashboard/dashboards/identity/application_credentials/panel.py index ab13db57a5..d3c54c1e65 100644 --- a/openstack_dashboard/dashboards/identity/application_credentials/panel.py +++ b/openstack_dashboard/dashboards/identity/application_credentials/panel.py @@ -24,10 +24,6 @@ class ApplicationCredentialsPanel(horizon.Panel): slug = 'application_credentials' policy_rules = (('identity', 'identity:list_application_credentials'),) - @staticmethod - def can_register(): - return keystone.VERSIONS.active >= 3 - def can_access(self, context): request = context['request'] keystone_version = keystone.get_identity_api_version(request) diff --git a/openstack_dashboard/dashboards/identity/domains/panel.py b/openstack_dashboard/dashboards/identity/domains/panel.py index 434ad8a76c..a6316c3757 100644 --- a/openstack_dashboard/dashboards/identity/domains/panel.py +++ b/openstack_dashboard/dashboards/identity/domains/panel.py @@ -16,8 +16,6 @@ from django.utils.translation import ugettext_lazy as _ import horizon -from openstack_dashboard.api import keystone - class Domains(horizon.Panel): name = _("Domains") @@ -25,10 +23,6 @@ class Domains(horizon.Panel): policy_rules = (("identity", "identity:get_domain"), ("identity", "identity:list_domains")) - @staticmethod - def can_register(): - return keystone.VERSIONS.active >= 3 - def can_access(self, context): request = context['request'] domain_token = request.session.get('domain_token') diff --git a/openstack_dashboard/dashboards/identity/groups/panel.py b/openstack_dashboard/dashboards/identity/groups/panel.py index bf540f227b..4edec674a2 100644 --- a/openstack_dashboard/dashboards/identity/groups/panel.py +++ b/openstack_dashboard/dashboards/identity/groups/panel.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +from django.conf import settings from django.utils.translation import ugettext_lazy as _ import horizon @@ -24,12 +25,8 @@ class Groups(horizon.Panel): slug = 'groups' policy_rules = (("identity", "identity:list_groups"),) - @staticmethod - def can_register(): - return keystone.VERSIONS.active >= 3 - def can_access(self, context): - if keystone.is_multi_domain_enabled() \ - and not keystone.is_domain_admin(context['request']): + if (settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT and + not keystone.is_domain_admin(context['request'])): return False return super(Groups, self).can_access(context) diff --git a/openstack_dashboard/dashboards/identity/groups/tests.py b/openstack_dashboard/dashboards/identity/groups/tests.py index 7ebf957941..a1e9dcb185 100644 --- a/openstack_dashboard/dashboards/identity/groups/tests.py +++ b/openstack_dashboard/dashboards/identity/groups/tests.py @@ -243,15 +243,10 @@ class GroupsViewTests(test.BaseAdminViewTests): self.mock_group_get.assert_called_once_with(test.IsHttpRequest(), group.id) - if api.keystone.VERSIONS.active >= 3: - self.mock_get_effective_domain_id.assert_called_once_with( - test.IsHttpRequest()) - self.mock_user_list.assert_called_once_with( - test.IsHttpRequest(), group=group.id, domain=domain_id) - else: - self.mock_get_effective_domain_id.assert_not_called() - self.mock_user_list.assert_called_once_with( - test.IsHttpRequest(), group=group.id) + self.mock_get_effective_domain_id.assert_called_once_with( + test.IsHttpRequest()) + self.mock_user_list.assert_called_once_with( + test.IsHttpRequest(), group=group.id, domain=domain_id) @test.create_mocks({api.keystone: ('get_effective_domain_id', 'user_list', @@ -271,15 +266,10 @@ class GroupsViewTests(test.BaseAdminViewTests): self.assertRedirectsNoFollow(res, GROUP_MANAGE_URL) self.assertMessageCount(success=1) - if api.keystone.VERSIONS.active >= 3: - self.mock_get_effective_domain_id.assert_called_once_with( - test.IsHttpRequest()) - self.mock_user_list.assert_called_once_with( - test.IsHttpRequest(), group=group.id, domain=domain_id) - else: - self.mock_get_effective_domain_id.assert_not_called() - self.mock_user_list.assert_called_once_with( - test.IsHttpRequest(), group=group.id) + self.mock_get_effective_domain_id.assert_called_once_with( + test.IsHttpRequest()) + self.mock_user_list.assert_called_once_with( + test.IsHttpRequest(), group=group.id, domain=domain_id) self.mock_remove_group_user.assert_called_once_with( test.IsHttpRequest(), diff --git a/openstack_dashboard/dashboards/identity/identity_providers/panel.py b/openstack_dashboard/dashboards/identity/identity_providers/panel.py index 30a7825ee0..339c596290 100644 --- a/openstack_dashboard/dashboards/identity/identity_providers/panel.py +++ b/openstack_dashboard/dashboards/identity/identity_providers/panel.py @@ -12,12 +12,11 @@ # License for the specific language governing permissions and limitations # under the License. +from django.conf import settings from django.utils.translation import ugettext_lazy as _ import horizon -from openstack_dashboard.api import keystone - class IdentityProviders(horizon.Panel): name = _("Identity Providers") @@ -26,5 +25,4 @@ class IdentityProviders(horizon.Panel): @staticmethod def can_register(): - return (keystone.VERSIONS.active >= 3 and - keystone.is_federation_management_enabled()) + return settings.OPENSTACK_KEYSTONE_FEDERATION_MANAGEMENT diff --git a/openstack_dashboard/dashboards/identity/mappings/panel.py b/openstack_dashboard/dashboards/identity/mappings/panel.py index 16875d7c25..7399cfd18c 100644 --- a/openstack_dashboard/dashboards/identity/mappings/panel.py +++ b/openstack_dashboard/dashboards/identity/mappings/panel.py @@ -12,12 +12,11 @@ # License for the specific language governing permissions and limitations # under the License. +from django.conf import settings from django.utils.translation import ugettext_lazy as _ import horizon -from openstack_dashboard.api import keystone - class Mappings(horizon.Panel): name = _("Mappings") @@ -26,5 +25,4 @@ class Mappings(horizon.Panel): @staticmethod def can_register(): - return (keystone.VERSIONS.active >= 3 and - keystone.is_federation_management_enabled()) + return settings.OPENSTACK_KEYSTONE_FEDERATION_MANAGEMENT diff --git a/openstack_dashboard/dashboards/identity/projects/tables.py b/openstack_dashboard/dashboards/identity/projects/tables.py index 079092b7d7..01445e4874 100644 --- a/openstack_dashboard/dashboards/identity/projects/tables.py +++ b/openstack_dashboard/dashboards/identity/projects/tables.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +from django.conf import settings from django.template import defaultfilters as filters from django.urls import reverse from django.utils.http import urlencode @@ -62,7 +63,7 @@ class UpdateMembersLink(tables.LinkAction): return "?".join([base_url, param]) def allowed(self, request, project): - if api.keystone.is_multi_domain_enabled(): + if settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT: # domain admin or cloud admin = True # project admin or member = False return api.keystone.is_domain_admin(request) @@ -79,7 +80,7 @@ class UpdateGroupsLink(tables.LinkAction): policy_rules = (("identity", "identity:list_groups"),) def allowed(self, request, project): - if api.keystone.is_multi_domain_enabled(): + if settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT: # domain admin or cloud admin = True # project admin or member = False return api.keystone.is_domain_admin(request) @@ -114,7 +115,7 @@ class CreateProject(tables.LinkAction): policy_rules = (('identity', 'identity:create_project'),) def allowed(self, request, project): - if api.keystone.is_multi_domain_enabled(): + if settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT: # domain admin or cloud admin = True # project admin or member = False return api.keystone.is_domain_admin(request) @@ -132,7 +133,7 @@ class UpdateProject(policy.PolicyTargetMixin, tables.LinkAction): policy_target_attrs = (("target.project.domain_id", "domain_id"),) def allowed(self, request, project): - if api.keystone.is_multi_domain_enabled(): + if settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT: # domain admin or cloud admin = True # project admin or member = False return api.keystone.is_domain_admin(request) @@ -180,8 +181,8 @@ class DeleteTenantsAction(policy.PolicyTargetMixin, tables.DeleteAction): policy_target_attrs = (("target.project.domain_id", "domain_id"),) def allowed(self, request, project): - if api.keystone.is_multi_domain_enabled() \ - and not api.keystone.is_domain_admin(request): + if (settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT and + not api.keystone.is_domain_admin(request)): return False return api.keystone.keystone_can_edit_project() @@ -220,11 +221,8 @@ class TenantsTable(tables.DataTable): widget=forms.Textarea(attrs={'rows': 4}), required=False)) id = tables.Column('id', verbose_name=_('Project ID')) - - if api.keystone.VERSIONS.active >= 3: - domain_name = tables.Column( - 'domain_name', verbose_name=_('Domain Name')) - + domain_name = tables.Column( + 'domain_name', verbose_name=_('Domain Name')) enabled = tables.Column('enabled', verbose_name=_('Enabled'), status=True, filters=(filters.yesno, filters.capfirst), form_field=forms.BooleanField( diff --git a/openstack_dashboard/dashboards/identity/projects/tabs.py b/openstack_dashboard/dashboards/identity/projects/tabs.py index b27d5601be..f4bf69213b 100644 --- a/openstack_dashboard/dashboards/identity/projects/tabs.py +++ b/openstack_dashboard/dashboards/identity/projects/tabs.py @@ -41,28 +41,24 @@ class OverviewTab(tabs.Tab): def _get_domain_name(self, project): domain_name = '' - if api.keystone.VERSIONS.active >= 3: - try: - if policy.check((("identity", "identity:get_domain"),), - self.request): - domain = api.keystone.domain_get( - self.request, project.domain_id) - domain_name = domain.name - else: - domain = api.keystone.get_default_domain(self.request) - domain_name = domain.get('name') - except Exception: - exceptions.handle(self.request, - _('Unable to retrieve project domain.')) + try: + if policy.check((("identity", "identity:get_domain"),), + self.request): + domain = api.keystone.domain_get( + self.request, project.domain_id) + domain_name = domain.name + else: + domain = api.keystone.get_default_domain(self.request) + domain_name = domain.get('name') + except Exception: + exceptions.handle(self.request, + _('Unable to retrieve project domain.')) return domain_name def _get_extras(self, project): - if api.keystone.VERSIONS.active >= 3: - extra_info = settings.PROJECT_TABLE_EXTRA_INFO - return dict((display_key, getattr(project, key, '')) - for key, display_key in extra_info.items()) - else: - return {} + extra_info = settings.PROJECT_TABLE_EXTRA_INFO + return dict((display_key, getattr(project, key, '')) + for key, display_key in extra_info.items()) class UsersTab(tabs.TableTab): diff --git a/openstack_dashboard/dashboards/identity/projects/tests.py b/openstack_dashboard/dashboards/identity/projects/tests.py index d42cdb4ac0..65d1a82c49 100644 --- a/openstack_dashboard/dashboards/identity/projects/tests.py +++ b/openstack_dashboard/dashboards/identity/projects/tests.py @@ -541,15 +541,12 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): 'role_list', 'role_assignments_list')}) def test_update_project_get(self): - keystone_api_version = api.keystone.VERSIONS.active - project = self.tenants.first() default_role = self.roles.first() domain_id = project.domain_id users = self._get_all_users(domain_id) groups = self._get_all_groups(domain_id) roles = self.roles.list() - proj_users = self._get_proj_users(project.id) role_assignments = self._get_proj_role_assignment(project.id) self.mock_tenant_get.return_value = project @@ -564,13 +561,7 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): self.mock_role_list.return_value = roles self.mock_group_list.return_value = groups - if keystone_api_version >= 3: - self.mock_role_assignments_list.return_value = role_assignments - else: - retvals_user_list.append(proj_users) - expected_user_list.append( - mock.call(test.IsHttpRequest(), project=self.tenant.id)) - self.mock_roles_for_user.return_value = roles + self.mock_role_assignments_list.return_value = role_assignments url = reverse('horizon:identity:projects:update', args=[self.tenant.id]) @@ -609,16 +600,10 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): self.mock_group_list.assert_called_once_with( test.IsHttpRequest(), domain=domain_id) - if keystone_api_version >= 3: - self.assert_mock_multiple_calls_with_same_arguments( - self.mock_role_assignments_list, 2, - mock.call(test.IsHttpRequest(), project=self.tenant.id)) - self.mock_roles_for_user.assert_not_called() - else: - self.mock_roles_for_user.assert_has_calls( - [mock.call(test.IsHttpRequest(), user.id, self.tenant.id) - for user in proj_users]) - self.mock_role_assignments_list.assert_not_called() + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_role_assignments_list, 2, + mock.call(test.IsHttpRequest(), project=self.tenant.id)) + self.mock_roles_for_user.assert_not_called() @test.create_mocks({api.keystone: ('tenant_get', 'domain_get', @@ -636,13 +621,10 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): 'role_list', 'role_assignments_list')}) def test_update_project_save(self): - keystone_api_version = api.keystone.VERSIONS.active - project = self.tenants.first() default_role = self.roles.first() domain_id = project.domain_id users = self._get_all_users(domain_id) - proj_users = self._get_proj_users(project.id) groups = self._get_all_groups(domain_id) roles = self.roles.list() role_assignments = self._get_proj_role_assignment(project.id) @@ -670,16 +652,6 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): expected_roles_for_user = [] self.mock_roles_for_user.side_effect = retvals_roles_for_user - if keystone_api_version < 3: - retvals_user_list.append(proj_users) - expected_user_list.append( - mock.call(test.IsHttpRequest(), - project=self.tenant.id)) - for user in proj_users: - retvals_roles_for_user.append(roles) - expected_roles_for_user.append( - mock.call(test.IsHttpRequest(), user.id, self.tenant.id)) - workflow_data[USER_ROLE_PREFIX + "1"] = ['3'] # admin role workflow_data[USER_ROLE_PREFIX + "2"] = ['2'] # member role # Group assignment form data @@ -716,95 +688,67 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): expected_remove_group_role = [] self.mock_remove_group_role.side_effect = retvals_remove_group_role - if keystone_api_version >= 3: - # admin role with attempt to remove current admin, results in - # warning message - workflow_data[USER_ROLE_PREFIX + "1"] = ['3'] + # admin role with attempt to remove current admin, results in + # warning message + workflow_data[USER_ROLE_PREFIX + "1"] = ['3'] - # member role - workflow_data[USER_ROLE_PREFIX + "2"] = ['1', '3'] + # member role + workflow_data[USER_ROLE_PREFIX + "2"] = ['1', '3'] - # admin role - workflow_data[GROUP_ROLE_PREFIX + "1"] = ['2', '3'] + # admin role + workflow_data[GROUP_ROLE_PREFIX + "1"] = ['2', '3'] - # member role - workflow_data[GROUP_ROLE_PREFIX + "2"] = ['1', '2', '3'] - self.mock_role_assignments_list.return_value = role_assignments - # Give user 1 role 2 - retvals_add_tenant_user_role.append(None) - expected_add_tenant_user_role.append( - mock.call(test.IsHttpRequest(), - project=self.tenant.id, - user='1', - role='2',)) - # remove role 2 from user 2 - retvals_remove_tenant_user_role.append(None) - expected_remove_tenant_user_role.append( - mock.call(test.IsHttpRequest(), - project=self.tenant.id, - user='2', - role='2')) + # member role + workflow_data[GROUP_ROLE_PREFIX + "2"] = ['1', '2', '3'] + self.mock_role_assignments_list.return_value = role_assignments + # Give user 1 role 2 + retvals_add_tenant_user_role.append(None) + expected_add_tenant_user_role.append( + mock.call(test.IsHttpRequest(), + project=self.tenant.id, + user='1', + role='2',)) + # remove role 2 from user 2 + retvals_remove_tenant_user_role.append(None) + expected_remove_tenant_user_role.append( + mock.call(test.IsHttpRequest(), + project=self.tenant.id, + user='2', + role='2')) - # Give user 3 role 1 - retvals_add_tenant_user_role.append(None) - expected_add_tenant_user_role.append( - mock.call(test.IsHttpRequest(), - project=self.tenant.id, - user='3', - role='1')) - retvals_group_list.append(groups) - expected_group_list.append( - mock.call(test.IsHttpRequest(), - domain=self.domain.id, - project=self.tenant.id)) - retvals_roles_for_group.append(roles) - expected_roles_for_group.append( - mock.call(test.IsHttpRequest(), - group='1', - project=self.tenant.id)) - retvals_remove_group_role.append(None) - expected_remove_group_role.append( - mock.call(test.IsHttpRequest(), - project=self.tenant.id, - group='1', - role='1')) - retvals_roles_for_group.append(roles) - expected_roles_for_group.append( - mock.call(test.IsHttpRequest(), - group='2', - project=self.tenant.id)) - retvals_roles_for_group.append(roles) - expected_roles_for_group.append( - mock.call(test.IsHttpRequest(), - group='3', - project=self.tenant.id)) - else: - retvals_user_list.append(proj_users) - expected_user_list.append( - mock.call(test.IsHttpRequest(), - project=self.tenant.id)) - - # admin user - try to remove all roles on current project, warning - retvals_roles_for_user.append(roles) - expected_roles_for_group.append( - mock.call(test.IsHttpRequest(), '1', self.tenant.id)) - - # member user 1 - has role 1, will remove it - retvals_roles_for_user.append((roles[1],)) - expected_roles_for_group.append( - mock.call(test.IsHttpRequest(), '2', self.tenant.id)) - - # member user 3 - has role 2 - retvals_roles_for_user.append((roles[0],)) - expected_roles_for_group.append( - mock.call(test.IsHttpRequest(), '3', self.tenant.id)) - # add role 2 - retvals_add_tenant_user_role.append(self.exceptions.keystone) - expected_add_tenant_user_role.append( - mock.call(test.IsHttpRequest(), - project=self.tenant.id, - user='3', - role='2')) + # Give user 3 role 1 + retvals_add_tenant_user_role.append(None) + expected_add_tenant_user_role.append( + mock.call(test.IsHttpRequest(), + project=self.tenant.id, + user='3', + role='1')) + retvals_group_list.append(groups) + expected_group_list.append( + mock.call(test.IsHttpRequest(), + domain=self.domain.id, + project=self.tenant.id)) + retvals_roles_for_group.append(roles) + expected_roles_for_group.append( + mock.call(test.IsHttpRequest(), + group='1', + project=self.tenant.id)) + retvals_remove_group_role.append(None) + expected_remove_group_role.append( + mock.call(test.IsHttpRequest(), + project=self.tenant.id, + group='1', + role='1')) + retvals_roles_for_group.append(roles) + expected_roles_for_group.append( + mock.call(test.IsHttpRequest(), + group='2', + project=self.tenant.id)) + retvals_roles_for_group.append(roles) + expected_roles_for_group.append( + mock.call(test.IsHttpRequest(), + group='3', + project=self.tenant.id)) # submit form data project_data = {"domain_id": project._info["domain_id"], @@ -859,12 +803,9 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): _check_mock_calls(self.mock_remove_group_role, expected_remove_group_role) - if keystone_api_version >= 3: - self.assert_mock_multiple_calls_with_same_arguments( - self.mock_role_assignments_list, 3, - mock.call(test.IsHttpRequest(), project=self.tenant.id)) - else: - self.mock_role_assignments_list.assert_not_called() + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_role_assignments_list, 3, + mock.call(test.IsHttpRequest(), project=self.tenant.id)) @test.create_mocks({api.keystone: ('tenant_get',)}) def test_update_project_get_error(self): @@ -895,8 +836,6 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): 'role_list', 'role_assignments_list')}) def test_update_project_tenant_update_error(self): - keystone_api_version = api.keystone.VERSIONS.active - project = self.tenants.first() default_role = self.roles.first() domain_id = project.domain_id @@ -925,16 +864,7 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): expected_roles_for_user = [] self.mock_roles_for_user.side_effect = retvals_roles_for_user - if keystone_api_version >= 3: - self.mock_role_assignments_list.return_value = role_assignments - else: - retvals_user_list.append(proj_users) - expected_roles_for_user.append( - mock.call(test.IsHttpRequest(), project=self.tenant.id)) - for user in proj_users: - retvals_roles_for_user.append(roles) - expected_roles_for_user.append( - mock.call(test.IsHttpRequest(), user.id, self.tenant.id)) + self.mock_role_assignments_list.return_value = role_assignments role_ids = [role.id for role in roles] for user in proj_users: @@ -992,12 +922,9 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): self.assertEqual(len(expected_roles_for_user), self.mock_roles_for_user.call_count) - if keystone_api_version >= 3: - self.assert_mock_multiple_calls_with_same_arguments( - self.mock_role_assignments_list, 2, - mock.call(test.IsHttpRequest(), project=self.tenant.id)) - else: - self.mock_role_assignments_list.assert_not_called() + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_role_assignments_list, 2, + mock.call(test.IsHttpRequest(), project=self.tenant.id)) self.mock_get_effective_domain_id.assert_called_once_with( test.IsHttpRequest()) diff --git a/openstack_dashboard/dashboards/identity/projects/views.py b/openstack_dashboard/dashboards/identity/projects/views.py index 25458b83ac..108b92bd9d 100644 --- a/openstack_dashboard/dashboards/identity/projects/views.py +++ b/openstack_dashboard/dashboards/identity/projects/views.py @@ -131,10 +131,9 @@ class IndexView(tables.DataTableView): _("Insufficient privilege level to view project information.") messages.info(self.request, msg) - if api.keystone.VERSIONS.active >= 3: - domain_lookup = api.keystone.domain_lookup(self.request) - for t in tenants: - t.domain_name = domain_lookup.get(t.domain_id) + domain_lookup = api.keystone.domain_lookup(self.request) + for t in tenants: + t.domain_name = domain_lookup.get(t.domain_id) return tenants @@ -182,28 +181,27 @@ class UpdateProjectView(workflows.WorkflowView): for field in PROJECT_INFO_FIELDS: initial[field] = getattr(project_info, field, None) - if keystone.VERSIONS.active >= 3: - # get extra columns info - ex_info = settings.PROJECT_TABLE_EXTRA_INFO - for ex_field in ex_info: - initial[ex_field] = getattr(project_info, ex_field, None) + # get extra columns info + ex_info = settings.PROJECT_TABLE_EXTRA_INFO + for ex_field in ex_info: + initial[ex_field] = getattr(project_info, ex_field, None) - # Retrieve the domain name where the project belong - try: - if policy.check((("identity", "identity:get_domain"),), - self.request): - domain = api.keystone.domain_get(self.request, - initial["domain_id"]) - initial["domain_name"] = domain.name + # Retrieve the domain name where the project belong + try: + if policy.check((("identity", "identity:get_domain"),), + self.request): + domain = api.keystone.domain_get(self.request, + initial["domain_id"]) + initial["domain_name"] = domain.name - else: - domain = api.keystone.get_default_domain(self.request) - initial["domain_name"] = domain.name + else: + domain = api.keystone.get_default_domain(self.request) + initial["domain_name"] = domain.name - except Exception: - exceptions.handle(self.request, - _('Unable to retrieve project domain.'), - redirect=reverse(INDEX_URL)) + except Exception: + exceptions.handle(self.request, + _('Unable to retrieve project domain.'), + redirect=reverse(INDEX_URL)) except Exception: exceptions.handle(self.request, _('Unable to retrieve project details.'), diff --git a/openstack_dashboard/dashboards/identity/projects/workflows.py b/openstack_dashboard/dashboards/identity/projects/workflows.py index d78c76d9fb..791db79457 100644 --- a/openstack_dashboard/dashboards/identity/projects/workflows.py +++ b/openstack_dashboard/dashboards/identity/projects/workflows.py @@ -43,7 +43,6 @@ LOG = logging.getLogger(__name__) INDEX_URL = "horizon:identity:projects:index" ADD_USER_URL = "horizon:identity:projects:create_user" -PROJECT_GROUP_ENABLED = keystone.VERSIONS.active >= 3 PROJECT_USER_MEMBER_SLUG = "update_members" PROJECT_GROUP_MEMBER_SLUG = "update_group_members" COMMON_HORIZONTAL_TEMPLATE = "identity/projects/_common_horizontal_form.html" @@ -232,12 +231,10 @@ class CreateProjectInfoAction(workflows.Action): super(CreateProjectInfoAction, self).__init__(request, *args, **kwargs) - # For keystone V3, display the two fields in read-only - if keystone.VERSIONS.active >= 3: - readonlyInput = forms.TextInput(attrs={'readonly': 'readonly'}) - self.fields["domain_id"].widget = readonlyInput - self.fields["domain_name"].widget = readonlyInput - self.add_extra_fields() + readonlyInput = forms.TextInput(attrs={'readonly': 'readonly'}) + self.fields["domain_id"].widget = readonlyInput + self.fields["domain_name"].widget = readonlyInput + self.add_extra_fields() def add_extra_fields(self): # add extra column defined by setting @@ -263,9 +260,8 @@ class CreateProjectInfo(workflows.Step): def __init__(self, workflow): super(CreateProjectInfo, self).__init__(workflow) - if keystone.VERSIONS.active >= 3: - EXTRA_INFO = settings.PROJECT_TABLE_EXTRA_INFO - self.contributes += tuple(EXTRA_INFO.keys()) + EXTRA_INFO = settings.PROJECT_TABLE_EXTRA_INFO + self.contributes += tuple(EXTRA_INFO.keys()) class UpdateProjectMembersAction(workflows.MembershipAction): @@ -478,10 +474,9 @@ class CreateProject(workflows.Workflow): def __init__(self, request=None, context_seed=None, entry_point=None, *args, **kwargs): - if PROJECT_GROUP_ENABLED: - self.default_steps = (CreateProjectInfo, - UpdateProjectMembers, - UpdateProjectGroups) + self.default_steps = (CreateProjectInfo, + UpdateProjectMembers, + UpdateProjectGroups) super(CreateProject, self).__init__(request=request, context_seed=context_seed, entry_point=entry_point, @@ -499,11 +494,8 @@ class CreateProject(workflows.Workflow): domain_id = data['domain_id'] try: # add extra information - if keystone.VERSIONS.active >= 3: - EXTRA_INFO = settings.PROJECT_TABLE_EXTRA_INFO - kwargs = dict((key, data.get(key)) for key in EXTRA_INFO) - else: - kwargs = {} + EXTRA_INFO = settings.PROJECT_TABLE_EXTRA_INFO + kwargs = dict((key, data.get(key)) for key in EXTRA_INFO) desc = data['description'] self.object = api.keystone.tenant_create(request, @@ -545,10 +537,7 @@ class CreateProject(workflows.Workflow): users_added += 1 users_to_add -= users_added except Exception: - if PROJECT_GROUP_ENABLED: - group_msg = _(", add project groups") - else: - group_msg = "" + group_msg = _(", add project groups") exceptions.handle(request, _('Failed to add %(users_to_add)s project ' 'members%(group_msg)s and set project quotas.') @@ -591,8 +580,7 @@ class CreateProject(workflows.Workflow): return False project_id = project.id self._update_project_members(request, data, project_id) - if PROJECT_GROUP_ENABLED: - self._update_project_groups(request, data, project_id) + self._update_project_groups(request, data, project_id) return True @@ -640,9 +628,8 @@ class UpdateProjectInfo(workflows.Step): def __init__(self, workflow): super(UpdateProjectInfo, self).__init__(workflow) - if keystone.VERSIONS.active >= 3: - EXTRA_INFO = settings.PROJECT_TABLE_EXTRA_INFO - self.contributes += tuple(EXTRA_INFO.keys()) + EXTRA_INFO = settings.PROJECT_TABLE_EXTRA_INFO + self.contributes += tuple(EXTRA_INFO.keys()) class UpdateProject(workflows.Workflow): @@ -657,10 +644,9 @@ class UpdateProject(workflows.Workflow): def __init__(self, request=None, context_seed=None, entry_point=None, *args, **kwargs): - if PROJECT_GROUP_ENABLED: - self.default_steps = (UpdateProjectInfo, - UpdateProjectMembers, - UpdateProjectGroups) + self.default_steps = (UpdateProjectInfo, + UpdateProjectMembers, + UpdateProjectGroups) super(UpdateProject, self).__init__(request=request, context_seed=context_seed, @@ -685,11 +671,8 @@ class UpdateProject(workflows.Workflow): project_id = data['project_id'] # add extra information - if keystone.VERSIONS.active >= 3: - EXTRA_INFO = settings.PROJECT_TABLE_EXTRA_INFO - kwargs = dict((key, data.get(key)) for key in EXTRA_INFO) - else: - kwargs = {} + EXTRA_INFO = settings.PROJECT_TABLE_EXTRA_INFO + kwargs = dict((key, data.get(key)) for key in EXTRA_INFO) return api.keystone.tenant_update( request, @@ -832,10 +815,7 @@ class UpdateProject(workflows.Workflow): users_to_modify -= users_added return True except Exception: - if PROJECT_GROUP_ENABLED: - group_msg = _(", update project groups") - else: - group_msg = "" + group_msg = _(", update project groups") exceptions.handle(request, _('Failed to modify %(users_to_modify)s' ' project members%(group_msg)s and ' @@ -935,10 +915,9 @@ class UpdateProject(workflows.Workflow): if not ret: return False - if PROJECT_GROUP_ENABLED: - ret = self._update_project_groups(request, data, - project_id, domain_id) - if not ret: - return False + ret = self._update_project_groups(request, data, + project_id, domain_id) + if not ret: + return False return True diff --git a/openstack_dashboard/dashboards/identity/roles/panel.py b/openstack_dashboard/dashboards/identity/roles/panel.py index 57f07398ce..43358bbf82 100644 --- a/openstack_dashboard/dashboards/identity/roles/panel.py +++ b/openstack_dashboard/dashboards/identity/roles/panel.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +from django.conf import settings from django.utils.translation import ugettext_lazy as _ import horizon @@ -25,11 +26,7 @@ class Roles(horizon.Panel): policy_rules = (("identity", "identity:list_roles"),) def can_access(self, context): - if keystone.is_multi_domain_enabled() \ - and not keystone.is_domain_admin(context['request']): + if (settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT and + not keystone.is_domain_admin(context['request'])): return False return super(Roles, self).can_access(context) - - @staticmethod - def can_register(): - return keystone.VERSIONS.active >= 3 diff --git a/openstack_dashboard/dashboards/identity/users/forms.py b/openstack_dashboard/dashboards/identity/users/forms.py index 8e794a4e3d..9fa2c75dc7 100644 --- a/openstack_dashboard/dashboards/identity/users/forms.py +++ b/openstack_dashboard/dashboards/identity/users/forms.py @@ -35,7 +35,6 @@ from horizon.utils import validators from openstack_dashboard import api LOG = logging.getLogger(__name__) -PROJECT_REQUIRED = api.keystone.VERSIONS.active < 3 class PasswordMixin(forms.SelfHandlingForm): @@ -72,12 +71,8 @@ class BaseUserForm(forms.SelfHandlingForm): default_project_id = kwargs['initial'].get('project', None) try: - if api.keystone.VERSIONS.active >= 3: - projects, has_more = api.keystone.tenant_list( - request, domain=domain_id) - else: - projects, has_more = api.keystone.tenant_list( - request, user=user_id) + projects, has_more = api.keystone.tenant_list( + request, domain=domain_id) for project in sorted(projects, key=lambda p: p.name.lower()): if project.enabled: @@ -96,14 +91,13 @@ class BaseUserForm(forms.SelfHandlingForm): class AddExtraColumnMixIn(object): def add_extra_fields(self, ordering=None): - if api.keystone.VERSIONS.active >= 3: - # add extra column defined by setting - EXTRA_INFO = settings.USER_TABLE_EXTRA_INFO - for key, value in EXTRA_INFO.items(): - self.fields[key] = forms.CharField(label=value, - required=False) - if ordering: - ordering.append(key) + # add extra column defined by setting + EXTRA_INFO = settings.USER_TABLE_EXTRA_INFO + for key, value in EXTRA_INFO.items(): + self.fields[key] = forms.CharField(label=value, + required=False) + if ordering: + ordering.append(key) ADD_PROJECT_URL = "horizon:identity:projects:create" @@ -126,10 +120,10 @@ class CreateUserForm(PasswordMixin, BaseUserForm, AddExtraColumnMixIn): label=_("Email"), required=False) project = forms.ThemableDynamicChoiceField(label=_("Primary Project"), - required=PROJECT_REQUIRED, + required=False, add_item_link=ADD_PROJECT_URL) role_id = forms.ThemableChoiceField(label=_("Role"), - required=PROJECT_REQUIRED) + required=False) enabled = forms.BooleanField(label=_("Enabled"), required=False, initial=True) @@ -155,13 +149,9 @@ class CreateUserForm(PasswordMixin, BaseUserForm, AddExtraColumnMixIn): self.fields['role_id'].choices = role_choices # For keystone V3, display the two fields in read-only - if api.keystone.VERSIONS.active >= 3: - readonlyInput = forms.TextInput(attrs={'readonly': 'readonly'}) - self.fields["domain_id"].widget = readonlyInput - self.fields["domain_name"].widget = readonlyInput - # For keystone V2.0, hide description field - else: - self.fields["description"].widget = forms.HiddenInput() + readonlyInput = forms.TextInput(attrs={'readonly': 'readonly'}) + self.fields["domain_id"].widget = readonlyInput + self.fields["domain_name"].widget = readonlyInput # We have to protect the entire "data" dict because it contains the # password and confirm_password strings. @@ -175,11 +165,8 @@ class CreateUserForm(PasswordMixin, BaseUserForm, AddExtraColumnMixIn): data['email'] = data['email'] or None # add extra information - if api.keystone.VERSIONS.active >= 3: - EXTRA_INFO = settings.USER_TABLE_EXTRA_INFO - kwargs = dict((key, data.get(key)) for key in EXTRA_INFO) - else: - kwargs = {} + EXTRA_INFO = settings.USER_TABLE_EXTRA_INFO + kwargs = dict((key, data.get(key)) for key in EXTRA_INFO) if "lock_password" in data: kwargs.update({'options': @@ -240,7 +227,7 @@ class UpdateUserForm(BaseUserForm, AddExtraColumnMixIn): label=_("Email"), required=False) project = forms.ThemableChoiceField(label=_("Primary Project"), - required=PROJECT_REQUIRED) + required=False) lock_password = forms.BooleanField(label=_("Lock password"), required=False, @@ -252,14 +239,9 @@ class UpdateUserForm(BaseUserForm, AddExtraColumnMixIn): if api.keystone.keystone_can_edit_user() is False: for field in ('name', 'email'): self.fields.pop(field) - # For keystone V3, display the two fields in read-only - if api.keystone.VERSIONS.active >= 3: - readonlyInput = forms.TextInput(attrs={'readonly': 'readonly'}) - self.fields["domain_id"].widget = readonlyInput - self.fields["domain_name"].widget = readonlyInput - # For keystone V2.0, hide description field - else: - self.fields["description"].widget = forms.HiddenInput() + readonlyInput = forms.TextInput(attrs={'readonly': 'readonly'}) + self.fields["domain_id"].widget = readonlyInput + self.fields["domain_name"].widget = readonlyInput def handle(self, request, data): user = data.pop('id') @@ -267,7 +249,7 @@ class UpdateUserForm(BaseUserForm, AddExtraColumnMixIn): data.pop('domain_id') data.pop('domain_name') - if not PROJECT_REQUIRED and 'project' not in self.changed_data: + if 'project' not in self.changed_data: data.pop('project') if 'description' not in self.changed_data: diff --git a/openstack_dashboard/dashboards/identity/users/panel.py b/openstack_dashboard/dashboards/identity/users/panel.py index 6c20f570b7..175f4447c8 100644 --- a/openstack_dashboard/dashboards/identity/users/panel.py +++ b/openstack_dashboard/dashboards/identity/users/panel.py @@ -16,6 +16,7 @@ # License for the specific language governing permissions and limitations # under the License. +from django.conf import settings from django.utils.translation import ugettext_lazy as _ import horizon @@ -30,7 +31,7 @@ class Users(horizon.Panel): ("identity", "identity:list_users")) def can_access(self, context): - if keystone.is_multi_domain_enabled() \ - and not keystone.is_domain_admin(context['request']): + if (settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT and + not keystone.is_domain_admin(context['request'])): return False return super(Users, self).can_access(context) diff --git a/openstack_dashboard/dashboards/identity/users/tables.py b/openstack_dashboard/dashboards/identity/users/tables.py index 8a9a59c900..d3345c01cd 100644 --- a/openstack_dashboard/dashboards/identity/users/tables.py +++ b/openstack_dashboard/dashboards/identity/users/tables.py @@ -159,13 +159,10 @@ class DeleteUsersAction(policy.PolicyTargetMixin, tables.DeleteAction): class UserFilterAction(tables.FilterAction): - if api.keystone.VERSIONS.active < 3: - filter_type = "query" - else: - filter_type = "server" - filter_choices = (("name", _("User Name ="), True), - ("id", _("User ID ="), True), - ("enabled", _("Enabled ="), True, _('e.g. Yes/No'))) + filter_type = "server" + filter_choices = (("name", _("User Name ="), True), + ("id", _("User ID ="), True), + ("enabled", _("Enabled ="), True, _('e.g. Yes/No'))) class UpdateRow(tables.Row): @@ -209,11 +206,9 @@ class UsersTable(tables.DataTable): filters=(defaultfilters.yesno, defaultfilters.capfirst), empty_value="False") - - if api.keystone.VERSIONS.active >= 3: - domain_name = tables.Column('domain_name', - verbose_name=_('Domain Name'), - attrs={'data-type': 'uuid'}) + domain_name = tables.Column('domain_name', + verbose_name=_('Domain Name'), + attrs={'data-type': 'uuid'}) class Meta(object): name = "users" diff --git a/openstack_dashboard/dashboards/identity/users/tabs.py b/openstack_dashboard/dashboards/identity/users/tabs.py index bd47925a52..1d483f3011 100644 --- a/openstack_dashboard/dashboards/identity/users/tabs.py +++ b/openstack_dashboard/dashboards/identity/users/tabs.py @@ -40,19 +40,18 @@ class OverviewTab(tabs.Tab): def _get_domain_name(self, user): domain_name = '' - if api.keystone.VERSIONS.active >= 3: - try: - if policy.check((("identity", "identity:get_domain"),), - self.request): - domain = api.keystone.domain_get( - self.request, user.domain_id) - domain_name = domain.name - else: - domain = api.keystone.get_default_domain(self.request) - domain_name = domain.get('name') - except Exception: - exceptions.handle(self.request, - _('Unable to retrieve user domain.')) + try: + if policy.check((("identity", "identity:get_domain"),), + self.request): + domain = api.keystone.domain_get( + self.request, user.domain_id) + domain_name = domain.name + else: + domain = api.keystone.get_default_domain(self.request) + domain_name = domain.get('name') + except Exception: + exceptions.handle(self.request, + _('Unable to retrieve user domain.')) return domain_name def _get_project_name(self, user): @@ -67,12 +66,9 @@ class OverviewTab(tabs.Tab): {'project_id': project_id, 'reason': e}) def _get_extras(self, user): - if api.keystone.VERSIONS.active >= 3: - extra_info = settings.USER_TABLE_EXTRA_INFO - return dict((display_key, getattr(user, key, '')) - for key, display_key in extra_info.items()) - else: - return {} + extra_info = settings.USER_TABLE_EXTRA_INFO + return dict((display_key, getattr(user, key, '')) + for key, display_key in extra_info.items()) def get_context_data(self, request): user = self.tab_group.kwargs['user'] diff --git a/openstack_dashboard/dashboards/identity/users/tests.py b/openstack_dashboard/dashboards/identity/users/tests.py index f93d6eb8a7..ebd532a243 100644 --- a/openstack_dashboard/dashboards/identity/users/tests.py +++ b/openstack_dashboard/dashboards/identity/users/tests.py @@ -143,12 +143,8 @@ class UsersViewTests(test.BaseAdminViewTests): ]) self.assertEqual(2, self.mock_get_default_domain.call_count) - if api.keystone.VERSIONS.active >= 3: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), domain=domain.id) - else: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), user=None) + self.mock_tenant_list.assert_called_once_with( + test.IsHttpRequest(), domain=domain.id) kwargs = {'phone_num': phone_number} self.mock_user_create.assert_called_once_with( @@ -217,12 +213,8 @@ class UsersViewTests(test.BaseAdminViewTests): mock.call(test.IsHttpRequest()), mock.call(test.IsHttpRequest(), False), ]) - if api.keystone.VERSIONS.active >= 3: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), domain=domain.id) - else: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), user=user.id) + self.mock_tenant_list.assert_called_once_with( + test.IsHttpRequest(), domain=domain.id) self.mock_user_create.assert_called_once_with( test.IsHttpRequest(), name=user.name, @@ -276,14 +268,9 @@ class UsersViewTests(test.BaseAdminViewTests): self.assert_mock_multiple_calls_with_same_arguments( self.mock_get_default_domain, 2, mock.call(test.IsHttpRequest())) - if api.keystone.VERSIONS.active >= 3: - self.assert_mock_multiple_calls_with_same_arguments( - self.mock_tenant_list, 2, - mock.call(test.IsHttpRequest(), domain=domain_id)) - else: - self.assert_mock_multiple_calls_with_same_arguments( - self.mock_tenant_list, 2, - mock.call(test.IsHttpRequest(), user=None)) + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_tenant_list, 2, + mock.call(test.IsHttpRequest(), domain=domain_id)) self.assert_mock_multiple_calls_with_same_arguments( self.mock_role_list, 2, mock.call(test.IsHttpRequest())) @@ -329,14 +316,9 @@ class UsersViewTests(test.BaseAdminViewTests): self.assert_mock_multiple_calls_with_same_arguments( self.mock_get_default_domain, 2, mock.call(test.IsHttpRequest())) - if api.keystone.VERSIONS.active >= 3: - self.assert_mock_multiple_calls_with_same_arguments( - self.mock_tenant_list, 2, - mock.call(test.IsHttpRequest(), domain=domain_id)) - else: - self.assert_mock_multiple_calls_with_same_arguments( - self.mock_tenant_list, 2, - mock.call(test.IsHttpRequest(), user=None)) + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_tenant_list, 2, + mock.call(test.IsHttpRequest(), domain=domain_id)) self.assert_mock_multiple_calls_with_same_arguments( self.mock_role_list, 2, mock.call(test.IsHttpRequest())) @@ -382,14 +364,9 @@ class UsersViewTests(test.BaseAdminViewTests): self.assert_mock_multiple_calls_with_same_arguments( self.mock_get_default_domain, 2, mock.call(test.IsHttpRequest())) - if api.keystone.VERSIONS.active >= 3: - self.assert_mock_multiple_calls_with_same_arguments( - self.mock_tenant_list, 2, - mock.call(test.IsHttpRequest(), domain=domain_id)) - else: - self.assert_mock_multiple_calls_with_same_arguments( - self.mock_tenant_list, 2, - mock.call(test.IsHttpRequest(), user=None)) + self.assert_mock_multiple_calls_with_same_arguments( + self.mock_tenant_list, 2, + mock.call(test.IsHttpRequest(), domain=domain_id)) self.assert_mock_multiple_calls_with_same_arguments( self.mock_role_list, 2, mock.call(test.IsHttpRequest())) @@ -493,12 +470,8 @@ class UsersViewTests(test.BaseAdminViewTests): admin=True) self.mock_domain_get.assert_called_once_with(test.IsHttpRequest(), domain_id) - if api.keystone.VERSIONS.active >= 3: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), domain=domain.id) - else: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), user=user.id) + self.mock_tenant_list.assert_called_once_with( + test.IsHttpRequest(), domain=domain.id) kwargs = {'phone_num': phone_number} self.mock_user_update.assert_called_once_with(test.IsHttpRequest(), user.id, @@ -538,12 +511,8 @@ class UsersViewTests(test.BaseAdminViewTests): admin=True) self.mock_domain_get.assert_called_once_with(test.IsHttpRequest(), domain_id) - if api.keystone.VERSIONS.active >= 3: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), domain=domain.id) - else: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), user=user.id) + self.mock_tenant_list.assert_called_once_with( + test.IsHttpRequest(), domain=domain.id) self.mock_user_update.assert_called_once_with(test.IsHttpRequest(), user.id, email=user.email, @@ -581,12 +550,8 @@ class UsersViewTests(test.BaseAdminViewTests): admin=True) self.mock_domain_get.assert_called_once_with(test.IsHttpRequest(), domain_id) - if api.keystone.VERSIONS.active >= 3: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), domain=domain.id) - else: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), user=user.id) + self.mock_tenant_list.assert_called_once_with( + test.IsHttpRequest(), domain=domain.id) self.mock_user_update.assert_called_once_with(test.IsHttpRequest(), user.id, email=user.email or "", @@ -657,12 +622,8 @@ class UsersViewTests(test.BaseAdminViewTests): admin=True) self.mock_domain_get.assert_called_once_with(test.IsHttpRequest(), domain_id) - if api.keystone.VERSIONS.active >= 3: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), domain=domain.id) - else: - self.mock_tenant_list.assert_called_once_with( - test.IsHttpRequest(), user=user.id) + self.mock_tenant_list.assert_called_once_with( + test.IsHttpRequest(), domain=domain.id) self.assert_mock_multiple_calls_with_same_arguments( self.mock_keystone_can_edit_user, 2, mock.call()) @@ -1339,12 +1300,8 @@ class UsersViewTests(test.BaseAdminViewTests): admin=True) self.mock_domain_get.assert_called_once_with(test.IsHttpRequest(), domain_id) - if api.keystone.VERSIONS.active >= 3: - self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest(), - domain=domain.id) - else: - self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest(), - user=user.id) + self.mock_tenant_list.assert_called_once_with(test.IsHttpRequest(), + domain=domain.id) self.mock_user_update.assert_called_once_with(test.IsHttpRequest(), user.id, email=user.email, diff --git a/openstack_dashboard/dashboards/identity/users/views.py b/openstack_dashboard/dashboards/identity/users/views.py index 4abb538a24..a0702b10a2 100644 --- a/openstack_dashboard/dashboards/identity/users/views.py +++ b/openstack_dashboard/dashboards/identity/users/views.py @@ -95,10 +95,9 @@ class IndexView(tables.DataTableView): msg = _("Insufficient privilege level to view user information.") messages.info(self.request, msg) - if api.keystone.VERSIONS.active >= 3: - domain_lookup = api.keystone.domain_lookup(self.request) - for u in users: - u.domain_name = domain_lookup.get(u.domain_id) + domain_lookup = api.keystone.domain_lookup(self.request) + for u in users: + u.domain_name = domain_lookup.get(u.domain_id) return users @@ -134,20 +133,19 @@ class UpdateView(forms.ModalFormView): domain_id = getattr(user, "domain_id", None) domain_name = '' # Retrieve the domain name where the project belongs - if api.keystone.VERSIONS.active >= 3: - try: - if policy.check((("identity", "identity:get_domain"),), - self.request): - domain = api.keystone.domain_get(self.request, domain_id) - domain_name = domain.name + try: + if policy.check((("identity", "identity:get_domain"),), + self.request): + domain = api.keystone.domain_get(self.request, domain_id) + domain_name = domain.name - else: - domain = api.keystone.get_default_domain(self.request) - domain_name = domain.get('name') + else: + domain = api.keystone.get_default_domain(self.request) + domain_name = domain.get('name') - except Exception: - exceptions.handle(self.request, - _('Unable to retrieve project domain.')) + except Exception: + exceptions.handle(self.request, + _('Unable to retrieve project domain.')) data = {'domain_id': domain_id, 'domain_name': domain_name, @@ -157,9 +155,8 @@ class UpdateView(forms.ModalFormView): 'email': getattr(user, 'email', None), 'description': getattr(user, 'description', None), 'lock_password': options.get("lock_password", False)} - if api.keystone.VERSIONS.active >= 3: - for key in settings.USER_TABLE_EXTRA_INFO: - data[key] = getattr(user, key, None) + for key in settings.USER_TABLE_EXTRA_INFO: + data[key] = getattr(user, key, None) return data diff --git a/openstack_dashboard/templatetags/context_selection.py b/openstack_dashboard/templatetags/context_selection.py index 23e38bc78e..da63c29cd0 100644 --- a/openstack_dashboard/templatetags/context_selection.py +++ b/openstack_dashboard/templatetags/context_selection.py @@ -17,8 +17,6 @@ from __future__ import absolute_import from django.conf import settings from django import template -from openstack_dashboard.api import keystone - register = template.Library() @@ -27,9 +25,10 @@ def is_multi_region_configured(request): return len(request.user.available_services_regions) > 1 +# TODO(e0ne): pass OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT to the template +# context and remove `is_multidomain` template tag def is_multidomain_supported(): - return (keystone.VERSIONS.active >= 3 and - settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT) + return settings.OPENSTACK_KEYSTONE_MULTIDOMAIN_SUPPORT @register.simple_tag(takes_context=True) diff --git a/openstack_dashboard/test/test_data/keystone_data.py b/openstack_dashboard/test/test_data/keystone_data.py index 9e2902d53f..e54fabbae2 100644 --- a/openstack_dashboard/test/test_data/keystone_data.py +++ b/openstack_dashboard/test/test_data/keystone_data.py @@ -36,86 +36,152 @@ from openstack_auth import user as auth_user from openstack_dashboard.test.test_data import utils -# Dummy service catalog with all service. +# Dummy service catalog with all service. All data should match +# scoped_auth_ref.service_catalog.catalog from keystoneauth1. # All endpoint URLs should point to example.com. # Try to keep them as accurate to real data as possible (ports, URIs, etc.) +# TODO(e0ne): make RegionOne and RegionTwo links unique. SERVICE_CATALOG = [ {"type": "compute", "name": "nova", "endpoints_links": [], "endpoints": [ {"region": "RegionOne", - "adminURL": "http://admin.nova.example.com:8774/v2", - "internalURL": "http://int.nova.example.com:8774/v2", - "publicURL": "http://public.nova.example.com:8774/v2"}, + "interface": "admin", + "url": "http://admin.nova.example.com:8774/v2"}, + {"region": "RegionOne", + "interface": "internal", + "url": "http://int.nova.example.com:8774/v2"}, + {"region": "RegionOne", + "interface": "public", + "url": "http://public.nova.example.com:8774/v2"}, {"region": "RegionTwo", - "adminURL": "http://admin.nova2.example.com:8774/v2", - "internalURL": "http://int.nova2.example.com:8774/v2", - "publicURL": "http://public.nova2.example.com:8774/v2"}]}, + "interface": "admin", + "url": "http://admin.nova2.example.com:8774/v2"}, + {"region": "RegionTwo", + "interface": "internal", + "url": "http://int.nova2.example.com:8774/v2"}, + {"region": "RegionTwo", + "interface": "public", + "url": "http://public.nova2.example.com:8774/v2"} + ]}, {"type": "volumev2", "name": "cinderv2", "endpoints_links": [], "endpoints": [ {"region": "RegionOne", - "adminURL": "http://admin.cinder.example.com:8776/v2", - "internalURL": "http://int.cinder.example.com:8776/v2", - "publicURL": "http://public.cinder.example.com:8776/v2"}, + "interface": "admin", + "url": "http://admin.cinder.example.com:8776/v2"}, + {"region": "RegionOne", + "interface": "internal", + "url": "http://int.cinder.example.com:8776/v2"}, + {"region": "RegionOne", + "interface": "public", + "url": "http://public.cinder.example.com:8776/v2"}, {"region": "RegionTwo", - "adminURL": "http://admin.cinder.example.com:8776/v2", - "internalURL": "http://int.cinder.example.com:8776/v2", - "publicURL": "http://public.cinder.example.com:8776/v2"}]}, + "interface": "admin", + "url": "http://admin.cinder2.example.com:8776/v2"}, + {"region": "RegionTwo", + "interface": "internal", + "url": "http://int.cinder2.example.com:8776/v2"}, + {"region": "RegionTwo", + "interface": "public", + "url": "http://public.cinder.example.com:8776/v2"} + ]}, {"type": "volumev3", "name": "cinderv3", "endpoints_links": [], "endpoints": [ {"region": "RegionOne", - "adminURL": "http://admin.cinder.example.com:8776/v3", - "internalURL": "http://int.cinder.example.com:8776/v3", - "publicURL": "http://public.cinder.example.com:8776/v3"}, + "interface": "admin", + "url": "http://admin.cinder.example.com:8776/v3"}, + {"region": "RegionOne", + "interface": "internal", + "url": "http://int.cinder.example.com:8776/v3"}, + {"region": "RegionOne", + "interface": "public", + "url": "http://public.cinder.example.com:8776/v3"}, {"region": "RegionTwo", - "adminURL": "http://admin.cinder.example.com:8776/v3", - "internalURL": "http://int.cinder.example.com:8776/v3", - "publicURL": "http://public.cinder.example.com:8776/v3"}]}, + "interface": "admin", + "url": "http://admin.cinder2.example.com:8776/v3"}, + {"region": "RegionTwo", + "interface": "internal", + "url": "http://int.cinder2.example.com:8776/v3"}, + {"region": "RegionTwo", + "interface": "public", + "url": "http://public.cinder2.example.com:8776/v3"} + ]}, {"type": "image", "name": "glance", "endpoints_links": [], "endpoints": [ {"region": "RegionOne", - "adminURL": "http://admin.glance.example.com:9292", - "internalURL": "http://int.glance.example.com:9292", - "publicURL": "http://public.glance.example.com:9292"}]}, + "interface": "admin", + "url": "http://admin.glance.example.com:9292", + }, + {"region": "RegionOne", + "interface": "internal", + "url": "http://int.glance.example.com:9292"}, + {"region": "RegionOne", + "interface": "public", + "url": "http://public.glance.example.com:9292"} + ]}, {"type": "identity", "name": "keystone", "endpoints_links": [], "endpoints": [ {"region": "RegionOne", - "adminURL": "http://admin.keystone.example.com/identity/v3", - "internalURL": "http://int.keystone.example.com/identity/v3", - "publicURL": "http://public.keystone.example.com/identity/v3"}]}, + "interface": "admin", + "url": "http://admin.keystone.example.com/identity/v3"}, + {"region": "RegionOne", + "interface": "internal", + "url": "http://int.keystone.example.com/identity/v3"}, + {"region": "RegionOne", + "interface": "public", + "url": "http://public.keystone.example.com/identity/v3"} + ]}, {"type": "object-store", "name": "swift", "endpoints_links": [], "endpoints": [ {"region": "RegionOne", - "adminURL": "http://admin.swift.example.com:8080/", - "internalURL": "http://int.swift.example.com:8080/", - "publicURL": "http://public.swift.example.com:8080/"}]}, + "interface": "admin", + "url": "http://admin.swift.example.com:8080/"}, + {"region": "RegionOne", + "interface": "internal", + "url": "http://int.swift.example.com:8080/"}, + {"region": "RegionOne", + "interface": "public", + "url": "http://public.swift.example.com:8080/"} + ]}, {"type": "network", "name": "neutron", "endpoints_links": [], "endpoints": [ {"region": "RegionOne", - "adminURL": "http://admin.neutron.example.com:9696/", - "internalURL": "http://int.neutron.example.com:9696/", - "publicURL": "http://public.neutron.example.com:9696/"}]}, + "interface": "admin", + "url": "http://admin.neutron.example.com:9696/"}, + {"region": "RegionOne", + "interface": "internal", + "url": "http://int.neutron.example.com:9696/"}, + {"region": "RegionOne", + "interface": "public", + "url": "http://public.neutron.example.com:9696/"} + ]}, {"type": "ec2", "name": "EC2 Service", "endpoints_links": [], "endpoints": [ {"region": "RegionOne", - "adminURL": "http://admin.nova.example.com:8773/services/Admin", - "publicURL": "http://public.nova.example.com:8773/services/Cloud", - "internalURL": "http://int.nova.example.com:8773/services/Cloud"}]}, + "interface": "admin", + "url": "http://admin.nova.example.com:8773/services/Admin"}, + {"region": "RegionOne", + "interface": "public", + "url": "http://public.nova.example.com:8773/services/Cloud"}, + {"region": "RegionOne", + "interface": "internal", + "url": "http://int.nova.example.com:8773/services/Cloud"} + ]}, ] diff --git a/openstack_dashboard/test/unit/api/test_keystone.py b/openstack_dashboard/test/unit/api/test_keystone.py index 3513a1a7c0..07e5702f75 100644 --- a/openstack_dashboard/test/unit/api/test_keystone.py +++ b/openstack_dashboard/test/unit/api/test_keystone.py @@ -74,12 +74,11 @@ class ServiceAPITests(test.APIMockTestCase): def test_service_wrapper(self): catalog = self.service_catalog identity_data = api.base.get_service_from_catalog(catalog, "identity") + # 'Service' class below requires 'id', so populate it here. identity_data['id'] = 1 - region = identity_data["endpoints"][0]["region"] - service = api.keystone.Service(identity_data, region) + service = api.keystone.Service(identity_data, "RegionOne") self.assertEqual(u"identity (native backend)", str(service)) - self.assertEqual(identity_data["endpoints"][0]["region"], - service.region) + self.assertEqual("RegionOne", service.region) self.assertEqual("http://int.keystone.example.com/identity/v3", service.url) self.assertEqual("http://public.keystone.example.com/identity/v3", @@ -89,12 +88,11 @@ class ServiceAPITests(test.APIMockTestCase): def test_service_wrapper_service_in_region(self): catalog = self.service_catalog compute_data = api.base.get_service_from_catalog(catalog, "compute") + # 'Service' class below requires 'id', so populate it here. compute_data['id'] = 1 - region = compute_data["endpoints"][1]["region"] - service = api.keystone.Service(compute_data, region) + service = api.keystone.Service(compute_data, 'RegionTwo') self.assertEqual(u"compute", str(service)) - self.assertEqual(compute_data["endpoints"][1]["region"], - service.region) + self.assertEqual("RegionTwo", service.region) self.assertEqual("http://int.nova2.example.com:8774/v2", service.url) self.assertEqual("http://public.nova2.example.com:8774/v2",