fix(tv): open VLC with the account download token, not the access token - #281
Conversation
Open in VLC built /files/{id}/stream with the session's full OAuth token.
It now loads the account's download token through the same account info
query playback uses; a missing token is a typed failure with no fallback.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
TV emulator (putio-tv, API 36): Open in VLC on an mp4 reaches the ready path with the download token; the image has no VLC installed.
|
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes which credential is exposed to an external app (VLC) and the session-termination semantics, a security-sensitive area that relies on SDK token behavior not inspectable here, warranting human review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
This PR hardens TV "Open in VLC" so the /files/{id}/stream URL handed to the external VLC app carries the account's narrow download token instead of the session's full OAuth access token. The token is fetched through the same account/info?download_token=1 lookup playback already uses (refactored into a shared PutioClient.loadMediaAccount() helper). FilesStreamUrls.originalStreamUrl becomes suspend and returns a typed FilesStreamUrlResult (Ready/DownloadTokenUnavailable/Failure) rather than a nullable String, with no fallback to the access token and a 401 ending the session like any other files action.
Changes:
- Replace nullable stream-URL string with a
suspendAPI returning aFilesStreamUrlResultsealed hierarchy; build the URL from the account download token. - Extract
PutioClient.loadMediaAccount()and reuse it in both playback and stream-URL building. - Wire the TV UI to the new result type (launch in a coroutine scope, distinct notices, 401 → session verdict) and update docs/strings/tests.
| File | Description |
|---|---|
app/src/main/kotlin/io/putdotio/android/files/FilesWatchedRepository.kt |
New FilesStreamUrlResult type; SdkFilesStreamUrls loads the download token and maps failures to the app taxonomy, with a redacting toString. |
app/src/main/kotlin/io/putdotio/android/playback/PlaybackRepository.kt |
Adds shared PutioClient.loadMediaAccount() and reuses it. |
app/src/tv/kotlin/io/putdotio/android/tv/TvSessionViewModel.kt |
originalStreamUrl now suspend, returns the result and records a 401 as the session verdict. |
app/src/tv/kotlin/io/putdotio/android/PutioApp.kt |
Launches the lookup in a coroutine scope and routes the three result cases to notices/session rejection. |
app/src/tv/kotlin/io/putdotio/android/tv/files/TvFilesAction.kt |
Doc comment updated to download token. |
app/src/tv/res/values/strings.xml |
Adds tv_files_stream_error for non-401 failures. |
docs/harness.md |
Documents the download-token behavior. |
app/src/test/.../SdkFilesStreamUrlsTest.kt |
New test covering ready/missing-token/401 cases against a real PutioClient. |
app/src/testTv/.../TvSessionViewModelTest.kt, app/src/androidTestTv/.../TvProofSession.kt, app/src/androidTestTv/.../TvAutoplayProofTest.kt |
Update stubs/assertions to the new result type. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| suspend fun originalStreamUrl(item: FilesItem): FilesStreamUrlResult = | ||
| streamUrls.originalStreamUrl(item.id).also { result -> | ||
| val failure = (result as? FilesStreamUrlResult.Failure)?.failure | ||
| if (failure is FilesFailure.AuthenticationRequired) mutableFileActionFailure.value = failure | ||
| } |
There was a problem hiding this comment.
Added in 65c8a8d: TvSessionViewModelTest now stubs a 401 stream lookup and asserts it becomes the session verdict that survives dismissal, which fails with the wiring removed. It also checks that a missing download token or another failure records no verdict.
Change
TV Open in VLC now hands VLC a
/files/{id}/streamURL that carries the account's download token instead of the session's full OAuth access token.account/info?download_token=1query playback already uses (loadMediaAccount), so no new fetch path was added.DownloadTokenUnavailableand the existing "Couldn't prepare the stream" notice shows. The app never falls back to the access token.Couldn't prepare the stream. <reason>.Authorizationheader, and streaming playback already uses the download token. Those paths are unchanged.Validation
./gradlew verify :app:assembleMobileProductionDebug :app:assembleTvProductionDebug :app:assembleMobileNightlyDebug :app:assembleTvNightlyDebug: passed (1075 mobile and 747 TV unit tests)SdkFilesStreamUrlsTestbuilds the URL through a realPutioClientwith a stubbedaccount/info. With the builder reverted toclient.config.accessToken, it fails withexpected ...oauth_token=narrow-download-token but was ...oauth_token=full-oauth-access-token.putio-tvemulator (API 36), signed in as the shared test identity: Files → Menu on an mp4 → Open in VLC reached the ready path ("VLC isn't installed", screenshot below). That shows the download-token lookup succeeded. VLC isn't installed on the image, so the URL VLC receives wasn't observed on device.55b0da6with a clean worktree, and SDKputio-sdk-kotlinmainat40736da(clean). The SDK API is unchanged;buildOriginalStreamUrl(fileId, accessToken: String)receives the download token's value.Written by an agent (Claude Code, Opus 5.5)