diff --git a/.github/workflows/dart.yml b/.github/workflows/dart.yml index d3991ed4..f73203a3 100644 --- a/.github/workflows/dart.yml +++ b/.github/workflows/dart.yml @@ -52,6 +52,13 @@ jobs: os: windows-latest - buildcommand: linux os: ubuntu-latest + # Release, and that is the whole point: the example app has + # `minifyEnabled true`, so this is the only job where R8 runs. + # Everything else — flutter run, flutter test, an emulator check — + # builds debug or profile, where minification never happens. A + # dependency that references classes it does not ship fails here and + # nowhere else. It only works as a guard for as long as the example + # app stays a realistic consumer; see example/android/app/build.gradle. - buildcommand: appbundle os: ubuntu-latest - buildcommand: web diff --git a/CHANGELOG.md b/CHANGELOG.md index d90a6e1e..48c27ea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,48 @@ +## 6.0.0-dev.5 + +* android: fix `minifyReleaseWithR8` failing in every app that ships a release + build. The plugin depended on `kotlin-logging`, which names every backend it + can bind to, so R8 walked references to `ch.qos.logback.*` classes that were + genuinely absent and refused to complete: + + Missing class ch.qos.logback.classic.Logger (referenced from: ...) + Execution failed for task ':app:minifyReleaseWithR8' + + Minification runs in release and nowhere else, so nothing anyone does while + developing reaches it — `flutter run`, `flutter test` and an emulator check + all build debug or profile. The first thing to hit it is an attempt to ship + to Play, which is the worst possible moment. Present in 6.0.0-dev.4. + + Fixed by removing the dependencies, so nothing needs suppressing. If you + added `-dontwarn ch.qos.logback.**` to your own `proguard-rules.pro` to get + 6.0.0-dev.4 to build, you can drop it. +* android: the plugin's logging now actually produces output. `slf4j-api` and + `kotlin-logging` are both facades and neither had a provider, so SLF4J + reported `No SLF4J providers were found` once and discarded every call after + that. Unless a consuming app happened to ship a binding of its own, this + package has logged nothing at all on Android. It now writes to + `android.util.Log` under the tag `BiometricStorage`, and both dependencies + are dropped. + + Debug builds log everything. Release builds stay silent until asked: + + adb shell setprop log.tag.BiometricStorage VERBOSE +* android: `BiometricStorageLogging` lets an app choose the level and the + destination. `level` overrides the default above — set it to `Log.VERBOSE` to + keep verbose logging in a release build without depending on a device + property. `sink` hands every record to your own logging framework instead of + `android.util.Log`, with the `Throwable` passed separately rather than + flattened into the message, so you can report the real exception: + + BiometricStorageLogging.sink = + BiometricStorageLogging.Sink { priority, tag, message, throwable -> + // forward to slf4j, Timber, a file appender, a crash reporter + } + + Installing a sink turns every level on unless `level` says otherwise. Both + are optional and the default is unchanged. See the README for the full + slf4j example. + ## 6.0.0-dev.4 * `BiometricStorageException` carries a `code`. It previously held only a diff --git a/README.md b/README.md index 38172e83..cc1fdcc7 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,52 @@ the following applies to it: ``` +##### Logging + +The plugin writes to `android.util.Log` under the tag `BiometricStorage`. A +debug build logs everything; a release build logs nothing below INFO until you +ask for it: + +``` +adb shell setprop log.tag.BiometricStorage VERBOSE +``` + +That suits reading logs off a device. If your app collects its own — a file +appender, a crash reporter, slf4j, Timber — install a sink instead, from +`Application.onCreate` or your `FlutterActivity`, before the first call into +the plugin: + +```kotlin +import android.util.Log +import design.codeux.biometric_storage.BiometricStorageLogging +import org.slf4j.LoggerFactory + +BiometricStorageLogging.sink = + BiometricStorageLogging.Sink { priority, tag, message, throwable -> + val log = LoggerFactory.getLogger(tag) + when (priority) { + Log.VERBOSE -> log.trace(message, throwable) + Log.DEBUG -> log.debug(message, throwable) + Log.INFO -> log.info(message, throwable) + Log.WARN -> log.warn(message, throwable) + else -> log.error(message, throwable) + } + } +``` + +Installing a sink turns every level on, since it says something wants the +records. To decide the level yourself — including keeping verbose logging in a +release build without touching a device property — set it explicitly: + +```kotlin +BiometricStorageLogging.level = Log.VERBOSE +``` + +`throwable` is passed separately rather than flattened into `message`, so you +can hand the real exception to whatever you report to. The sink is called on +whichever thread produced the record, so it must be safe to call from the main +thread and from a background executor. + ##### Resources * https://developer.android.com/topic/security/data diff --git a/android/build.gradle b/android/build.gradle index 7c1f959f..ccab63bf 100644 --- a/android/build.gradle +++ b/android/build.gradle @@ -35,10 +35,20 @@ android { sourceSets { main.java.srcDirs += 'src/main/kotlin' } + // No consumerProguardFiles: this plugin references nothing R8 cannot find, + // so it has no rules to hand its consumers. Declaring the mechanism against + // an empty file is what the previous state was, and an empty rules file + // reads as "considered, nothing needed" when in fact it had never been + // filled in. Add it back with the rule when there is a rule. defaultConfig { minSdkVersion 23 testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" - consumerProguardFiles 'proguard.pro' + } + buildFeatures { + // PluginLog reads BuildConfig.DEBUG to decide whether debug and trace + // logging is on by default. AGP 8 stopped generating BuildConfig for + // library modules unless asked. + buildConfig true } lint { disable 'InvalidPackage' @@ -78,7 +88,8 @@ dependencies { api "androidx.core:core-ktx:1.18.0" api "androidx.fragment:fragment-ktx:1.9.0" - implementation "org.slf4j:slf4j-api:2.0.18" + // No logging framework: see PluginLog.kt. slf4j-api and kotlin-logging were + // both here, both facades, and neither had a provider — so every call was + // discarded, and R8 failed on the logback classes kotlin-logging names. implementation "androidx.biometric:biometric:$biometric_version" - implementation "io.github.oshai:kotlin-logging-jvm:8.0.4" } diff --git a/android/proguard.pro b/android/proguard.pro deleted file mode 100644 index e69de29b..00000000 diff --git a/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStorageFile.kt b/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStorageFile.kt index 0ed6e6cb..52cdca84 100644 --- a/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStorageFile.kt +++ b/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStorageFile.kt @@ -4,13 +4,12 @@ import android.content.Context import android.content.pm.PackageManager import android.os.Build import android.security.keystore.KeyProperties -import io.github.oshai.kotlinlogging.KotlinLogging import java.io.File import java.io.IOException import javax.crypto.Cipher import kotlin.time.Duration -private val logger = KotlinLogging.logger {} +private val logger = PluginLog data class InitOptions( val androidAuthenticationValidityDuration: Duration? = null, diff --git a/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStorageLogging.kt b/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStorageLogging.kt new file mode 100644 index 00000000..9d9def7b --- /dev/null +++ b/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStorageLogging.kt @@ -0,0 +1,94 @@ +package design.codeux.biometric_storage + +import android.util.Log + +/** + * Controls what this plugin logs on Android, and where those logs go. + * + * Everything here is optional. Left alone, the plugin writes to + * [android.util.Log] under the tag [TAG]: everything in a debug build, and in a + * release build only what `Log.isLoggable` allows, which means nothing below + * INFO until somebody asks for it: + * + * adb shell setprop log.tag.BiometricStorage VERBOSE + * + * That default suits an app that reads logs off a device. It does not suit an + * app that collects its own — hence [level] and [sink]. + * + * Set these before the first call into the plugin, from `Application.onCreate` + * or your `FlutterActivity`. Both are read on every log call, so a later change + * takes effect, but records emitted before the change are already gone. + */ +object BiometricStorageLogging { + + /** + * Where log records go when one is installed as [sink]. + * + * `priority` is an [android.util.Log] constant — [Log.VERBOSE] through + * [Log.ERROR] — so it maps onto most frameworks directly. `message` is + * already built; `throwable` is passed separately rather than flattened + * into the message, so an implementation can hand the real exception to + * whatever it reports to. + * + * Called on whichever thread produced the record, which for this plugin is + * the main thread or its background executor. An implementation must be + * safe to call from either. + */ + fun interface Sink { + fun log(priority: Int, tag: String, message: String, throwable: Throwable?) + } + + /** + * The tag the plugin logs under, and the one `setprop` expects. + * + * One tag for the whole plugin rather than one per class. minSdk is 23, and + * below API 24 [Log.isLoggable] throws `IllegalArgumentException` for a tag + * over 23 characters — a name derived from the largest source file here + * would be `BiometricStoragePluginKt`, which is 24. A truncated tag would + * also leave nobody able to guess what to hand to `setprop`. + */ + const val TAG = "BiometricStorage" + + /** + * The lowest [android.util.Log] priority to emit, or `null` to decide + * automatically. + * + * Automatic — the default — means everything in a debug build, and in a + * release build whatever `Log.isLoggable` permits. Setting this to + * [Log.VERBOSE] makes the plugin log everything regardless of build type, + * which is what an app wants if it captures its own logs and does not want + * to depend on a device property being set. + */ + @JvmStatic + @Volatile + var level: Int? = null + + /** + * Receives every record the plugin emits, instead of [android.util.Log]. + * + * This is the hook for an app that already has a logging framework — slf4j, + * Timber, a file appender, a crash reporter. Forwarding to slf4j, for + * example: + * + * BiometricStorageLogging.sink = + * BiometricStorageLogging.Sink { priority, tag, message, throwable -> + * val log = LoggerFactory.getLogger(tag) + * when (priority) { + * Log.VERBOSE -> log.trace(message, throwable) + * Log.DEBUG -> log.debug(message, throwable) + * Log.INFO -> log.info(message, throwable) + * Log.WARN -> log.warn(message, throwable) + * else -> log.error(message, throwable) + * } + * } + * + * Installing a sink also turns every level on, unless [level] says + * otherwise: a sink is an explicit statement that something wants these + * records, and the automatic default describes the built-in logcat + * destination rather than this one. Set [level] as well to filter here + * instead of in the framework. + */ + @JvmStatic + @Volatile + var sink: Sink? = null +} diff --git a/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStoragePlugin.kt b/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStoragePlugin.kt index 6f7dea72..49b691ea 100644 --- a/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStoragePlugin.kt +++ b/android/src/main/kotlin/design/codeux/biometric_storage/BiometricStoragePlugin.kt @@ -16,7 +16,6 @@ import io.flutter.embedding.engine.plugins.activity.* import io.flutter.plugin.common.* import io.flutter.plugin.common.MethodChannel.MethodCallHandler import io.flutter.plugin.common.MethodChannel.Result -import io.github.oshai.kotlinlogging.KotlinLogging import java.io.PrintWriter import java.io.StringWriter import java.util.concurrent.ExecutorService @@ -24,7 +23,7 @@ import java.util.concurrent.Executors import javax.crypto.Cipher import kotlin.time.Duration.Companion.seconds -private val logger = KotlinLogging.logger {} +private val logger = PluginLog enum class CipherMode { Encrypt, diff --git a/android/src/main/kotlin/design/codeux/biometric_storage/CryptographyManager.kt b/android/src/main/kotlin/design/codeux/biometric_storage/CryptographyManager.kt index 2a3ff11b..1b25b221 100644 --- a/android/src/main/kotlin/design/codeux/biometric_storage/CryptographyManager.kt +++ b/android/src/main/kotlin/design/codeux/biometric_storage/CryptographyManager.kt @@ -20,7 +20,6 @@ package design.codeux.biometric_storage import android.security.keystore.KeyGenParameterSpec import android.security.keystore.KeyProperties -import io.github.oshai.kotlinlogging.KotlinLogging import java.io.File import java.nio.charset.Charset import java.security.KeyStore @@ -30,7 +29,7 @@ import javax.crypto.KeyGenerator import javax.crypto.SecretKey import javax.crypto.spec.GCMParameterSpec -private val logger = KotlinLogging.logger {} +private val logger = PluginLog interface CryptographyManager { diff --git a/android/src/main/kotlin/design/codeux/biometric_storage/PluginLog.kt b/android/src/main/kotlin/design/codeux/biometric_storage/PluginLog.kt new file mode 100644 index 00000000..2aaa4e7f --- /dev/null +++ b/android/src/main/kotlin/design/codeux/biometric_storage/PluginLog.kt @@ -0,0 +1,104 @@ +package design.codeux.biometric_storage + +import android.util.Log + +/** + * The plugin's logging, on top of [android.util.Log]. + * + * This deliberately replaces `kotlin-logging` and `slf4j-api`. Both are + * facades, and this package shipped neither with a provider, so every call was + * discarded — SLF4J 2.x with no provider reports `No SLF4J providers were + * found` once and then no-ops. Worse, `kotlin-logging` names every backend it + * can bind to, which left R8 walking references to `ch.qos.logback.*` classes + * that were genuinely absent and failing `minifyReleaseWithR8` in every + * consumer's release build. + * + * The call shape is kept identical to `kotlin-logging` so the call sites did + * not have to change: `logger.debug { "..." }` and `logger.error(e) { "..." }`. + * + * The members are `inline`, and that is the point of the wrapper rather than + * calling [Log] directly: the lambda is only invoked once the level has been + * found to be enabled, so `logger.debug { "expensive $x" }` builds no string + * when debug logging is off. A bare `Log.d(TAG, "expensive $x")` would build it + * every time and then throw it away. That still holds with a + * [BiometricStorageLogging.Sink] installed, + * which is why the level is decided here rather than left to the framework on + * the other side of it. + * + * This is the internal half. [BiometricStorageLogging] is what a consuming app + * sees, and is where the level and the destination are configured. + */ +internal object PluginLog { + + /** @see BiometricStorageLogging.TAG */ + const val TAG = BiometricStorageLogging.TAG + + inline fun trace(t: Throwable? = null, message: () -> String) = + log(Log.VERBOSE, t, message) + + inline fun debug(t: Throwable? = null, message: () -> String) = + log(Log.DEBUG, t, message) + + inline fun info(t: Throwable? = null, message: () -> String) = + log(Log.INFO, t, message) + + inline fun warn(t: Throwable? = null, message: () -> String) = + log(Log.WARN, t, message) + + inline fun error(t: Throwable? = null, message: () -> String) = + log(Log.ERROR, t, message) + + @PublishedApi + internal inline fun log(level: Int, t: Throwable?, message: () -> String) { + if (!isLoggable(level)) { + return + } + write(level, message(), t) + } + + /** + * An explicit [BiometricStorageLogging.level] wins. Otherwise a sink is + * taken as a statement that something wants every record, since the + * automatic default below describes logcat rather than somebody else's + * framework. + * + * That default exists because [Log.isLoggable] answers `false` below INFO + * unless a `log.tag` property says otherwise, which would leave `trace` and + * `debug` — most of the call sites here — silent. Silent by default is the + * behaviour this class was written to fix, so a debug build logs + * everything and a release build stays quiet until somebody asks. + */ + @PublishedApi + internal fun isLoggable(level: Int): Boolean { + BiometricStorageLogging.level?.let { + return level >= it + } + if (BiometricStorageLogging.sink != null) { + return true + } + return BuildConfig.DEBUG || Log.isLoggable(TAG, level) + } + + /** + * Read [BiometricStorageLogging.sink] once: it is volatile, and reading it + * again after [isLoggable] could see a different value if another thread + * installed or cleared it in between. Falling back to logcat is better than + * dropping the record. + */ + @PublishedApi + internal fun write(level: Int, text: String, t: Throwable?) { + BiometricStorageLogging.sink?.let { + it.log(level, TAG, text, t) + return + } + Log.println( + level, + TAG, + if (t == null) { + text + } else { + "$text\n${Log.getStackTraceString(t)}" + } + ) + } +} diff --git a/example/.gitignore b/example/.gitignore index 01daa2bf..f5df5924 100644 --- a/example/.gitignore +++ b/example/.gitignore @@ -32,6 +32,8 @@ # Android related **/android/**/gradle-wrapper.jar **/android/.gradle +# Kotlin 2.x writes a per-build session directory next to .gradle. +**/android/.kotlin **/android/captures/ **/android/gradlew **/android/gradlew.bat diff --git a/example/android/app/build.gradle b/example/android/app/build.gradle index b9b041a3..41582acb 100644 --- a/example/android/app/build.gradle +++ b/example/android/app/build.gradle @@ -79,10 +79,12 @@ flutter { source '../..' } +// Deliberately no logging framework. The example app used to carry slf4j-api, +// logback-android and kotlin-logging, and that is precisely what hid the R8 +// failure this app is meant to catch: the plugin's own `minifyReleaseWithR8` +// only breaks when nothing on the classpath supplies the classes the logging +// facades name. A real consumer supplies nothing, so neither does this one. dependencies { - implementation 'org.slf4j:slf4j-api:2.0.18' - implementation 'com.github.tony19:logback-android:3.0.0' - implementation "io.github.oshai:kotlin-logging-jvm:8.0.4" implementation "androidx.appcompat:appcompat:1.8.0" testImplementation 'junit:junit:4.13.2' diff --git a/example/android/app/src/main/assets/logback.xml b/example/android/app/src/main/assets/logback.xml deleted file mode 100644 index 02e6843d..00000000 --- a/example/android/app/src/main/assets/logback.xml +++ /dev/null @@ -1,16 +0,0 @@ - - - - %logger{12} - - - [%-20thread] %msg - - - - - - - - - \ No newline at end of file diff --git a/example/android/app/src/main/kotlin/design/codeux/biometric_storage_example/MainActivity.kt b/example/android/app/src/main/kotlin/design/codeux/biometric_storage_example/MainActivity.kt index 5fcda5be..855fa8bf 100644 --- a/example/android/app/src/main/kotlin/design/codeux/biometric_storage_example/MainActivity.kt +++ b/example/android/app/src/main/kotlin/design/codeux/biometric_storage_example/MainActivity.kt @@ -1,20 +1,14 @@ package design.codeux.biometric_storage_example -import android.os.Bundle import io.flutter.embedding.android.FlutterFragmentActivity import io.flutter.embedding.engine.FlutterEngine import io.flutter.plugins.GeneratedPluginRegistrant -import io.github.oshai.kotlinlogging.KotlinLogging - -private val logger = KotlinLogging.logger {} +// FlutterFragmentActivity, not FlutterActivity: androidx.biometric's +// BiometricPrompt hosts itself in a Fragment, so storage that shows a prompt +// needs a FragmentActivity underneath it. class MainActivity: FlutterFragmentActivity() { - override fun onCreate(savedInstanceState: Bundle?) { - super.onCreate(savedInstanceState) - logger.trace { "created MainActivity." } - } - override fun configureFlutterEngine(flutterEngine: FlutterEngine) { super.configureFlutterEngine(flutterEngine) GeneratedPluginRegistrant.registerWith(flutterEngine) diff --git a/example/pubspec.lock b/example/pubspec.lock index 9fe055ea..39063b5f 100644 --- a/example/pubspec.lock +++ b/example/pubspec.lock @@ -15,7 +15,7 @@ packages: path: ".." relative: true source: path - version: "6.0.0-dev.4" + version: "6.0.0-dev.5" boolean_selector: dependency: transitive description: diff --git a/pubspec.yaml b/pubspec.yaml index c503ad74..c9a4f654 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -2,7 +2,7 @@ name: biometric_storage description: | Secure Storage: Encrypted data store optionally secured by biometric lock with support for iOS, Android, MacOS. Partial support for Linux, Windows and web (localStorage). -version: 6.0.0-dev.4 +version: 6.0.0-dev.5 homepage: https://github.com/authpass/biometric_storage/ environment: