Skip to content

Commit

Permalink
Fix: don't fetch manifest for unknown Safe Apps (#3400)
Browse files Browse the repository at this point in the history
* Fix: don't fetch manifest for unknown Safe Apps

* Update e2e tests

* Fix tx_modal test
  • Loading branch information
katspaugh authored Mar 7, 2024
1 parent 36b8ad2 commit 5714bca
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 15 deletions.
5 changes: 3 additions & 2 deletions cypress/e2e/safe-apps/browser_permissions.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ describe('Browser permissions tests', () => {
main.acceptCookies()
})

it('Verify a permissions slide to the user is displayed', () => {
// @TODO: unknown apps don't have permissions
xit('Verify a permissions slide to the user is displayed', () => {
safeapps.clickOnContinueBtn()
safeapps.verifyCameraCheckBoxExists()
safeapps.verifyMicrofoneCheckBoxExists()
})

it('Verify the selection can be changed, accepted and stored', () => {
xit('Verify the selection can be changed, accepted and stored', () => {
safeapps.verifyMicrofoneCheckBoxExists().click()
safeapps.clickOnContinueBtn()
safeapps.verifyWarningDefaultAppMsgIsDisplayed()
Expand Down
5 changes: 4 additions & 1 deletion cypress/e2e/safe-apps/tx_modal.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import * as safeapps from '../pages/safeapps.pages'

const testAppName = 'Cypress Test App'
const testAppDescr = 'Cypress Test App Description'
const unknownApp = 'unknown'
const confirmTx = 'Confirm transaction'

describe('Transaction modal tests', () => {
beforeEach(() => {
Expand All @@ -28,7 +30,8 @@ describe('Transaction modal tests', () => {
safeapps.verifyWarningDefaultAppMsgIsDisplayed()
safeapps.clickOnContinueBtn()
cy.findByRole('dialog').within(() => {
cy.findByText(testAppName)
cy.findByText(confirmTx)
cy.findByText(unknownApp)
})
},
)
Expand Down
2 changes: 1 addition & 1 deletion src/components/safe-apps/SafeAppLandingPage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ type Props = {
const CHAIN_ID_WITH_A_DEMO = '1'

const SafeAppLanding = ({ appUrl, chain }: Props) => {
const { safeApp, isLoading } = useSafeAppFromManifest(appUrl, chain.chainId)
const [backendApp, , backendAppLoading] = useSafeAppFromBackend(appUrl, chain.chainId)
const { safeApp, isLoading } = useSafeAppFromManifest(appUrl, chain.chainId, backendApp)
const wallet = useWallet()
const onboard = useOnboard()
// show demo if the app was shared for mainnet or we can find the mainnet chain id on the backend
Expand Down
7 changes: 4 additions & 3 deletions src/components/safe-apps/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export const getLegacyChainName = (chainName: string, chainId: string): string =
return network
}

export const getEmptySafeApp = (url = ''): SafeAppDataWithPermissions => {
export const getEmptySafeApp = (url = '', appData?: SafeAppData): SafeAppDataWithPermissions => {
return {
id: Math.random(),
url,
Expand All @@ -75,10 +75,11 @@ export const getEmptySafeApp = (url = ''): SafeAppDataWithPermissions => {
type: SafeAppAccessPolicyTypes.NoRestrictions,
},
tags: [],
safeAppsPermissions: [],
features: [],
socialProfiles: [],
developerWebsite: '',
socialProfiles: [],
...appData,
safeAppsPermissions: [],
}
}

Expand Down
13 changes: 9 additions & 4 deletions src/hooks/safe-apps/useSafeAppFromManifest.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useEffect, useMemo } from 'react'
import type { SafeAppData } from '@safe-global/safe-gateway-typescript-sdk'
import { Errors, logError } from '@/services/exceptions'
import { fetchSafeAppFromManifest } from '@/services/safe-apps/manifest'
import useAsync from '@/hooks/useAsync'
Expand All @@ -11,12 +12,16 @@ type UseSafeAppFromManifestReturnType = {
isLoading: boolean
}

const useSafeAppFromManifest = (appUrl: string, chainId: string): UseSafeAppFromManifestReturnType => {
const useSafeAppFromManifest = (
appUrl: string,
chainId: string,
safeAppData?: SafeAppData,
): UseSafeAppFromManifestReturnType => {
const [data, error, isLoading] = useAsync<SafeAppDataWithPermissions>(() => {
if (appUrl && chainId) return fetchSafeAppFromManifest(appUrl, chainId)
}, [appUrl, chainId])
if (appUrl && chainId && safeAppData) return fetchSafeAppFromManifest(appUrl, chainId)
}, [appUrl, chainId, safeAppData])

const emptyApp = useMemo(() => getEmptySafeApp(appUrl), [appUrl])
const emptyApp = useMemo(() => getEmptySafeApp(appUrl, safeAppData), [appUrl, safeAppData])

useEffect(() => {
if (!error) return
Expand Down
8 changes: 4 additions & 4 deletions src/pages/apps/open.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ const SafeApps: NextPage = () => {
const chainId = useChainId()
const router = useRouter()
const appUrl = useSafeAppUrl()
const { safeApp, isLoading } = useSafeAppFromManifest(appUrl || '', chainId)
const { allSafeApps, remoteSafeAppsLoading } = useSafeApps()
const safeAppData = allSafeApps.find((app) => app.url === appUrl)
const { safeApp, isLoading } = useSafeAppFromManifest(appUrl || '', chainId, safeAppData)
const isSafeAppsEnabled = useHasFeature(FEATURES.SAFE_APPS)
const isWalletConnectEnabled = useHasFeature(FEATURES.NATIVE_WALLETCONNECT)

const { remoteSafeApps, remoteSafeAppsLoading } = useSafeApps()

const { addPermissions, getPermissions, getAllowedFeaturesList } = useBrowserPermissions()
const origin = getOrigin(appUrl)
const {
Expand All @@ -41,7 +41,7 @@ const SafeApps: NextPage = () => {
onComplete,
} = useSafeAppsInfoModal({
url: origin,
safeApp: remoteSafeApps.find((app) => app.url === appUrl),
safeApp: safeAppData,
permissions: safeApp?.safeAppsPermissions || [],
addPermissions,
getPermissions,
Expand Down

0 comments on commit 5714bca

Please sign in to comment.