Merge pull request #1566 from dpalou/MOBILE-2553

Mobile 2553

scripts/langindex.json

@@ -1349,6 +1349,7 @@
   "core.login.selectsite": "local_moodlemobileapp",
   "core.login.signupplugindisabled": "local_moodlemobileapp",
   "core.login.siteaddress": "local_moodlemobileapp",
+  "core.login.sitehasredirect": "local_moodlemobileapp",
   "core.login.siteinmaintenance": "local_moodlemobileapp",
   "core.login.sitepolicynotagreederror": "local_moodlemobileapp",
   "core.login.siteurl": "local_moodlemobileapp",

src/core/login/lang/en.json

@@ -71,6 +71,7 @@
     "selectsite": "Please select your site:",
     "signupplugindisabled": "{{$a}} is not enabled.",
     "siteaddress": "Site address",
+    "sitehasredirect": "Your site contains at least one HTTP redirect. The app cannot follow redirects, this could be the issue that's preventing the app from connecting to your site.",
     "siteinmaintenance": "Your site is in maintenance mode",
     "sitepolicynotagreederror": "Site policy not agreed.",
     "siteurl": "Site URL",

src/lang/en.json

@@ -9,7 +9,7 @@
     "areyousure": "Are you sure?",
     "back": "Back",
     "cancel": "Cancel",
-    "cannotconnect": "Cannot connect: Verify that you have correctly typed the URL and that your site uses Moodle 2.4 or later.",
+    "cannotconnect": "Cannot connect: verify that you have typed correctly the URL.",
     "cannotdownloadfiles": "File downloading is disabled. Please contact your site administrator.",
     "captureaudio": "Record audio",
     "capturedimage": "Taken picture.",

src/providers/sites.ts

@@ -21,6 +21,7 @@ import { CoreLoggerProvider } from './logger';
 import { CoreSitesFactoryProvider } from './sites-factory';
 import { CoreTextUtilsProvider } from './utils/text';
 import { CoreUrlUtilsProvider } from './utils/url';
+import { CoreUtilsProvider } from './utils/utils';
 import { CoreConstants } from '@core/constants';
 import { CoreConfigConstants } from '../configconstants';
 import { CoreSite } from '@classes/site';
@@ -211,7 +212,8 @@ export class CoreSitesProvider {
 
     constructor(logger: CoreLoggerProvider, private http: HttpClient, private sitesFactory: CoreSitesFactoryProvider,
             private appProvider: CoreAppProvider, private translate: TranslateService, private urlUtils: CoreUrlUtilsProvider,
-            private eventsProvider: CoreEventsProvider,  private textUtils: CoreTextUtilsProvider) {
+            private eventsProvider: CoreEventsProvider,  private textUtils: CoreTextUtilsProvider,
+            private utils: CoreUtilsProvider) {
         this.logger = logger.getInstance('CoreSitesProvider');
 
         this.appDB = appProvider.getDB();
@@ -258,14 +260,10 @@ export class CoreSitesProvider {
                 protocol = protocol == 'https://' ? 'http://' : 'https://';
 
                 return this.checkSiteWithProtocol(siteUrl, protocol).catch((secondError) => {
-                    // Site doesn't exist.
+                    // Site doesn't exist. Return the error message.
-                    if (secondError.error) {
+                    return Promise.reject(this.textUtils.getErrorMessageFromError(error) ||
-                        return Promise.reject(secondError.error);
+                            this.textUtils.getErrorMessageFromError(secondError) ||
-                    } else if (error.error) {
+                            this.translate.instant('core.cannotconnect'));
-                        return Promise.reject(error.error);
-                    }
-
-                    return Promise.reject(this.translate.instant('core.login.checksiteversion'));
                 });
             });
         }
@@ -286,7 +284,7 @@ export class CoreSitesProvider {
 
         return this.siteExists(siteUrl).catch((error) => {
             // Do not continue checking if WS are not enabled.
-            if (error.errorcode && error.errorcode == 'enablewsdescription') {
+            if (error.errorcode == 'enablewsdescription') {
                 return rejectWithCriticalError(error.error, error.errorcode);
             }
 
@@ -298,13 +296,13 @@ export class CoreSitesProvider {
                 siteUrl = treatedUrl;
             }).catch((secondError) => {
                 // Do not continue checking if WS are not enabled.
-                if (secondError.errorcode && secondError.errorcode == 'enablewsdescription') {
+                if (secondError.errorcode == 'enablewsdescription') {
                     return rejectWithCriticalError(secondError.error, secondError.errorcode);
                 }
 
-                error = secondError || error;
+                // Return the error message.
-
+                return Promise.reject(this.textUtils.getErrorMessageFromError(error) ||
-                return Promise.reject({ error: typeof error == 'object' ? error.error : error });
+                        this.textUtils.getErrorMessageFromError(secondError));
             });
         }).then(() => {
             // Create a temporary site to check if local_mobile is installed.
@@ -385,11 +383,11 @@ export class CoreSitesProvider {
             data.service = 'c';
         }
 
-        const promise = this.http.post(siteUrl + '/login/token.php', data).timeout(CoreConstants.WS_TIMEOUT).toPromise();
+        return this.http.post(siteUrl + '/login/token.php', data).timeout(CoreConstants.WS_TIMEOUT).toPromise().catch(() => {
-
+            // Default error messages are kinda bad, return our own message.
-        return promise.catch((error) => {
+            return Promise.reject({error: this.translate.instant('core.cannotconnect')});
-            return Promise.reject(error);
         }).then((data: any) => {
+
             if (data.errorcode && (data.errorcode == 'enablewsdescription' || data.errorcode == 'requirecorrectaccess')) {
                 return Promise.reject({ errorcode: data.errorcode, error: data.error });
             } else if (data.error && data.error == 'Web services must be enabled in Advanced features.') {
@@ -424,7 +422,8 @@ export class CoreSitesProvider {
                 password: password,
                 service: service
             },
-            promise = this.http.post(siteUrl + '/login/token.php', params).timeout(CoreConstants.WS_TIMEOUT).toPromise();
+            loginUrl = siteUrl + '/login/token.php',
+            promise = this.http.post(loginUrl, params).timeout(CoreConstants.WS_TIMEOUT).toPromise();
 
         return promise.then((data: any): any => {
             if (typeof data == 'undefined') {
@@ -433,12 +432,22 @@ export class CoreSitesProvider {
                 if (typeof data.token != 'undefined') {
                     return { token: data.token, siteUrl: siteUrl, privateToken: data.privatetoken };
                 } else {
+
                     if (typeof data.error != 'undefined') {
                         // We only allow one retry (to avoid loops).
                         if (!retry && data.errorcode == 'requirecorrectaccess') {
                             siteUrl = this.urlUtils.addOrRemoveWWW(siteUrl);
 
                             return this.getUserToken(siteUrl, username, password, service, true);
+                        } else if (data.errorcode == 'missingparam') {
+                            // It seems the server didn't receive all required params, it could be due to a redirect.
+                            return this.utils.checkRedirect(loginUrl).then((redirect) => {
+                                if (redirect) {
+                                    return Promise.reject({ error: this.translate.instant('core.login.sitehasredirect') });
+                                } else {
+                                    return Promise.reject({ error: data.error, errorcode: data.errorcode });
+                                }
+                            });
                         } else if (typeof data.errorcode != 'undefined') {
                             return Promise.reject({ error: data.error, errorcode: data.errorcode });
                         } else {

src/providers/utils/text.ts

@@ -407,6 +407,10 @@ export class CoreTextUtilsProvider {
      * @return {string} Error message, undefined if not found.
      */
     getErrorMessageFromError(error: any): string {
+        if (typeof error == 'string') {
+            return error;
+        }
+
         return error && (error.message || error.error || error.content || error.body);
     }
 

src/providers/utils/utils.ts

@@ -26,6 +26,7 @@ import { CoreLoggerProvider } from '../logger';
 import { TranslateService } from '@ngx-translate/core';
 import { CoreLangProvider } from '../lang';
 import { CoreWSProvider, CoreWSError } from '../ws';
+import { CoreConstants } from '@core/constants';
 
 /**
  * Deferred promise. It's similar to the result of $q.defer() in AngularJS.
@@ -182,6 +183,43 @@ export class CoreUtilsProvider {
         return;
     }
 
+    /**
+     * Check if a URL has a redirect.
+     *
+     * @param {string} url The URL to check.
+     * @return {Promise<boolean>} Promise resolved with boolean_ whether there is a redirect.
+     */
+    checkRedirect(url: string): Promise<boolean> {
+        if (window.fetch) {
+            const win = <any> window, // Convert to <any> to be able to use AbortController (not supported by our TS version).
+                initOptions: any = {
+                    redirect: 'follow'
+                };
+            let controller;
+
+            // Some browsers implement fetch but no AbortController.
+            if (win.AbortController) {
+                controller = new win.AbortController();
+                initOptions.signal = controller.signal;
+            }
+
+            return this.timeoutPromise(window.fetch(url, initOptions), CoreConstants.WS_TIMEOUT).then((response: Response) => {
+                return response.redirected;
+            }).catch((error) => {
+                if (error.timeout && controller) {
+                    // Timeout, abort the request.
+                    controller.abort();
+                }
+
+                // There was a timeout, cannot determine if there's a redirect. Assume it's false.
+                return false;
+            });
+        } else {
+            // Cannot check if there is a redirect, assume it's false.
+            return Promise.resolve(false);
+        }
+    }
+
     /**
      * Close the InAppBrowser window.
      *
@@ -1082,6 +1120,26 @@ export class CoreUtilsProvider {
         return result;
     }
 
+    /**
+     * Set a timeout to a Promise. If the time passes before the Promise is resolved or rejected, it will be automatically
+     * rejected.
+     *
+     * @param {Promise<T>} promise The promise to timeout.
+     * @param {number} time Number of milliseconds of the timeout.
+     * @return {Promise<T>} Promise with the timeout.
+     */
+    timeoutPromise<T>(promise: Promise<T>, time: number): Promise<T> {
+        return new Promise((resolve, reject): void => {
+            const timeout = setTimeout(() => {
+                reject({timeout: true});
+            }, time);
+
+            promise.then(resolve).catch(reject).finally(() => {
+                clearTimeout(timeout);
+            });
+        });
+    }
+
     /**
      * Converts locale specific floating point/comma number back to standard PHP float value.
      * Do NOT try to do any math operations before this conversion on any user submitted floats!