Fix permissions connect close and redirect behavior (#8751)

* fix permissions connect close/redirect behavior

* improve permissions connect fullscreen close behavior

* fix multi ui perm conf redirect; consolidate perm selectors

ui/app/pages/permissions-connect/permissions-connect.component.js

@@ -6,7 +6,6 @@ import { getEnvironmentType } from '../../../../app/scripts/lib/util'
 import {
   ENVIRONMENT_TYPE_FULLSCREEN,
   ENVIRONMENT_TYPE_NOTIFICATION,
-  ENVIRONMENT_TYPE_POPUP,
 } from '../../../../app/scripts/lib/enums'
 import {
   DEFAULT_ROUTE,
@@ -20,30 +19,29 @@ export default class PermissionConnect extends Component {
     getRequestAccountTabIds: PropTypes.func.isRequired,
     getCurrentWindowTab: PropTypes.func.isRequired,
     accounts: PropTypes.array.isRequired,
-    originName: PropTypes.string,
+    origin: PropTypes.string,
     showNewAccountModal: PropTypes.func.isRequired,
     newAccountNumber: PropTypes.number.isRequired,
     nativeCurrency: PropTypes.string,
     permissionsRequest: PropTypes.object,
-    addressLastConnectedMap: PropTypes.object,
+    addressLastConnectedMap: PropTypes.object.isRequired,
+    lastConnectedInfo: PropTypes.object.isRequired,
     permissionsRequestId: PropTypes.string,
-    domains: PropTypes.object,
     history: PropTypes.object.isRequired,
     connectPath: PropTypes.string.isRequired,
     confirmPermissionPath: PropTypes.string.isRequired,
     page: PropTypes.string.isRequired,
-    redirecting: PropTypes.bool,
     targetDomainMetadata: PropTypes.object,
+    location: PropTypes.shape({
+      pathname: PropTypes.string,
+    }).isRequired,
   }
 
   static defaultProps = {
-    originName: '',
+    origin: '',
     nativeCurrency: '',
     permissionsRequest: undefined,
-    addressLastConnectedMap: {},
     permissionsRequestId: '',
-    domains: {},
-    redirecting: false,
   }
 
   static contextTypes = {
@@ -56,7 +54,7 @@ export default class PermissionConnect extends Component {
       ? new Set([this.props.accounts[0].address])
       : new Set(),
     permissionAccepted: null,
-    originName: this.props.originName,
+    origin: this.props.origin,
   }
 
   beforeUnload = () => {
@@ -69,22 +67,25 @@ export default class PermissionConnect extends Component {
   }
 
   removeBeforeUnload = () => {
-    if (getEnvironmentType() === ENVIRONMENT_TYPE_FULLSCREEN || getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
+    const environmentType = getEnvironmentType()
+    if (
+      environmentType === ENVIRONMENT_TYPE_FULLSCREEN ||
+      environmentType === ENVIRONMENT_TYPE_NOTIFICATION
+    ) {
       window.removeEventListener('beforeunload', this.beforeUnload)
     }
   }
 
   componentDidUpdate (prevProps) {
-    const { domains, permissionsRequest, redirecting } = this.props
+    const { permissionsRequest, lastConnectedInfo } = this.props
-    const { originName } = this.state
+    const { redirecting, origin } = this.state
 
     if (!permissionsRequest && prevProps.permissionsRequest && !redirecting) {
-      const permissionDataForDomain = (domains && domains[originName]) || {}
+
-      const permissionsForDomain = permissionDataForDomain.permissions || []
+      const accountsLastApprovedTime = lastConnectedInfo[origin]?.lastApproved || 0
-      const prevPermissionDataForDomain = (prevProps.domains && prevProps.domains[originName]) || {}
+      const initialAccountsLastApprovedTime = prevProps.lastConnectedInfo[origin]?.lastApproved || 0
-      const prevPermissionsForDomain = prevPermissionDataForDomain.permissions || []
+
-      const addedAPermission = permissionsForDomain.length > prevPermissionsForDomain.length
+      if (accountsLastApprovedTime > initialAccountsLastApprovedTime) {
-      if (addedAPermission) {
         this.redirectFlow(true)
       } else {
         this.redirectFlow(false)
@@ -99,7 +100,7 @@ export default class PermissionConnect extends Component {
   }
 
   redirectFlow (accepted) {
-    const { history } = this.props
+    const { history, location, confirmPermissionPath } = this.props
 
     this.setState({
       redirecting: true,
@@ -107,16 +108,15 @@ export default class PermissionConnect extends Component {
     })
     this.removeBeforeUnload()
 
-    if (getEnvironmentType() === ENVIRONMENT_TYPE_FULLSCREEN) {
+    if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
-      setTimeout(async () => {
-        const currentTab = await global.platform.currentTab()
-        global.platform.closeTab(currentTab.id)
-      }, 2000)
-    } else if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
       setTimeout(async () => {
         global.platform.closeCurrentWindow()
-      }, 2000)
+      }, 1500)
-    } else if (getEnvironmentType() === ENVIRONMENT_TYPE_POPUP) {
+    } else if (location.pathname === confirmPermissionPath) {
+      setTimeout(async () => {
+        history.push(DEFAULT_ROUTE)
+      }, 1500)
+    } else {
       history.push(DEFAULT_ROUTE)
     }
   }
@@ -135,19 +135,25 @@ export default class PermissionConnect extends Component {
       return history.push(DEFAULT_ROUTE)
     }
 
-    if (getEnvironmentType() === ENVIRONMENT_TYPE_FULLSCREEN || getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
+    const environmentType = getEnvironmentType()
+    if (
+      environmentType === ENVIRONMENT_TYPE_FULLSCREEN ||
+      environmentType === ENVIRONMENT_TYPE_NOTIFICATION
+    ) {
       window.addEventListener('beforeunload', this.beforeUnload)
     }
   }
 
   cancelPermissionsRequest = async (requestId) => {
+
     const { history, rejectPermissionsRequest } = this.props
+
     if (requestId) {
       await rejectPermissionsRequest(requestId)
 
-      if (getEnvironmentType() === ENVIRONMENT_TYPE_FULLSCREEN || getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
+      if (getEnvironmentType() === ENVIRONMENT_TYPE_NOTIFICATION) {
         window.close()
-      } else if (getEnvironmentType() === ENVIRONMENT_TYPE_POPUP) {
+      } else {
         history.push(DEFAULT_ROUTE)
       }
     }
@@ -202,7 +208,7 @@ export default class PermissionConnect extends Component {
     const {
       selectedAccountAddresses,
       permissionAccepted,
-      originName,
+      origin,
       redirecting,
     } = this.state
 
@@ -216,7 +222,6 @@ export default class PermissionConnect extends Component {
             render={() => (
               <ChooseAccount
                 accounts={accounts}
-                originName={originName}
                 nativeCurrency={nativeCurrency}
                 selectAccounts={(addresses) => this.selectAccounts(addresses)}
                 selectNewAccountViaModal={(handleAccountClick) => {
@@ -250,7 +255,7 @@ export default class PermissionConnect extends Component {
                 selectedIdentities={accounts.filter((account) => selectedAccountAddresses.has(account.address))}
                 redirect={redirecting}
                 permissionRejected={ permissionAccepted === false }
-                cachedOrigin={originName}
+                cachedOrigin={origin}
               />
             )}
           />

ui/app/pages/permissions-connect/permissions-connect.container.js

@@ -7,11 +7,16 @@ import {
   getAccountsWithLabels,
   getLastConnectedInfo,
   getTargetDomainMetadata,
-  getPermissionDomains,
 } from '../../selectors'
 
 import { formatDate } from '../../helpers/utils/util'
-import { approvePermissionsRequest, rejectPermissionsRequest, showModal, getCurrentWindowTab, getRequestAccountTabIds } from '../../store/actions'
+import {
+  approvePermissionsRequest,
+  rejectPermissionsRequest,
+  showModal,
+  getCurrentWindowTab,
+  getRequestAccountTabIds,
+} from '../../store/actions'
 import {
   CONNECT_ROUTE,
   CONNECT_CONFIRM_PERMISSIONS_ROUTE,
@@ -34,7 +39,7 @@ const mapStateToProps = (state, ownProps) => {
   const accountsWithLabels = getAccountsWithLabels(state)
 
   const lastConnectedInfo = getLastConnectedInfo(state) || {}
-  const addressLastConnectedMap = lastConnectedInfo[origin] || {}
+  const addressLastConnectedMap = lastConnectedInfo[origin]?.accounts || {}
 
   Object.keys(addressLastConnectedMap).forEach((key) => {
     addressLastConnectedMap[key] = formatDate(addressLastConnectedMap[key], 'yyyy-MM-dd')
@@ -58,11 +63,11 @@ const mapStateToProps = (state, ownProps) => {
     permissionsRequest,
     permissionsRequestId,
     accounts: accountsWithLabels,
-    originName: origin,
+    origin,
     newAccountNumber: accountsWithLabels.length + 1,
     nativeCurrency,
     addressLastConnectedMap,
-    domains: getPermissionDomains(state),
+    lastConnectedInfo,
     connectPath,
     confirmPermissionPath,
     page,

ui/app/selectors/permissions.js

@@ -249,3 +249,32 @@ export function getPermissionsForActiveTab (state) {
     }
   })
 }
+
+export function getLastConnectedInfo (state) {
+  const { permissionsHistory = {} } = state.metamask
+  return Object.keys(permissionsHistory).reduce((acc, origin) => {
+    const ethAccountsHistory = JSON.parse(JSON.stringify(permissionsHistory[origin]['eth_accounts']))
+    return {
+      ...acc,
+      [origin]: ethAccountsHistory,
+    }
+  }, {})
+}
+
+export function getPermissionsRequests (state) {
+  return state.metamask.permissionsRequests || []
+}
+
+export function getPermissionsRequestCount (state) {
+  const permissionsRequests = getPermissionsRequests(state)
+  return permissionsRequests.length
+}
+
+export function getFirstPermissionRequest (state) {
+  const requests = getPermissionsRequests(state)
+  return requests && requests[0] ? requests[0] : null
+}
+
+export function hasPermissionRequests (state) {
+  return Boolean(getFirstPermissionRequest(state))
+}

ui/app/selectors/selectors.js

@@ -6,6 +6,7 @@ import {
   checksumAddress,
   getAccountByAddress,
 } from '../helpers/utils/util'
+import { getPermissionsRequestCount } from './permissions'
 
 export function getNetworkIdentifier (state) {
   const { metamask: { provider: { type, nickname, rpcTarget } } } = state
@@ -282,15 +283,6 @@ export function getCustomNonceValue (state) {
   return String(state.metamask.customNonceValue)
 }
 
-export function getPermissionsRequests (state) {
-  return state.metamask.permissionsRequests || []
-}
-
-export function getPermissionsRequestCount (state) {
-  const permissionsRequests = getPermissionsRequests(state)
-  return permissionsRequests.length
-}
-
 export function getDomainMetadata (state) {
   return state.metamask.domainMetadata
 }
@@ -339,31 +331,10 @@ export function getFeatureFlags (state) {
   return state.metamask.featureFlags
 }
 
-export function getFirstPermissionRequest (state) {
-  const requests = getPermissionsRequests(state)
-  return requests && requests[0] ? requests[0] : null
-}
-
-export function hasPermissionRequests (state) {
-  return Boolean(getFirstPermissionRequest(state))
-}
-
 export function getOriginOfCurrentTab (state) {
   return state.activeTab?.origin
 }
 
-export function getLastConnectedInfo (state) {
-  const { permissionsHistory = {} } = state.metamask
-  const lastConnectedInfoData = Object.keys(permissionsHistory).reduce((acc, origin) => {
-    const ethAccountsHistory = JSON.parse(JSON.stringify(permissionsHistory[origin]['eth_accounts']))
-    return {
-      ...acc,
-      [origin]: ethAccountsHistory.accounts,
-    }
-  }, {})
-  return lastConnectedInfoData
-}
-
 export function getIpfsGateway (state) {
   return state.metamask.ipfsGateway
 }