From 294dcb4efffabc30fb85f50f2e01af2904b476a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Sat, 6 Jun 2015 22:43:41 +0200 Subject: [PATCH 1/4] Adding global error handler for ajax calls which run into redirections or unauthorized responses --- core/js/js.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/core/js/js.js b/core/js/js.js index 83658a537b8f..b646ff5c6f90 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -1311,6 +1311,13 @@ function initCore() { $('html').addClass('edge'); } + $( document ).ajaxError(function( event, request, settings ) { + if (_.contains([302, 307, 401], request.status)) { + var app = $('#content').attr('class').substring(4); + OC.redirect(OC.generateUrl('apps/' + app)); + } + }); + /** * Calls the server periodically to ensure that session doesn't * time out From b8b77709c05df3d820af2bfc83ff9d386bc19990 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 10 Dec 2015 18:08:40 +0100 Subject: [PATCH 2/4] Add handler for global ajax errors --- apps/files/js/filelist.js | 7 ---- apps/files/tests/js/filelistSpec.js | 7 ---- core/js/files/client.js | 2 + core/js/js.js | 65 +++++++++++++++++++++++++++-- core/js/setupchecks.js | 9 ++-- core/js/tests/specHelper.js | 7 +++- core/js/tests/specs/coreSpec.js | 42 +++++++++++++++++++ 7 files changed, 117 insertions(+), 22 deletions(-) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 1a6f38d3d7c0..3338d2b58278 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -1427,13 +1427,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 diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 0091a9ee6e46..122e618de960 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -2441,13 +2441,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); diff --git a/core/js/files/client.js b/core/js/files/client.js index 55a8e2c485a5..6797ba84e7f6 100644 --- a/core/js/files/client.js +++ b/core/js/files/client.js @@ -137,6 +137,8 @@ }); return result; }; + + OC.registerXHRForErrorProcessing(xhr); return xhr; }, diff --git a/core/js/js.js b/core/js/js.js index b646ff5c6f90..fac9c45f6687 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -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 @@ -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); + } + + } }; /** @@ -1311,11 +1368,11 @@ function initCore() { $('html').addClass('edge'); } - $( document ).ajaxError(function( event, request, settings ) { - if (_.contains([302, 307, 401], request.status)) { - var app = $('#content').attr('class').substring(4); - OC.redirect(OC.generateUrl('apps/' + app)); + $(document).on('ajaxError.main', function( event, request, settings ) { + if (settings && settings.allowAuthErrors) { + return; } + OC._processAjaxError(request); }); /** diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index de41b66ec329..1819b5a9c1ec 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -40,7 +40,8 @@ '' + '' + '', - complete: afterCall + complete: afterCall, + allowAuthErrors: true }); return deferred.promise(); }, @@ -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(); }, @@ -181,7 +183,8 @@ $.ajax({ type: 'GET', - url: OC.generateUrl('heartbeat') + url: OC.generateUrl('heartbeat'), + allowAuthErrors: true }).then(afterCall, afterCall); return deferred.promise(); diff --git a/core/js/tests/specHelper.js b/core/js/tests/specHelper.js index d13691845a7d..f9bdeae0d64b 100644 --- a/core/js/tests/specHelper.js +++ b/core/js/tests/specHelper.js @@ -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 @@ -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() { @@ -172,6 +175,8 @@ window.isPhantom = /phantom/i.test(navigator.userAgent); $testArea.remove(); delete($.fn.select2); + + ajaxErrorStub.restore(); }); })(); diff --git a/core/js/tests/specs/coreSpec.js b/core/js/tests/specs/coreSpec.js index 2e970f7e7071..32eb8df32d12 100644 --- a/core/js/tests/specs/coreSpec.js +++ b/core/js/tests/specs/coreSpec.js @@ -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 */ @@ -473,6 +474,7 @@ describe('Core base tests', function() { }); afterEach(function() { clock.restore(); + $(document).off('ajaxError'); }); it('Sets up menu toggle', function() { window.initCore(); @@ -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); + } + } + }); + }) }); From dfc3536d2b95fea1b54b3e85651e3a66c2d0088e Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 15 Feb 2016 15:38:37 +0100 Subject: [PATCH 3/4] Catch auth coming from JS in OCS --- lib/private/api.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/private/api.php b/lib/private/api.php index 452612d4c16e..6c6be233c9df 100644 --- a/lib/private/api.php +++ b/lib/private/api.php @@ -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'); } @@ -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; From b99c6f1f67a207984b8b5355703cabd89d1e7c73 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 16 Feb 2016 09:48:40 +0100 Subject: [PATCH 4/4] Send 401 header for OC_JSON::checkLoggedIn() --- lib/private/json.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/private/json.php b/lib/private/json.php index adee28a1593c..74aebd476fb2 100644 --- a/lib/private/json.php +++ b/lib/private/json.php @@ -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(); }