diff --git a/data/model/organization.py b/data/model/organization.py index 2d7d21593..949e6766a 100644 --- a/data/model/organization.py +++ b/data/model/organization.py @@ -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() diff --git a/data/model/user.py b/data/model/user.py index a94bc4cb8..14069ce32 100644 --- a/data/model/user.py +++ b/data/model/user.py @@ -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() diff --git a/data/users/__init__.py b/data/users/__init__.py index 210902115..11fc92458 100644 --- a/data/users/__init__.py +++ b/data/users/__init__.py @@ -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) diff --git a/data/users/externaljwt.py b/data/users/externaljwt.py index 696b081d5..444ada43a 100644 --- a/data/users/externaljwt.py +++ b/data/users/externaljwt.py @@ -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) diff --git a/data/users/externalldap.py b/data/users/externalldap.py index c56b24eed..2417584f5 100644 --- a/data/users/externalldap.py +++ b/data/users/externalldap.py @@ -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): diff --git a/data/users/federated.py b/data/users/federated.py index 7ba1f4a80..52f7c15cf 100644 --- a/data/users/federated.py +++ b/data/users/federated.py @@ -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) diff --git a/data/users/keystone.py b/data/users/keystone.py index ecff5b701..c018c83d7 100644 --- a/data/users/keystone.py +++ b/data/users/keystone.py @@ -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): diff --git a/endpoints/api/organization.py b/endpoints/api/organization.py index 999ecf8e1..29585b3f1 100644 --- a/endpoints/api/organization.py +++ b/endpoints/api/organization.py @@ -69,7 +69,6 @@ class OrganizationList(ApiResource): 'description': 'Description of a new organization.', 'required': [ 'name', - 'email', ], 'properties': { 'name': { @@ -105,8 +104,12 @@ class OrganizationList(ApiResource): msg = 'A user or organization with this name already exists' raise request_error(message=msg) + if features.MAILING and not org_data.get('email'): + raise request_error(message='Email address is required') + try: - model.organization.create_organization(org_data['name'], org_data['email'], user) + model.organization.create_organization(org_data['name'], org_data.get('email'), user, + email_required=features.MAILING) return 'Created', 201 except model.DataModelException as ex: raise request_error(exception=ex) diff --git a/endpoints/api/superuser.py b/endpoints/api/superuser.py index 477e3d7d6..e12070a1f 100644 --- a/endpoints/api/superuser.py +++ b/endpoints/api/superuser.py @@ -210,7 +210,7 @@ class SuperUserList(ApiResource): 'CreateInstallUser': { 'id': 'CreateInstallUser', 'description': 'Data for creating a user', - 'required': ['username', 'email'], + 'required': ['username'], 'properties': { 'username': { 'type': 'string', @@ -253,15 +253,15 @@ class SuperUserList(ApiResource): user_information = request.get_json() if SuperUserPermission().can(): - username = user_information['username'] - email = user_information['email'] - # Generate a temporary password for the user. random = SystemRandom() password = ''.join([random.choice(string.ascii_uppercase + string.digits) for _ in range(32)]) # Create the user. - user = model.user.create_user(username, password, email, auto_verify=not features.MAILING) + username = user_information['username'] + email = user_information.get('email') + user = model.user.create_user(username, password, email, auto_verify=not features.MAILING, + email_required=features.MAILING) # If mailing is turned on, send the user a verification email. if features.MAILING: diff --git a/endpoints/api/user.py b/endpoints/api/user.py index 4f09704eb..8e4a6b43f 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -163,7 +163,6 @@ class User(ApiResource): 'required': [ 'username', 'password', - 'email', ], 'properties': { 'username': { @@ -355,9 +354,14 @@ class User(ApiResource): if existing_user: raise request_error(message='The username already exists') + if features.MAILING and not user_data.get('email'): + raise request_error(message='Email address is required') + try: new_user = model.user.create_user(user_data['username'], user_data['password'], - user_data['email'], auto_verify=not features.MAILING) + user_data.get('email'), + auto_verify=not features.MAILING, + email_required=features.MAILING) email_address_confirmed = handle_invite_code(invite_code, new_user) if features.MAILING and not email_address_confirmed: diff --git a/static/js/pages/org-view.js b/static/js/pages/org-view.js index ad09fdd97..61f224b04 100644 --- a/static/js/pages/org-view.js +++ b/static/js/pages/org-view.js @@ -10,7 +10,7 @@ }) }]); - function OrgViewCtrl($scope, $routeParams, $timeout, ApiService, UIService, AvatarService) { + function OrgViewCtrl($scope, $routeParams, $timeout, ApiService, UIService, AvatarService, Config, Features) { var orgname = $routeParams.orgname; $scope.namespace = orgname; @@ -22,6 +22,9 @@ $scope.changeEmailInfo = null; $scope.context = {}; + $scope.Config = Config; + $scope.Features = Features; + $scope.orgScope = { 'changingOrganization': false, 'organizationEmail': '' diff --git a/static/partials/new-organization.html b/static/partials/new-organization.html index 1951c781c..5022182d1 100644 --- a/static/partials/new-organization.html +++ b/static/partials/new-organization.html @@ -54,7 +54,7 @@ This will also be the namespace for your repositories. Must be alphanumeric, all lowercase and at least four characters long. -