feat: 🎸 Add refresh token compat + group messaging by highesttt · Pull Request #9 · beeper/line

highesttt · 2025-12-25T19:56:24Z

No description provided.

Copilot

Pull request overview

This PR adds two major features to the LINE Messenger bridge: refresh token support for automatic token renewal and end-to-end encrypted group messaging capabilities.

Implements refresh token flow with automatic retry logic when access tokens expire
Adds complete E2EE group messaging support including key unwrapping, encryption, and decryption
Replaces the deprecated GetMessageBoxes API with GetAllChatMids and GetChats for better chat synchronization

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 19 comments.

Show a summary per file

File	Description
pkg/runner.go	Adds `ChannelUnwrapGroupSharedKey` method to unwrap encrypted group shared keys
pkg/line/structs.go	Defines new data structures for group keys, chats, and token refresh requests/responses
pkg/line/methods.go	Adds group key fetching methods and refactors public key parsing; replaces message box API
pkg/line/client.go	Implements `RefreshAccessToken` method for token renewal
pkg/internal/runner.js	Adds JavaScript handler for channel-based group key unwrapping
pkg/e2ee/manager.go	Implements group key management with unwrapping, encryption, and decryption capabilities
pkg/connector/connector.go	Extends user login metadata to store and persist refresh tokens
pkg/connector/client.go	Integrates token refresh throughout API calls; implements group message handling and chat sync
README.md	Updates feature checklist to mark group chats as complete

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-25T20:01:48Z

-	if err != nil {
-		return nil, fmt.Errorf("missing own E2EE key: %w", err)
-	}
+	fromMid := lc.midOrFallback()


The variable fromMid is declared here but not used until much later in the function (line 932 or 959). Consider moving this declaration closer to where it's first used to improve code readability and reduce the scope of the variable.

Copilot · 2025-12-25T20:01:49Z

+			res, err := client.GetContactsV2(missingMids)
+			if err == nil && res != nil && res.Contacts != nil {
+				for mid, wrapper := range res.Contacts {
+					lc.contactCache[mid] = wrapper.Contact


The contactCache map is accessed concurrently from multiple goroutines without synchronization. It's written to in syncChats (line 491), syncSingleChat (line 491), and getContact (line 732), and read from in generateNameFromMembers (line 315) and syncSingleChat (line 483). Since these operations can happen from different goroutines (e.g., syncSingleChat is called from a goroutine in handleOperation), this creates a data race condition. Consider protecting contactCache with a mutex or using a concurrent-safe map implementation.

Copilot · 2025-12-25T20:01:49Z

+				client = line.NewClient(lc.AccessToken)
+				ei3, err = client.GetEncryptedIdentityV3()
+			} else {
+				lc.UserLogin.Bridge.Log.Error().Err(errRefresh).Msg("Failed to refresh token")


When refreshing the token fails in the retry logic, the original error is returned but the client instance is not reset. This means subsequent operations will continue using the old (expired) client. Consider resetting the client or handling this case more explicitly to avoid using a stale client instance.

Suggested change

lc.UserLogin.Bridge.Log.Error().Err(errRefresh).Msg("Failed to refresh token")

lc.UserLogin.Bridge.Log.Error().Err(errRefresh).Msg("Failed to refresh token")

// Invalidate tokens to avoid using a known-stale client state.

lc.AccessToken = ""

lc.RefreshToken = ""

// Propagate the refresh failure so callers see a hard error.

err = fmt.Errorf("token refresh failed: %w", errRefresh)

Copilot · 2025-12-25T20:01:49Z

-		portal, err := lc.UserLogin.Bridge.GetPortalByKey(ctx, portalKey)
-		if err != nil || portal == nil {
-			continue
+	chunkSize := 20


The chunk size of 20 is a magic number without explanation. Consider extracting this as a named constant (e.g., const maxChatsPerBatch = 20) to clarify why this specific value is chosen and make it easier to adjust if needed.

Copilot · 2025-12-25T20:01:49Z

 	}
 	if pub == "" || keyID == 0 {
-		return nil, fmt.Errorf("negotiateE2EEPublicKey: missing fields (pub=%t keyID=%d raw=%s)", pub != "", keyID, string(wrapper.Data))
+		return nil, fmt.Errorf("missing fields (pub=%t keyID=%d raw=%s)", pub != "", keyID, string(rawData))


The error message references 'negotiateE2EEPublicKey' but this code has been refactored into a separate parseE2EEPublicKey function. The error message should be updated to reflect the actual function name where the error occurs, which would be more accurate for debugging purposes.

Suggested change

return nil, fmt.Errorf("missing fields (pub=%t keyID=%d raw=%s)", pub != "", keyID, string(rawData))

return nil, fmt.Errorf("parseE2EEPublicKey: missing fields (pub=%t keyID=%d raw=%s)", pub != "", keyID, string(rawData))

Copilot · 2025-12-25T20:01:52Z

+		if _, _, err2 := lc.ensurePeerKey(ctx, msg.From); err2 != nil {
+			lc.UserLogin.Bridge.Log.Warn().Err(err).Err(err2).Str("peer", msg.From).Int("key_id", peerRaw).Msg("Failed to fetch peer key for decrypt")
+		}


The fallback to ensurePeerKey (line 905) may overwrite a previously cached key with different keyID. If ensurePeerKeyByID fails for keyID X, then ensurePeerKey succeeds and caches a key with keyID Y, the cache entry for mid will now point to keyID Y. This could lead to using the wrong key for subsequent operations if the specific keyID is important.

Suggested change

if _, _, err2 := lc.ensurePeerKey(ctx, msg.From); err2 != nil {

lc.UserLogin.Bridge.Log.Warn().Err(err).Err(err2).Str("peer", msg.From).Int("key_id", peerRaw).Msg("Failed to fetch peer key for decrypt")

}

lc.UserLogin.Bridge.Log.Warn().

Err(err).

Str("peer", msg.From).

Int("key_id", peerRaw).

Msg("Failed to fetch peer key by ID for decrypt")

Copilot · 2025-12-25T20:01:52Z

+	// keyVersion 1
+	res, err := client.GetE2EEPublicKey(mid, 1, keyID)


The keyVersion parameter is hardcoded to 1 without explanation. If the API supports multiple key versions, this should either be made configurable, derived from context, or documented why version 1 is always appropriate. Consider adding a comment explaining this choice.

Copilot · 2025-12-25T20:01:53Z

@@ -104,8 +104,44 @@ func (c *Client) GetEncryptedIdentityV3() (*EncryptedIdentityV3, error) {
 	if err := json.Unmarshal(resp, &wrapper); err != nil {
 		return nil, err
 	}


The GetEncryptedIdentityV3 function does not check wrapper.Code before returning the data, unlike other similar functions in this file (e.g., GetProfile, GetE2EEGroupSharedKey, GetLastE2EEGroupSharedKey). This inconsistency means API errors might not be properly detected and could lead to returning invalid data. The function should check if wrapper.Code != 0 and return an appropriate error.

Suggested change

}

}

if wrapper.Code != 0 {

return nil, fmt.Errorf("getEncryptedIdentityV3 failed: %s", wrapper.Message)

}

Copilot · 2025-12-25T20:01:53Z

+	}
+	var keyID int
+	if v, ok := resp["keyId"]; ok {
+		_ = json.Unmarshal(v, &keyID)


The error returned from json.Unmarshal is being silently discarded. If unmarshaling fails, keyID will remain 0, and the function will return 0 without indicating that an error occurred during parsing. This could mask JSON parsing issues and make debugging difficult.

Copilot · 2025-12-25T20:01:53Z

 	client := line.NewClient(lc.AccessToken)
 	res, err := client.GetContactsV2([]string{mid})
+	if err != nil && lc.isRefreshRequired(err) {
+		if errRefresh := lc.refreshAndSave(context.TODO()); errRefresh == nil {


Using context.TODO() indicates that a proper context should be passed but isn't available. This is called from getContact which may not have a context parameter. Consider adding a context parameter to getContact or using context.Background() with a comment explaining why a proper context can't be used here.

Suggested change

if errRefresh := lc.refreshAndSave(context.TODO()); errRefresh == nil {

// getContact does not currently accept a context; use Background for this

// non-interactive refresh operation instead of a placeholder context.TODO().

if errRefresh := lc.refreshAndSave(context.Background()); errRefresh == nil {

feat: 🎸 Add refresh token compat + group messaging

1adc168

highesttt requested a review from Copilot December 25, 2025 19:56

highesttt linked an issue Dec 25, 2025 that may be closed by this pull request

bug: 🐛 Group chats are broken #4

Closed

3 tasks

Copilot started reviewing on behalf of highesttt December 25, 2025 19:56 View session

Copilot AI reviewed Dec 25, 2025

View reviewed changes

highesttt merged commit 7d46e44 into main Dec 25, 2025
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: 🎸 Add refresh token compat + group messaging#9

feat: 🎸 Add refresh token compat + group messaging#9
highesttt merged 1 commit into
mainfrom
4-bug-group-chats-are-broken

highesttt commented Dec 25, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 25, 2025

Uh oh!

Copilot AI Dec 25, 2025

Uh oh!

Copilot AI Dec 25, 2025

Uh oh!

Copilot AI Dec 25, 2025

Uh oh!

Copilot AI Dec 25, 2025

Uh oh!

Copilot AI Dec 25, 2025

Uh oh!

Copilot AI Dec 25, 2025

Uh oh!

Copilot AI Dec 25, 2025

Uh oh!

Copilot AI Dec 25, 2025

Uh oh!

Copilot AI Dec 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

-				lc.UserLogin.Bridge.Log.Error().Err(errRefresh).Msg("Failed to refresh token")
+				lc.UserLogin.Bridge.Log.Error().Err(errRefresh).Msg("Failed to refresh token")
+				// Invalidate tokens to avoid using a known-stale client state.
+				lc.AccessToken = ""
+				lc.RefreshToken = ""
+				// Propagate the refresh failure so callers see a hard error.
+				err = fmt.Errorf("token refresh failed: %w", errRefresh)

	return nil, fmt.Errorf("missing fields (pub=%t keyID=%d raw=%s)", pub != "", keyID, string(rawData))
	return nil, fmt.Errorf("parseE2EEPublicKey: missing fields (pub=%t keyID=%d raw=%s)", pub != "", keyID, string(rawData))

		// keyVersion 1
		res, err := client.GetE2EEPublicKey(mid, 1, keyID)

-	}
+	}
+	if wrapper.Code != 0 {
+		return nil, fmt.Errorf("getEncryptedIdentityV3 failed: %s", wrapper.Message)
+	}

-		if errRefresh := lc.refreshAndSave(context.TODO()); errRefresh == nil {
+		// getContact does not currently accept a context; use Background for this
+		// non-interactive refresh operation instead of a placeholder context.TODO().
+		if errRefresh := lc.refreshAndSave(context.Background()); errRefresh == nil {

Uh oh!

Conversation

highesttt commented Dec 25, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants