Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removal of Third Party Auth #15571

Open
wants to merge 21 commits into
base: devel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ tools/docker-compose/_build
tools/docker-compose/_sources
tools/docker-compose/overrides/
tools/docker-compose-minikube/_sources
tools/docker-compose/keycloak.awx.realm.json

!tools/docker-compose/editable_dependencies
tools/docker-compose/editable_dependencies/*
Expand Down
18 changes: 4 additions & 14 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ COMPOSE_TAG ?= $(GIT_BRANCH)
MAIN_NODE_TYPE ?= hybrid
# If set to true docker-compose will also start a pgbouncer instance and use it
PGBOUNCER ?= false
# If set to true docker-compose will also start a keycloak instance
KEYCLOAK ?= false
# If set to true docker-compose will also start an ldap instance
LDAP ?= false
# If set to true docker-compose will also start a splunk instance
SPLUNK ?= false
# If set to true docker-compose will also start a prometheus instance
Expand All @@ -45,8 +41,6 @@ GRAFANA ?= false
VAULT ?= false
# If set to true docker-compose will also start a hashicorp vault instance with TLS enabled
VAULT_TLS ?= false
# If set to true docker-compose will also start a tacacs+ instance
TACACS ?= false
# If set to true docker-compose will also start an OpenTelemetry Collector instance
OTEL ?= false
# If set to true docker-compose will also start a Loki instance
Expand Down Expand Up @@ -345,7 +339,7 @@ api-lint:
awx-link:
[ -d "/awx_devel/awx.egg-info" ] || $(PYTHON) /awx_devel/tools/scripts/egg_info_dev

TEST_DIRS ?= awx/main/tests/unit awx/main/tests/functional awx/conf/tests awx/sso/tests
TEST_DIRS ?= awx/main/tests/unit awx/main/tests/functional awx/conf/tests
PYTEST_ARGS ?= -n auto
## Run all API unit tests.
test:
Expand Down Expand Up @@ -446,7 +440,7 @@ test_unit:
@if [ "$(VENV_BASE)" ]; then \
. $(VENV_BASE)/awx/bin/activate; \
fi; \
py.test awx/main/tests/unit awx/conf/tests/unit awx/sso/tests/unit
py.test awx/main/tests/unit awx/conf/tests/unit

## Output test coverage as HTML (into htmlcov directory).
coverage_html:
Expand Down Expand Up @@ -507,14 +501,11 @@ docker-compose-sources: .git/hooks/pre-commit
-e execution_node_count=$(EXECUTION_NODE_COUNT) \
-e minikube_container_group=$(MINIKUBE_CONTAINER_GROUP) \
-e enable_pgbouncer=$(PGBOUNCER) \
-e enable_keycloak=$(KEYCLOAK) \
-e enable_ldap=$(LDAP) \
-e enable_splunk=$(SPLUNK) \
-e enable_prometheus=$(PROMETHEUS) \
-e enable_grafana=$(GRAFANA) \
-e enable_vault=$(VAULT) \
-e vault_tls=$(VAULT_TLS) \
-e enable_tacacs=$(TACACS) \
-e enable_otel=$(OTEL) \
-e enable_loki=$(LOKI) \
-e install_editable_dependencies=$(EDITABLE_DEPENDENCIES) \
Expand All @@ -525,8 +516,7 @@ docker-compose: awx/projects docker-compose-sources
ansible-galaxy install --ignore-certs -r tools/docker-compose/ansible/requirements.yml;
$(ANSIBLE_PLAYBOOK) -i tools/docker-compose/inventory tools/docker-compose/ansible/initialize_containers.yml \
-e enable_vault=$(VAULT) \
-e vault_tls=$(VAULT_TLS) \
-e enable_ldap=$(LDAP); \
-e vault_tls=$(VAULT_TLS); \
$(MAKE) docker-compose-up

docker-compose-up:
Expand Down Expand Up @@ -598,7 +588,7 @@ docker-clean:
-$(foreach image_id,$(shell docker images --filter=reference='*/*/*awx_devel*' --filter=reference='*/*awx_devel*' --filter=reference='*awx_devel*' -aq),docker rmi --force $(image_id);)

docker-clean-volumes: docker-compose-clean docker-compose-container-group-clean
docker volume rm -f tools_var_lib_awx tools_awx_db tools_awx_db_15 tools_vault_1 tools_ldap_1 tools_grafana_storage tools_prometheus_storage $(shell docker volume ls --filter name=tools_redis_socket_ -q)
docker volume rm -f tools_var_lib_awx tools_awx_db tools_awx_db_15 tools_vault_1 tools_grafana_storage tools_prometheus_storage $(shell docker volume ls --filter name=tools_redis_socket_ -q)

docker-refresh: docker-clean docker-compose

Expand Down
12 changes: 4 additions & 8 deletions awx/api/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from awx.conf import fields, register, register_validate
from awx.api.fields import OAuth2ProviderField
from oauth2_provider.settings import oauth2_settings
from awx.sso.common import is_remote_auth_enabled


register(
Expand All @@ -35,10 +34,7 @@
'DISABLE_LOCAL_AUTH',
field_class=fields.BooleanField,
label=_('Disable the built-in authentication system'),
help_text=_(
"Controls whether users are prevented from using the built-in authentication system. "
"You probably want to do this if you are using an LDAP or SAML integration."
),
help_text=_("Controls whether users are prevented from using the built-in authentication system. "),
category=_('Authentication'),
category_slug='authentication',
)
Expand Down Expand Up @@ -77,8 +73,8 @@
default=False,
label=_('Allow External Users to Create OAuth2 Tokens'),
help_text=_(
'For security reasons, users from external auth providers (LDAP, SAML, '
'SSO, Radius, and others) are not allowed to create OAuth2 tokens. '
'For security reasons, users from external auth providers '
'are not allowed to create OAuth2 tokens. '
'To change this behavior, enable this setting. Existing tokens will '
'not be deleted when this setting is toggled off.'
),
Expand Down Expand Up @@ -109,7 +105,7 @@


def authentication_validate(serializer, attrs):
if attrs.get('DISABLE_LOCAL_AUTH', False) and not is_remote_auth_enabled():
if attrs.get('DISABLE_LOCAL_AUTH', False):
raise serializers.ValidationError(_("There are no remote authentication systems configured."))
return attrs

Expand Down
54 changes: 2 additions & 52 deletions awx/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,6 @@
# AWX Utils
from awx.api.validators import HostnameRegexValidator

from awx.sso.common import get_external_account

logger = logging.getLogger('awx.api.serializers')

# Fields that should be summarized regardless of object type.
Expand Down Expand Up @@ -961,8 +959,6 @@ def get_types(self):

class UserSerializer(BaseSerializer):
password = serializers.CharField(required=False, default='', help_text=_('Field used to change the password.'))
ldap_dn = serializers.CharField(source='profile.ldap_dn', read_only=True)
external_account = serializers.SerializerMethodField(help_text=_('Set if the account is managed by an external service'))
is_system_auditor = serializers.BooleanField(default=False)
show_capabilities = ['edit', 'delete']

Expand All @@ -979,22 +975,13 @@ class Meta:
'is_superuser',
'is_system_auditor',
'password',
'ldap_dn',
'last_login',
'external_account',
)
extra_kwargs = {'last_login': {'read_only': True}}

def to_representation(self, obj):
ret = super(UserSerializer, self).to_representation(obj)
if self.get_external_account(obj):
# If this is an external account it shouldn't have a password field
ret.pop('password', None)
else:
# If its an internal account lets assume there is a password and return $encrypted$ to the user
ret['password'] = '$encrypted$'
if obj and type(self) is UserSerializer:
ret['auth'] = obj.social_auth.values('provider', 'uid')
ret['password'] = '$encrypted$'
return ret

def get_validation_exclusions(self, obj=None):
Expand Down Expand Up @@ -1027,10 +1014,7 @@ def validate_password(self, value):
return value

def _update_password(self, obj, new_password):
# For now we're not raising an error, just not saving password for
# users managed by LDAP who already have an unusable password set.
# Get external password will return something like ldap or enterprise or None if the user isn't external. We only want to allow a password update for a None option
if new_password and new_password != '$encrypted$' and not self.get_external_account(obj):
if new_password and new_password != '$encrypted$':
obj.set_password(new_password)
obj.save(update_fields=['password'])

Expand All @@ -1045,9 +1029,6 @@ def _update_password(self, obj, new_password):
obj.set_unusable_password()
obj.save(update_fields=['password'])

def get_external_account(self, obj):
return get_external_account(obj)

def create(self, validated_data):
new_password = validated_data.pop('password', None)
is_system_auditor = validated_data.pop('is_system_auditor', None)
Expand Down Expand Up @@ -1085,37 +1066,6 @@ def get_related(self, obj):
)
return res

def _validate_ldap_managed_field(self, value, field_name):
if not getattr(settings, 'AUTH_LDAP_SERVER_URI', None):
return value
try:
is_ldap_user = bool(self.instance and self.instance.profile.ldap_dn)
except AttributeError:
is_ldap_user = False
if is_ldap_user:
ldap_managed_fields = ['username']
ldap_managed_fields.extend(getattr(settings, 'AUTH_LDAP_USER_ATTR_MAP', {}).keys())
ldap_managed_fields.extend(getattr(settings, 'AUTH_LDAP_USER_FLAGS_BY_GROUP', {}).keys())
if field_name in ldap_managed_fields:
if value != getattr(self.instance, field_name):
raise serializers.ValidationError(_('Unable to change %s on user managed by LDAP.') % field_name)
return value

def validate_username(self, value):
return self._validate_ldap_managed_field(value, 'username')

def validate_first_name(self, value):
return self._validate_ldap_managed_field(value, 'first_name')

def validate_last_name(self, value):
return self._validate_ldap_managed_field(value, 'last_name')

def validate_email(self, value):
return self._validate_ldap_managed_field(value, 'email')

def validate_is_superuser(self, value):
return self._validate_ldap_managed_field(value, 'is_superuser')


class UserActivityStreamSerializer(UserSerializer):
"""Changes to system auditor status are shown as separate entries,
Expand Down
40 changes: 14 additions & 26 deletions awx/api/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@
# ansi2html
from ansi2html import Ansi2HTMLConverter

# Python Social Auth
from social_core.backends.utils import load_backends

# Django OAuth Toolkit
from oauth2_provider.models import get_access_token_model

Expand Down Expand Up @@ -129,6 +126,9 @@
from awx.api.pagination import UnifiedJobEventPagination
from awx.main.utils import set_environ

if 'ansible_base.authentication' in getattr(settings, "INSTALLED_APPS", []):
from ansible_base.authentication.models.authenticator import Authenticator as AnsibleBaseAuthenticator

Check warning on line 130 in awx/api/views/__init__.py

View check run for this annotation

Codecov / codecov/patch

awx/api/views/__init__.py#L130

Added line #L130 was not covered by tests

logger = logging.getLogger('awx.api.views')


Expand Down Expand Up @@ -684,30 +684,18 @@
swagger_topic = 'System Configuration'

def get(self, request):
from rest_framework.reverse import reverse

data = OrderedDict()
err_backend, err_message = request.session.get('social_auth_error', (None, None))
auth_backends = list(load_backends(settings.AUTHENTICATION_BACKENDS, force_load=True).items())
# Return auth backends in consistent order: Google, GitHub, SAML.
auth_backends.sort(key=lambda x: 'g' if x[0] == 'google-oauth2' else x[0])
for name, backend in auth_backends:
login_url = reverse('social:begin', args=(name,))
complete_url = request.build_absolute_uri(reverse('social:complete', args=(name,)))
backend_data = {'login_url': login_url, 'complete_url': complete_url}
if name == 'saml':
backend_data['metadata_url'] = reverse('sso:saml_metadata')
for idp in sorted(settings.SOCIAL_AUTH_SAML_ENABLED_IDPS.keys()):
saml_backend_data = dict(backend_data.items())
saml_backend_data['login_url'] = '%s?idp=%s' % (login_url, idp)
full_backend_name = '%s:%s' % (name, idp)
if (err_backend == full_backend_name or err_backend == name) and err_message:
saml_backend_data['error'] = err_message
data[full_backend_name] = saml_backend_data
else:
if err_backend == name and err_message:
backend_data['error'] = err_message
data[name] = backend_data
if 'ansible_base.authentication' in getattr(settings, "INSTALLED_APPS", []):
jessicamack marked this conversation as resolved.
Show resolved Hide resolved
# app is using ansible_base authentication
# add ansible_base authenticators
authenticators = AnsibleBaseAuthenticator.objects.filter(enabled=True, category="sso")

Check warning on line 691 in awx/api/views/__init__.py

View check run for this annotation

Codecov / codecov/patch

awx/api/views/__init__.py#L691

Added line #L691 was not covered by tests
for authenticator in authenticators:
login_url = authenticator.get_login_url()
data[authenticator.name] = {

Check warning on line 694 in awx/api/views/__init__.py

View check run for this annotation

Codecov / codecov/patch

awx/api/views/__init__.py#L693-L694

Added lines #L693 - L694 were not covered by tests
'login_url': login_url,
'name': authenticator.name,
}

return Response(data)


Expand Down
9 changes: 0 additions & 9 deletions awx/api/views/root.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,15 +295,6 @@ def get(self, request, format=None):
become_methods=PRIVILEGE_ESCALATION_METHODS,
)

# If LDAP is enabled, user_ldap_fields will return a list of field
# names that are managed by LDAP and should be read-only for users with
# a non-empty ldap_dn attribute.
if getattr(settings, 'AUTH_LDAP_SERVER_URI', None):
user_ldap_fields = ['username', 'password']
user_ldap_fields.extend(getattr(settings, 'AUTH_LDAP_USER_ATTR_MAP', {}).keys())
user_ldap_fields.extend(getattr(settings, 'AUTH_LDAP_USER_FLAGS_BY_GROUP', {}).keys())
data['user_ldap_fields'] = user_ldap_fields

if (
request.user.is_superuser
or request.user.is_system_auditor
Expand Down
6 changes: 2 additions & 4 deletions awx/conf/migrations/0006_v331_ldap_group_type.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

# AWX
from awx.conf.migrations._ldap_group_type import fill_ldap_group_type_params

from django.db import migrations


class Migration(migrations.Migration):
dependencies = [('conf', '0005_v330_rename_two_session_settings')]

operations = [migrations.RunPython(fill_ldap_group_type_params)]
# this migration is doing nothing, and is here to preserve migrations files integrity
operations = []
Loading
Loading