Add the concept of require_fresh_login to both the backend and frontend. Sensitive methods will now be marked with the annotation, which requires that the user has performed a login within 10 minutes or they are asked to do so in the UI before running the operation again.

This commit is contained in:
Joseph Schorr 2014-09-04 14:24:20 -04:00
parent 1e7e012b92
commit e783df31e0
9 changed files with 174 additions and 61 deletions

View file

@ -1,7 +1,8 @@
import logging
import json
import datetime
from flask import Blueprint, request, make_response, jsonify
from flask import Blueprint, request, make_response, jsonify, session
from flask.ext.restful import Resource, abort, Api, reqparse
from flask.ext.restful.utils.cors import crossdomain
from werkzeug.exceptions import HTTPException
@ -66,6 +67,11 @@ class Unauthorized(ApiException):
ApiException.__init__(self, 'insufficient_scope', 403, 'Unauthorized', payload)
class FreshLoginRequired(ApiException):
def __init__(self, payload=None):
ApiException.__init__(self, 'fresh_login_required', 401, "Requires fresh login", payload)
class ExceedsLicenseException(ApiException):
def __init__(self, payload=None):
ApiException.__init__(self, None, 402, 'Payment Required', payload)
@ -264,6 +270,26 @@ def require_user_permission(permission_class, scope=None):
require_user_read = require_user_permission(UserReadPermission, scopes.READ_USER)
require_user_admin = require_user_permission(UserAdminPermission, None)
require_fresh_user_admin = require_user_permission(UserAdminPermission, None)
def require_fresh_login(func):
@add_method_metadata('requires_fresh_login', True)
@wraps(func)
def wrapped(*args, **kwargs):
user = get_authenticated_user()
if not user:
raise Unauthorized()
logger.debug('Checking fresh login for user %s', user.username)
last_login = session.get('login_time', datetime.datetime.now() - datetime.timedelta(minutes=60))
valid_span = datetime.datetime.now() - datetime.timedelta(minutes=10)
if last_login >= valid_span:
return func(*args, **kwargs)
raise FreshLoginRequired()
return wrapped
def require_scope(scope_object):

View file

@ -119,6 +119,11 @@ def swagger_route_data(include_internal=False, compact=False):
if internal is not None:
new_operation['internal'] = True
if include_internal:
requires_fresh_login = method_metadata(method, 'requires_fresh_login')
if requires_fresh_login is not None:
new_operation['requires_fresh_login'] = True
if not internal or (internal and include_internal):
operations.append(new_operation)

View file

@ -9,7 +9,7 @@ from app import app, billing as stripe, authentication
from endpoints.api import (ApiResource, nickname, resource, validate_json_request, request_error,
log_action, internal_only, NotFound, require_user_admin, parse_args,
query_param, InvalidToken, require_scope, format_date, hide_if, show_if,
license_error)
license_error, require_fresh_login)
from endpoints.api.subscribe import subscribe
from endpoints.common import common_login
from data import model
@ -117,10 +117,6 @@ class User(ApiResource):
'type': 'object',
'description': 'Fields which can be updated in a user.',
'properties': {
'current_password': {
'type': 'string',
'description': 'The user\'s current password',
},
'password': {
'type': 'string',
'description': 'The user\'s password',
@ -148,6 +144,7 @@ class User(ApiResource):
return user_view(user)
@require_user_admin
@require_fresh_login
@nickname('changeUserDetails')
@internal_only
@validate_json_request('UpdateUser')
@ -156,22 +153,8 @@ class User(ApiResource):
user = get_authenticated_user()
user_data = request.get_json()
def verify_current_password(user, user_data):
current_password = user_data.get('current_password', '')
verified = False
try:
verified = model.verify_user(user.username, current_password)
except:
pass
if not verified:
raise request_error(message='Current password does not match')
try:
if 'password' in user_data:
verify_current_password(user, user_data)
logger.debug('Changing password for user: %s', user.username)
log_action('account_change_password', user.username)
model.change_password(user, user_data['password'])
@ -181,8 +164,6 @@ class User(ApiResource):
model.change_invoice_email(user, user_data['invoice_email'])
if 'email' in user_data and user_data['email'] != user.email:
verify_current_password(user, user_data)
new_email = user_data['email']
if model.find_user_by_email(new_email):
# Email already used.
@ -377,6 +358,37 @@ class Signin(ApiResource):
return conduct_signin(username, password)
@resource('/v1/signin/verify')
@internal_only
class VerifyUser(ApiResource):
""" Operations for verifying the existing user. """
schemas = {
'VerifyUser': {
'id': 'VerifyUser',
'type': 'object',
'description': 'Information required to verify the signed in user.',
'required': [
'password',
],
'properties': {
'password': {
'type': 'string',
'description': 'The user\'s password',
},
},
},
}
@require_user_admin
@nickname('verifyUser')
@validate_json_request('VerifyUser')
def post(self):
""" Verifies the signed in the user with the specified credentials. """
signin_data = request.get_json()
password = signin_data['password']
return conduct_signin(get_authenticated_user().username, password)
@resource('/v1/signout')
@internal_only
class Signout(ApiResource):

View file

@ -2,8 +2,9 @@ import logging
import urlparse
import json
import string
import datetime
from flask import make_response, render_template, request, abort
from flask import make_response, render_template, request, abort, session
from flask.ext.login import login_user, UserMixin
from flask.ext.principal import identity_changed
from random import SystemRandom
@ -112,6 +113,7 @@ def common_login(db_user):
logger.debug('Successfully signed in as: %s' % db_user.username)
new_identity = QuayDeferredPermissionUser(db_user.username, 'username', {scopes.DIRECT_LOGIN})
identity_changed.send(app, identity=new_identity)
session['login_time'] = datetime.datetime.now()
return True
else:
logger.debug('User could not be logged in, inactive?.')

View file

@ -713,7 +713,7 @@ quayApp = angular.module('quay', quayDependencies, function($provide, cfpLoading
return config;
}]);
$provide.factory('ApiService', ['Restangular', function(Restangular) {
$provide.factory('ApiService', ['Restangular', '$q', function(Restangular, $q) {
var apiService = {};
var getResource = function(path, opt_background) {
@ -810,6 +810,65 @@ quayApp = angular.module('quay', quayDependencies, function($provide, cfpLoading
}
};
var freshLoginFailCheck = function(opName, opArgs) {
return function(resp) {
var deferred = $q.defer();
// If the error is a fresh login required, show the dialog.
if (resp.status == 401 && resp.data['error_type'] == 'fresh_login_required') {
bootbox.dialog({
"message": 'It has been more than a few minutes since you last logged in, ' +
'so please verify your password to perform this sensitive operation:' +
'<form style="margin-top: 10px" action="javascript:void(0)">' +
'<input id="freshPassword" class="form-control" type="password" placeholder="Current Password">' +
'</form>',
"title": 'Please Verify',
"buttons": {
"verify": {
"label": "Verify",
"className": "btn-success",
"callback": function() {
var info = {
'password': $('#freshPassword').val()
};
$('#freshPassword').val('');
// Conduct the sign in of the user.
apiService.verifyUser(info).then(function() {
// On success, retry the operation. if it succeeds, then resolve the
// deferred promise with the result. Otherwise, reject the same.
apiService[opName].apply(apiService, opArgs).then(function(resp) {
deferred.resolve(resp);
}, function(resp) {
deferred.reject(resp);
});
}, function(resp) {
// Reject with the sign in error.
deferred.reject({'data': {'message': 'Invalid verification credentials'}});
});
}
},
"close": {
"label": "Cancel",
"className": "btn-default",
"callback": function() {
deferred.reject(resp);
}
}
}
});
// Return a new promise. We'll accept or reject it based on the result
// of the login.
return deferred.promise;
}
// Otherwise, we just 'raise' the error via the reject method on the promise.
return $q.reject(resp);
};
};
var buildMethodsForOperation = function(operation, resource, resourceMap) {
var method = operation['method'].toLowerCase();
var operationName = operation['nickname'];
@ -823,7 +882,15 @@ quayApp = angular.module('quay', quayDependencies, function($provide, cfpLoading
'ignoreLoadingBar': true
});
}
return one['custom' + method.toUpperCase()](opt_options);
var opObj = one['custom' + method.toUpperCase()](opt_options);
// If the operation requires_fresh_login, then add a specialized error handler that
// will defer the operation's result if sudo is requested.
if (operation['requires_fresh_login']) {
opObj = opObj.catch(freshLoginFailCheck(operationName, arguments));
}
return opObj;
};
// If the method for the operation is a GET, add an operationAsResource method.
@ -3923,9 +3990,11 @@ quayApp.directive('billingOptions', function () {
var save = function() {
$scope.working = true;
var errorHandler = ApiService.errorDisplay('Could not change user details');
ApiService.changeDetails($scope.organization, $scope.obj).then(function(resp) {
$scope.working = false;
});
}, errorHandler);
};
var checkSave = function() {
@ -5699,11 +5768,10 @@ quayApp.run(['$location', '$rootScope', 'Restangular', 'UserService', 'PlanServi
// Handle session expiration.
Restangular.setErrorInterceptor(function(response) {
if (response.status == 401) {
if (response.data['session_required'] == null || response.data['session_required'] === true) {
$('#sessionexpiredModal').modal({});
return false;
}
if (response.status == 401 && response.data['error_type'] == 'invalid_token' &&
response.data['session_required'] !== false) {
$('#sessionexpiredModal').modal({});
return false;
}
if (response.status == 503) {

View file

@ -1763,12 +1763,11 @@ function UserAdminCtrl($scope, $timeout, $location, ApiService, PlanService, Use
// Reset the form.
delete $scope.cuser['repeatEmail'];
delete $scope.cuser['current_password'];
$scope.changeEmailForm.$setPristine();
}, function(result) {
$scope.updatingUser = false;
UIService.showFormError('#changeEmailForm', result);
UIService.showFormError('#changeEmailForm', result);
});
};
@ -1778,14 +1777,14 @@ function UserAdminCtrl($scope, $timeout, $location, ApiService, PlanService, Use
$scope.updatingUser = true;
$scope.changePasswordSuccess = false;
ApiService.changeUserDetails($scope.cuser).then(function() {
ApiService.changeUserDetails($scope.cuser).then(function(resp) {
$scope.updatingUser = false;
$scope.changePasswordSuccess = true;
// Reset the form
delete $scope.cuser['password']
delete $scope.cuser['repeatPassword']
delete $scope.cuser['current_password'];
$scope.changePasswordForm.$setPristine();

View file

@ -128,8 +128,6 @@
<div class="panel-body">
<form class="form-change col-md-6" id="changeEmailForm" name="changeEmailForm" ng-submit="changeEmail()"
ng-show="!awaitingConfirmation && !registering">
<input type="password" class="form-control" placeholder="Your current password" ng-model="cuser.current_password" required
ng-pattern="/^.{8,}$/">
<input type="email" class="form-control" placeholder="Your new e-mail address" ng-model="cuser.email" required>
<button class="btn btn-primary" ng-disabled="changeEmailForm.$invalid || cuser.email == user.email" type="submit">Change E-mail Address</button>
</form>
@ -153,8 +151,6 @@
<div ng-show="!updatingUser" class="panel-body">
<form class="form-change col-md-6" id="changePasswordForm" name="changePasswordForm" ng-submit="changePassword()"
ng-show="!awaitingConfirmation && !registering">
<input type="password" class="form-control" placeholder="Your current password" ng-model="cuser.current_password" required
ng-pattern="/^.{8,}$/">
<input type="password" class="form-control" placeholder="Your new password" ng-model="cuser.password" required
ng-pattern="/^.{8,}$/">
<input type="password" class="form-control" placeholder="Verify your new password" ng-model="cuser.repeatPassword"

View file

@ -23,7 +23,8 @@ from endpoints.api.trigger import (BuildTriggerActivate, BuildTriggerSources, Bu
from endpoints.api.repoemail import RepositoryAuthorizedEmail
from endpoints.api.repositorynotification import RepositoryNotification, RepositoryNotificationList
from endpoints.api.user import (PrivateRepositories, ConvertToOrganization, Recovery, Signout,
Signin, User, UserAuthorizationList, UserAuthorization, UserNotification)
Signin, User, UserAuthorizationList, UserAuthorization, UserNotification,
VerifyUser)
from endpoints.api.repotoken import RepositoryToken, RepositoryTokenList
from endpoints.api.prototype import PermissionPrototype, PermissionPrototypeList
from endpoints.api.logs import UserLogs, OrgLogs, RepositoryLogs
@ -434,6 +435,24 @@ class TestSignin(ApiTestCase):
self._run_test('POST', 403, 'devtable', {u'username': 'E9RY', u'password': 'LQ0N'})
class TestVerifyUser(ApiTestCase):
def setUp(self):
ApiTestCase.setUp(self)
self._set_url(VerifyUser)
def test_post_anonymous(self):
self._run_test('POST', 401, None, {u'password': 'LQ0N'})
def test_post_freshuser(self):
self._run_test('POST', 403, 'freshuser', {u'password': 'LQ0N'})
def test_post_reader(self):
self._run_test('POST', 403, 'reader', {u'password': 'LQ0N'})
def test_post_devtable(self):
self._run_test('POST', 200, 'devtable', {u'password': 'password'})
class TestListPlans(ApiTestCase):
def setUp(self):
ApiTestCase.setUp(self)
@ -473,13 +492,13 @@ class TestUser(ApiTestCase):
self._run_test('PUT', 401, None, {})
def test_put_freshuser(self):
self._run_test('PUT', 200, 'freshuser', {})
self._run_test('PUT', 401, 'freshuser', {})
def test_put_reader(self):
self._run_test('PUT', 200, 'reader', {})
self._run_test('PUT', 401, 'reader', {})
def test_put_devtable(self):
self._run_test('PUT', 200, 'devtable', {})
self._run_test('PUT', 401, 'devtable', {})
def test_post_anonymous(self):
self._run_test('POST', 400, None, {u'username': 'T946', u'password': '0SG4', u'email': 'MENT'})

View file

@ -172,14 +172,14 @@ class TestCSRFFailure(ApiTestCase):
# Make sure a simple post call succeeds.
self.putJsonResponse(User,
data=dict(password='newpasswordiscool', current_password='password'))
data=dict(password='newpasswordiscool'))
# Change the session's CSRF token.
self.setCsrfToken('someinvalidtoken')
# Verify that the call now fails.
self.putJsonResponse(User,
data=dict(password='newpasswordiscool', current_password='password'),
data=dict(password='newpasswordiscool'),
expected_code=403)
@ -325,29 +325,15 @@ class TestChangeUserDetails(ApiTestCase):
def test_changepassword(self):
self.login(READ_ACCESS_USER)
self.putJsonResponse(User,
data=dict(password='newpasswordiscool', current_password='password'))
data=dict(password='newpasswordiscool'))
self.login(READ_ACCESS_USER, password='newpasswordiscool')
def test_changepassword_invalidpasswor(self):
self.login(READ_ACCESS_USER)
self.putJsonResponse(User,
data=dict(password='newpasswordiscool', current_password='notcorrect'),
expected_code=400)
def test_changeeemail(self):
self.login(READ_ACCESS_USER)
self.putJsonResponse(User,
data=dict(email='test+foo@devtable.com', current_password='password'))
data=dict(email='test+foo@devtable.com'))
def test_changeeemail_invalidpassword(self):
self.login(READ_ACCESS_USER)
self.putJsonResponse(User,
data=dict(email='test+foo@devtable.com', current_password='notcorrect'),
expected_code=400)
def test_changeinvoiceemail(self):
self.login(READ_ACCESS_USER)