Make email addresses optional in external auth if email feature is turned off
Before this change, external auth such as Keystone would fail if a user without an email address tried to login, even if the email feature was disabled.
This commit is contained in:
parent
934cdecbd6
commit
d7f56350a4
18 changed files with 206 additions and 93 deletions
|
@ -5,10 +5,10 @@ from data.model import (user, team, DataModelException, InvalidOrganizationExcep
|
|||
InvalidUsernameException, db_transaction, _basequery)
|
||||
|
||||
|
||||
def create_organization(name, email, creating_user):
|
||||
def create_organization(name, email, creating_user, email_required=True):
|
||||
try:
|
||||
# Create the org
|
||||
new_org = user.create_user_noverify(name, email)
|
||||
new_org = user.create_user_noverify(name, email, email_required=email_required)
|
||||
new_org.organization = True
|
||||
new_org.save()
|
||||
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
import bcrypt
|
||||
import logging
|
||||
import json
|
||||
import uuid
|
||||
|
||||
from peewee import JOIN_LEFT_OUTER, IntegrityError, fn
|
||||
from uuid import uuid4
|
||||
|
@ -31,13 +32,12 @@ def hash_password(password, salt=None):
|
|||
salt = salt or bcrypt.gensalt()
|
||||
return bcrypt.hashpw(password.encode('utf-8'), salt)
|
||||
|
||||
|
||||
def create_user(username, password, email, auto_verify=False):
|
||||
def create_user(username, password, email, auto_verify=False, email_required=True):
|
||||
""" Creates a regular user, if allowed. """
|
||||
if not validate_password(password):
|
||||
raise InvalidPasswordException(INVALID_PASSWORD_MESSAGE)
|
||||
|
||||
created = create_user_noverify(username, email)
|
||||
created = create_user_noverify(username, email, email_required=email_required)
|
||||
created.password_hash = hash_password(password)
|
||||
created.verified = auto_verify
|
||||
created.save()
|
||||
|
@ -45,9 +45,14 @@ def create_user(username, password, email, auto_verify=False):
|
|||
return created
|
||||
|
||||
|
||||
def create_user_noverify(username, email):
|
||||
if not validate_email(email):
|
||||
raise InvalidEmailAddressException('Invalid email address: %s' % email)
|
||||
def create_user_noverify(username, email, email_required=True):
|
||||
if email_required:
|
||||
if not validate_email(email):
|
||||
raise InvalidEmailAddressException('Invalid email address: %s' % email)
|
||||
else:
|
||||
# If email addresses are not required and none was specified, then we just use a unique
|
||||
# ID to ensure that the database consistency check remains intact.
|
||||
email = email or str(uuid.uuid4())
|
||||
|
||||
(username_valid, username_issue) = validate_username(username)
|
||||
if not username_valid:
|
||||
|
@ -300,8 +305,8 @@ def list_entity_robot_permission_teams(entity_name, include_permissions=False):
|
|||
|
||||
|
||||
def create_federated_user(username, email, service_name, service_ident,
|
||||
set_password_notification, metadata={}):
|
||||
new_user = create_user_noverify(username, email)
|
||||
set_password_notification, metadata={}, email_required=True):
|
||||
new_user = create_user_noverify(username, email, email_required=email_required)
|
||||
new_user.verified = True
|
||||
new_user.save()
|
||||
|
||||
|
|
|
@ -29,7 +29,7 @@ def get_federated_service_name(authentication_type):
|
|||
|
||||
LDAP_CERT_FILENAME = 'ldap.crt'
|
||||
|
||||
def get_users_handler(config, config_provider, override_config_dir):
|
||||
def get_users_handler(config, _, override_config_dir):
|
||||
""" Returns a users handler for the authentication configured in the given config object. """
|
||||
authentication_type = config.get('AUTHENTICATION_TYPE', 'Database')
|
||||
|
||||
|
@ -48,7 +48,8 @@ def get_users_handler(config, config_provider, override_config_dir):
|
|||
|
||||
allow_tls_fallback = config.get('LDAP_ALLOW_INSECURE_FALLBACK', False)
|
||||
return LDAPUsers(ldap_uri, base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr,
|
||||
allow_tls_fallback, secondary_user_rdns=secondary_user_rdns)
|
||||
allow_tls_fallback, secondary_user_rdns=secondary_user_rdns,
|
||||
requires_email=features.MAILING)
|
||||
|
||||
if authentication_type == 'JWT':
|
||||
verify_url = config.get('JWT_VERIFY_ENDPOINT')
|
||||
|
@ -59,7 +60,8 @@ def get_users_handler(config, config_provider, override_config_dir):
|
|||
getuser_url = config.get('JWT_GETUSER_ENDPOINT', None)
|
||||
|
||||
return ExternalJWTAuthN(verify_url, query_url, getuser_url, issuer, override_config_dir,
|
||||
config['HTTPCLIENT'], max_fresh_s)
|
||||
config['HTTPCLIENT'], max_fresh_s,
|
||||
requires_email=features.MAILING)
|
||||
|
||||
if authentication_type == 'Keystone':
|
||||
auth_url = config.get('KEYSTONE_AUTH_URL')
|
||||
|
@ -68,9 +70,9 @@ def get_users_handler(config, config_provider, override_config_dir):
|
|||
keystone_admin_username = config.get('KEYSTONE_ADMIN_USERNAME')
|
||||
keystone_admin_password = config.get('KEYSTONE_ADMIN_PASSWORD')
|
||||
keystone_admin_tenant = config.get('KEYSTONE_ADMIN_TENANT')
|
||||
|
||||
return get_keystone_users(auth_version, auth_url, keystone_admin_username,
|
||||
keystone_admin_password, keystone_admin_tenant, timeout)
|
||||
keystone_admin_password, keystone_admin_tenant, timeout,
|
||||
requires_email=features.MAILING)
|
||||
|
||||
raise RuntimeError('Unknown authentication type: %s' % authentication_type)
|
||||
|
||||
|
|
|
@ -14,8 +14,8 @@ class ExternalJWTAuthN(FederatedUsers):
|
|||
PUBLIC_KEY_FILENAME = 'jwt-authn.cert'
|
||||
|
||||
def __init__(self, verify_url, query_url, getuser_url, issuer, override_config_dir, http_client,
|
||||
max_fresh_s, public_key_path=None):
|
||||
super(ExternalJWTAuthN, self).__init__('jwtauthn')
|
||||
max_fresh_s, public_key_path=None, requires_email=True):
|
||||
super(ExternalJWTAuthN, self).__init__('jwtauthn', requires_email)
|
||||
self.verify_url = verify_url
|
||||
self.query_url = query_url
|
||||
self.getuser_url = getuser_url
|
||||
|
@ -23,6 +23,7 @@ class ExternalJWTAuthN(FederatedUsers):
|
|||
self.issuer = issuer
|
||||
self.client = http_client
|
||||
self.max_fresh_s = max_fresh_s
|
||||
self.requires_email = requires_email
|
||||
|
||||
default_key_path = os.path.join(override_config_dir, ExternalJWTAuthN.PUBLIC_KEY_FILENAME)
|
||||
public_key_path = public_key_path or default_key_path
|
||||
|
@ -48,11 +49,12 @@ class ExternalJWTAuthN(FederatedUsers):
|
|||
if not 'sub' in payload:
|
||||
raise Exception('Missing sub field in JWT')
|
||||
|
||||
if not 'email' in payload:
|
||||
if self.requires_email and not 'email' in payload:
|
||||
raise Exception('Missing email field in JWT')
|
||||
|
||||
# Parse out the username and email.
|
||||
user_info = UserInformation(username=payload['sub'], email=payload['email'], id=payload['sub'])
|
||||
user_info = UserInformation(username=payload['sub'], email=payload.get('email'),
|
||||
id=payload['sub'])
|
||||
return (user_info, None)
|
||||
|
||||
|
||||
|
@ -67,7 +69,7 @@ class ExternalJWTAuthN(FederatedUsers):
|
|||
|
||||
query_results = []
|
||||
for result in payload['results'][0:limit]:
|
||||
user_info = UserInformation(username=result['username'], email=result['email'],
|
||||
user_info = UserInformation(username=result['username'], email=result.get('email'),
|
||||
id=result['username'])
|
||||
query_results.append(user_info)
|
||||
|
||||
|
@ -83,10 +85,11 @@ class ExternalJWTAuthN(FederatedUsers):
|
|||
if not 'sub' in payload:
|
||||
raise Exception('Missing sub field in JWT')
|
||||
|
||||
if not 'email' in payload:
|
||||
if self.requires_email and not 'email' in payload:
|
||||
raise Exception('Missing email field in JWT')
|
||||
|
||||
user_info = UserInformation(username=payload['sub'], email=payload['email'], id=payload['sub'])
|
||||
user_info = UserInformation(username=payload['sub'], email=payload.get('email'),
|
||||
id=payload['sub'])
|
||||
return (user_info, None)
|
||||
|
||||
|
||||
|
|
|
@ -53,15 +53,14 @@ class LDAPUsers(FederatedUsers):
|
|||
_LDAPResult = namedtuple('LDAPResult', ['dn', 'attrs'])
|
||||
|
||||
def __init__(self, ldap_uri, base_dn, admin_dn, admin_passwd, user_rdn, uid_attr, email_attr,
|
||||
allow_tls_fallback=False, secondary_user_rdns=None):
|
||||
|
||||
super(LDAPUsers, self).__init__('ldap')
|
||||
|
||||
allow_tls_fallback=False, secondary_user_rdns=None, requires_email=True):
|
||||
super(LDAPUsers, self).__init__('ldap', requires_email)
|
||||
self._ldap = LDAPConnectionBuilder(ldap_uri, admin_dn, admin_passwd, allow_tls_fallback)
|
||||
self._ldap_uri = ldap_uri
|
||||
self._uid_attr = uid_attr
|
||||
self._email_attr = email_attr
|
||||
self._allow_tls_fallback = allow_tls_fallback
|
||||
self._requires_email = requires_email
|
||||
|
||||
# Note: user_rdn is a list of RDN pieces (for historical reasons), and secondary_user_rds
|
||||
# is a list of RDN strings.
|
||||
|
@ -167,11 +166,11 @@ class LDAPUsers(FederatedUsers):
|
|||
if not response.get(self._uid_attr):
|
||||
return (None, 'Missing uid field "%s" in user record' % self._uid_attr)
|
||||
|
||||
if not response.get(self._email_attr):
|
||||
if self._requires_email and not response.get(self._email_attr):
|
||||
return (None, 'Missing mail field "%s" in user record' % self._email_attr)
|
||||
|
||||
username = response[self._uid_attr][0].decode('utf-8')
|
||||
email = response[self._email_attr][0]
|
||||
email = response.get(self._email_attr, [None])[0]
|
||||
return (UserInformation(username=username, email=email, id=username), None)
|
||||
|
||||
def get_user(self, username_or_email):
|
||||
|
|
|
@ -12,8 +12,9 @@ UserInformation = namedtuple('UserInformation', ['username', 'email', 'id'])
|
|||
class FederatedUsers(object):
|
||||
""" Base class for all federated users systems. """
|
||||
|
||||
def __init__(self, federated_service):
|
||||
def __init__(self, federated_service, requires_email):
|
||||
self._federated_service = federated_service
|
||||
self._requires_email = requires_email
|
||||
|
||||
@property
|
||||
def federated_service(self):
|
||||
|
@ -50,11 +51,13 @@ class FederatedUsers(object):
|
|||
|
||||
db_user = model.user.create_federated_user(valid_username, email, self._federated_service,
|
||||
username,
|
||||
set_password_notification=False)
|
||||
set_password_notification=False,
|
||||
email_required=self._requires_email)
|
||||
else:
|
||||
# Update the db attributes from the federated service.
|
||||
db_user.email = email
|
||||
db_user.save()
|
||||
if email:
|
||||
db_user.email = email
|
||||
db_user.save()
|
||||
|
||||
return (db_user, None)
|
||||
|
||||
|
|
|
@ -18,23 +18,27 @@ def _take(n, iterable):
|
|||
|
||||
|
||||
def get_keystone_users(auth_version, auth_url, admin_username, admin_password, admin_tenant,
|
||||
timeout=None):
|
||||
timeout=None, requires_email=True):
|
||||
if auth_version == 3:
|
||||
return KeystoneV3Users(auth_url, admin_username, admin_password, admin_tenant, timeout)
|
||||
return KeystoneV3Users(auth_url, admin_username, admin_password, admin_tenant, timeout,
|
||||
requires_email)
|
||||
else:
|
||||
return KeystoneV2Users(auth_url, admin_username, admin_password, admin_tenant, timeout)
|
||||
return KeystoneV2Users(auth_url, admin_username, admin_password, admin_tenant, timeout,
|
||||
requires_email)
|
||||
|
||||
|
||||
class KeystoneV2Users(FederatedUsers):
|
||||
""" Delegates authentication to OpenStack Keystone V2. """
|
||||
def __init__(self, auth_url, admin_username, admin_password, admin_tenant, timeout=None):
|
||||
super(KeystoneV2Users, self).__init__('keystone')
|
||||
def __init__(self, auth_url, admin_username, admin_password, admin_tenant, timeout=None,
|
||||
requires_email=True):
|
||||
super(KeystoneV2Users, self).__init__('keystone', requires_email)
|
||||
self.auth_url = auth_url
|
||||
self.admin_username = admin_username
|
||||
self.admin_password = admin_password
|
||||
self.admin_tenant = admin_tenant
|
||||
self.timeout = timeout or DEFAULT_TIMEOUT
|
||||
self.debug = os.environ.get('USERS_DEBUG') == '1'
|
||||
self.requires_email = requires_email
|
||||
|
||||
def verify_credentials(self, username_or_email, password):
|
||||
try:
|
||||
|
@ -58,7 +62,11 @@ class KeystoneV2Users(FederatedUsers):
|
|||
logger.exception('Keystone unauthorized admin')
|
||||
return (None, 'Keystone admin credentials are invalid: %s' % kut.message)
|
||||
|
||||
return (UserInformation(username=username_or_email, email=user.email, id=user_id), None)
|
||||
if self.requires_email and not hasattr(user, 'email'):
|
||||
return (None, 'Missing email field for user %s' % user_id)
|
||||
|
||||
email = user.email if hasattr(user, 'email') else None
|
||||
return (UserInformation(username=username_or_email, email=email, id=user_id), None)
|
||||
|
||||
def query_users(self, query, limit=20):
|
||||
return (None, 'Unsupported in Keystone V2')
|
||||
|
@ -69,14 +77,16 @@ class KeystoneV2Users(FederatedUsers):
|
|||
|
||||
class KeystoneV3Users(FederatedUsers):
|
||||
""" Delegates authentication to OpenStack Keystone V3. """
|
||||
def __init__(self, auth_url, admin_username, admin_password, admin_tenant, timeout=None):
|
||||
super(KeystoneV3Users, self).__init__('keystone')
|
||||
def __init__(self, auth_url, admin_username, admin_password, admin_tenant, timeout=None,
|
||||
requires_email=True):
|
||||
super(KeystoneV3Users, self).__init__('keystone', requires_email)
|
||||
self.auth_url = auth_url
|
||||
self.admin_username = admin_username
|
||||
self.admin_password = admin_password
|
||||
self.admin_tenant = admin_tenant
|
||||
self.timeout = timeout or DEFAULT_TIMEOUT
|
||||
self.debug = os.environ.get('USERS_DEBUG') == '1'
|
||||
self.requires_email = requires_email
|
||||
|
||||
def verify_credentials(self, username_or_email, password):
|
||||
try:
|
||||
|
@ -85,6 +95,10 @@ class KeystoneV3Users(FederatedUsers):
|
|||
debug=self.debug)
|
||||
user_id = keystone_client.user_id
|
||||
user = keystone_client.users.get(user_id)
|
||||
|
||||
if self.requires_email and not hasattr(user, 'email'):
|
||||
return (None, 'Missing email field for user %s' % user_id)
|
||||
|
||||
return (self._user_info(user), None)
|
||||
except KeystoneAuthorizationFailure as kaf:
|
||||
logger.exception('Keystone auth failure for user: %s', username_or_email)
|
||||
|
@ -101,12 +115,15 @@ class KeystoneV3Users(FederatedUsers):
|
|||
if len(users_found) != 1:
|
||||
return (None, 'Single user not found')
|
||||
|
||||
return (users_found[0], None)
|
||||
user = users_found[0]
|
||||
if self.requires_email and not user.email:
|
||||
return (None, 'Missing email field for user %s' % user.id)
|
||||
|
||||
return (user, None)
|
||||
|
||||
@staticmethod
|
||||
def _user_info(user):
|
||||
# Because Keystone uses defined attributes...
|
||||
email = user.email if hasattr(user, 'email') else ''
|
||||
email = user.email if hasattr(user, 'email') else None
|
||||
return UserInformation(user.name, email, user.id)
|
||||
|
||||
def query_users(self, query, limit=20):
|
||||
|
|
Reference in a new issue