Repository navigation
feat: support mutual TLS client certificates and show playback buffering indicator - #52
Open
AbsoluteMate wants to merge 7 commits into
Open
AbsoluteMate wants to merge 7 commits into
AbsoluteMate wants to merge 7 commits into
Conversation
Self-hosted Downtify instances behind reverse proxies such as Cloudflare Access / Zero Trust mTLS, Caddy or nginx can require a client certificate during the TLS handshake. Until now OkHttp used the default platform SSLContext with no KeyManager, so TLS handshakes asking for a client certificate always presented none and were rejected before reaching the server. This change adds optional Android KeyChain client certificate selection to both the initial Connect screen and Settings, and wires an X509ExtendedKeyManager into the shared OkHttpClient pool so REST calls, WebSocket events, image loading and Media3 audio streaming authenticate transparently over mTLS. ### `core:network` - `SessionStore.kt`: Persist the chosen KeyChain alias (`client_cert_alias`) alongside session preferences, expose `clientCertificate` (`StateFlow`) and `currentClientCertificate`, preserve the certificate alias across sign-out (`clear`), and notify `onClientCertificateChanged` when the alias changes. - `ClientCertificateManager.kt`: Implement `X509ExtendedKeyManager` backed by `KeyChain.getPrivateKey` and `KeyChain.getCertificateChain` with in-memory credential caching and TLS session invalidation (`clearCache()`). Scope certificate presentation via `allowsHost` so that once paired, only the paired server's host receives the certificate (never third-party cover or preview CDNs). Configure OkHttp's `sslSocketFactory` while preserving the platform `X509TrustManager` (and `network_security_config.xml`). - `NetworkModule.kt`: Apply `ClientCertificateManager.configureTls` to the root `@DefaultClient` (`@StreamingClient` shares its connection pool and TLS configuration via `newBuilder()`), and evict idle pooled connections whenever the selected certificate changes. - `ClientCertificateManagerTest.kt`: Unit tests covering host scoping before and after pairing, alias selection, credential caching and cache invalidation. ### `core:data` - `ServerRepository.kt`: Expose `clientCertificate: StateFlow<String?>` and `setClientCertificate(alias: String?)` on IO. ### `app` - `ClientCertificatePicker.kt`: Wrap `KeyChain.choosePrivateKeyAlias`, pre-filling host/port from the typed or paired server URL, dispatching the result back on the main thread, and falling back to `KeyChain.createInstallIntent()` when no user certificate is installed on the device yet. - `ConnectViewModel.kt` / `ConnectScreen.kt`: Expose `clientCertificate` in `ConnectUiState` and add a compact trailing action (`DowntifyIcons.Terms` to select, `DowntifyIcons.Close` to clear) plus supporting text on the server address field so the step keeps its zero-scroll height budget on `360 × 640 dp` screens across all locales. - `SettingsViewModel.kt` / `SettingsScreen.kt`: Add a `Client certificate` row under the `Server` section that opens the system `KeyChain` picker when unset, or a dialog to change/remove the active certificate and re-probe the server. - `res/values*/strings.xml`: Add client certificate strings across all 8 supported locales (`values`, `values-bg`, `values-el`, `values-es`, `values-fr`, `values-hu`, `values-pt-rBR`, `values-tr`). - `ConnectScreenTest.kt` / `SettingsScreenTest.kt`: UI tests for selecting and clearing the client certificate in the Connect and Settings screens.
…ption card below address input
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hi! This PR brings a few features and fixes that I needed for my setup and daily use:
KeyChainboth on the initial Connect screen (via a dedicated card below the server address) and in Settings.MiniPlayerandNowPlayingScreennow shows aCircularProgressIndicatorso there's clear visual feedback that the track is loading.Songsquick-access tiles on the Home screen, and simplified top-level tab backstack handling.What Changed
Mutual TLS (mTLS) Client Certificates
core:networkClientCertificateManager(X509ExtendedKeyManager) backed byKeyChainwith in-memory caching and strict fail-closed host scoping (allowsHost), ensuring the certificate is only presented to valid peer hosts (and, once paired, strictly to the paired Downtify server—never leaked to third-party CDNs or unknown peers).SessionStoreandNetworkModuleto persist the selected certificate alias (client_cert_alias), configure TLS on the sharedOkHttpClient, and evict pooled connections when the certificate changes.core:data&appClientCertificatePickeraroundKeyChain.choosePrivateKeyAlias(with fallback to certificate installation if none is installed).ClientCertificateOptioncard inConnectScreen(with adjusted vertical spacing so small-screen fit tests pass) and a setting row inSettingsScreen.Offline Storage Path Validation
core:dataOfflineStorageby sanitizingtrackIdvalues (sanitize) and verifying canonical paths (safeFile) to prevent path traversal outside the app'sofflinedirectory.Player Buffering Indicator
core:designsystem&appMiniPlayerButtonandPlaybackControls(TransportButton) to display aCircularProgressIndicatorwhileisBufferingistrue, keeping the active button shape and preserving thecontentDescriptionsemantics for accessibility.Home & Navigation Fixes
HomeScreen: HandledPlaybackContextType.SongsinQuickAccessTileso direct song playback tiles show the localized "Songs" label and song icon.DowntifyApp: SimplifiedopenTabinMainShellso switching or re-tapping a top-level tab consistently pops any nested destination above the tab's start route.Tests
ClientCertificateManager(ClientCertificateManagerTest), including null/empty host rejection and host scoping before/after pairing.ConnectScreenTest,ConnectScreenFitTest,SettingsScreenTest,HomeScreenTest,MiniPlayerTest, andPlayerComponentsTest.Note
I'm still relatively new to Kotlin and Android development, so I relied on AI assistance during the implementation to better understand the codebase and ensure that my changes followed the project's existing architecture and conventions.
That said, I've made an effort to carefully review and understand the generated code rather than simply relying on it. I've also tested the implementation extensively in my own environment, including both the mTLS connection flow and playback buffering, and verified that all unit and UI tests are passing.
I fully understand that there may be things I've overlooked or approaches that could be improved. I'm genuinely open to any feedback, suggestions, or requested changes, and I'd be more than happy to make adjustments based on your review.
Thanks for taking the time to look at my contribution!