From 8e0d167f95ad57cf2fb9585e71e72d6b066c9487 Mon Sep 17 00:00:00 2001 From: Elias Nahum Date: Tue, 22 May 2018 11:02:14 -0400 Subject: [PATCH] Fix push notification and emm race conditions (#1691) * Fix push notification and emm race conditions * feedback review * Removed unnecesary code --- android/app/src/main/AndroidManifest.xml | 2 - .../rnbeta/InitializationModule.java | 8 --- app/app.js | 71 ++++--------------- app/mattermost.js | 24 ++++--- .../mattermost-managed.android.js | 2 +- .../mattermost-managed.ios.js | 12 +++- .../push_notifications.android.js | 10 ++- .../push_notifications.ios.js | 9 +-- app/screens/entry/entry.js | 31 +++++--- app/utils/push_notifications.js | 35 ++++----- index.js | 5 +- .../android/extension_post/extension_post.js | 3 +- 12 files changed, 89 insertions(+), 123 deletions(-) diff --git a/android/app/src/main/AndroidManifest.xml b/android/app/src/main/AndroidManifest.xml index dd03b9e4f..22529a023 100644 --- a/android/app/src/main/AndroidManifest.xml +++ b/android/app/src/main/AndroidManifest.xml @@ -57,8 +57,6 @@ android:name="com.reactnativenavigation.controllers.NavigationActivity" android:configChanges="keyboard|keyboardHidden|orientation|screenSize"/> { - try { - const shouldStart = await avoidNativeBridge( - () => { - return Initialization.managedConfigResult; - }, - () => { - return Initialization.managedConfigResult; - }, - () => { - return AsyncStorage.getItem(DEVICE_SECURE_CACHE_KEY); - } - ); - if (shouldStart !== null) { - return shouldStart === 'true'; - } - } catch (error) { - return false; - } - return false; - }; - getAppCredentials = async () => { try { const credentials = await avoidNativeBridge( @@ -116,6 +94,8 @@ export default class App { this.currentUserId = currentUserId; this.token = token; this.url = url; + Client4.setUrl(url); + Client4.setToken(token); } } } catch (error) { @@ -163,8 +143,8 @@ export default class App { return null; }; - setManagedConfig = (shouldStart) => { - AsyncStorage.setItem(DEVICE_SECURE_CACHE_KEY, shouldStart.toString()); + setPerformingEMMAuthentication = (authenticating) => { + this.performingEMMAuthentication = authenticating; }; setAppCredentials = (deviceToken, currentUserId, token, url) => { @@ -221,37 +201,15 @@ export default class App { clearNativeCache = () => { resetGenericPassword(); AsyncStorage.multiRemove([ - DEVICE_SECURE_CACHE_KEY, TOOLBAR_BACKGROUND, TOOLBAR_TEXT_COLOR, APP_BACKGROUND, ]); }; - verifyManagedConfigCache = async (shouldStartCache) => { - // since we are caching managed device results - // we should verify after using the cache - const shouldStart = await handleManagedConfig(); - if (shouldStartCache && !shouldStart) { - this.setManagedConfig(false); - mattermostManaged.quitApp(); - return; - } - - this.setManagedConfig(true); - }; - launchApp = async () => { - const shouldStartCache = await this.getManagedConfig(); - if (shouldStartCache) { - this.startApp(); - this.verifyManagedConfigCache(shouldStartCache); - return; - } - const shouldStart = await handleManagedConfig(); if (shouldStart) { - this.setManagedConfig(shouldStart); this.startApp(); } }; @@ -261,18 +219,13 @@ export default class App { return; } - const {dispatch, getState} = store; - const {entities} = getState(); + const {dispatch} = store; let screen = 'SelectServer'; - if (entities) { - const {credentials} = entities.general; - - if (credentials.token && credentials.url) { - screen = 'Channel'; - tracker.initialLoad = Date.now(); - loadMe()(dispatch, getState); - } + if (this.token && this.url) { + screen = 'Channel'; + tracker.initialLoad = Date.now(); + dispatch(loadMe()); } switch (screen) { diff --git a/app/mattermost.js b/app/mattermost.js index 42c80ea55..f51e426df 100644 --- a/app/mattermost.js +++ b/app/mattermost.js @@ -1,4 +1,4 @@ -// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See License.txt for license information. /* eslint-disable global-require*/ @@ -94,7 +94,9 @@ const initializeModules = () => { EventEmitter.on(General.CONFIG_CHANGED, handleConfigChanged); EventEmitter.on(General.DEFAULT_CHANNEL, handleResetChannelDisplayName); Orientation.addOrientationListener(handleOrientationChange); - mattermostManaged.addEventListener('managedConfigDidChange', handleManagedConfig); + mattermostManaged.addEventListener('managedConfigDidChange', () => { + handleManagedConfig(true); + }); if (config) { configureAnalytics(config); @@ -135,7 +137,7 @@ const resetBadgeAndVersion = () => { }; const handleLogout = () => { - app.setAppStarted(false); + app.setAppStarted(true); app.clearNativeCache(); deleteFileCache(); resetBadgeAndVersion(); @@ -207,7 +209,11 @@ const handleOrientationChange = (orientation) => { }, 100); }; -export const handleManagedConfig = async (serverConfig) => { +export const handleManagedConfig = async (eventFromEmmServer = false) => { + if (app.performingEMMAuthentication) { + return true; + } + const {dispatch, getState} = store; const state = getState(); @@ -235,6 +241,7 @@ export const handleManagedConfig = async (serverConfig) => { return mattermostManaged.getConfig(); } ); + if (config && Object.keys(config).length) { app.setEMMEnabled(true); authNeeded = config.inAppPinCode && config.inAppPinCode === 'true'; @@ -272,7 +279,7 @@ export const handleManagedConfig = async (serverConfig) => { } } - if (authNeeded && !serverConfig) { + if (authNeeded && !eventFromEmmServer) { const authenticated = await handleAuthentication(vendor); if (!authenticated) { return false; @@ -299,6 +306,7 @@ export const handleManagedConfig = async (serverConfig) => { }; const handleAuthentication = async (vendor) => { + app.setPerformingEMMAuthentication(true); const isSecured = await mattermostManaged.isDeviceSecure(); const translations = app.getTranslations(); @@ -316,6 +324,7 @@ const handleAuthentication = async (vendor) => { } } + app.setPerformingEMMAuthentication(false); return true; }; @@ -391,10 +400,7 @@ const handleAppActive = async () => { const config = await mattermostManaged.getConfig(); const authNeeded = config.inAppPinCode && config.inAppPinCode === 'true'; if (authNeeded) { - const authenticated = await handleAuthentication(config.vendor); - if (!authenticated) { - mattermostManaged.quitApp(); - } + await handleAuthentication(config.vendor); } } catch (error) { // do nothing diff --git a/app/mattermost_managed/mattermost-managed.android.js b/app/mattermost_managed/mattermost-managed.android.js index f103f58b9..9fef9d4fa 100644 --- a/app/mattermost_managed/mattermost-managed.android.js +++ b/app/mattermost_managed/mattermost-managed.android.js @@ -34,7 +34,7 @@ export default { listeners.splice(index, 1); } }, - authenticate: LocalAuth.authenticate, + authenticate: LocalAuth.auth, blurAppScreen: MattermostManaged.blurAppScreen, getConfig: MattermostManaged.getConfig, getLocalConfig: async () => { diff --git a/app/mattermost_managed/mattermost-managed.ios.js b/app/mattermost_managed/mattermost-managed.ios.js index db515973a..d04c3ef7c 100644 --- a/app/mattermost_managed/mattermost-managed.ios.js +++ b/app/mattermost_managed/mattermost-managed.ios.js @@ -35,7 +35,17 @@ export default { }, authenticate: LocalAuth.authenticate, blurAppScreen: BlurAppScreen.enabled, - getConfig: MattermostManaged.getConfig, + getConfig: async () => { + return { + inAppPinCode: 'true', + blurApplicationScreen: 'true', + jailbreakDetection: 'true', + allowOtherServers: 'true', + vendor: 'Local Test', + serverUrl: 'http://192.168.0.18:8065', + username: 'elias', + }; + }, getLocalConfig: async () => { if (!localConfig) { try { diff --git a/app/push_notifications/push_notifications.android.js b/app/push_notifications/push_notifications.android.js index 2c7e004a2..9af06dab9 100644 --- a/app/push_notifications/push_notifications.android.js +++ b/app/push_notifications/push_notifications.android.js @@ -1,4 +1,4 @@ -// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See License.txt for license information. import {AppRegistry, AppState} from 'react-native'; @@ -15,6 +15,9 @@ class PushNotification { NotificationsAndroid.setRegistrationTokenUpdateListener((deviceToken) => { this.deviceToken = deviceToken; + if (this.onRegister) { + this.onRegister({token: this.deviceToken}); + } }); NotificationsAndroid.setNotificationReceivedListener((notification) => { @@ -42,13 +45,14 @@ class PushNotification { data, text: data.text, badge: parseInt(data.badge, 10) - parseInt(data.msg_count, 10), + completed: true, // used to identify that the notification belongs to a reply }; } }); } handleNotification = (data, userInteraction) => { - const deviceNotification = { + this.deviceNotification = { data, foreground: !userInteraction && AppState.currentState === 'active', message: data.message, @@ -57,7 +61,7 @@ class PushNotification { }; if (this.onNotification) { - this.onNotification(deviceNotification); + this.onNotification(this.deviceNotification); } }; diff --git a/app/push_notifications/push_notifications.ios.js b/app/push_notifications/push_notifications.ios.js index 12c6d69df..3766cd474 100644 --- a/app/push_notifications/push_notifications.ios.js +++ b/app/push_notifications/push_notifications.ios.js @@ -1,4 +1,4 @@ -// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See License.txt for license information. import {AppState} from 'react-native'; @@ -12,6 +12,7 @@ const replies = new Set(); class PushNotification { constructor() { + this.deviceNotification = null; this.onRegister = null; this.onNotification = null; this.onReply = null; @@ -56,7 +57,7 @@ class PushNotification { } handleNotification = (data, foreground, userInteraction) => { - const deviceNotification = { + this.deviceNotification = { data, foreground: foreground || (!userInteraction && AppState.currentState === 'active'), message: data.message, @@ -65,7 +66,7 @@ class PushNotification { }; if (this.onNotification) { - this.onNotification(deviceNotification); + this.onNotification(this.deviceNotification); } }; @@ -130,7 +131,7 @@ class PushNotification { } getNotification() { - return null; + return this.deviceNotification; } resetNotification() { diff --git a/app/screens/entry/entry.js b/app/screens/entry/entry.js index 4a8af53f9..7a4f33591 100644 --- a/app/screens/entry/entry.js +++ b/app/screens/entry/entry.js @@ -1,4 +1,4 @@ -// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See License.txt for license information. /* eslint-disable global-require*/ @@ -19,10 +19,12 @@ import { app, store, } from 'app/mattermost'; +import {loadFromPushNotification} from 'app/actions/views/root'; import {ViewTypes} from 'app/constants'; import PushNotifications from 'app/push_notifications'; import {stripTrailingSlashes} from 'app/utils/url'; +import ChannelLoader from 'app/components/channel_loader'; import EmptyToolbar from 'app/components/start/empty_toolbar'; import Loading from 'app/components/loading'; import SafeAreaView from 'app/components/safe_area_view'; @@ -116,7 +118,7 @@ export default class Entry extends PureComponent { this.setAppCredentials(); this.setStartupThemes(); - this.setReplyNotifications(); + this.handleNotification(); if (Platform.OS === 'android') { this.launchForAndroid(); @@ -173,7 +175,7 @@ export default class Entry extends PureComponent { ); }; - setReplyNotifications = () => { + handleNotification = async () => { const notification = PushNotifications.getNotification(); // If notification exists, it means that the app was started through a reply @@ -183,16 +185,18 @@ export default class Entry extends PureComponent { const notificationData = notification || app.replyNotificationData; const {data, text, badge, completed} = notificationData; - onPushNotificationReply(data, text, badge, completed); + // if the notification has a completed property it means that we are replying to a notification + // and in case it doesn't it means we just opened the notification + if (completed) { + onPushNotificationReply(data, text, badge, completed); + } else { + await store.dispatch(loadFromPushNotification(notification)); + } PushNotifications.resetNotification(); } }; launchForAndroid = () => { - if (app.startAppFromPushNotification) { - return; - } - app.launchApp(); }; @@ -242,6 +246,7 @@ export default class Entry extends PureComponent { } let toolbar = null; + let loading = null; const backgroundColor = app.appBackground ? app.appBackground : '#ffff'; if (app.token && app.toolbarBackground) { const toolbarTheme = { @@ -258,6 +263,14 @@ export default class Entry extends PureComponent { /> ); + + loading = ( + + + + ); + } else { + loading = ; } return ( @@ -267,7 +280,7 @@ export default class Entry extends PureComponent { navigator={navigator} > {toolbar} - + {loading} ); } diff --git a/app/utils/push_notifications.js b/app/utils/push_notifications.js index 61f4514a6..8ea25b4bd 100644 --- a/app/utils/push_notifications.js +++ b/app/utils/push_notifications.js @@ -1,7 +1,7 @@ -import { - Platform, - InteractionManager, -} from 'react-native'; +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +import {Platform} from 'react-native'; import PushNotifications from 'app/push_notifications'; import DeviceInfo from 'react-native-device-info'; @@ -18,8 +18,6 @@ import { loadFromPushNotification, } from 'app/actions/views/root'; -import {stripTrailingSlashes} from 'app/utils/url'; - import { app, store, @@ -42,16 +40,14 @@ const onRegisterDevice = (data) => { const token = `${prefix}:${data.token}`; if (state.views.root.hydrationComplete) { app.setDeviceToken(token); - setDeviceToken(token)(store.dispatch, store.getState); + store.dispatch(setDeviceToken(token)); } else { app.setDeviceToken(token); } }; -const onPushNotification = (deviceNotification) => { +const onPushNotification = async (deviceNotification) => { const {dispatch, getState} = store; - const state = getState(); - const {token, url} = state.entities.general.credentials; // mark the app as started as soon as possible if (Platform.OS === 'android' && !app.appStarted) { @@ -69,25 +65,18 @@ const onPushNotification = (deviceNotification) => { } if (data.type === 'clear') { - markChannelAsRead(data.channel_id, null, false)(dispatch, getState); + dispatch(markChannelAsRead(data.channel_id, null, false)); } else if (foreground) { EventEmitter.emit(ViewTypes.NOTIFICATION_IN_APP, notification); } else if (userInteraction && !notification.localNotification) { EventEmitter.emit('close_channel_drawer'); - if (app.startAppFromPushNotification) { - Client4.setToken(token); - Client4.setUrl(stripTrailingSlashes(url)); - } - - InteractionManager.runAfterInteractions(async () => { - await loadFromPushNotification(notification)(dispatch, getState); - - if (app.startAppFromPushNotification) { - app.launchApp(); - } else { + if (getState().views.root.hydrationComplete) { + await dispatch(loadFromPushNotification(notification)); + if (!app.startAppFromPushNotification) { EventEmitter.emit(ViewTypes.NOTIFICATION_TAPPED); + PushNotifications.resetNotification(); } - }); + } } }; diff --git a/index.js b/index.js index bd56c7a9f..effcd7828 100644 --- a/index.js +++ b/index.js @@ -1,10 +1,9 @@ -// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See License.txt for license information. -/* eslint-disable no-unused-vars */ import {AppRegistry, Platform} from 'react-native'; -import Mattermost from 'app/mattermost'; +import 'app/mattermost'; import ShareExtension from 'share_extension/android'; if (Platform.OS === 'android') { diff --git a/share_extension/android/extension_post/extension_post.js b/share_extension/android/extension_post/extension_post.js index 9a5f1fbb4..9851aaa08 100644 --- a/share_extension/android/extension_post/extension_post.js +++ b/share_extension/android/extension_post/extension_post.js @@ -153,6 +153,7 @@ export default class ExtensionPost extends PureComponent { if (config) { const authNeeded = config.inAppPinCode && config.inAppPinCode === 'true'; const vendor = config.vendor || 'Mattermost'; + if (authNeeded) { const isSecured = await mattermostManaged.isDeviceSecure(); if (isSecured) { @@ -166,7 +167,7 @@ export default class ExtensionPost extends PureComponent { suppressEnterPassword: false, }); } catch (err) { - return this.onClose(); + return this.onClose({nativeEvent: true}); } } }