also looking at this
fix(android): enforce the capability flags the bridge already reports
#233
7 files
+187
-10
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -132,6 +132,14 @@ Edit `craft.config.json` in your Android project: | ||
| 132 | 132 | } |
| 133 | 133 | ``` |
| 134 | 134 | |
| 135 | Every `enable*` flag is enforced, not only reported. A call whose capability is | |
| 136 | off rejects with `CAPABILITY_DISABLED`, as it does on iOS, and | |
| 137 | `window.craft.capabilities` reports the same answer, so a page that | |
| 138 | feature-detects and a page that just calls behave the same. Calls that answer | |
| 139 | nothing yet, such as `haptic` and `vibrate`, do nothing and say so on the | |
| 140 | console. Every flag defaults to off, so an app that shipped without setting one | |
| 141 | and relied on Android serving it anyway has to turn it on. | |
| 142 | ||
| 135 | 143 | Only the fields shown above are generator options. Enabling background location |
| 136 | 144 | also enables foreground geolocation. Push notifications require a regular |
| 137 | 145 | `googleServicesFile` whose JSON contains a client matching the generated |
| @@ -359,6 +359,24 @@ describe('Craft Android builder', () => { | ||
| 359 | 359 | expect(holder).toContain('private external fun nativeDispatchDeepLink(url: String, initial: Boolean): Boolean') |
| 360 | 360 | }) |
| 361 | 361 | |
| 362 | it('enforces the capability flags it reports, in the page and in Kotlin', async () => { | |
| 363 | // #209: the flags shaped the manifest and craft.capabilities, and then | |
| 364 | // every call was served anyway. | |
| 365 | const output = mkdtempSync(join(tmpdir(), 'craft-android-capability-gate-')) | |
| 366 | await init({ name: 'WildLoop', packageName: 'org.wildloop.app', output, config: { enableShare: true } }) | |
| 367 | ||
| 368 | const bridge = readFileSync(join(output, 'app/src/main/java/org/wildloop/app/CraftBridge.kt'), 'utf8') | |
| 369 | expect(bridge).toContain('installCapabilityGates') | |
| 370 | expect(bridge).toContain("error.code = 'CAPABILITY_DISABLED'") | |
| 371 | // The gate reads the object the page reads, so the two cannot disagree. | |
| 372 | expect(bridge).toContain('if (craft.capabilities[capability]) { return served.apply(this, arguments); }') | |
| 373 | // Generated from this app's config, not hardcoded. | |
| 374 | expect(bridge).toContain('private val shareEnabled = true') | |
| 375 | expect(bridge).toContain('private val hapticsEnabled = false') | |
| 376 | expect(bridge).toContain('if (!hapticsEnabled && disabled("haptics", "haptic")) return') | |
| 377 | expect(bridge).toContain('if (!keepAwakeEnabled && disabled("keepAwake", "setKeepAwake")) return false') | |
| 378 | }) | |
| 379 | ||
| 362 | 380 | it('routes external Activity results back to every pending media promise', async () => { |
| 363 | 381 | const output = mkdtempSync(join(tmpdir(), 'craft-android-activity-results-')) |
| 364 | 382 | await init({ name: 'WildLoop', packageName: 'org.wildloop.app', output }) |
| @@ -9,8 +9,8 @@ import { renderAndroidPromiseRuntime } from './promise-runtime' | ||
| 9 | 9 | |
| 10 | 10 | const template = readFileSync(join(import.meta.dir, '../templates/CraftBridge.kt.template'), 'utf8') |
| 11 | 11 | |
| 12 | /** The page script, as Kotlin evaluates it, with every flag off. */ | |
| 13 | function pageScript(): string { | |
| 12 | /** The page script, as Kotlin evaluates it, with every capability on or off. */ | |
| 13 | function pageScript(enabled = true): string { | |
| 14 | 14 | const opening = ' val script = """\n' |
| 15 | 15 | const start = template.indexOf(opening) |
| 16 | 16 | if (start === -1) throw new Error('CraftBridge.kt.template no longer builds `val script = """`') |
| @@ -18,7 +18,7 @@ function pageScript(): string { | ||
| 18 | 18 | const script = template |
| 19 | 19 | .slice(start + opening.length, end) |
| 20 | 20 | .replace(/\{\{PROMISE_RUNTIME\}\}/g, () => renderAndroidPromiseRuntime(' ')) |
| 21 | .replace(/\{\{ENABLE_[A-Z_]+\}\}/g, 'false') | |
| 21 | .replace(/\{\{ENABLE_[A-Z_]+\}\}/g, String(enabled)) | |
| 22 | 22 | // Kotlin string templates, `${isBiometricAvailable()}` and the like. Each |
| 23 | 23 | // is a Boolean in the capabilities object. |
| 24 | 24 | .replace(/\$\{(?:[^{}]|\{[^{}]*\})*\}/g, 'false') |
| @@ -29,7 +29,7 @@ function pageScript(): string { | ||
| 29 | 29 | type Listener = (event: { type: string, detail: unknown }) => void |
| 30 | 30 | |
| 31 | 31 | /** `beforeInject` runs first, the way a page's own script runs before Android injects the bridge. */ |
| 32 | function loadPage(beforeInject?: (page: Record<string, any>) => void) { | |
| 32 | function loadPage(beforeInject?: (page: Record<string, any>) => void, enabled = true) { | |
| 33 | 33 | const calls: string[] = [] |
| 34 | 34 | const listeners: Record<string, Listener[]> = {} |
| 35 | 35 | const page: Record<string, any> = { |
| @@ -66,7 +66,7 @@ function loadPage(beforeInject?: (page: Record<string, any>) => void) { | ||
| 66 | 66 | |
| 67 | 67 | const inject = () => { |
| 68 | 68 | // eslint-disable-next-line no-new-func |
| 69 | new Function('window', 'document', 'CraftAndroid', 'CustomEvent', 'console', pageScript())( | |
| 69 | new Function('window', 'document', 'CraftAndroid', 'CustomEvent', 'console', pageScript(enabled))( | |
| 70 | 70 | page, |
| 71 | 71 | document, |
| 72 | 72 | CraftAndroid, |
| @@ -197,6 +197,43 @@ describe('the injected Android page script', () => { | ||
| 197 | 197 | expect(seen).toEqual([{ url: 'app://warm', initial: false }]) |
| 198 | 198 | }) |
| 199 | 199 | |
| 200 | // #209: the flags craft.capabilities reports are enforced, as on iOS. | |
| 201 | it('refuses a call whose capability the app was not built with', async () => { | |
| 202 | const page = loadPage(undefined, false) | |
| 203 | ||
| 204 | expect(page.craft.capabilities.share).toBe(false) | |
| 205 | await expect(page.craft.share('hello')).rejects.toMatchObject({ code: 'CAPABILITY_DISABLED' }) | |
| 206 | await expect(page.craft.deepLinks.getInitialURL()).rejects.toMatchObject({ code: 'CAPABILITY_DISABLED' }) | |
| 207 | await expect(page.craft.secureStore.set('k', 'v')).rejects.toMatchObject({ code: 'CAPABILITY_DISABLED' }) | |
| 208 | expect(page.calls).toEqual([]) | |
| 209 | }, 5000) | |
| 210 | ||
| 211 | it('refuses through the v1 contract as well, since it wraps the same call', async () => { | |
| 212 | const page = loadPage(undefined, false) | |
| 213 | await expect(page.craft.share.share({ text: 'hello' })).rejects.toMatchObject({ code: 'CAPABILITY_DISABLED' }) | |
| 214 | }, 5000) | |
| 215 | ||
| 216 | it('does nothing, rather than refusing, for a call with no answer to give', async () => { | |
| 217 | // haptic and vibrate still return undefined (#219), so there is no | |
| 218 | // promise to reject; native must not be asked to buzz either. | |
| 219 | const page = loadPage(undefined, false) | |
| 220 | ||
| 221 | expect(page.craft.haptic('light')).toBeUndefined() | |
| 222 | expect(page.craft.vibrate([100])).toBeUndefined() | |
| 223 | expect(page.calls).toEqual([]) | |
| 224 | }) | |
| 225 | ||
| 226 | it('serves the same calls when the app was built with them', async () => { | |
| 227 | const page = loadPage() | |
| 228 | ||
| 229 | expect(page.craft.capabilities.share).toBe(true) | |
| 230 | void page.craft.share('hello') | |
| 231 | void page.craft.secureStore.set('k', 'v') | |
| 232 | page.craft.haptic('light') | |
| 233 | ||
| 234 | expect(page.calls).toEqual(['share', 'secureSet', 'haptic']) | |
| 235 | }) | |
| 236 | ||
| 200 | 237 | it('does not hand a link to a second subscriber after the script runs again in the same page', async () => { |
| 201 | 238 | const page = loadPage() |
| 202 | 239 | const first: unknown[] = [] |
| @@ -221,6 +221,20 @@ class CraftBridge( | ||
| 221 | 221 | private var bridgeReady = false |
| 222 | 222 | private val pendingEvents = mutableListOf<Pair<String, Map<String, Any>>>() |
| 223 | 223 | |
| 224 | // What the app was built with (#209). The page refuses a disabled call | |
| 225 | // before it gets here; these are the second check, for a caller that | |
| 226 | // reaches CraftAndroid directly, on the capabilities Android itself does | |
| 227 | // not gate with a permission. | |
| 228 | private val hapticsEnabled = | |
| 229 | private val speechEnabled = | |
| 230 | private val shareEnabled = | |
| 231 | private val keepAwakeEnabled = | |
| 232 | ||
| 233 | private fun disabled(capability: String, call: String): Boolean { | |
| 234 | android.util.Log.d("CraftBridge", "$call refused: $capability is disabled in craft.config.json") | |
| 235 | return true | |
| 236 | } | |
| 237 | ||
| 224 | 238 | // Deep links (#215), on the UI thread only, the way iOS's DeepLinkManager |
| 225 | 239 | // holds them: a link that arrives before the first page has its bridge is |
| 226 | 240 | // the launch link, and every link waits for an injected bridge. |
| @@ -1367,6 +1381,70 @@ class CraftBridge( | ||
| 1367 | 1381 | } |
| 1368 | 1382 | }; |
| 1369 | 1383 | |
| 1384 | // What the app was built with, enforced and not just reported | |
| 1385 | // (#209). iOS refuses a call whose capability is off with | |
| 1386 | // CAPABILITY_DISABLED, in Swift and in Zig; Android shaped the | |
| 1387 | // manifest and craft.capabilities from the same flags and then | |
| 1388 | // served the call anyway, so a page that feature-detected and a | |
| 1389 | // page that just called got different behaviour. | |
| 1390 | // | |
| 1391 | // The refusal is made here, from the same capabilities object the | |
| 1392 | // page can read, because that object is what this bridge promises. | |
| 1393 | // Calls that answer nothing yet (#219) cannot carry a refusal, so | |
| 1394 | // they do nothing and say so on the console, which is what a | |
| 1395 | // capability the app never asked for should do. | |
| 1396 | // | |
| 1397 | // Kotlin refuses the ungated-by-Android ones a second time, for a | |
| 1398 | // caller that reaches CraftAndroid without going through here. | |
| 1399 | (function installCapabilityGates(craft) { | |
| 1400 | var gated = [ | |
| 1401 | ['haptic', 'haptics'], | |
| 1402 | ['vibrate', 'haptics'], | |
| 1403 | ['startListening', 'speechRecognition'], | |
| 1404 | ['stopListening', 'speechRecognition'], | |
| 1405 | ['setFlashlight', 'flashlight'], | |
| 1406 | ['toggleFlashlight', 'flashlight'], | |
| 1407 | ['setKeepAwake', 'keepAwake'], | |
| 1408 | ['watchPosition', 'geolocation'], | |
| 1409 | ['clearWatch', 'geolocation'], | |
| 1410 | ['share', 'share', true], | |
| 1411 | ['openCamera', 'camera', true], | |
| 1412 | ['pickImage', 'camera', true], | |
| 1413 | ['startVideoRecording', 'camera', true], | |
| 1414 | ['authenticate', 'biometric', true], | |
| 1415 | ['registerPush', 'pushNotifications', true], | |
| 1416 | ['getCurrentPosition', 'geolocation', true], | |
| 1417 | ['secureStore.set', 'secureStorage', true], | |
| 1418 | ['secureStore.get', 'secureStorage', true], | |
| 1419 | ['secureStore.remove', 'secureStorage', true], | |
| 1420 | ['notifications.schedule', 'localNotifications', true], | |
| 1421 | ['notifications.cancel', 'localNotifications', true], | |
| 1422 | ['notifications.cancelAll', 'localNotifications', true], | |
| 1423 | ['deepLinks.getInitialURL', 'deepLinks', true] | |
| 1424 | ]; | |
| 1425 | function refusal(capability) { | |
| 1426 | var error = new Error(capability + ' is disabled: this app was built without it'); | |
| 1427 | error.code = 'CAPABILITY_DISABLED'; | |
| 1428 | error.bridge = true; | |
| 1429 | return error; | |
| 1430 | } | |
| 1431 | gated.forEach(function(entry) { | |
| 1432 | var path = entry[0].split('.'); | |
| 1433 | var capability = entry[1]; | |
| 1434 | var awaited = entry[2] === true; | |
| 1435 | var owner = path.length > 1 ? craft[path[0]] : craft; | |
| 1436 | var name = path[path.length - 1]; | |
| 1437 | if (!owner || typeof owner[name] !== 'function') { return; } | |
| 1438 | var served = owner[name]; | |
| 1439 | owner[name] = function() { | |
| 1440 | if (craft.capabilities[capability]) { return served.apply(this, arguments); } | |
| 1441 | if (awaited) { return Promise.reject(refusal(capability)); } | |
| 1442 | console.warn('craft: ' + name + ' did nothing because ' + capability + ' is disabled'); | |
| 1443 | return undefined; | |
| 1444 | }; | |
| 1445 | }); | |
| 1446 | })(window.craft); | |
| 1447 | ||
| 1370 | 1448 | // Links that arrive before anything is listening (#215, as iOS |
| 1371 | 1449 | // does since #198). |
| 1372 | 1450 | // |
| @@ -1571,6 +1649,7 @@ class CraftBridge( | ||
| 1571 | 1649 | |
| 1572 | 1650 | @JavascriptInterface |
| 1573 | 1651 | fun haptic(style: String) { |
| 1652 | if (!hapticsEnabled && disabled("haptics", "haptic")) return | |
| 1574 | 1653 | if (CraftNative.haptic(activity, style)) return |
| 1575 | 1654 | |
| 1576 | 1655 | val vibrator = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { |
| @@ -1603,6 +1682,7 @@ class CraftBridge( | ||
| 1603 | 1682 | |
| 1604 | 1683 | @JavascriptInterface |
| 1605 | 1684 | fun startListening() { |
| 1685 | if (!speechEnabled && disabled("speechRecognition", "startListening")) return | |
| 1606 | 1686 | if (CraftNative.startListening(activity)) return |
| 1607 | 1687 | |
| 1608 | 1688 | if (!SpeechRecognizer.isRecognitionAvailable(activity)) { |
| @@ -1711,6 +1791,11 @@ class CraftBridge( | ||
| 1711 | 1791 | |
| 1712 | 1792 | @JavascriptInterface |
| 1713 | 1793 | fun share(text: String, title: String) { |
| 1794 | if (!shareEnabled) { | |
| 1795 | rejectShare("share is disabled: this app was built without it") | |
| 1796 | disabled("share", "share") | |
| 1797 | return | |
| 1798 | } | |
| 1714 | 1799 | // The same line iOS draws: an empty string is nothing to share, and a |
| 1715 | 1800 | // chooser opened over nothing would offer apps a blank message. |
| 1716 | 1801 | if (text.isEmpty()) { |
| @@ -2743,6 +2828,7 @@ class CraftBridge( | ||
| 2743 | 2828 | |
| 2744 | 2829 | @JavascriptInterface |
| 2745 | 2830 | fun vibrate(patternJson: String) { |
| 2831 | if (!hapticsEnabled && disabled("haptics", "vibrate")) return | |
| 2746 | 2832 | if (CraftNative.vibrate(activity, patternJson)) return |
| 2747 | 2833 | |
| 2748 | 2834 | val vibrator = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { |
| @@ -3342,6 +3428,7 @@ class CraftBridge( | ||
| 3342 | 3428 | |
| 3343 | 3429 | @JavascriptInterface |
| 3344 | 3430 | fun setKeepAwake(enabled: Boolean): Boolean { |
| 3431 | if (!keepAwakeEnabled && disabled("keepAwake", "setKeepAwake")) return false | |
| 3345 | 3432 | // Zig queues the same window call on the same looper. isKeepingAwake |
| 3346 | 3433 | // below is written here and read nowhere, so the Kotlin body not |
| 3347 | 3434 | // running leaves nothing stale — unlike the flashlight pair. |
| @@ -27,7 +27,12 @@ import { command, driverPage, waitForFile } from './support' | ||
| 27 | 27 | const DEEP_LINK_SCHEME = 'crafte2eprobe' |
| 28 | 28 | |
| 29 | 29 | const CONFIG = { |
| 30 | // Off, so the capability gate has something to refuse (#209), and on, so it | |
| 31 | // has something to serve: the share cases below need sharing enabled now | |
| 32 | // that the flag is enforced. | |
| 30 | 33 | enablePushNotifications: false, |
| 34 | enableSecureStorage: false, | |
| 35 | enableShare: true, | |
| 31 | 36 | enableGeolocation: true, |
| 32 | 37 | enableDeepLinks: true, |
| 33 | 38 | urlSchemes: [DEEP_LINK_SCHEME], |
| @@ -355,17 +355,38 @@ | ||
| 355 | 355 | } |
| 356 | 356 | }, |
| 357 | 357 | { |
| 358 | // Android's equivalent gate. With push disabled the generator emits a | |
| 359 | // literal reject call rather than a Firebase block, so the rejection | |
| 360 | // string is fixed at generation time and is safe to match on. | |
| 358 | // Android's equivalent gate, and since #209 the same code iOS sends: | |
| 359 | // push is off in this app's config, so the call is refused rather | |
| 360 | // than reaching Firebase. capabilities says so too, and the case | |
| 361 | // checks that the two agree. | |
| 361 | 362 | name: 'push.disabled.rejects', |
| 362 | 363 | platforms: ['android'], |
| 363 | 364 | run: function () { |
| 365 | if (window.craft.capabilities.pushNotifications !== false) | |
| 366 | return 'capabilities.pushNotifications is ' + JSON.stringify(window.craft.capabilities.pushNotifications) | |
| 364 | 367 | return withTimeout(function () { return window.craft.registerPush() }).then(function (outcome) { |
| 365 | 368 | if (outcome.state === 'timeout') return 'registerPush never settled' |
| 366 | 369 | if (outcome.state === 'resolved') return 'registerPush resolved while push is disabled' |
| 367 | if (outcome.error.message.indexOf('Push notifications are disabled') === -1) | |
| 368 | return 'message is ' + JSON.stringify(outcome.error.message) | |
| 370 | if (outcome.error.code !== 'CAPABILITY_DISABLED') return 'code is ' + JSON.stringify(outcome.error.code) | |
| 371 | }) | |
| 372 | } | |
| 373 | }, | |
| 374 | { | |
| 375 | // #209: a capability the app was built with is served, and the page | |
| 376 | // can tell which is which from capabilities alone. Sharing is on in | |
| 377 | // this app precisely so the refusal above is not the only answer the | |
| 378 | // gate can give. | |
| 379 | name: 'capabilities.match.behaviour', | |
| 380 | platforms: ['android'], | |
| 381 | run: function () { | |
| 382 | if (window.craft.capabilities.share !== true) | |
| 383 | return 'capabilities.share is ' + JSON.stringify(window.craft.capabilities.share) | |
| 384 | if (window.craft.capabilities.secureStorage !== false) | |
| 385 | return 'capabilities.secureStorage is ' + JSON.stringify(window.craft.capabilities.secureStorage) | |
| 386 | return withTimeout(function () { return window.craft.secureStore.set('craft-e2e', NONCE) }).then(function (outcome) { | |
| 387 | if (outcome.state === 'timeout') return 'secureStore.set never settled' | |
| 388 | if (outcome.state === 'resolved') return 'secureStore.set resolved while secure storage is disabled' | |
| 389 | if (outcome.error.code !== 'CAPABILITY_DISABLED') return 'code is ' + JSON.stringify(outcome.error.code) | |
| 369 | 390 | }) |
| 370 | 391 | } |
| 371 | 392 | } |