Skip to content

Commit

Permalink
Merge pull request #16783 from owncloud/handle-redirects-global
Browse files Browse the repository at this point in the history
Adding global error handler for ajax calls which run into redirection…
  • Loading branch information
DeepDiver1975 committed Feb 17, 2016
2 parents add696b + b99c6f1 commit 7af7d18
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 20 deletions.
7 changes: 0 additions & 7 deletions apps/files/js/filelist.js
Original file line number Diff line number Diff line change
Expand Up @@ -1433,13 +1433,6 @@
delete this._reloadCall;
this.hideMask();

if (status === 401) {
// TODO: append current URL to be able to get back after logging in again
OC.redirect(OC.generateUrl('apps/files'));
OC.Notification.show(result);
return false;
}

// Firewall Blocked request?
if (status === 403) {
// Go home
Expand Down
7 changes: 0 additions & 7 deletions apps/files/tests/js/filelistSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2462,13 +2462,6 @@ describe('OCA.Files.FileList tests', function() {
getFolderContentsStub.restore();
fileList = undefined;
});
it('redirects to files app in case of auth error', function () {
deferredList.reject(401, 'Authentication error');

expect(redirectStub.calledOnce).toEqual(true);
expect(redirectStub.getCall(0).args[0]).toEqual(OC.webroot + '/index.php/apps/files');
expect(getFolderContentsStub.calledOnce).toEqual(true);
});
it('redirects to root folder in case of forbidden access', function () {
deferredList.reject(403);

Expand Down
2 changes: 2 additions & 0 deletions core/js/files/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@
});
return result;
};

OC.registerXHRForErrorProcessing(xhr);
return xhr;
},

Expand Down
64 changes: 64 additions & 0 deletions core/js/js.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,13 @@ var OC={
window.location = targetURL;
},

/**
* Reloads the current page
*/
reload: function() {
window.location.reload();
},

/**
* Protocol that is used to access this ownCloud instance
* @return {string} Used protocol
Expand Down Expand Up @@ -727,6 +734,56 @@ var OC={
isUserAdmin: function() {
return oc_isadmin;
},

/**
* Process ajax error, redirects to main page
* if an error/auth error status was returned.
*/
_processAjaxError: function(xhr) {
// purposefully aborted request ?
if (xhr.status === 0 && (xhr.statusText === 'abort' || xhr.statusText === 'timeout')) {
return;
}

if (_.contains([0, 302, 307, 401], xhr.status)) {
OC.reload();
}
},

/**
* Registers XmlHttpRequest object for global error processing.
*
* This means that if this XHR object returns 401 or session timeout errors,
* the current page will automatically be reloaded.
*
* @param {XMLHttpRequest} xhr
*/
registerXHRForErrorProcessing: function(xhr) {
var loadCallback = function() {
if (xhr.readyState !== 4) {
return;
}

if (xhr.status >= 200 && xhr.status < 300 || xhr.status === 304) {
return;
}

// fire jquery global ajax error handler
$(document).trigger(new $.Event('ajaxError'), xhr);
};

var errorCallback = function() {
// fire jquery global ajax error handler
$(document).trigger(new $.Event('ajaxError'), xhr);
};

// FIXME: also needs an IE8 way
if (xhr.addEventListener) {
xhr.addEventListener('load', loadCallback);
xhr.addEventListener('error', errorCallback);
}

}
};

/**
Expand Down Expand Up @@ -1311,6 +1368,13 @@ function initCore() {
$('html').addClass('edge');
}

$(document).on('ajaxError.main', function( event, request, settings ) {
if (settings && settings.allowAuthErrors) {
return;
}
OC._processAjaxError(request);
});

/**
* Calls the server periodically to ensure that session doesn't
* time out
Expand Down
9 changes: 6 additions & 3 deletions core/js/setupchecks.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
'<d:propfind xmlns:d="DAV:">' +
'<d:prop><d:resourcetype/></d:prop>' +
'</d:propfind>',
complete: afterCall
complete: afterCall,
allowAuthErrors: true
});
return deferred.promise();
},
Expand Down Expand Up @@ -157,7 +158,8 @@

$.ajax({
type: 'GET',
url: OC.generateUrl('settings/ajax/checksetup')
url: OC.generateUrl('settings/ajax/checksetup'),
allowAuthErrors: true
}).then(afterCall, afterCall);
return deferred.promise();
},
Expand All @@ -181,7 +183,8 @@

$.ajax({
type: 'GET',
url: OC.generateUrl('heartbeat')
url: OC.generateUrl('heartbeat'),
allowAuthErrors: true
}).then(afterCall, afterCall);

return deferred.promise();
Expand Down
7 changes: 6 additions & 1 deletion core/js/tests/specHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ window.isPhantom = /phantom/i.test(navigator.userAgent);
// global setup for all tests
(function setupTests() {
var fakeServer = null,
$testArea = null;
$testArea = null,
ajaxErrorStub = null;

/**
* Utility functions for testing
Expand Down Expand Up @@ -162,6 +163,8 @@ window.isPhantom = /phantom/i.test(navigator.userAgent);

// dummy select2 (which isn't loaded during the tests)
$.fn.select2 = function() { return this; };

ajaxErrorStub = sinon.stub(OC, '_processAjaxError');
});

afterEach(function() {
Expand All @@ -172,6 +175,8 @@ window.isPhantom = /phantom/i.test(navigator.userAgent);
$testArea.remove();

delete($.fn.select2);

ajaxErrorStub.restore();
});
})();

42 changes: 42 additions & 0 deletions core/js/tests/specs/coreSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ describe('Core base tests', function() {
/* jshint camelcase: false */
window.oc_config = oldConfig;
routeStub.restore();
$(document).off('ajaxError');
});
it('sends heartbeat half the session lifetime when heartbeat enabled', function() {
/* jshint camelcase: false */
Expand Down Expand Up @@ -473,6 +474,7 @@ describe('Core base tests', function() {
});
afterEach(function() {
clock.restore();
$(document).off('ajaxError');
});
it('Sets up menu toggle', function() {
window.initCore();
Expand Down Expand Up @@ -841,5 +843,45 @@ describe('Core base tests', function() {
// verification is done in afterEach
});
});
describe('global ajax errors', function() {
var reloadStub, ajaxErrorStub;

beforeEach(function() {
reloadStub = sinon.stub(OC, 'reload');
// unstub the error processing method
ajaxErrorStub = OC._processAjaxError;
ajaxErrorStub.restore();
window.initCore();
});
afterEach(function() {
reloadStub.restore();
$(document).off('ajaxError');
});

it('reloads current page in case of auth error', function () {
var dataProvider = [
[200, false],
[400, false],
[401, true],
[302, true],
[307, true]
];

for (var i = 0; i < dataProvider.length; i++) {
var xhr = { status: dataProvider[i][0] };
var expectedCall = dataProvider[i][1];

reloadStub.reset();

$(document).trigger(new $.Event('ajaxError'), xhr);

if (expectedCall) {
expect(reloadStub.calledOnce).toEqual(true);
} else {
expect(reloadStub.notCalled).toEqual(true);
}
}
});
})
});

11 changes: 9 additions & 2 deletions lib/private/api.php
Original file line number Diff line number Diff line change
Expand Up @@ -377,9 +377,16 @@ private static function loginUser() {
* @param string $format the format xml|json
*/
public static function respond($result, $format='xml') {
$request = \OC::$server->getRequest();

// Send 401 headers if unauthorised
if($result->getStatusCode() === API::RESPOND_UNAUTHORISED) {
header('WWW-Authenticate: Basic realm="Authorisation Required"');
// If request comes from JS return dummy auth request
if($request->getHeader('X-Requested-With') === 'XMLHttpRequest') {
header('WWW-Authenticate: DummyBasic realm="Authorisation Required"');
} else {
header('WWW-Authenticate: Basic realm="Authorisation Required"');
}
header('HTTP/1.0 401 Unauthorized');
}

Expand All @@ -389,7 +396,7 @@ public static function respond($result, $format='xml') {

$meta = $result->getMeta();
$data = $result->getData();
if (self::isV2(\OC::$server->getRequest())) {
if (self::isV2($request)) {
$statusCode = self::mapStatusCodes($result->getStatusCode());
if (!is_null($statusCode)) {
$meta['statuscode'] = $statusCode;
Expand Down
1 change: 1 addition & 0 deletions lib/private/json.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public static function checkAppEnabled($app) {
public static function checkLoggedIn() {
if( !OC_User::isLoggedIn()) {
$l = \OC::$server->getL10N('lib');
http_response_code(\OCP\AppFramework\Http::STATUS_UNAUTHORIZED);
self::error(array( 'data' => array( 'message' => $l->t('Authentication error'), 'error' => 'authentication_error' )));
exit();
}
Expand Down

0 comments on commit 7af7d18

Please sign in to comment.