diff --git a/pkg/connector/auth_recovery.go b/pkg/connector/auth_recovery.go index 853d764..3ee5901 100644 --- a/pkg/connector/auth_recovery.go +++ b/pkg/connector/auth_recovery.go @@ -9,7 +9,7 @@ import ( type lineCallDeps[T any] struct { newClient func() *line.Client - recover func(context.Context) error + recover func(context.Context, *line.Client, error) (*line.Client, error) isAuthError func(error) bool call func(*line.Client) (T, error) } @@ -27,13 +27,23 @@ func callLineWithRecovery[T any](ctx context.Context, client *line.Client, deps return client, res, err } - if errRecover := deps.recover(ctx); errRecover != nil { + recoveredClient, errRecover := deps.recover(ctx, client, err) + if errRecover != nil { var zero T return client, zero, fmt.Errorf("failed to recover token after LINE auth error (%w): %w", err, errRecover) } + if recoveredClient == nil { + return client, res, err + } - client = deps.newClient() + client = recoveredClient res, err = deps.call(client) + if line.IsLoggedOut(err) { + // The retry is the final attempt, but a current-token logout still needs + // to transition the login to BAD_CREDENTIALS. The source-aware recovery + // callback will ignore it if another token rotation made this retry stale. + _, _ = deps.recover(ctx, client, err) + } return client, res, err } @@ -44,10 +54,74 @@ func (lc *LineClient) isTokenError(err error) bool { if lc.isSessionInvalidated() { return false } + return line.IsAuthError(err) +} + +// recoverClientAfterAuthError classifies an auth error using the exact client +// that produced it. Logged-out responses from an older access token are safe to +// retry after a concurrent refresh/re-login; the same response from the current +// token is a genuine forced logout and must invalidate the session. +func (lc *LineClient) recoverClientAfterAuthError(ctx context.Context, failedClient *line.Client, err error) (*line.Client, error) { + if !lc.isTokenError(err) { + return nil, nil + } + + // Wait behind any in-flight refresh/re-login before comparing tokens. This + // makes the comparison authoritative even when the failed request completed + // while another goroutine was rotating the access token. + lc.recoverMu.Lock() + if ctx.Err() != nil { + lc.recoverMu.Unlock() + return nil, ctx.Err() + } + + currentToken := lc.getAccessToken() + if failedClient != nil && failedClient.AccessToken != "" && currentToken != "" && failedClient.AccessToken != currentToken && !lc.isSessionInvalidated() { + if lc.UserLogin != nil && lc.UserLogin.Bridge != nil { + lc.UserLogin.Bridge.Log.Debug(). + Bool("logged_out", line.IsLoggedOut(err)). + Bool("stale_access_token", true). + Msg("Retrying LINE request after response from stale access token") + } + lc.recoverMu.Unlock() + return newLineAPIClient(currentToken), nil + } + if line.IsLoggedOut(err) { - return false + lc.markLoggedOutByOtherClientLocked(ctx, err) + lc.recoverMu.Unlock() + return nil, nil } - return line.IsAuthError(err) + if lc.recoveryStopped || lc.superseded.Load() { + lc.recoverMu.Unlock() + return nil, errLineClientSuperseded + } + if lc.isSessionInvalidated() { + lc.recoverMu.Unlock() + return nil, errLineSessionInvalidated + } + lc.recoverMu.Unlock() + + recoveryToken := lc.getAccessToken() + if errRecover := recoverLineToken(lc, ctx); errRecover != nil { + if line.IsLoggedOut(errRecover) { + // Refresh/re-login errors come from the token that was current when + // recovery started. Classify them with the same source-aware path in + // case another serialized recovery rotated that token first. + return lc.recoverClientAfterAuthError(ctx, newLineAPIClient(recoveryToken), errRecover) + } + return nil, errRecover + } + if ctx.Err() != nil { + return nil, ctx.Err() + } + if lc.superseded.Load() { + return nil, errLineClientSuperseded + } + if lc.isSessionInvalidated() { + return nil, errLineSessionInvalidated + } + return lc.newClient(), nil } func (lc *LineClient) callLine(ctx context.Context, call func(*line.Client) error) (*line.Client, error) { @@ -57,15 +131,12 @@ func (lc *LineClient) callLine(ctx context.Context, call func(*line.Client) erro func (lc *LineClient) callLineUsing(ctx context.Context, client *line.Client, call func(*line.Client) error) (*line.Client, error) { client, _, err := callLineWithRecovery(ctx, client, lineCallDeps[struct{}]{ newClient: func() *line.Client { return lc.newClient() }, - recover: func(ctx context.Context) error { return recoverLineToken(lc, ctx) }, + recover: lc.recoverClientAfterAuthError, isAuthError: lc.isTokenError, call: func(client *line.Client) (struct{}, error) { return struct{}{}, call(client) }, }) - if lc.isLoggedOut(err) { - lc.markLoggedOutByOtherClient(ctx, err) - } return client, err } @@ -76,12 +147,9 @@ func callLineResult[T any](lc *LineClient, ctx context.Context, call func(*line. func callLineResultUsing[T any](lc *LineClient, ctx context.Context, client *line.Client, call func(*line.Client) (T, error)) (*line.Client, T, error) { client, res, err := callLineWithRecovery(ctx, client, lineCallDeps[T]{ newClient: func() *line.Client { return lc.newClient() }, - recover: func(ctx context.Context) error { return recoverLineToken(lc, ctx) }, + recover: lc.recoverClientAfterAuthError, isAuthError: lc.isTokenError, call: call, }) - if lc.isLoggedOut(err) { - lc.markLoggedOutByOtherClient(ctx, err) - } return client, res, err } diff --git a/pkg/connector/auth_recovery_test.go b/pkg/connector/auth_recovery_test.go index e6f2453..1eed26c 100644 --- a/pkg/connector/auth_recovery_test.go +++ b/pkg/connector/auth_recovery_test.go @@ -90,9 +90,12 @@ func TestCallLineWithRecovery(t *testing.T) { newClient: func() *line.Client { return line.NewClient("token") }, - recover: func(context.Context) error { + recover: func(context.Context, *line.Client, error) (*line.Client, error) { recoveries++ - return tt.recoverErr + if tt.recoverErr != nil { + return nil, tt.recoverErr + } + return line.NewClient("recovered"), nil }, isAuthError: line.IsAuthError, call: func(*line.Client) (struct{}, error) { @@ -138,8 +141,8 @@ func TestCallLineWithRecoveryReusesClientUntilRecovery(t *testing.T) { newClients++ return refreshedClient }, - recover: func(context.Context) error { - return nil + recover: func(context.Context, *line.Client, error) (*line.Client, error) { + return refreshedClient, nil }, isAuthError: line.IsAuthError, call: func(client *line.Client) (struct{}, error) { @@ -156,8 +159,8 @@ func TestCallLineWithRecoveryReusesClientUntilRecovery(t *testing.T) { if client != refreshedClient { t.Fatal("expected recovered client to be returned") } - if newClients != 1 { - t.Fatalf("new clients = %d, want 1", newClients) + if newClients != 0 { + t.Fatalf("new clients = %d, want 0 because recovery returned the retry client", newClients) } if len(calls) != 2 || calls[0] != "initial" || calls[1] != "refreshed" { t.Fatalf("calls used clients %v, want [initial refreshed]", calls) @@ -174,7 +177,9 @@ func TestCallLineWithRecoveryUsesProvidedClientWithoutRecreating(t *testing.T) { newClients++ return line.NewClient("unexpected") }, - recover: func(context.Context) error { return nil }, + recover: func(context.Context, *line.Client, error) (*line.Client, error) { + return line.NewClient("unexpected"), nil + }, isAuthError: line.IsAuthError, call: func(client *line.Client) (struct{}, error) { if client.AccessToken != "initial" { @@ -194,16 +199,16 @@ func TestCallLineWithRecoveryUsesProvidedClientWithoutRecreating(t *testing.T) { } } -func TestLineClientIsTokenErrorExcludesNonRecoverableErrors(t *testing.T) { +func TestLineClientIsTokenErrorClassifiesRecoverableErrors(t *testing.T) { lc := &LineClient{} if !lc.isTokenError(errAuthRequired) { t.Fatal("expected auth-required error to be classified as token error") } - if lc.isTokenError(errLoggedOut) { - t.Fatal("logged-out sessions must not trigger token recovery") + if !lc.isTokenError(errLoggedOut) { + t.Fatal("logged-out sessions must reach source-aware auth handling") } - if lc.isTokenError(errSenderKey) { - t.Fatal("invalid sender key sessions must not trigger token recovery") + if !lc.isTokenError(errSenderKey) { + t.Fatal("invalid sender key sessions must reach source-aware auth handling") } lc.sessionInvalidated = true if lc.isTokenError(errAuthRequired) { @@ -218,6 +223,125 @@ func TestLineClientIsTokenErrorExcludesNonRecoverableErrors(t *testing.T) { } } +func TestCallLineUsingRetriesStaleLogoutWithCurrentToken(t *testing.T) { + lc := &LineClient{AccessToken: "current-token"} + var calls []string + client, err := lc.callLineUsing(context.Background(), line.NewClient("old-token"), func(client *line.Client) error { + calls = append(calls, client.AccessToken) + if client.AccessToken == "old-token" { + return errLoggedOut + } + return nil + }) + if err != nil { + t.Fatalf("callLineUsing returned error: %v", err) + } + if client == nil || client.AccessToken != "current-token" { + t.Fatalf("client = %#v, want current-token", client) + } + if len(calls) != 2 || calls[0] != "old-token" || calls[1] != "current-token" { + t.Fatalf("call tokens = %v, want [old-token current-token]", calls) + } + if lc.isSessionInvalidated() { + t.Fatal("stale logout invalidated the current session") + } +} + +func TestRecoveryLoggedOutErrorInvalidatesCurrentSession(t *testing.T) { + oldRecover := recoverLineToken + t.Cleanup(func() { + recoverLineToken = oldRecover + }) + recoverLineToken = func(*LineClient, context.Context) error { + return errLoggedOut + } + + lc := &LineClient{AccessToken: "current-token"} + retryClient, err := lc.recoverClientAfterAuthError(context.Background(), line.NewClient("current-token"), errAuthRequired) + if err != nil { + t.Fatalf("recoverClientAfterAuthError returned error: %v", err) + } + if retryClient != nil { + t.Fatalf("retry client = %#v, want nil", retryClient) + } + if lc.hasAccessToken() || !lc.isSessionInvalidated() { + t.Fatal("logged-out recovery error did not invalidate the current session") + } +} + +func TestConcurrentOldTokenFailuresWaitForSingleRecovery(t *testing.T) { + oldRecover := recoverLineToken + t.Cleanup(func() { + recoverLineToken = oldRecover + }) + + lc := &LineClient{AccessToken: "old-token"} + recoveryStarted := make(chan struct{}) + allowRecovery := make(chan struct{}) + var recoveryCalls atomic.Int32 + recoverLineToken = func(lc *LineClient, ctx context.Context) error { + return lc.runTokenRecovery(ctx, func(context.Context) error { + if recoveryCalls.Add(1) == 1 { + close(recoveryStarted) + } + <-allowRecovery + lc.setTokens("new-token", "") + return nil + }) + } + + primaryDone := make(chan error, 1) + go func() { + _, err := lc.callLineUsing(context.Background(), line.NewClient("old-token"), func(client *line.Client) error { + if client.AccessToken == "old-token" { + return errAuthRequired + } + return nil + }) + primaryDone <- err + }() + + select { + case <-recoveryStarted: + case <-time.After(time.Second): + t.Fatal("primary recovery did not start") + } + + const staleCalls = 8 + var started sync.WaitGroup + started.Add(staleCalls) + staleDone := make(chan error, staleCalls) + for range staleCalls { + go func() { + _, err := lc.callLineUsing(context.Background(), line.NewClient("old-token"), func(client *line.Client) error { + if client.AccessToken == "old-token" { + started.Done() + return errLoggedOut + } + return nil + }) + staleDone <- err + }() + } + started.Wait() + close(allowRecovery) + + if err := <-primaryDone; err != nil { + t.Fatalf("primary call returned error: %v", err) + } + for range staleCalls { + if err := <-staleDone; err != nil { + t.Fatalf("stale call returned error: %v", err) + } + } + if got := recoveryCalls.Load(); got != 1 { + t.Fatalf("recovery calls = %d, want 1", got) + } + if lc.getAccessToken() != "new-token" || lc.isSessionInvalidated() { + t.Fatal("concurrent stale failures clobbered the recovered session") + } +} + func TestRunTokenRecoverySkipsRecentRecovery(t *testing.T) { lc := &LineClient{recoverTime: time.Now()} var calls int @@ -308,8 +432,15 @@ func TestRecoverTokenDoesNotReloginAfterCancellation(t *testing.T) { } } -func TestForcedLogoutWinsOverInFlightRecovery(t *testing.T) { +func TestStaleForcedLogoutDoesNotClobberInFlightRecovery(t *testing.T) { lc := &LineClient{AccessToken: "old-token"} + runCtx, _, started := lc.beginRun(context.Background()) + if !started { + t.Fatal("beginRun unexpectedly rejected startup") + } + defer lc.wg.Done() + defer lc.cancelActiveRun() + recoveryStarted := make(chan struct{}) allowRecovery := make(chan struct{}) recoveryDone := make(chan error, 1) @@ -323,10 +454,14 @@ func TestForcedLogoutWinsOverInFlightRecovery(t *testing.T) { }() <-recoveryStarted - logoutDone := make(chan struct{}) + type recoveryResult struct { + client *line.Client + err error + } + logoutDone := make(chan recoveryResult, 1) go func() { - lc.markLoggedOutByOtherClient(context.Background(), errLoggedOut) - close(logoutDone) + client, err := lc.recoverClientAfterAuthError(context.Background(), line.NewClient("old-token"), errLoggedOut) + logoutDone <- recoveryResult{client: client, err: err} }() close(allowRecovery) @@ -334,15 +469,52 @@ func TestForcedLogoutWinsOverInFlightRecovery(t *testing.T) { t.Fatalf("recovery returned error: %v", err) } select { - case <-logoutDone: + case result := <-logoutDone: + if result.err != nil { + t.Fatalf("stale logout handling returned error: %v", result.err) + } + if result.client == nil || result.client.AccessToken != "recovered-token" { + t.Fatalf("retry client = %#v, want recovered-token", result.client) + } case <-time.After(time.Second): - t.Fatal("forced logout did not complete after recovery") + t.Fatal("stale logout handling did not complete after recovery") } - if lc.hasAccessToken() { - t.Fatal("in-flight recovery resurrected the invalidated session") + if lc.getAccessToken() != "recovered-token" { + t.Fatalf("access token = %q, want recovered-token", lc.getAccessToken()) } - if !lc.isSessionInvalidated() { - t.Fatal("session was not invalidated after in-flight recovery") + if lc.isSessionInvalidated() { + t.Fatal("stale logout invalidated the recovered session") + } + select { + case <-runCtx.Done(): + t.Fatal("stale logout canceled the active run") + default: + } +} + +func TestCurrentTokenLoggedOutErrorsInvalidateSession(t *testing.T) { + tests := []struct { + name string + err error + }{ + {name: "client logged out", err: errLoggedOut}, + {name: "invalid sender key", err: errSenderKey}, + {name: "request need login", err: errors.New(`SSE error: 401: {"code":10004,"message":"REQUEST_NEED_LOGIN"}`)}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + lc := &LineClient{AccessToken: "current-token"} + retryClient, err := lc.recoverClientAfterAuthError(context.Background(), line.NewClient("current-token"), tt.err) + if err != nil { + t.Fatalf("recoverClientAfterAuthError returned error: %v", err) + } + if retryClient != nil { + t.Fatalf("retry client = %#v, want nil", retryClient) + } + if lc.hasAccessToken() || !lc.isSessionInvalidated() { + t.Fatal("current-token logout did not invalidate the session") + } + }) } } diff --git a/pkg/connector/client.go b/pkg/connector/client.go index 4415b17..b0e3786 100644 --- a/pkg/connector/client.go +++ b/pkg/connector/client.go @@ -307,35 +307,20 @@ func (lc *LineClient) refreshAndSave(ctx context.Context) error { return nil } -func (lc *LineClient) isRefreshRequired(err error) bool { - return line.IsRefreshRequired(err) -} - func (lc *LineClient) isLoggedOut(err error) bool { return line.IsLoggedOut(err) } -func (lc *LineClient) shouldAttemptTokenRecovery(ctx context.Context, err error) bool { - if err == nil { - return false - } - if ctx.Err() != nil || lc.superseded.Load() || lc.isSessionInvalidated() { - return false - } - if lc.isLoggedOut(err) { - lc.markLoggedOutByOtherClient(ctx, err) - return false - } - return lc.isRefreshRequired(err) || line.IsUnauthorizedStatus(err) -} - func (lc *LineClient) markLoggedOutByOtherClient(ctx context.Context, err error) { - // Serialize invalidation with token recovery. If a refresh/re-login was - // already in flight, it may finish first, but this transition always runs - // afterward so recovery cannot resurrect a forcefully logged-out session. lc.recoverMu.Lock() defer lc.recoverMu.Unlock() + lc.markLoggedOutByOtherClientLocked(ctx, err) +} +// markLoggedOutByOtherClientLocked applies the persistent forced-logout +// transition. The caller must hold recoverMu so token rotation and invalidation +// cannot interleave. +func (lc *LineClient) markLoggedOutByOtherClientLocked(ctx context.Context, err error) { if lc.UserLogin == nil { lc.invalidateAccessToken() line.InvalidateOBSTokenCache() @@ -783,42 +768,20 @@ func (lc *LineClient) refreshLoginE2EEKeys(res *line.LoginResult, meta *UserLogi } func (lc *LineClient) ensureValidToken(ctx context.Context) error { - return lc.ensureValidTokenWith( - ctx, - func(ctx context.Context) error { - _, err := getProfileWithToken(ctx, lc.getAccessToken()) - return err - }, - lc.refreshAndSave, - lc.tryLogin, - ) -} - -func (lc *LineClient) ensureValidTokenWith( - ctx context.Context, - profile func(context.Context) error, - refresh func(context.Context) error, - relogin func(context.Context) error, -) error { - err := profile(ctx) + _, _, err := callLineResult(lc, ctx, func(client *line.Client) (*line.Profile, error) { + return getProfileWithToken(ctx, client.AccessToken) + }) if err == nil { return nil } if ctx.Err() != nil { return ctx.Err() } - - if lc.isLoggedOut(err) { + if line.IsAuthError(err) { return err } - - if !lc.isRefreshRequired(err) { - lc.UserLogin.Bridge.Log.Warn().Err(err).Msg("GetProfile failed with non-auth error, continuing anyway") - return nil - } - - lc.UserLogin.Bridge.Log.Info().Msg("Access token expired, attempting refresh...") - return lc.recoverTokenWith(ctx, refresh, relogin) + lc.UserLogin.Bridge.Log.Warn().Err(err).Msg("GetProfile failed with non-auth error, continuing anyway") + return nil } func (lc *LineClient) Disconnect() { diff --git a/pkg/connector/creategroup.go b/pkg/connector/creategroup.go index 73b6d2d..28f9646 100644 --- a/pkg/connector/creategroup.go +++ b/pkg/connector/creategroup.go @@ -32,18 +32,11 @@ func (lc *LineClient) CreateGroup(ctx context.Context, params *bridgev2.GroupCre name = params.Name.Name } - client := lc.newClient() - var chat *line.Chat - var err error chatType := 1 // ROOM: members join automatically. lineName := name - chat, err = client.CreateChat(participantMids, lineName, chatType) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - chat, err = client.CreateChat(participantMids, lineName, chatType) - } - } + _, chat, err := callLineResult(lc, ctx, func(client *line.Client) (*line.Chat, error) { + return client.CreateChat(participantMids, lineName, chatType) + }) if err != nil { return nil, fmt.Errorf("failed to create LINE chat: %w", err) } @@ -274,16 +267,11 @@ func (lc *LineClient) registerGroupKey(ctx context.Context, chatMid string, memb keyIds = append(keyIds, selfRawID) encryptedKeys = append(encryptedKeys, selfEncryptedKey) - if err := client.RegisterE2EEGroupKey(1, chatMid, apiMembers, keyIds, encryptedKeys); err != nil { - if lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - err = client.RegisterE2EEGroupKey(1, chatMid, apiMembers, keyIds, encryptedKeys) - } - } - if err != nil { - return fmt.Errorf("registerE2EEGroupKey failed: %w", err) - } + _, err = lc.callLineUsing(ctx, client, func(client *line.Client) error { + return client.RegisterE2EEGroupKey(1, chatMid, apiMembers, keyIds, encryptedKeys) + }) + if err != nil { + return fmt.Errorf("registerE2EEGroupKey failed: %w", err) } lc.UserLogin.Bridge.Log.Info(). diff --git a/pkg/connector/e2ee_keys.go b/pkg/connector/e2ee_keys.go index 51b2f0b..2fc2821 100644 --- a/pkg/connector/e2ee_keys.go +++ b/pkg/connector/e2ee_keys.go @@ -39,7 +39,7 @@ func (lc *LineClient) fetchAndUnwrapGroupKey(ctx context.Context, chatMid string } client := lc.newClient() - fetch := func() (*line.E2EEGroupSharedKey, error) { + fetch := func(client *line.Client) (*line.E2EEGroupSharedKey, error) { var sharedKey *line.E2EEGroupSharedKey var err error if groupKeyID > 0 { @@ -50,7 +50,7 @@ func (lc *LineClient) fetchAndUnwrapGroupKey(ctx context.Context, chatMid string return sharedKey, groupKeyFetchError(groupKeyID, err) } - sharedKey, err := fetch() + client, sharedKey, err := callLineResultUsing(lc, ctx, client, fetch) // No group key exists yet — auto-register one so the group can use E2EE. if err != nil && line.IsGroupKeyNotFound(err) { lc.UserLogin.Bridge.Log.Info().Str("chat_mid", chatMid). @@ -60,16 +60,7 @@ func (lc *LineClient) fetchAndUnwrapGroupKey(ctx context.Context, chatMid string Msg("Auto-register group key failed") return fmt.Errorf("auto-register group key: %w", registerErr) } - sharedKey, err = fetch() - } - // Token recovery for other error types - if err != nil && !line.IsNoUsableE2EEGroupKey(err) && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - sharedKey, err = fetch() - } else { - return fmt.Errorf("failed to recover token before fetching group key: %w", errRecover) - } + client, sharedKey, err = callLineResultUsing(lc, ctx, client, fetch) } if err != nil { return err @@ -115,7 +106,7 @@ func (lc *LineClient) fetchAndUnwrapGroupKey(ctx context.Context, chatMid string if registerErr := lc.autoRegisterGroupKey(ctx, chatMid); registerErr != nil { return fmt.Errorf("%w (fresh group key registration failed: %v)", err, registerErr) } - sharedKey, err = fetch() + _, sharedKey, err = callLineResultUsing(lc, ctx, client, fetch) if err != nil { return fmt.Errorf("failed to fetch fresh group key after registration: %w", err) } @@ -242,18 +233,11 @@ func joinedGroupMemberMIDs(group *line.GroupExtra, ownMID string) ([]string, boo // Pending invitees are deliberately excluded: LINE validates group keys against the // current joined-member set and rejects keys that include invitees. func (lc *LineClient) getChatMemberMIDs(ctx context.Context, chatMid string) ([]string, bool, error) { - client := lc.newClient() - chats, err := client.GetChats([]string{chatMid}, true, true) + _, chats, err := callLineResult(lc, ctx, func(client *line.Client) (*line.GetChatsResponse, error) { + return client.GetChats([]string{chatMid}, true, true) + }) if err != nil { - if lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - chats, err = client.GetChats([]string{chatMid}, true, true) - } - } - if err != nil { - return nil, false, fmt.Errorf("getChats failed for %s: %w", chatMid, err) - } + return nil, false, fmt.Errorf("getChats failed for %s: %w", chatMid, err) } if len(chats.Chats) == 0 { return nil, false, fmt.Errorf("chat %s not found", chatMid) diff --git a/pkg/connector/forced_logout_test.go b/pkg/connector/forced_logout_test.go index 572d09e..0253999 100644 --- a/pkg/connector/forced_logout_test.go +++ b/pkg/connector/forced_logout_test.go @@ -50,30 +50,57 @@ func TestEnsureValidTokenReturnsLoggedOutWithoutRelogin(t *testing.T) { } func TestEnsureValidTokenDoesNotReloginAfterLoggedOutRefresh(t *testing.T) { + oldGetProfile := getProfileWithToken + oldRecover := recoverLineToken + t.Cleanup(func() { + getProfileWithToken = oldGetProfile + recoverLineToken = oldRecover + }) + lc := &LineClient{ + AccessToken: "expired", UserLogin: &bridgev2.UserLogin{ Bridge: &bridgev2.Bridge{Log: zerolog.New(io.Discard)}, }, } var reloginCalls int - err := lc.ensureValidTokenWith( - context.Background(), - func(context.Context) error { return errAuthRequired }, - func(context.Context) error { return errLoggedOut }, - func(context.Context) error { - reloginCalls++ - return nil - }, - ) - if !line.IsLoggedOut(err) { - t.Fatalf("ensureValidTokenWith error = %v, want logged-out error", err) + getProfileWithToken = func(_ context.Context, token string) (*line.Profile, error) { + if token != "expired" { + t.Fatalf("profile token = %q, want expired", token) + } + return nil, errAuthRequired + } + recoverLineToken = func(lc *LineClient, ctx context.Context) error { + return lc.recoverTokenWith( + ctx, + func(context.Context) error { return errLoggedOut }, + func(context.Context) error { + reloginCalls++ + return nil + }, + ) + } + + err := lc.ensureValidToken(context.Background()) + if !line.IsAuthError(err) { + t.Fatalf("ensureValidToken error = %v, want auth error", err) } if reloginCalls != 0 { t.Fatalf("relogin calls = %d, want 0", reloginCalls) } + if lc.hasAccessToken() || !lc.isSessionInvalidated() { + t.Fatal("logged-out refresh did not invalidate the session") + } } func TestForcedLogoutWinsOverEnsureValidTokenRefresh(t *testing.T) { + oldGetProfile := getProfileWithToken + oldRecover := recoverLineToken + t.Cleanup(func() { + getProfileWithToken = oldGetProfile + recoverLineToken = oldRecover + }) + lc := &LineClient{ AccessToken: "old-token", UserLogin: &bridgev2.UserLogin{ @@ -84,10 +111,18 @@ func TestForcedLogoutWinsOverEnsureValidTokenRefresh(t *testing.T) { allowRefresh := make(chan struct{}) ensureDone := make(chan error, 1) var reloginCalls int - go func() { - ensureDone <- lc.ensureValidTokenWith( - context.Background(), - func(context.Context) error { return errAuthRequired }, + getProfileWithToken = func(_ context.Context, token string) (*line.Profile, error) { + if token == "recovered-token" { + return &line.Profile{}, nil + } + if token != "old-token" { + t.Fatalf("profile token = %q, want old-token or recovered-token", token) + } + return nil, errAuthRequired + } + recoverLineToken = func(lc *LineClient, ctx context.Context) error { + return lc.recoverTokenWith( + ctx, func(context.Context) error { close(refreshStarted) <-allowRefresh @@ -99,6 +134,9 @@ func TestForcedLogoutWinsOverEnsureValidTokenRefresh(t *testing.T) { return nil }, ) + } + go func() { + ensureDone <- lc.ensureValidToken(context.Background()) }() <-refreshStarted @@ -110,7 +148,7 @@ func TestForcedLogoutWinsOverEnsureValidTokenRefresh(t *testing.T) { close(allowRefresh) if err := <-ensureDone; err != nil { - t.Fatalf("ensureValidTokenWith returned error: %v", err) + t.Fatalf("ensureValidToken returned error: %v", err) } if reloginCalls != 0 { t.Fatalf("relogin calls = %d, want 0", reloginCalls) diff --git a/pkg/connector/handle_message.go b/pkg/connector/handle_message.go index 685a050..3e92a56 100644 --- a/pkg/connector/handle_message.go +++ b/pkg/connector/handle_message.go @@ -29,15 +29,11 @@ const ( func (lc *LineClient) newMessageHandler() *handlers.Handler { return &handlers.Handler{ - Log: lc.UserLogin.Bridge.Log, - HTTPClient: lc.HTTPClient, - RecoverToken: lc.recoverToken, - ShouldRecover: lc.shouldAttemptTokenRecovery, - IsRefreshRequired: lc.isRefreshRequired, - IsLoggedOut: lc.isLoggedOut, - HandleLoggedOut: lc.markLoggedOutByOtherClient, - NewClient: func() *line.Client { return lc.newClient() }, - DecryptMedia: lc.decryptImageData, + Log: lc.UserLogin.Bridge.Log, + HTTPClient: lc.HTTPClient, + RecoverClient: lc.recoverClientAfterAuthError, + NewClient: func() *line.Client { return lc.newClient() }, + DecryptMedia: lc.decryptImageData, } } diff --git a/pkg/connector/handlers/audio.go b/pkg/connector/handlers/audio.go index 54e4832..1e2236b 100644 --- a/pkg/connector/handlers/audio.go +++ b/pkg/connector/handlers/audio.go @@ -48,10 +48,11 @@ func (h *Handler) ConvertAudio(ctx context.Context, portal *bridgev2.Portal, int talkMetaMessageID := obsTalkMetaMessageID(data.ID, isPlainMedia) audioData, err := client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions) - if newClient, ok := h.tryRecoverClient(ctx, err); ok { + if newClient, ok := h.tryRecoverClient(ctx, client, err); ok { client = newClient audioData, err = client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions) } + h.handleFinalAuthError(ctx, client, err) if err != nil { h.Log.Warn(). diff --git a/pkg/connector/handlers/file.go b/pkg/connector/handlers/file.go index d4be6f1..e2eac47 100644 --- a/pkg/connector/handlers/file.go +++ b/pkg/connector/handlers/file.go @@ -39,10 +39,11 @@ func (h *Handler) ConvertFile(ctx context.Context, portal *bridgev2.Portal, inte talkMetaMessageID := obsTalkMetaMessageID(data.ID, isPlainMedia) fileData, err := client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions) - if newClient, ok := h.tryRecoverClient(ctx, err); ok { + if newClient, ok := h.tryRecoverClient(ctx, client, err); ok { client = newClient fileData, err = client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions) } + h.handleFinalAuthError(ctx, client, err) if err != nil { h.Log.Warn(). diff --git a/pkg/connector/handlers/handler.go b/pkg/connector/handlers/handler.go index 3a2b1cd..1e34086 100644 --- a/pkg/connector/handlers/handler.go +++ b/pkg/connector/handlers/handler.go @@ -19,12 +19,9 @@ type Handler struct { Log zerolog.Logger HTTPClient *http.Client - // RecoverToken attempts to restore a valid session by refreshing or re-logging in. - RecoverToken func(ctx context.Context) error - ShouldRecover func(ctx context.Context, err error) bool - IsRefreshRequired func(err error) bool - IsLoggedOut func(err error) bool - HandleLoggedOut func(ctx context.Context, err error) + // RecoverClient classifies auth errors using the client that made the failed + // request, and returns a client that may be used for one retry. + RecoverClient func(ctx context.Context, failedClient *line.Client, err error) (*line.Client, error) // NewClient creates a new LINE API client with the current access token. NewClient func() *line.Client @@ -77,26 +74,26 @@ func mediaDownloadFailure(kind string, err error, relatesTo *event.RelatesTo) (* // tryRecoverClient attempts token recovery on auth errors and returns a fresh client. // Returns (newClient, true) on success, (nil, false) if recovery was not needed or failed. -func (h *Handler) tryRecoverClient(ctx context.Context, err error) (*line.Client, bool) { - if err == nil { +func (h *Handler) tryRecoverClient(ctx context.Context, failedClient *line.Client, err error) (*line.Client, bool) { + if err == nil || h.RecoverClient == nil { return nil, false } - if h.IsLoggedOut(err) { - if h.HandleLoggedOut != nil { - h.HandleLoggedOut(ctx, err) - } + recoveredClient, errRecover := h.RecoverClient(ctx, failedClient, err) + if errRecover != nil { + h.Log.Warn().Err(errRecover).Msg("Failed to recover token for media download") return nil, false } - if h.ShouldRecover != nil { - if !h.ShouldRecover(ctx, err) { - return nil, false - } - } else if !line.IsUnauthorizedStatus(err) && !h.IsRefreshRequired(err) { - return nil, false + return recoveredClient, recoveredClient != nil +} + +// handleFinalAuthError applies forced-logout handling to the one allowed retry +// without requesting another retry. Source-aware recovery will ignore the error +// if another token rotation already made the retry client stale. +func (h *Handler) handleFinalAuthError(ctx context.Context, failedClient *line.Client, err error) { + if !line.IsLoggedOut(err) || h.RecoverClient == nil { + return } - if errRecover := h.RecoverToken(ctx); errRecover != nil { - h.Log.Warn().Err(errRecover).Msg("Failed to recover token for media download") - return nil, false + if _, errRecover := h.RecoverClient(ctx, failedClient, err); errRecover != nil { + h.Log.Warn().Err(errRecover).Msg("Failed to handle LINE logout after media retry") } - return h.NewClient(), true } diff --git a/pkg/connector/handlers/handler_test.go b/pkg/connector/handlers/handler_test.go index 606adad..4e16257 100644 --- a/pkg/connector/handlers/handler_test.go +++ b/pkg/connector/handlers/handler_test.go @@ -11,60 +11,79 @@ import ( "github.com/highesttt/matrix-line-messenger/pkg/line" ) -func TestTryRecoverClientUsesShouldRecover(t *testing.T) { +func TestTryRecoverClientPassesOriginatingClient(t *testing.T) { errAuth := errors.New("SSE error: 401") var recoverCalled bool + failedClient := line.NewClient("failed-token") h := &Handler{ - ShouldRecover: func(context.Context, error) bool { - return false - }, - IsLoggedOut: func(error) bool { - return false - }, - IsRefreshRequired: func(error) bool { - return true - }, - RecoverToken: func(context.Context) error { + RecoverClient: func(_ context.Context, client *line.Client, err error) (*line.Client, error) { recoverCalled = true - return nil + if client != failedClient { + t.Fatalf("failed client = %#v, want originating client", client) + } + if !errors.Is(err, errAuth) { + t.Fatalf("auth error = %v, want %v", err, errAuth) + } + return nil, nil }, } - client, ok := h.tryRecoverClient(context.Background(), errAuth) + client, ok := h.tryRecoverClient(context.Background(), failedClient, errAuth) if ok || client != nil { t.Fatalf("tryRecoverClient returned client=%v ok=%v, want no recovery", client, ok) } - if recoverCalled { - t.Fatal("RecoverToken was called despite ShouldRecover returning false") + if !recoverCalled { + t.Fatal("RecoverClient was not called") } } func TestTryRecoverClientRecoversOBSObjectInfoUnauthorized(t *testing.T) { recoveredClient := line.NewClient("refreshed-token") + failedClient := line.NewClient("expired-token") var recoverCalled bool h := &Handler{ - ShouldRecover: func(_ context.Context, err error) bool { - return line.IsUnauthorizedStatus(err) - }, - IsLoggedOut: func(error) bool { - return false - }, - RecoverToken: func(context.Context) error { + RecoverClient: func(_ context.Context, client *line.Client, err error) (*line.Client, error) { recoverCalled = true - return nil - }, - NewClient: func() *line.Client { - return recoveredClient + if client != failedClient { + t.Fatalf("failed client = %#v, want originating client", client) + } + if !line.IsUnauthorizedStatus(err) { + t.Fatalf("error = %v, want unauthorized status", err) + } + return recoveredClient, nil }, } - client, ok := h.tryRecoverClient(context.Background(), errors.New("OBS object info failed (401): unauthorized")) + client, ok := h.tryRecoverClient(context.Background(), failedClient, errors.New("OBS object info failed (401): unauthorized")) if !ok || client != recoveredClient { t.Fatalf("tryRecoverClient returned client=%v ok=%v, want refreshed client", client, ok) } if !recoverCalled { - t.Fatal("RecoverToken was not called for OBS object-info 401") + t.Fatal("RecoverClient was not called for OBS object-info 401") + } +} + +func TestHandleFinalAuthErrorKeepsRetrySourceClient(t *testing.T) { + errLoggedOut := errors.New("V3_TOKEN_CLIENT_LOGGED_OUT") + retryClient := line.NewClient("retry-token") + var calls int + h := &Handler{ + RecoverClient: func(_ context.Context, client *line.Client, err error) (*line.Client, error) { + calls++ + if client != retryClient { + t.Fatalf("failed client = %#v, want retry client", client) + } + if !errors.Is(err, errLoggedOut) { + t.Fatalf("error = %v, want logged-out error", err) + } + return nil, nil + }, + } + + h.handleFinalAuthError(context.Background(), retryClient, errLoggedOut) + if calls != 1 { + t.Fatalf("RecoverClient calls = %d, want 1", calls) } } diff --git a/pkg/connector/handlers/image.go b/pkg/connector/handlers/image.go index 1201d9c..83f315b 100644 --- a/pkg/connector/handlers/image.go +++ b/pkg/connector/handlers/image.go @@ -50,7 +50,7 @@ func (h *Handler) ConvertImage(ctx context.Context, portal *bridgev2.Portal, int } // Refresh token if we get a 401 - if newClient, ok := h.tryRecoverClient(ctx, err); ok { + if newClient, ok := h.tryRecoverClient(ctx, client, err); ok { client = newClient if isPlainMedia { imgData, err = client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, "m", downloadOptions) @@ -58,6 +58,7 @@ func (h *Handler) ConvertImage(ctx context.Context, portal *bridgev2.Portal, int imgData, err = client.DownloadOBSWithOptions(ctx, oid, talkMetaMessageID, downloadOptions) } } + h.handleFinalAuthError(ctx, client, err) downloadDuration := time.Since(dlStart) if err != nil { diff --git a/pkg/connector/handlers/post_notification.go b/pkg/connector/handlers/post_notification.go index fa80627..19a4226 100644 --- a/pkg/connector/handlers/post_notification.go +++ b/pkg/connector/handlers/post_notification.go @@ -211,15 +211,17 @@ func (h *Handler) convertAlbumPreview( previewContext.ChatID, previewContext.AlbumID, ) - if newClient, ok := h.tryRecoverClient(ctx, err); ok { + if newClient, ok := h.tryRecoverClient(ctx, client, err); ok { + client = newClient imageData, err = h.downloadAlbumPreview( ctx, - newClient, + client, media.OID, previewContext.ChatID, previewContext.AlbumID, ) } + h.handleFinalAuthError(ctx, client, err) if errors.Is(err, line.ErrOBSObjectNotFound) { h.Log.Warn(). Str("msg_id", messageID). diff --git a/pkg/connector/handlers/post_notification_test.go b/pkg/connector/handlers/post_notification_test.go index ffe33fb..035e16d 100644 --- a/pkg/connector/handlers/post_notification_test.go +++ b/pkg/connector/handlers/post_notification_test.go @@ -310,12 +310,6 @@ func TestConvertPostNotificationCancelsQueuedAlbumPreviewsAfterFailure(t *testin NewClient: func() *line.Client { return line.NewClient("token") }, - IsLoggedOut: func(error) bool { - return false - }, - ShouldRecover: func(context.Context, error) bool { - return false - }, DownloadAlbumPreview: func(ctx context.Context, _ *line.Client, oid, _, _ string) ([]byte, error) { calls.Add(1) if oid == "fatal-oid" { @@ -397,12 +391,6 @@ func TestConvertPostNotificationKeepsSuccessfulAlbumImagesWhenOneExpired(t *test NewClient: func() *line.Client { return line.NewClient("token") }, - IsLoggedOut: func(error) bool { - return false - }, - ShouldRecover: func(context.Context, error) bool { - return false - }, DownloadAlbumPreview: func(_ context.Context, _ *line.Client, oid, _, _ string) ([]byte, error) { if oid == "expired-oid" { return nil, line.ErrOBSObjectNotFound @@ -440,12 +428,6 @@ func TestConvertPostNotificationLeavesTransientAlbumFailureRetryable(t *testing. NewClient: func() *line.Client { return line.NewClient("token") }, - IsLoggedOut: func(error) bool { - return false - }, - ShouldRecover: func(context.Context, error) bool { - return false - }, DownloadAlbumPreview: func(context.Context, *line.Client, string, string, string) ([]byte, error) { return nil, line.ErrOBSEncodingIncomplete }, diff --git a/pkg/connector/handlers/video.go b/pkg/connector/handlers/video.go index f9195a1..1aca568 100644 --- a/pkg/connector/handlers/video.go +++ b/pkg/connector/handlers/video.go @@ -50,10 +50,11 @@ func (h *Handler) ConvertVideo(ctx context.Context, portal *bridgev2.Portal, int dlStart := time.Now() videoData, err := client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions) - if newClient, ok := h.tryRecoverClient(ctx, err); ok { + if newClient, ok := h.tryRecoverClient(ctx, client, err); ok { client = newClient videoData, err = client.DownloadOBSWithSIDOptions(ctx, oid, talkMetaMessageID, sid, downloadOptions) } + h.handleFinalAuthError(ctx, client, err) if err != nil { h.Log.Warn(). diff --git a/pkg/connector/sync.go b/pkg/connector/sync.go index 1239219..376eb69 100644 --- a/pkg/connector/sync.go +++ b/pkg/connector/sync.go @@ -117,14 +117,9 @@ var ( ) func (lc *LineClient) getMessageBoxesWithRecovery(ctx context.Context, opts line.MessageBoxesOptions) (*line.MessageBoxesResponse, error) { - client := lc.newClient() - res, err := client.GetMessageBoxes(opts) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - res, err = client.GetMessageBoxes(opts) - } - } + _, res, err := callLineResult(lc, ctx, func(client *line.Client) (*line.MessageBoxesResponse, error) { + return client.GetMessageBoxes(opts) + }) return res, err } @@ -157,14 +152,9 @@ func (lc *LineClient) fetchAllMessageBoxes(ctx context.Context, opts line.Messag } func (lc *LineClient) refreshBlockedContacts(ctx context.Context) ([]string, error) { - client := lc.newClient() - blockedMIDs, err := client.GetBlockedContactIds() - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - blockedMIDs, err = client.GetBlockedContactIds() - } - } + _, blockedMIDs, err := callLineResult(lc, ctx, func(client *line.Client) ([]string, error) { + return client.GetBlockedContactIds() + }) if err != nil { return nil, err } @@ -600,14 +590,9 @@ func (lc *LineClient) FetchMessages(ctx context.Context, params bridgev2.FetchMe limit = 50 } - client := lc.newClient() - msgs, err := client.GetRecentMessagesV2(chatMID, limit) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - msgs, err = client.GetRecentMessagesV2(chatMID, limit) - } - } + _, msgs, err := callLineResult(lc, ctx, func(client *line.Client) ([]*line.Message, error) { + return client.GetRecentMessagesV2(chatMID, limit) + }) if err != nil { if unblockState != nil { unblockState.complete() @@ -771,14 +756,9 @@ func collectStartupBackfillChatMIDs(messageBoxes []line.MessageBox, memberChatMI // (live) message path. Used by prefetchMessages on startup. func (lc *LineClient) backfillRecentMessages(ctx context.Context, chatMID string, limit int) bool { start := time.Now() - client := lc.newClient() - msgs, err := client.GetRecentMessagesV2(chatMID, limit) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - msgs, err = client.GetRecentMessagesV2(chatMID, limit) - } - } + _, msgs, err := callLineResult(lc, ctx, func(client *line.Client) ([]*line.Message, error) { + return client.GetRecentMessagesV2(chatMID, limit) + }) if err != nil { lc.UserLogin.Bridge.Log.Warn().Err(err).Str("chat_mid", chatMID).Msg("Failed to fetch recent messages") return false @@ -845,14 +825,9 @@ func (lc *LineClient) syncChats(ctx context.Context) { } func (lc *LineClient) syncChatsNow(ctx context.Context) { - client := lc.newClient() - midsResp, err := client.GetAllChatMids(true, true) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - midsResp, err = client.GetAllChatMids(true, true) - } - } + client, midsResp, err := callLineResult(lc, ctx, func(client *line.Client) (*line.GetAllChatMidsResponse, error) { + return client.GetAllChatMids(true, true) + }) if err != nil { lc.UserLogin.Bridge.Log.Warn().Err(err).Msg("Failed to fetch all chat mids") return @@ -877,13 +852,10 @@ func (lc *LineClient) syncChatsNow(ctx context.Context) { end = len(allMids) } batch := allMids[i:end] - chatsResp, err := client.GetChats(batch, true, true) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - chatsResp, err = client.GetChats(batch, true, true) - } - } + var chatsResp *line.GetChatsResponse + client, chatsResp, err = callLineResultUsing(lc, ctx, client, func(client *line.Client) (*line.GetChatsResponse, error) { + return client.GetChats(batch, true, true) + }) if err != nil { lc.UserLogin.Bridge.Log.Warn().Err(err).Msg("Failed to fetch batch of chats") continue @@ -1210,14 +1182,9 @@ func (lc *LineClient) cacheGroupMembersFromRecentMessages(ctx context.Context, c if len(lc.getCachedGroupMembers(chatMid)) > 1 { return } - client := lc.newClient() - msgs, err := client.GetRecentMessagesV2(chatMid, 50) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - msgs, err = client.GetRecentMessagesV2(chatMid, 50) - } - } + _, msgs, err := callLineResult(lc, ctx, func(client *line.Client) ([]*line.Message, error) { + return client.GetRecentMessagesV2(chatMid, 50) + }) if err != nil { lc.UserLogin.Bridge.Log.Debug().Err(err).Str("chat_mid", chatMid).Msg("Failed to fetch recent messages for group member cache") return @@ -1552,20 +1519,13 @@ func (lc *LineClient) pollLoop(ctx context.Context) { client := lc.newClient() lc.UserLogin.Bridge.Log.Info().Msg("Starting LINE SSE loop...") - rev, err := getLastOpRevisionWithClient(ctx, client) - if err != nil && lc.isLoggedOut(err) { - lc.markLoggedOutByOtherClient(ctx, err) - return - } - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - rev, err = getLastOpRevisionWithClient(ctx, client) - } else { - lc.UserLogin.Bridge.Log.Warn().Err(errRecover).Msg("Failed to recover token for getLastOpRevision") - } - } + _, rev, err := callLineResultUsing(lc, ctx, client, func(client *line.Client) (int64, error) { + return getLastOpRevisionWithClient(ctx, client) + }) if err != nil { + if lc.isSessionInvalidated() { + return + } lc.UserLogin.Bridge.Log.Warn().Err(err).Msg("Failed to get last op revision") } else { localRev = rev @@ -1677,13 +1637,8 @@ func (lc *LineClient) pollLoop(ctx context.Context) { continue } - if lc.isLoggedOut(err) { - lc.markLoggedOutByOtherClient(ctx, err) - return - } - - if line.IsUnauthorizedStatus(err) { - if lc.handleReceiveAuthError(ctx, err) { + if line.IsAuthError(err) { + if lc.handleReceiveAuthError(ctx, client, err) { return } } @@ -1708,7 +1663,8 @@ func (lc *LineClient) handleReceiveAuthProbe(ctx context.Context) bool { // This is only a health probe. Keep localRev unchanged so the reconnected // stream replays operations that arrived while the old stream was stalled. - _, probeErr := getLastOpRevisionWithClient(ctx, lc.newClient()) + probeClient := lc.newClient() + _, probeErr := getLastOpRevisionWithClient(ctx, probeClient) if probeErr == nil { return false } @@ -1716,27 +1672,26 @@ func (lc *LineClient) handleReceiveAuthProbe(ctx context.Context) bool { return true } - if lc.isLoggedOut(probeErr) { - lc.markLoggedOutByOtherClient(ctx, probeErr) - return true - } - if line.IsUnauthorizedStatus(probeErr) { - return lc.handleReceiveAuthError(ctx, probeErr) + return lc.handleReceiveAuthError(ctx, probeClient, probeErr) } - if lc.shouldAttemptTokenRecovery(ctx, probeErr) { - if errRecover := lc.recoverToken(ctx); errRecover != nil { - if errors.Is(errRecover, errLineSessionInvalidated) || lc.isLoggedOut(errRecover) { - lc.markLoggedOutByOtherClient(ctx, errRecover) - return true - } - if lc.UserLogin != nil && lc.UserLogin.Bridge != nil { - lc.UserLogin.Bridge.Log.Warn().Err(errRecover).Msg("Failed to recover token after receive auth probe") - } + recoveredClient, errRecover := lc.recoverClientAfterAuthError(ctx, probeClient, probeErr) + if errRecover != nil { + if errors.Is(errRecover, errLineSessionInvalidated) || errors.Is(errRecover, errLineClientSuperseded) || ctx.Err() != nil { + return true + } + if lc.UserLogin != nil && lc.UserLogin.Bridge != nil { + lc.UserLogin.Bridge.Log.Warn().Err(errRecover).Msg("Failed to recover token after receive auth probe") } return false } + if recoveredClient != nil { + return false + } + if lc.isSessionInvalidated() { + return true + } if lc.UserLogin != nil && lc.UserLogin.Bridge != nil { lc.UserLogin.Bridge.Log.Warn().Err(probeErr).Msg("Receive auth probe failed; reconnecting SSE") @@ -1747,50 +1702,61 @@ func (lc *LineClient) handleReceiveAuthProbe(ctx context.Context) bool { // handleReceiveAuthError handles auth failures from /operation/receive. The // receive endpoint may return only a bare 401/403, so probe getProfile to reveal // the detailed forced-logout envelope before deciding whether recovery is safe. -func (lc *LineClient) handleReceiveAuthError(ctx context.Context, err error) bool { +func (lc *LineClient) handleReceiveAuthError(ctx context.Context, failedClient *line.Client, err error) bool { if lc.isLoggedOut(err) { - lc.markLoggedOutByOtherClient(ctx, err) - return true + recoveredClient, errRecover := lc.recoverClientAfterAuthError(ctx, failedClient, err) + if errRecover != nil { + return true + } + return recoveredClient == nil } - _, profileErr := getProfileWithToken(ctx, lc.getAccessToken()) + profileToken := lc.getAccessToken() + if profileToken == "" && failedClient != nil { + profileToken = failedClient.AccessToken + } + profileClient := newLineAPIClient(profileToken) + _, profileErr := getProfileWithToken(ctx, profileToken) if ctx.Err() != nil { return true } if lc.isLoggedOut(profileErr) { - lc.markLoggedOutByOtherClient(ctx, profileErr) - return true + recoveredClient, errRecover := lc.recoverClientAfterAuthError(ctx, profileClient, profileErr) + if errRecover != nil { + return true + } + return recoveredClient == nil } if profileErr == nil { return false } - if !lc.shouldAttemptTokenRecovery(ctx, err) { + recoveredClient, errRecover := lc.recoverClientAfterAuthError(ctx, failedClient, err) + if recoveredClient != nil { + return false + } + if errRecover == nil { return true } - if errRecover := lc.recoverToken(ctx); errRecover != nil { - if ctx.Err() != nil { - return true - } - if errors.Is(errRecover, errLineSessionInvalidated) || lc.isLoggedOut(errRecover) { - lc.markLoggedOutByOtherClient(ctx, errRecover) - return true - } - if lc.UserLogin != nil && lc.UserLogin.Bridge != nil { - lc.UserLogin.Bridge.Log.Error().Err(errRecover).Msg("Failed to recover session, stopping poll loop") - } - if lc.UserLogin != nil && lc.UserLogin.BridgeState != nil { - lc.UserLogin.BridgeState.Send(status.BridgeState{ - StateEvent: status.StateBadCredentials, - Error: "line-logged-out", - Message: "LINE session was invalidated (logged out by another client). Please re-authenticate the bridge.", - UserAction: status.UserActionRelogin, - }) - } + if ctx.Err() != nil { return true } - return false + if errors.Is(errRecover, errLineSessionInvalidated) || errors.Is(errRecover, errLineClientSuperseded) { + return true + } + if lc.UserLogin != nil && lc.UserLogin.Bridge != nil { + lc.UserLogin.Bridge.Log.Error().Err(errRecover).Msg("Failed to recover session, stopping poll loop") + } + if lc.UserLogin != nil && lc.UserLogin.BridgeState != nil { + lc.UserLogin.BridgeState.Send(status.BridgeState{ + StateEvent: status.StateBadCredentials, + Error: "line-logged-out", + Message: "LINE session was invalidated (logged out by another client). Please re-authenticate the bridge.", + UserAction: status.UserActionRelogin, + }) + } + return true } func (lc *LineClient) handleOperation(ctx context.Context, op line.Operation) { @@ -1979,7 +1945,7 @@ func (lc *LineClient) handleOperation(ctx context.Context, op line.Operation) { // Curr == nil signals a reaction removal/clear from LINE. if param2.Curr == nil { lc.UserLogin.Bridge.Log.Debug().Str("msg_id", op.Param1).Str("chat_mid", param2.ChatMid).Msg("Received reaction removal (self)") - lc.handleReactionRemove(op, param2.ChatMid, []networkid.UserID{makeUserID(string(lc.UserLogin.ID))}) + lc.handleReactionRemove(op, param2.ChatMid, makeUserID(string(lc.UserLogin.ID))) return } @@ -2018,7 +1984,7 @@ func (lc *LineClient) handleOperation(ctx context.Context, op line.Operation) { // use the type 140 actor from param3, so the sender is unambiguous. if param2.Curr == nil { lc.UserLogin.Bridge.Log.Debug().Str("msg_id", op.Param1).Str("chat_mid", param2.ChatMid).Msg("Received reaction removal (other)") - lc.handleReactionRemove(op, param2.ChatMid, []networkid.UserID{makeUserID(op.Param3)}) + lc.handleReactionRemove(op, param2.ChatMid, makeUserID(op.Param3)) return } @@ -2126,29 +2092,21 @@ func (lc *LineClient) liveReactionSyncEvent( } } -// handleReactionRemove queues an authoritative empty reaction sync for each -// candidate sender. LINE only allows one reaction per sender, so this removes -// both legacy empty-ID rows and stable paid/predefined reaction IDs without -// needing the previous reaction type. -func (lc *LineClient) handleReactionRemove(op line.Operation, chatMid string, senders []networkid.UserID) { - for _, sender := range senders { - lc.UserLogin.Bridge.QueueRemoteEvent( - lc.UserLogin, - lc.liveReactionSyncEvent(op, chatMid, sender, nil), - ) - } +// handleReactionRemove queues an authoritative empty reaction sync for the +// sender, removing both legacy empty-ID rows and stable paid/predefined reaction +// IDs without needing the previous reaction type. +func (lc *LineClient) handleReactionRemove(op line.Operation, chatMid string, sender networkid.UserID) { + lc.UserLogin.Bridge.QueueRemoteEvent( + lc.UserLogin, + lc.liveReactionSyncEvent(op, chatMid, sender, nil), + ) } func (lc *LineClient) syncSingleChat(ctx context.Context, op line.Operation) { chatMid := op.Param1 - client := lc.newClient() - chatsResp, err := client.GetChats([]string{chatMid}, true, true) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - chatsResp, err = client.GetChats([]string{chatMid}, true, true) - } - } + _, chatsResp, err := callLineResult(lc, ctx, func(client *line.Client) (*line.GetChatsResponse, error) { + return client.GetChats([]string{chatMid}, true, true) + }) if err != nil { lc.UserLogin.Bridge.Log.Warn().Err(err).Str("chat_mid", chatMid).Msg("Failed to fetch chat info") // Only emit leave if we confirm the user is definitively not a member @@ -2202,14 +2160,9 @@ func (lc *LineClient) syncSingleChat(ctx context.Context, op line.Operation) { // checkChatMembership calls GetAllChatMids to verify whether the bridge user // is a member or invitee of the given chat. func (lc *LineClient) checkChatMembership(ctx context.Context, chatMid string) (isMember, isInvitee bool) { - client := lc.newClient() - midsResp, err := client.GetAllChatMids(true, true) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - midsResp, err = client.GetAllChatMids(true, true) - } - } + _, midsResp, err := callLineResult(lc, ctx, func(client *line.Client) (*line.GetAllChatMidsResponse, error) { + return client.GetAllChatMids(true, true) + }) if err != nil { lc.UserLogin.Bridge.Log.Warn().Err(err).Msg("checkChatMembership: GetAllChatMids failed") return false, false @@ -2329,14 +2282,9 @@ func (lc *LineClient) handleMemberJoin(chatMid, joinerMid string) { } func (lc *LineClient) handleInvite(ctx context.Context, chatMid string, opType OperationType) { - client := lc.newClient() - chatsResp, err := client.GetChats([]string{chatMid}, true, true) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - chatsResp, err = client.GetChats([]string{chatMid}, true, true) - } - } + _, chatsResp, err := callLineResult(lc, ctx, func(client *line.Client) (*line.GetChatsResponse, error) { + return client.GetChats([]string{chatMid}, true, true) + }) if err != nil { lc.UserLogin.Bridge.Log.Warn().Err(err).Str("chat_mid", chatMid).Msg("Failed to fetch chat info for invite") return @@ -2377,14 +2325,9 @@ func (lc *LineClient) handleInvite(ctx context.Context, chatMid string, opType O } func (lc *LineClient) handleInviteForSelf(ctx context.Context, chatMid string) { - client := lc.newClient() - chatsResp, err := client.GetChats([]string{chatMid}, true, true) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - chatsResp, err = client.GetChats([]string{chatMid}, true, true) - } - } + _, chatsResp, err := callLineResult(lc, ctx, func(client *line.Client) (*line.GetChatsResponse, error) { + return client.GetChats([]string{chatMid}, true, true) + }) if err != nil { lc.UserLogin.Bridge.Log.Warn().Err(err).Str("chat_mid", chatMid).Msg("Failed to fetch invited chat info") return diff --git a/pkg/connector/sync_test.go b/pkg/connector/sync_test.go index d16b7c8..c9bb435 100644 --- a/pkg/connector/sync_test.go +++ b/pkg/connector/sync_test.go @@ -848,7 +848,7 @@ func TestReceiveRequestNeedLoginMarksLoggedOutImmediately(t *testing.T) { } lc := &LineClient{AccessToken: "stale"} - stopped := lc.handleReceiveAuthError(context.Background(), errors.New(`SSE error: 401: {"code":10004,"message":"REQUEST_NEED_LOGIN"}`)) + stopped := lc.handleReceiveAuthError(context.Background(), line.NewClient("stale"), errors.New(`SSE error: 401: {"code":10004,"message":"REQUEST_NEED_LOGIN"}`)) if !stopped { t.Fatal("receive auth handler should stop on REQUEST_NEED_LOGIN") @@ -877,7 +877,7 @@ func TestReceiveAuthErrorWithValidProfileDoesNotRecover(t *testing.T) { } lc := &LineClient{AccessToken: "valid"} - stopped := lc.handleReceiveAuthError(context.Background(), errors.New("SSE error: 401")) + stopped := lc.handleReceiveAuthError(context.Background(), line.NewClient("valid"), errors.New("SSE error: 401")) if stopped { t.Fatal("receive auth handler should reconnect without stopping when the profile probe succeeds") @@ -893,6 +893,59 @@ func TestReceiveAuthErrorWithValidProfileDoesNotRecover(t *testing.T) { } } +func TestReceiveAuthErrorFromStaleSSEClientReconnectsCurrentToken(t *testing.T) { + oldGetProfile := getProfileWithToken + t.Cleanup(func() { + getProfileWithToken = oldGetProfile + }) + + var profileCalls int + getProfileWithToken = func(_ context.Context, token string) (*line.Profile, error) { + profileCalls++ + if token != "current-token" { + t.Fatalf("profile token = %q, want current token", token) + } + return &line.Profile{}, nil + } + + lc := &LineClient{AccessToken: "current-token"} + stopped := lc.handleReceiveAuthError(context.Background(), line.NewClient("old-token"), errors.New("SSE error: 401")) + + if stopped { + t.Fatal("stale SSE auth error stopped the current session") + } + if profileCalls != 1 { + t.Fatalf("profile calls = %d, want 1", profileCalls) + } + if lc.getAccessToken() != "current-token" || lc.isSessionInvalidated() { + t.Fatal("stale SSE auth error changed current session state") + } +} + +func TestReceiveAuthErrorFromStaleSSEClientClassifiesCurrentProbeLogout(t *testing.T) { + oldGetProfile := getProfileWithToken + t.Cleanup(func() { + getProfileWithToken = oldGetProfile + }) + + getProfileWithToken = func(_ context.Context, token string) (*line.Profile, error) { + if token != "current-token" { + t.Fatalf("profile token = %q, want current token", token) + } + return nil, errLoggedOut + } + + lc := &LineClient{AccessToken: "current-token"} + stopped := lc.handleReceiveAuthError(context.Background(), line.NewClient("old-token"), errors.New("SSE error: 401")) + + if !stopped { + t.Fatal("current-token profile logout should stop the session") + } + if lc.hasAccessToken() || !lc.isSessionInvalidated() { + t.Fatal("current-token profile logout was misclassified as a stale SSE response") + } +} + func TestReceiveAuthErrorCancellationDuringProfileDoesNotInvalidate(t *testing.T) { oldGetProfile := getProfileWithToken t.Cleanup(func() { @@ -913,7 +966,7 @@ func TestReceiveAuthErrorCancellationDuringProfileDoesNotInvalidate(t *testing.T lc := &LineClient{AccessToken: "valid"} result := make(chan bool, 1) go func() { - result <- lc.handleReceiveAuthError(ctx, errors.New("SSE error: 401")) + result <- lc.handleReceiveAuthError(ctx, line.NewClient("valid"), errors.New("SSE error: 401")) }() <-profileStarted cancel() diff --git a/pkg/connector/userinfo.go b/pkg/connector/userinfo.go index b2f6eb6..1d0f2b1 100644 --- a/pkg/connector/userinfo.go +++ b/pkg/connector/userinfo.go @@ -144,14 +144,9 @@ func (lc *LineClient) GetChatInfo(ctx context.Context, portal *bridgev2.Portal) mid := string(portal.ID) lowerMid := strings.ToLower(mid) if strings.HasPrefix(lowerMid, "c") || strings.HasPrefix(lowerMid, "r") { - client := lc.newClient() - res, err := client.GetChats([]string{mid}, true, true) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - res, err = client.GetChats([]string{mid}, true, true) - } - } + _, res, err := callLineResult(lc, ctx, func(client *line.Client) (*line.GetChatsResponse, error) { + return client.GetChats([]string{mid}, true, true) + }) if err != nil { return nil, err } @@ -211,14 +206,9 @@ func (lc *LineClient) getContact(ctx context.Context, mid string) line.Contact { // Use GetProfile for our own user data if mid == lc.Mid || mid == string(lc.UserLogin.ID) { - client := lc.newClient() - profile, err := client.GetProfile() - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - profile, err = client.GetProfile() - } - } + _, profile, err := callLineResult(lc, ctx, func(client *line.Client) (*line.Profile, error) { + return client.GetProfile() + }) if err == nil && profile != nil { contact := line.Contact{Mid: mid, DisplayName: profile.DisplayName, PicturePath: profile.PicturePath} lc.setCachedContact(mid, contact) @@ -228,13 +218,9 @@ func (lc *LineClient) getContact(ctx context.Context, mid string) line.Contact { } client := lc.newClient() - res, err := client.GetContactsV2([]string{mid}) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - res, err = client.GetContactsV2([]string{mid}) - } - } + client, res, err := callLineResultUsing(lc, ctx, client, func(client *line.Client) (*line.ContactsResponse, error) { + return client.GetContactsV2([]string{mid}) + }) if err == nil && res != nil && res.Contacts != nil { if wrapper, ok := res.Contacts[mid]; ok { lc.setCachedContact(mid, wrapper.Contact) @@ -244,13 +230,9 @@ func (lc *LineClient) getContact(ctx context.Context, mid string) line.Contact { // Fall back to BuddyService for official/business accounts lc.UserLogin.Bridge.Log.Debug().Str("mid", mid).Msg("Contact not found via GetContactsV2, trying BuddyService") - buddy, err := client.GetBuddyProfile(mid) - if err != nil && lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - buddy, err = client.GetBuddyProfile(mid) - } - } + _, buddy, err := callLineResultUsing(lc, ctx, client, func(client *line.Client) (*line.BuddyProfile, error) { + return client.GetBuddyProfile(mid) + }) if err == nil && buddy != nil { lc.UserLogin.Bridge.Log.Debug().Str("mid", mid).Str("display_name", buddy.DisplayName).Str("picture_path", buddy.PicturePath).Msg("Got buddy profile") contact := line.Contact{Mid: mid, DisplayName: buddy.DisplayName, PicturePath: buddy.PicturePath} @@ -332,19 +314,12 @@ func (lc *LineClient) SearchUsers(ctx context.Context, query string) ([]*bridgev } // Search contacts by display name - client := lc.newClient() - allMids, err := client.GetAllContactIds() + client, allMids, err := callLineResult(lc, ctx, func(client *line.Client) ([]string, error) { + return client.GetAllContactIds() + }) if err != nil { - if lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - allMids, err = client.GetAllContactIds() - } - } - if err != nil { - lc.UserLogin.Bridge.Log.Warn().Err(err).Msg("Failed to get all contact IDs for search") - return results, nil - } + lc.UserLogin.Bridge.Log.Warn().Err(err).Msg("Failed to get all contact IDs for search") + return results, nil } // Fetch contacts in batches to check display names @@ -378,18 +353,11 @@ func (lc *LineClient) SearchUsers(ctx context.Context, query string) ([]*bridgev var _ bridgev2.UserSearchingNetworkAPI = (*LineClient)(nil) func (lc *LineClient) GetContactList(ctx context.Context) ([]*bridgev2.ResolveIdentifierResponse, error) { - client := lc.newClient() - allMids, err := client.GetAllContactIds() + client, allMids, err := callLineResult(lc, ctx, func(client *line.Client) ([]string, error) { + return client.GetAllContactIds() + }) if err != nil { - if lc.shouldAttemptTokenRecovery(ctx, err) { - if errRecover := lc.recoverToken(ctx); errRecover == nil { - client = lc.newClient() - allMids, err = client.GetAllContactIds() - } - } - if err != nil { - return nil, err - } + return nil, err } var results []*bridgev2.ResolveIdentifierResponse