* fix: prevent singleflight self-deadlock in reentrant OAuth auth check [IDE-2178]
The OAuth token refresher closure calls AuthenticationService.IsAuthenticated()
again on refresh failure, from the same goroutine that is already inside
doAuthCheck()'s authCheckGroup.Do() for the same token. singleflight.Group
blocks a second same-key caller until the first in-flight call returns, but
here the "first caller" is the same goroutine still executing — it blocks on
itself forever, so IsAuthenticated() never returns, logout never fires, and
the IDE keeps showing the user as authenticated indefinitely.
Track tokens currently inside authCheckGroup.Do via inFlightAuthChecks and
short-circuit a reentrant call for the same token instead of deadlocking.
* fix: recognize structured 401 as permanent auth failure in shouldCauseLogout [IDE-2178]
net/http.Client.Do always wraps a RoundTripper-returned error in *url.Error,
regardless of cause, so isTransientNetworkError's errors.As(err, &url.Error)
check matches every HTTP-path error unconditionally -- including a genuine,
permanent 401 from the Snyk API. That misclassifies real auth failures as
transient network noise before shouldCauseLogout ever reaches its
"(status: 401)" or "failed to invoke whoami workflow" string checks, so
logout never fires and the user stays "authenticated" against a dead token.
GAF's error-catalog already carries this permanent/transient distinction
structurally: a real 401/400 comes back as snyk_errors.Error{StatusCode: ...},
preserved through the wrapping chain via Unwrap(). Check for that status code
before the transient-network guard instead of relying on wording, which is
fragile against error text changes (observed in the wild as "Authentication
error", not any of the substrings shouldCauseLogout previously matched on).
* fix: remove token-keyed in-flight guard that over-blocked concurrent auth checks [IDE-2178]
inFlightAuthChecks.LoadOrStore short-circuited every concurrent
IsAuthenticated() caller for a token, not just the same-goroutine
reentrant one, so legitimate concurrent callers got a hardcoded false
instead of the correct shared singleflight result. The actual reentrant
deadlock is fixed at its source instead: the OAuth token refresher no
longer calls back into IsAuthenticated() on refresh failure, since the
in-flight check that triggered the refresh observes the same failure
itself and already runs the notification/logout handling.
Verified the notification/logout side effect is preserved via the
pre-existing authentication_smoke_test.go (real network, real 401),
plus a new unit test proving concurrent callers now share the correct
result.
* test: tighten re-auth assertion and cover structured 400 in shouldCauseLogout [IDE-2178]
Assert the re-auth ShowMessageRequest carries a LoginCommand action instead of
just checking that some ShowMessageRequest fired. Add a structured 400
snyk_errors.Error case to Test_shouldCauseLogout, mirroring the existing 401
coverage.
* test: pin down OAuth refresh notification behavior and reachability [IDE-2178]
Default()'s custom refresherFunc is only ever wired into OAuth2Provider's own
authenticator, used by the explicit login flow. Real IsAuthenticated()/scan/
whoami traffic goes through a separate authenticator GAF builds internally,
which never receives this closure. Add a test proving that empirically via
the real IsAuthenticated() entry point, and a test pinning down that a
refresh failure reached outside IsAuthenticated() produces no proactive
notification today. Clarify the stale comment above refresherFunc accordingly.
* fix: guard lastUsedToken with dedicated mutex to fix data race [IDE-2178]
doAuthCheck() runs under m.RLock(), letting multiple goroutines execute it
concurrently for the same token (singleflight only dedupes the API call
itself). The lastUsedToken read-check-write ran unsynchronized outside that
call, so concurrent IsAuthenticated() calls raced on it under -race, caught
by TestIsAuthenticated_ConcurrentCallsAllReturnSharedResult. Guard it with
its own mutex, mirroring the existing notifDedup pattern in this same struct.
* fix: reset lastUsedToken on logout [IDE-2178]
Logout left lastUsedToken holding the old value, so re-authenticating
with the same PAT/API token afterward made isNewToken false in
doAuthCheck and silently skipped sendAuthenticationAnalytics.
* test: assert stale OAuth token is cleared from storage on invalid_grant [IDE-2178]
Test_IsAuthenticated_DoesNotUseOAuth2ProviderCustomRefresherFunc only checked
IsAuthenticated() == false. Both the "temporary error, don't log out" and the
"logout fired" branches of doAuthCheck return false, so this didn't prove the
ticket's actual symptom (stale token left in storage) was fixed. Assert the
token is cleared from CONFIG_KEY_OAUTH_TOKEN after the call.
