FormidableLabs / FormidableLabs/react-native-app-auth

[Android] AuthorizationService is never disposed: Custom Tabs bind leak breaks refresh() permanently after ~1000 calls

Aperta
#1,126 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Java
Stelle
2.3k
Fork
473
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

## Issue

On Android, the native module creates `new AuthorizationService(...)` at every call site and never calls `dispose()` on it. The string `dispose` does not appear anywhere in `RNAppAuthModule.java` — I checked `main` today as well as the published 8.1.0 and 8.4.1 artifacts.

`AuthorizationService`'s constructor eagerly binds to the selected browser's Custom Tabs service:

```
new AuthorizationService(context, config)
-> BrowserSelector.select(...)
-> CustomTabManager.bind(browserPackage)
-> CustomTabsClient.bindCustomTabsService(...)
-> Context.bindService(...)
```

That bind is released *only* by `AuthorizationService.dispose()` -> `CustomTabManager.dispose()` -> `unbindService`. AppAuth states the contract explicitly in the `AuthorizationService` javadoc:

> instances of this class **must be manually disposed** when no longer required, to avoid leaks

So each call through this library leaks one `ServiceConnection` to the default browser's `CustomTabsService`. Android's ActivityManager caps outstanding bind requests per process/service at ~1000. Once that is crossed, `bindService` throws:

```
Error: Too many bind requests(999+) for service Intent { act=android.support.customtabs.action.CustomTabsService pkg=com.sec.android.app.sbrowser }
```

The exception is thrown synchronously from the `AuthorizationService` constructor, so the JS promise rejects. Crucially, the bind counter only resets when the process dies — so from that moment on **every** subsequent call fails for the rest of the process lifetime. In a long-lived app the user is silently unable to refresh their token until they force-stop it.

`refresh()` is by far the worst affected, because it is the call an app makes repeatedly and unattended — and it needs no browser at all. The Custom Tabs bind is pure collateral damage from the constructor.

### Affected call sites

All six `new AuthorizationService(...)` in `packages/react-native-app-auth/android/src/main/java/com/rnappauth/RNAppAuthModule.java` on `main` (lines ~532, 614, 740, 753, 799, 849). None are disposed. The one in `refreshWithConfiguration` (~799) is the one that bites in practice.

### Reproduction

1. Android device with a Custom Tabs–capable default browser (seen with Samsung Internet).
2. Call `refresh()` repeatedly within a single process — e.g. an OP with a short access-token lifetime, refreshed on demand before API calls.
3. After ~1000 refreshes without the process being killed, every `refresh()` rejects with the error above and never recovers.

Observed in production on a device whose app process had stayed alive for ~10 days.

### Suggested fix

Minimal, per call site — dispose in the response callback:

```java
final AuthorizationService authService = new AuthorizationService(context, appAuthConfiguration);

AuthorizationService.TokenResponseCallback tokenResponseCallback = new AuthorizationService.TokenResponseCallback() {
@Override
public void onTokenRequestCompleted(@Nullable TokenResponse response, @Nullable AuthorizationException ex) {
authService.dispose();
// ...existing handling
}
};
```

Note that the `authorize` / `endSession` flows hand the service off to an Activity-based redirect flow, so disposal there has to happen after the redirect is handled rather than immediately.

Better still, mirror what [`flutter_appauth`](https://github.com/MaikuB/flutter_appauth) does with the same underlying `net.openid:appauth` library: keep a **single** lazily-created `AuthorizationService`, reuse it across calls, and dispose it on lifecycle teardown (`onDetachedFromEngine` in their case). See `FlutterAppauthPlugin.java` — `createAuthorizationServices()` / `disposeAuthorizationServices()`, plus their hardening PRs [#170](https://github.com/MaikuB/flutter_appauth/pull/170), [#556](https://github.com/MaikuB/flutter_appauth/pull/556) and [#600](https://github.com/MaikuB/flutter_appauth/pull/600).

### Prior art

- [auth0/Auth0.Android#517](https://github.com/auth0/Auth0.Android/pull/517) — "Fix memory leak in CustomTabsService", the same class of leak in another OAuth SDK's Custom Tabs controller.
- [openid/AppAuth-Android#1085](https://github.com/openid/AppAuth-Android/issues/1085), [#91](https://github.com/openid/AppAuth-Android/issues/91), [#134](https://github.com/openid/AppAuth-Android/issues/134), [#166](https://github.com/openid/AppAuth-Android/issues/166) — recurring reports around the bind/unbind side of `CustomTabManager`.
- `expo-web-browser` pairs `bindCustomTabsService` with `unbindService` and exposes `warmUp` / `coolDown` to callers, so the contract is well established elsewhere in the React Native ecosystem.

I'm happy to open a PR for the `refresh` / token-request paths if you'd like — let me know whether you'd prefer the minimal per-call `dispose()` or the cached-instance approach.

---

## Environment

* **Your Identity Provider**: Keycloak (not provider-specific — any OP with short-lived access tokens will reach the threshold)
* **Platform that you're experiencing the issue on**: Android
* **Your `react-native` Version**: 0.81.5 (Hermes, New Architecture)
* **Your `react-native-app-auth` Version**: 8.1.0 — but confirmed still present on `main` / 8.4.1
* **Are you using Expo?** Yes
* **Device**: Samsung Galaxy Tab A7 (SM-T505), Android 12, default browser Samsung Internet

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Inizia da packages/react-native-app-auth/android/src/main/java/com/rnappauth/RNAppAuthModule.java e ispeziona i sei punti di chiamata di AuthorizationService, in particolare refreshWithConfiguration e il relativo callback della risposta del token. Confronta il suggerimento di chiamare dispose per ogni chiamata con l’approccio basato sul ciclo di vita di un servizio memorizzato nella cache, includendo la gestione dei redirect di authorize ed endSession. Il lavoro è completato quando le chiamate ripetute a refresh non accumulano più bind di Custom Tabs e i flussi interessati continuano a completarsi correttamente.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
android, java
Ambito
authentication, mobile
Tipo di issue
Bug
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Tranquilla
Chiarezza
Abbastanza chiara
Idoneità per principianti
62/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.