Skip to content

Commit a5d41d0

Browse files
highesttthighesttt
committed
fix: address auth recovery review feedback
1 parent fc15510 commit a5d41d0

4 files changed

Lines changed: 94 additions & 65 deletions

File tree

‎pkg/connector/client.go‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -307,10 +307,6 @@ func (lc *LineClient) refreshAndSave(ctx context.Context) error {
307307
return nil
308308
}
309309

310-
func (lc *LineClient) isRefreshRequired(err error) bool {
311-
return line.IsRefreshRequired(err)
312-
}
313-
314310
func (lc *LineClient) isLoggedOut(err error) bool {
315311
return line.IsLoggedOut(err)
316312
}
@@ -788,33 +784,6 @@ func (lc *LineClient) ensureValidToken(ctx context.Context) error {
788784
return nil
789785
}
790786

791-
func (lc *LineClient) ensureValidTokenWith(
792-
ctx context.Context,
793-
profile func(context.Context) error,
794-
refresh func(context.Context) error,
795-
relogin func(context.Context) error,
796-
) error {
797-
err := profile(ctx)
798-
if err == nil {
799-
return nil
800-
}
801-
if ctx.Err() != nil {
802-
return ctx.Err()
803-
}
804-
805-
if lc.isLoggedOut(err) {
806-
return err
807-
}
808-
809-
if !lc.isRefreshRequired(err) {
810-
lc.UserLogin.Bridge.Log.Warn().Err(err).Msg("GetProfile failed with non-auth error, continuing anyway")
811-
return nil
812-
}
813-
814-
lc.UserLogin.Bridge.Log.Info().Msg("Access token expired, attempting refresh...")
815-
return lc.recoverTokenWith(ctx, refresh, relogin)
816-
}
817-
818787
func (lc *LineClient) Disconnect() {
819788
// Disconnect is terminal for this NetworkAPI instance. Framework reconnects
820789
// create a replacement client, so late handlers on this one must not mutate

‎pkg/connector/forced_logout_test.go‎

Lines changed: 54 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,30 +50,57 @@ func TestEnsureValidTokenReturnsLoggedOutWithoutRelogin(t *testing.T) {
5050
}
5151

5252
func TestEnsureValidTokenDoesNotReloginAfterLoggedOutRefresh(t *testing.T) {
53+
oldGetProfile := getProfileWithToken
54+
oldRecover := recoverLineToken
55+
t.Cleanup(func() {
56+
getProfileWithToken = oldGetProfile
57+
recoverLineToken = oldRecover
58+
})
59+
5360
lc := &LineClient{
61+
AccessToken: "expired",
5462
UserLogin: &bridgev2.UserLogin{
5563
Bridge: &bridgev2.Bridge{Log: zerolog.New(io.Discard)},
5664
},
5765
}
5866
var reloginCalls int
59-
err := lc.ensureValidTokenWith(
60-
context.Background(),
61-
func(context.Context) error { return errAuthRequired },
62-
func(context.Context) error { return errLoggedOut },
63-
func(context.Context) error {
64-
reloginCalls++
65-
return nil
66-
},
67-
)
68-
if !line.IsLoggedOut(err) {
69-
t.Fatalf("ensureValidTokenWith error = %v, want logged-out error", err)
67+
getProfileWithToken = func(_ context.Context, token string) (*line.Profile, error) {
68+
if token != "expired" {
69+
t.Fatalf("profile token = %q, want expired", token)
70+
}
71+
return nil, errAuthRequired
72+
}
73+
recoverLineToken = func(lc *LineClient, ctx context.Context) error {
74+
return lc.recoverTokenWith(
75+
ctx,
76+
func(context.Context) error { return errLoggedOut },
77+
func(context.Context) error {
78+
reloginCalls++
79+
return nil
80+
},
81+
)
82+
}
83+
84+
err := lc.ensureValidToken(context.Background())
85+
if !line.IsAuthError(err) {
86+
t.Fatalf("ensureValidToken error = %v, want auth error", err)
7087
}
7188
if reloginCalls != 0 {
7289
t.Fatalf("relogin calls = %d, want 0", reloginCalls)
7390
}
91+
if lc.hasAccessToken() || !lc.isSessionInvalidated() {
92+
t.Fatal("logged-out refresh did not invalidate the session")
93+
}
7494
}
7595

7696
func TestForcedLogoutWinsOverEnsureValidTokenRefresh(t *testing.T) {
97+
oldGetProfile := getProfileWithToken
98+
oldRecover := recoverLineToken
99+
t.Cleanup(func() {
100+
getProfileWithToken = oldGetProfile
101+
recoverLineToken = oldRecover
102+
})
103+
77104
lc := &LineClient{
78105
AccessToken: "old-token",
79106
UserLogin: &bridgev2.UserLogin{
@@ -84,10 +111,18 @@ func TestForcedLogoutWinsOverEnsureValidTokenRefresh(t *testing.T) {
84111
allowRefresh := make(chan struct{})
85112
ensureDone := make(chan error, 1)
86113
var reloginCalls int
87-
go func() {
88-
ensureDone <- lc.ensureValidTokenWith(
89-
context.Background(),
90-
func(context.Context) error { return errAuthRequired },
114+
getProfileWithToken = func(_ context.Context, token string) (*line.Profile, error) {
115+
if token == "recovered-token" {
116+
return &line.Profile{}, nil
117+
}
118+
if token != "old-token" {
119+
t.Fatalf("profile token = %q, want old-token or recovered-token", token)
120+
}
121+
return nil, errAuthRequired
122+
}
123+
recoverLineToken = func(lc *LineClient, ctx context.Context) error {
124+
return lc.recoverTokenWith(
125+
ctx,
91126
func(context.Context) error {
92127
close(refreshStarted)
93128
<-allowRefresh
@@ -99,6 +134,9 @@ func TestForcedLogoutWinsOverEnsureValidTokenRefresh(t *testing.T) {
99134
return nil
100135
},
101136
)
137+
}
138+
go func() {
139+
ensureDone <- lc.ensureValidToken(context.Background())
102140
}()
103141
<-refreshStarted
104142

@@ -110,7 +148,7 @@ func TestForcedLogoutWinsOverEnsureValidTokenRefresh(t *testing.T) {
110148
close(allowRefresh)
111149

112150
if err := <-ensureDone; err != nil {
113-
t.Fatalf("ensureValidTokenWith returned error: %v", err)
151+
t.Fatalf("ensureValidToken returned error: %v", err)
114152
}
115153
if reloginCalls != 0 {
116154
t.Fatalf("relogin calls = %d, want 0", reloginCalls)

‎pkg/connector/sync.go‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,15 +1712,16 @@ func (lc *LineClient) handleReceiveAuthError(ctx context.Context, failedClient *
17121712
}
17131713

17141714
profileToken := lc.getAccessToken()
1715-
if failedClient != nil && failedClient.AccessToken != "" {
1715+
if profileToken == "" && failedClient != nil {
17161716
profileToken = failedClient.AccessToken
17171717
}
1718+
profileClient := newLineAPIClient(profileToken)
17181719
_, profileErr := getProfileWithToken(ctx, profileToken)
17191720
if ctx.Err() != nil {
17201721
return true
17211722
}
17221723
if lc.isLoggedOut(profileErr) {
1723-
recoveredClient, errRecover := lc.recoverClientAfterAuthError(ctx, failedClient, profileErr)
1724+
recoveredClient, errRecover := lc.recoverClientAfterAuthError(ctx, profileClient, profileErr)
17241725
if errRecover != nil {
17251726
return true
17261727
}
@@ -1944,7 +1945,7 @@ func (lc *LineClient) handleOperation(ctx context.Context, op line.Operation) {
19441945
// Curr == nil signals a reaction removal/clear from LINE.
19451946
if param2.Curr == nil {
19461947
lc.UserLogin.Bridge.Log.Debug().Str("msg_id", op.Param1).Str("chat_mid", param2.ChatMid).Msg("Received reaction removal (self)")
1947-
lc.handleReactionRemove(op, param2.ChatMid, []networkid.UserID{makeUserID(string(lc.UserLogin.ID))})
1948+
lc.handleReactionRemove(op, param2.ChatMid, makeUserID(string(lc.UserLogin.ID)))
19481949
return
19491950
}
19501951

@@ -1983,7 +1984,7 @@ func (lc *LineClient) handleOperation(ctx context.Context, op line.Operation) {
19831984
// use the type 140 actor from param3, so the sender is unambiguous.
19841985
if param2.Curr == nil {
19851986
lc.UserLogin.Bridge.Log.Debug().Str("msg_id", op.Param1).Str("chat_mid", param2.ChatMid).Msg("Received reaction removal (other)")
1986-
lc.handleReactionRemove(op, param2.ChatMid, []networkid.UserID{makeUserID(op.Param3)})
1987+
lc.handleReactionRemove(op, param2.ChatMid, makeUserID(op.Param3))
19871988
return
19881989
}
19891990

@@ -2091,17 +2092,14 @@ func (lc *LineClient) liveReactionSyncEvent(
20912092
}
20922093
}
20932094

2094-
// handleReactionRemove queues an authoritative empty reaction sync for each
2095-
// candidate sender. LINE only allows one reaction per sender, so this removes
2096-
// both legacy empty-ID rows and stable paid/predefined reaction IDs without
2097-
// needing the previous reaction type.
2098-
func (lc *LineClient) handleReactionRemove(op line.Operation, chatMid string, senders []networkid.UserID) {
2099-
for _, sender := range senders {
2100-
lc.UserLogin.Bridge.QueueRemoteEvent(
2101-
lc.UserLogin,
2102-
lc.liveReactionSyncEvent(op, chatMid, sender, nil),
2103-
)
2104-
}
2095+
// handleReactionRemove queues an authoritative empty reaction sync for the
2096+
// sender, removing both legacy empty-ID rows and stable paid/predefined reaction
2097+
// IDs without needing the previous reaction type.
2098+
func (lc *LineClient) handleReactionRemove(op line.Operation, chatMid string, sender networkid.UserID) {
2099+
lc.UserLogin.Bridge.QueueRemoteEvent(
2100+
lc.UserLogin,
2101+
lc.liveReactionSyncEvent(op, chatMid, sender, nil),
2102+
)
21052103
}
21062104

21072105
func (lc *LineClient) syncSingleChat(ctx context.Context, op line.Operation) {

‎pkg/connector/sync_test.go‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -902,10 +902,10 @@ func TestReceiveAuthErrorFromStaleSSEClientReconnectsCurrentToken(t *testing.T)
902902
var profileCalls int
903903
getProfileWithToken = func(_ context.Context, token string) (*line.Profile, error) {
904904
profileCalls++
905-
if token != "old-token" {
906-
t.Fatalf("profile token = %q, want failed SSE token", token)
905+
if token != "current-token" {
906+
t.Fatalf("profile token = %q, want current token", token)
907907
}
908-
return nil, errLoggedOut
908+
return &line.Profile{}, nil
909909
}
910910

911911
lc := &LineClient{AccessToken: "current-token"}
@@ -922,6 +922,30 @@ func TestReceiveAuthErrorFromStaleSSEClientReconnectsCurrentToken(t *testing.T)
922922
}
923923
}
924924

925+
func TestReceiveAuthErrorFromStaleSSEClientClassifiesCurrentProbeLogout(t *testing.T) {
926+
oldGetProfile := getProfileWithToken
927+
t.Cleanup(func() {
928+
getProfileWithToken = oldGetProfile
929+
})
930+
931+
getProfileWithToken = func(_ context.Context, token string) (*line.Profile, error) {
932+
if token != "current-token" {
933+
t.Fatalf("profile token = %q, want current token", token)
934+
}
935+
return nil, errLoggedOut
936+
}
937+
938+
lc := &LineClient{AccessToken: "current-token"}
939+
stopped := lc.handleReceiveAuthError(context.Background(), line.NewClient("old-token"), errors.New("SSE error: 401"))
940+
941+
if !stopped {
942+
t.Fatal("current-token profile logout should stop the session")
943+
}
944+
if lc.hasAccessToken() || !lc.isSessionInvalidated() {
945+
t.Fatal("current-token profile logout was misclassified as a stale SSE response")
946+
}
947+
}
948+
925949
func TestReceiveAuthErrorCancellationDuringProfileDoesNotInvalidate(t *testing.T) {
926950
oldGetProfile := getProfileWithToken
927951
t.Cleanup(func() {

0 commit comments

Comments
 (0)