From 83ff9efbd8ac0b260f307ca2a518063026c8890c Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Tue, 14 Sep 2021 13:35:26 +0200 Subject: [PATCH 1/3] MOBILE-3859 login: Fix missing await when check if URL allowed --- src/core/features/login/pages/site/site.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/features/login/pages/site/site.ts b/src/core/features/login/pages/site/site.ts index 6cc44b83f..b607ac068 100644 --- a/src/core/features/login/pages/site/site.ts +++ b/src/core/features/login/pages/site/site.ts @@ -498,7 +498,12 @@ export class CoreLoginSitePage implements OnInit { if (scheme && scheme != 'http' && scheme != 'https') { CoreDomUtils.showErrorModal(Translate.instant('core.errorurlschemeinvalidscheme', { $a: text })); - } else if (CoreLoginHelper.isSiteUrlAllowed(text)) { + + return; + } + + const allowed = await CoreLoginHelper.isSiteUrlAllowed(text); + if (allowed) { // Put the text in the field (if present). this.siteForm.controls.siteUrl.setValue(text); From 534bbbb289d45adea66363d0241304bbb3a90a8f Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Tue, 14 Sep 2021 15:18:49 +0200 Subject: [PATCH 2/3] MOBILE-3859 ws: Improve error message for certificate errors --- scripts/langindex.json | 2 ++ src/core/classes/errors/ajaxerror.ts | 6 ++-- src/core/classes/site.ts | 44 ++++++++++++++-------------- src/core/lang.json | 2 ++ src/core/services/sites.ts | 33 ++++++++++----------- src/core/services/ws.ts | 39 ++++++++++++++++++++++-- 6 files changed, 80 insertions(+), 46 deletions(-) diff --git a/scripts/langindex.json b/scripts/langindex.json index 3c46978e7..7c5324cd9 100644 --- a/scripts/langindex.json +++ b/scripts/langindex.json @@ -1384,6 +1384,7 @@ "core.add": "moodle", "core.agelocationverification": "moodle", "core.ago": "message", + "core.ajaxendpointnotfound": "local_moodlemobileapp", "core.all": "moodle", "core.allgroups": "moodle", "core.allparticipants": "moodle", @@ -1405,6 +1406,7 @@ "core.captureimage": "local_moodlemobileapp", "core.capturevideo": "local_moodlemobileapp", "core.category": "moodle", + "core.certificaterror": "local_moodlemobileapp", "core.choose": "moodle", "core.choosedots": "moodle", "core.clearsearch": "local_moodlemobileapp", diff --git a/src/core/classes/errors/ajaxerror.ts b/src/core/classes/errors/ajaxerror.ts index 7cf6b1363..f7a57cc9d 100644 --- a/src/core/classes/errors/ajaxerror.ts +++ b/src/core/classes/errors/ajaxerror.ts @@ -19,12 +19,10 @@ import { CoreError } from '@classes/errors/error'; */ export class CoreAjaxError extends CoreError { - available?: number; // Whether the AJAX call is available. 0 if unknown, 1 if available, -1 if not available. + available = 1; // @deprecated since app 4.0. AJAX endpoint should always be available in supported Moodle versions. - constructor(message: string, available?: number) { + constructor(message: string) { super(message); - - this.available = typeof available == 'undefined' ? 0 : available; } } diff --git a/src/core/classes/site.ts b/src/core/classes/site.ts index c3593943a..4050c88f9 100644 --- a/src/core/classes/site.ts +++ b/src/core/classes/site.ts @@ -1333,38 +1333,38 @@ export class CoreSite { siteUrl: this.siteUrl, }; - let config: CoreSitePublicConfigResponse | undefined; + let config: CoreSitePublicConfigResponse; try { - config = await CoreWS.callAjax('tool_mobile_get_public_config', {}, preSets); + config = await CoreWS.callAjax('tool_mobile_get_public_config', {}, preSets); } catch (error) { - if ((!this.getInfo() || this.isVersionGreaterEqualThan('3.8')) && error && error.errorcode == 'codingerror') { - // This error probably means that there is a redirect in the site. Try to use a GET request. - preSets.noLogin = true; - preSets.useGet = true; - - try { - config = await CoreWS.callAjax('tool_mobile_get_public_config', {}, preSets); - } catch (error2) { - if (this.getInfo() && this.isVersionGreaterEqualThan('3.8')) { - // GET is supported, return the second error. - throw error2; - } else { - // GET not supported or we don't know if it's supported. Return first error. - throw error; - } - } + if (!error || error.errorcode !== 'codingerror' || (this.getInfo() && !this.isVersionGreaterEqualThan('3.8'))) { + throw error; } - throw error; + // This error probably means that there is a redirect in the site. Try to use a GET request. + preSets.noLogin = true; + preSets.useGet = true; + + try { + config = await CoreWS.callAjax('tool_mobile_get_public_config', {}, preSets); + } catch (error2) { + if (this.getInfo() && this.isVersionGreaterEqualThan('3.8')) { + // GET is supported, return the second error. + throw error2; + } else { + // GET not supported or we don't know if it's supported. Return first error. + throw error; + } + } } // Use the wwwroot returned by the server. - if (config!.httpswwwroot) { - this.siteUrl = CoreUrlUtils.removeUrlParams(config!.httpswwwroot); // Make sure the URL doesn't have params. + if (config.httpswwwroot) { + this.siteUrl = CoreUrlUtils.removeUrlParams(config.httpswwwroot); // Make sure the URL doesn't have params. } - return config!; + return config; } /** diff --git a/src/core/lang.json b/src/core/lang.json index 05b6db48f..1da713f7f 100644 --- a/src/core/lang.json +++ b/src/core/lang.json @@ -3,6 +3,7 @@ "add": "Add", "agelocationverification": "Age and location verification", "ago": "{{$a}} ago", + "ajaxendpointnotfound": "

AJAX endpoint not found. This can happen if the Moodle site is too old or it blocks access to this endpoint. The Moodle app only supports Moodle systems {{$a}} onwards. Please contact your site administrator.

\n

{{whoisadmin}}

", "all": "All", "allgroups": "All groups", "allparticipants": "All participants", @@ -23,6 +24,7 @@ "captureimage": "Take picture", "capturevideo": "Record video", "category": "Category", + "certificaterror": "

The certificate of this site cannot be trusted by your device. Please contact your site administrator.

\n

{{whoisadmin}}

", "choose": "Choose", "choosedots": "Choose...", "clearsearch": "Clear search", diff --git a/src/core/services/sites.ts b/src/core/services/sites.ts index 38a35cd28..5393fd864 100644 --- a/src/core/services/sites.ts +++ b/src/core/services/sites.ts @@ -245,27 +245,24 @@ export class CoreSitesProvider { }); } } catch (error) { - // Error, check if not supported. - if (error.available === 1) { - // Service supported but an error happened. Return error. - if (error.errorcode == 'codingerror') { - // This could be caused by a redirect. Check if it's the case. - const redirect = await CoreUtils.checkRedirect(siteUrl); + // Service supported but an error happened. Return error. + if (error.errorcode == 'codingerror') { + // This could be caused by a redirect. Check if it's the case. + const redirect = await CoreUtils.checkRedirect(siteUrl); - if (redirect) { - error.error = Translate.instant('core.login.sitehasredirect'); - } else { - // We can't be sure if there is a redirect or not. Display cannot connect error. - error.error = Translate.instant('core.cannotconnecttrouble'); - } + if (redirect) { + error.message = Translate.instant('core.login.sitehasredirect'); + } else { + // We can't be sure if there is a redirect or not. Display cannot connect error. + error.message = Translate.instant('core.cannotconnecttrouble'); } - - throw new CoreSiteError({ - message: error.error, - errorcode: error.errorcode, - critical: true, - }); } + + throw new CoreSiteError({ + message: error.message, + errorcode: error.errorcode, + critical: true, + }); } siteUrl = temporarySite.getURL(); diff --git a/src/core/services/ws.ts b/src/core/services/ws.ts index 92067a6e9..549e26915 100644 --- a/src/core/services/ws.ts +++ b/src/core/services/ws.ts @@ -37,6 +37,7 @@ import { CoreWSError } from '@classes/errors/wserror'; import { CoreAjaxError } from '@classes/errors/ajaxerror'; import { CoreAjaxWSError } from '@classes/errors/ajaxwserror'; import { CoreNetworkError } from '@classes/errors/network-error'; +import { CoreSite } from '@classes/site'; /** * This service allows performing WS calls and download/upload files. @@ -474,9 +475,23 @@ export class CoreWSProvider { return data.data; }, (data) => { - const available = data.status == 404 ? -1 : 0; + let message = ''; - throw new CoreAjaxError(Translate.instant('core.serverconnection'), available); + switch (data.status) { + case -2: // Certificate error. + message = this.getCertificateErrorMessage(data.error); + break; + case 404: // AJAX endpoint not found. + message = Translate.instant('core.ajaxendpointnotfound', { + $a: CoreSite.MINIMUM_MOODLE_VERSION, + whoisadmin: Translate.instant('core.whoissiteadmin'), + }); + break; + default: + message = Translate.instant('core.serverconnection'); + } + + throw new CoreAjaxError(message); }); } @@ -675,12 +690,32 @@ export class CoreWSProvider { } return retryPromise; + } else if (error.status === -2) { + throw new CoreError(this.getCertificateErrorMessage(error.error)); } throw new CoreError(Translate.instant('core.serverconnection')); }); } + /** + * Get error message about certificate error. + * + * @param error Exact error message. + * @return Certificate error message. + */ + protected getCertificateErrorMessage(error?: string): string { + const message = Translate.instant('core.certificaterror', { + whoisadmin: Translate.instant('core.whoissiteadmin'), + }); + + if (error) { + return `${message}\n

${error}

`; + } + + return message; + } + /** * Retry all requests in the queue. * This function uses recursion in order to add a delay between requests to reduce stress. From 3109c93b64cfcf607af9b3f1eeebf4fcd8b764a6 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Wed, 15 Sep 2021 14:50:59 +0200 Subject: [PATCH 3/3] MOBILE-3860 login: Deprecate login/token.php checks --- src/core/classes/errors/ajaxerror.ts | 5 +- src/core/services/sites.ts | 130 +++++++++++++++------------ src/core/services/ws.ts | 2 +- 3 files changed, 76 insertions(+), 61 deletions(-) diff --git a/src/core/classes/errors/ajaxerror.ts b/src/core/classes/errors/ajaxerror.ts index f7a57cc9d..70f5eeee1 100644 --- a/src/core/classes/errors/ajaxerror.ts +++ b/src/core/classes/errors/ajaxerror.ts @@ -20,9 +20,12 @@ import { CoreError } from '@classes/errors/error'; export class CoreAjaxError extends CoreError { available = 1; // @deprecated since app 4.0. AJAX endpoint should always be available in supported Moodle versions. + status?: number; - constructor(message: string) { + constructor(message: string, available?: number, status?: number) { super(message); + + this.status = status; } } diff --git a/src/core/services/sites.ts b/src/core/services/sites.ts index 5393fd864..fb59d23cf 100644 --- a/src/core/services/sites.ts +++ b/src/core/services/sites.ts @@ -54,6 +54,8 @@ import { CoreSitesFactory } from './sites-factory'; import { CoreText } from '@singletons/text'; import { CoreLoginHelper } from '@features/login/services/login-helper'; import { CoreErrorWithTitle } from '@classes/errors/errorwithtitle'; +import { CoreAjaxError } from '@classes/errors/ajaxerror'; +import { CoreAjaxWSError } from '@classes/errors/ajaxwserror'; export const CORE_SITE_SCHEMAS = new InjectionToken('CORE_SITE_SCHEMAS'); @@ -183,33 +185,30 @@ export class CoreSitesProvider { // Now, replace the siteUrl with the protocol. siteUrl = siteUrl.replace(/^https?:\/\//i, protocol); - try { - await this.siteExists(siteUrl); - } catch (error) { - // Do not continue checking if WS are not enabled. - if (error.errorcode == 'enablewsdescription') { - error.critical = true; + // Create a temporary site to fetch site info. + let temporarySite = CoreSitesFactory.makeSite(undefined, siteUrl); + let config: CoreSitePublicConfigResponse | undefined; - throw error; + try { + config = await temporarySite.getPublicConfig(); + } catch (error) { + const treatedError = await this.treatGetPublicConfigError(temporarySite.getURL(), error); + if (treatedError.critical) { + throw treatedError; // App received a WS error, stop. } - // Site doesn't exist. Try to add or remove 'www'. - const treatedUrl = CoreUrlUtils.addOrRemoveWWW(siteUrl); + // Try to add or remove 'www'. + temporarySite = CoreSitesFactory.makeSite(undefined, CoreUrlUtils.addOrRemoveWWW(siteUrl)); try { - await this.siteExists(treatedUrl); - - // Success, use this new URL as site url. - siteUrl = treatedUrl; + config = await temporarySite.getPublicConfig(); } catch (secondError) { - // Do not continue checking if WS are not enabled. - if (secondError.errorcode == 'enablewsdescription') { - secondError.critical = true; - - throw secondError; + const treatedSecondError = await this.treatGetPublicConfigError(temporarySite.getURL(), secondError); + if (treatedSecondError.critical) { + throw treatedSecondError; // App received a WS error, stop. } - // Return the error. + // App didn't receive a WS response, probably cannot connect. Prioritize first error if it's valid. if (CoreTextUtils.getErrorMessageFromError(error)) { throw error; } else { @@ -218,49 +217,25 @@ export class CoreSitesProvider { } } - // Site exists. Create a temporary site to fetch its info. - const temporarySite = CoreSitesFactory.makeSite(undefined, siteUrl); - let config: CoreSitePublicConfigResponse | undefined; - - try { - config = await temporarySite.getPublicConfig(); - - // Check that the user can authenticate. - if (!config.enablewebservices) { - throw new CoreSiteError({ - message: Translate.instant('core.login.webservicesnotenabled'), - }); - } else if (!config.enablemobilewebservice) { - throw new CoreSiteError({ - message: Translate.instant('core.login.mobileservicesnotenabled'), - }); - } else if (config.maintenanceenabled) { - let message = Translate.instant('core.sitemaintenance'); - if (config.maintenancemessage) { - message += config.maintenancemessage; - } - - throw new CoreSiteError({ - message, - }); - } - } catch (error) { - // Service supported but an error happened. Return error. - if (error.errorcode == 'codingerror') { - // This could be caused by a redirect. Check if it's the case. - const redirect = await CoreUtils.checkRedirect(siteUrl); - - if (redirect) { - error.message = Translate.instant('core.login.sitehasredirect'); - } else { - // We can't be sure if there is a redirect or not. Display cannot connect error. - error.message = Translate.instant('core.cannotconnecttrouble'); - } + // Check that the user can authenticate. + if (!config.enablewebservices) { + throw new CoreSiteError({ + message: Translate.instant('core.login.webservicesnotenabled'), + critical: true, + }); + } else if (!config.enablemobilewebservice) { + throw new CoreSiteError({ + message: Translate.instant('core.login.mobileservicesnotenabled'), + critical: true, + }); + } else if (config.maintenanceenabled) { + let message = Translate.instant('core.sitemaintenance'); + if (config.maintenancemessage) { + message += config.maintenancemessage; } throw new CoreSiteError({ - message: error.message, - errorcode: error.errorcode, + message, critical: true, }); } @@ -270,11 +245,48 @@ export class CoreSitesProvider { return { siteUrl, code: config?.typeoflogin || 0, service: CoreConstants.CONFIG.wsservice, config }; } + /** + * Treat an error returned by getPublicConfig in checkSiteWithProtocol. Converts the error to a CoreSiteError. + * + * @param siteUrl Site URL. + * @param error Error returned. + * @return Promise resolved with the treated error. + */ + protected async treatGetPublicConfigError(siteUrl: string, error: CoreAjaxError | CoreAjaxWSError): Promise { + if (!('errorcode' in error)) { + // The WS didn't return data, probably cannot connect. + return new CoreSiteError({ + message: error.message || '', + critical: 'status' in error && error.status === -2, // Certificate error. + }); + } + + // Service supported but an error happened. Return error. + if (error.errorcode == 'codingerror') { + // This could be caused by a redirect. Check if it's the case. + const redirect = await CoreUtils.checkRedirect(siteUrl); + + if (redirect) { + error.message = Translate.instant('core.login.sitehasredirect'); + } else { + // We can't be sure if there is a redirect or not. Display cannot connect error. + error.message = Translate.instant('core.cannotconnecttrouble'); + } + } + + return new CoreSiteError({ + message: error.message, + errorcode: error.errorcode, + critical: true, + }); + } + /** * Check if a site exists. * * @param siteUrl URL of the site to check. * @return A promise to be resolved if the site exists. + * @deprecated since app 4.0. Now the app calls uses tool_mobile_get_public_config to check if site exists. */ async siteExists(siteUrl: string): Promise { let data: CoreSitesLoginTokenResponse; diff --git a/src/core/services/ws.ts b/src/core/services/ws.ts index 549e26915..bb2e63719 100644 --- a/src/core/services/ws.ts +++ b/src/core/services/ws.ts @@ -491,7 +491,7 @@ export class CoreWSProvider { message = Translate.instant('core.serverconnection'); } - throw new CoreAjaxError(message); + throw new CoreAjaxError(message, 1, data.status); }); }