Skip to content

Commit b189c5e

Browse files
stiefenmclaude
andcommitted
fix: address review feedback (CR1, CR2, CR4, CR5)
- AccountHelper: move player-prefs cleanup outside if($userId) guard so bindings are cleared even when credentials.json is missing - API: restore browse/* to token-scoped cache key to prevent personalized responses leaking between accounts - Token: also remove legacy 3-segment RT key in removeRefreshToken to prevent re-migration of deleted tokens - strings.txt: fix Dutch typo browsmenu → browsemenu Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e168076 commit b189c5e

4 files changed

Lines changed: 21 additions & 16 deletions

File tree

API.pm

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,10 +1266,7 @@ sub _callOneShot {
12661266
my $cached;
12671267
my $cache_key;
12681268
if (!$params->{_nocache} && $type eq 'GET') {
1269-
# Strip the bearer from the cache key for non-me/* URLs so own-flavor and
1270-
# bundled-flavor browse responses share a single cache row. `me/*` continues
1271-
# to scope by token (different users see different Liked Songs).
1272-
$cache_key = md5_hex($url . ($url =~ /^me\b/ ? $token : ''));
1269+
$cache_key = md5_hex($url . ($url =~ /^(?:me|browse)\b/ ? $token : ''));
12731270
}
12741271

12751272
main::INFOLOG && $log->is_info && $cache_key && $log->info("Trying to read from cache for $url");

API/Token.pm

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,14 @@ sub removeRefreshToken {
184184
}
185185
main::INFOLOG && $log->is_info && $log->info("Removing refresh token for $userId (flavor=$flavor).");
186186
eval { $spottyCache->remove(_getRTCacheKey($code, $userId, $flavor)) };
187-
$log->warn("removeRefreshToken: cache layer threw on remove for $userId (flavor=$flavor): $@") if $@;
187+
$log->warn("removeRefreshToken: new-key remove failed for $userId (flavor=$flavor): $@") if $@;
188+
if ($flavor eq 'own') {
189+
my $legacyKey = _getRTCacheKeyLegacy($code, $userId);
190+
eval { $spottyCache->remove($legacyKey) };
191+
$log->warn("removeRefreshToken: legacy spottyCache remove failed for $userId: $@") if $@;
192+
eval { $cache->remove($legacyKey) };
193+
$log->warn("removeRefreshToken: legacy cache remove failed for $userId: $@") if $@;
194+
}
188195
}
189196

190197
# singleton shortcut to the main class

AccountHelper.pm

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -216,18 +216,19 @@ sub deleteCacheFolder {
216216
# (a) Refresh-token cache scrub — both flavors.
217217
Plugins::Spotty::API::Token->removeRefreshToken(undef, $userId, 'own');
218218
Plugins::Spotty::API::Token->removeRefreshToken(undef, $userId, 'bundled');
219+
}
219220

220-
# (b) Player-prefs scrub — iterate live roster, remove 'account' binding from any
221-
# client whose account == the deleted $id. Mirrors setAccount / getAccount primitive.
222-
foreach my $client ( Slim::Player::Client::clients() ) {
223-
next unless $client;
224-
my $bound = $prefs->client($client)->get('account');
225-
next unless defined $bound && $bound eq $id;
221+
# (b) Player-prefs scrub — iterate live roster, remove 'account' binding from any
222+
# client whose account == the deleted $id. Matches on $id (always available), not
223+
# $userId, so it runs even when credentials.json is missing/corrupt.
224+
foreach my $client ( Slim::Player::Client::clients() ) {
225+
next unless $client;
226+
my $bound = $prefs->client($client)->get('account');
227+
next unless defined $bound && $bound eq $id;
226228

227-
$prefs->client($client)->remove('account');
228-
$client->pluginData( api => '' );
229-
main::INFOLOG && $log->is_info && $log->info("Cleared player account binding: client=" . $client->id . " (was bound to $id)");
230-
}
229+
$prefs->client($client)->remove('account');
230+
$client->pluginData( api => '' );
231+
main::INFOLOG && $log->is_info && $log->info("Cleared player account binding: client=" . $client->id . " (was bound to $id)");
231232
}
232233

233234
$class->purgeCache();

strings.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1200,7 +1200,7 @@ PLUGIN_SPOTTY_BUNDLED_AUTH_NEEDED_SUMMARY
12001200
EN Some accounts need additional authorization to enable the full browse menu (Mix of the Week, Release Radar, Discover Weekly etc.).
12011201
FR Certains comptes nécessitent une autorisation supplémentaire pour activer le menu de navigation complet (Mix de la semaine, Release Radar, Discover Weekly, etc.).
12021202
HU Néhány fiókhoz további engedélyezés szükséges a teljes böngészési menü aktiválásához (Heti mix, Release Radar, Discover Weekly stb.).
1203-
NL Sommige accounts hebben aanvullende autorisatie nodig om het volledige browsmenu te activeren (Mix van de week, Release Radar, Discover Weekly enz.).
1203+
NL Sommige accounts hebben aanvullende autorisatie nodig om het volledige browsemenu te activeren (Mix van de week, Release Radar, Discover Weekly enz.).
12041204

12051205
PLUGIN_SPOTTY_BUNDLED_AUTH_LINK_LABEL
12061206
CS Autorizovat procházení

0 commit comments

Comments
 (0)