diff --git a/data/model.py b/data/model.py index d6d3c9154..1a6e91f1e 100644 --- a/data/model.py +++ b/data/model.py @@ -15,18 +15,52 @@ class DataModelException(Exception): pass -def create_user(username, password, email): - pw_hash = bcrypt.hashpw(password, bcrypt.gensalt()) +class InvalidEmailAddressException(DataModelException): + pass + +class InvalidUsernameException(DataModelException): + pass + + +class InvalidPasswordException(DataModelException): + pass + + +def create_user(username, password, email): if not validate_email(email): - raise DataModelException('Invalid email address: %s' % email) + raise InvalidEmailAddressException('Invalid email address: %s' % email) if not validate_username(username): - raise DataModelException('Invalid username: %s' % username) - if not validate_password(password): - raise DataModelException('Invalid password, password must be at least ' + - '8 characters and contain no whitespace.') + raise InvalidUsernameException('Invalid username: %s' % username) + + # We allow password none for the federated login case. + if password is not None and not validate_password(password): + raise InvalidPasswordException('Invalid password, password must be at ' + + 'least 8 characters and contain no ' + + 'whitespace.') try: + existing = User.get((User.username == username) | (User.email == email)) + + logger.debug('Existing user with same username or email.') + + # A user already exists with either the same username or email + if existing.username == username: + raise InvalidUsernameException('Username has already been taken: %s' % + username) + raise InvalidEmailAddressException('Email has already been used: %s' % + email) + + except User.DoesNotExist: + # This is actually the happy path + logger.debug('Email and username are unique!') + pass + + try: + pw_hash = None + if password is not None: + pw_hash = bcrypt.hashpw(password, bcrypt.gensalt()) + new_user = User.create(username=username, password_hash=pw_hash, email=email) return new_user @@ -35,18 +69,16 @@ def create_user(username, password, email): def create_federated_user(username, email, service_name, service_id): - try: - new_user = User.create(username=username, email=email, verified=True) + new_user = create_user(username, None, email) + new_user.verified = True + new_user.save() + service = LoginService.get(LoginService.name == service_name) federated_user = FederatedLogin.create(user=new_user, service=service, service_ident=service_id) return new_user - except Exception as ex: - raise DataModelException(ex.message) - - def verify_federated_login(service_name, service_id): selected = FederatedLogin.select(FederatedLogin, User) with_service = selected.join(LoginService) @@ -98,7 +130,9 @@ def verify_user(username, password): except User.DoesNotExist: return None - if bcrypt.hashpw(password, fetched.password_hash) == fetched.password_hash: + if (fetched.password_hash and + bcrypt.hashpw(password, fetched.password_hash) == + fetched.password_hash): return fetched # We weren't able to authorize the user diff --git a/endpoints/api.py b/endpoints/api.py index 0e5a9cdd5..2c6dcf6a8 100644 --- a/endpoints/api.py +++ b/endpoints/api.py @@ -72,13 +72,8 @@ def create_user_api(): send_confirmation_email(new_user.username, new_user.email, code.code) return make_response('Created', 201) except model.DataModelException as ex: - message = ex.message - m = re.search('column ([a-zA-Z]+) is not unique', message) - if m and m.group(1): - message = m.group(1) + ' already exists' - error_resp = jsonify({ - 'message': message, + 'message': ex.message, }) error_resp.status_code = 400 return error_resp diff --git a/endpoints/web.py b/endpoints/web.py index f253264b5..6f170cbdc 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -135,14 +135,18 @@ def github_oauth_callback(): to_login = model.verify_federated_login('github', github_id) if not to_login: # try to create the user - to_login = model.create_federated_user(username, found_email, 'github', - github_id) + + try: + to_login = model.create_federated_user(username, found_email, 'github', + github_id) + except model.DataModelException, ex: + return render_template('githuberror.html', error_message=ex.message) if common_login(to_login): return redirect(url_for('index')) # TODO something bad happened, we need to tell the user somehow - return redirect(url_for('signin')) + return render_template('githuberror.html') @app.route('/confirm', methods=['GET']) diff --git a/templates/githuberror.html b/templates/githuberror.html new file mode 100644 index 000000000..05370326b --- /dev/null +++ b/templates/githuberror.html @@ -0,0 +1,29 @@ + + +
+