Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions pkg/connector/creategroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,8 @@ func (lc *LineClient) CreateGroup(ctx context.Context, params *bridgev2.GroupCre
lc.generatedGroupNameCache[chat.ChatMid] = name == ""
lc.cacheMu.Unlock()

// Register E2EE group key so members can decrypt group messages.
// This is best-effort: if E2EE isn't available for a member we skip them,
// and if the entire registration fails we log a warning without aborting.
// Registration is best-effort: an incomplete E2EE member list leaves the
// chat on its plaintext fallback without aborting group creation.
if lc.E2EE != nil && len(participantMids) > 0 {
if err := lc.registerGroupKey(ctx, chat.ChatMid, participantMids); err != nil {
lc.UserLogin.Bridge.Log.Warn().Err(err).
Expand Down Expand Up @@ -213,18 +212,22 @@ func (lc *LineClient) resolveGroupMemberPublicKeys(ctx context.Context, client *
// can decrypt group messages.
func (lc *LineClient) registerGroupKey(ctx context.Context, chatMid string, members []string) error {
members = lc.groupKeyMemberMIDs(chatMid, members)
if len(members) == 0 {
return fmt.Errorf("no other members to register group key for")
if lc.E2EE == nil {
return fmt.Errorf("%w: E2EE manager not initialized", line.ErrNoUsableE2EEGroupKey)
}

client := lc.newClient()

// Batch responses can be partial without returning an error. Resolve every
// missing member individually so registration arrays retain the server's
// expected member count.
client, pubKeys, err := lc.resolveGroupMemberPublicKeys(ctx, client, chatMid, members)
if err != nil {
return err
pubKeys := make(map[string]line.E2EEPeerPublicKey, len(members))
if len(members) > 0 {
var err error
client, pubKeys, err = lc.resolveGroupMemberPublicKeys(ctx, client, chatMid, members)
if err != nil {
return err
}
}

// Generate group key in WASM (same approach as LINE Chrome Extension).
Expand Down Expand Up @@ -255,10 +258,6 @@ func (lc *LineClient) registerGroupKey(ctx context.Context, chatMid string, memb
encryptedKeys = append(encryptedKeys, encryptedKey)
}

if len(apiMembers) == 0 {
return fmt.Errorf("no members with valid E2EE keys")
}

// LINE's registerE2EEGroupKey requires the caller's own key entry as well — without it the
// server rejects the request with "empty caller key". The Chrome extension wraps the group
// key for every member returned by getLastE2EEPublicKeys, which includes the caller. Mirror
Expand Down
80 changes: 47 additions & 33 deletions pkg/connector/e2ee_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ var (
}
)

func groupKeyFetchError(groupKeyID int, err error) error {
if groupKeyID == 0 && line.IsNotAMemberError(err) {
return fmt.Errorf("%w: latest group key lookup reported not a member: %w", line.ErrNoUsableE2EEGroupKey, err)
}
return err
}

// fetchAndUnwrapGroupKey retrieves a specific group key (or the latest when groupKeyID == 0)
// and unwraps it so the E2EE manager can encrypt/decrypt group messages.
// If no group key exists yet (TalkException code 5), it auto-registers one and retries.
Expand All @@ -33,10 +40,14 @@ func (lc *LineClient) fetchAndUnwrapGroupKey(ctx context.Context, chatMid string

client := lc.newClient()
fetch := func() (*line.E2EEGroupSharedKey, error) {
var sharedKey *line.E2EEGroupSharedKey
var err error
if groupKeyID > 0 {
return client.GetE2EEGroupSharedKey(chatMid, groupKeyID)
sharedKey, err = client.GetE2EEGroupSharedKey(chatMid, groupKeyID)
} else {
sharedKey, err = client.GetLastE2EEGroupSharedKey(chatMid)
}
return client.GetLastE2EEGroupSharedKey(chatMid)
return sharedKey, groupKeyFetchError(groupKeyID, err)
}

sharedKey, err := fetch()
Expand Down Expand Up @@ -207,10 +218,30 @@ func (lc *LineClient) cacheGroupMemberMIDs(chatMid string, mids []string) {
lc.groupMemberCache[chatMid] = append([]string(nil), mids...)
}

// getChatMemberMIDs fetches the member and invitee MIDs for a group chat via GetChats.
// Invitees are included because group key registration must happen before they accept,
// otherwise the key won't be available when they start sending messages.
func (lc *LineClient) getChatMemberMIDs(ctx context.Context, chatMid string) ([]string, error) {
func joinedGroupMemberMIDs(group *line.GroupExtra, ownMID string) ([]string, bool) {
seen := make(map[string]struct{})
if group != nil {
for mid := range group.MemberMids {
if isUserMID(mid) {
seen[mid] = struct{}{}
}
}
}
if isUserMID(ownMID) {
seen[ownMID] = struct{}{}
}

mids := make([]string, 0, len(seen))
for mid := range seen {
mids = append(mids, mid)
}
return mids, group != nil && len(group.InviteeMids) > 0
}

// getChatMemberMIDs fetches joined member MIDs for a group chat via GetChats.
// 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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc missing new return value: Comment above explains "why invitees are excluded" but doesn't document that the signature now returns (mids []string, hasPendingInvitees bool, error). A short line noting that the bool is used by autoRegisterGroupKey to gate self-only fallbacks would keep the contract clear for future readers.

client := lc.newClient()
chats, err := client.GetChats([]string{chatMid}, true, true)
if err != nil {
Expand All @@ -221,58 +252,41 @@ func (lc *LineClient) getChatMemberMIDs(ctx context.Context, chatMid string) ([]
}
}
if err != nil {
return nil, 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, fmt.Errorf("chat %s not found", chatMid)
return nil, false, fmt.Errorf("chat %s not found", chatMid)
}
chat := chats.Chats[0]
if chat.Extra.GroupExtra == nil {
return nil, fmt.Errorf("chat %s has no group extra", chatMid)
}
seen := make(map[string]struct{})
for mid := range chat.Extra.GroupExtra.MemberMids {
if isUserMID(mid) {
seen[mid] = struct{}{}
}
}
for mid := range chat.Extra.GroupExtra.InviteeMids {
if isUserMID(mid) {
seen[mid] = struct{}{}
}
}
// Include the bridge user's MID when it has a valid user prefix.
if isUserMID(lc.Mid) {
seen[lc.Mid] = struct{}{}
}
mids := make([]string, 0, len(seen))
for mid := range seen {
mids = append(mids, mid)
return nil, false, fmt.Errorf("chat %s has no group extra", chatMid)
}
group := chat.Extra.GroupExtra
mids, hasPendingInvitees := joinedGroupMemberMIDs(group, lc.Mid)
if len(mids) == 0 {
return nil, fmt.Errorf("chat %s has no members or invitees", chatMid)
return nil, hasPendingInvitees, fmt.Errorf("chat %s has no joined members", chatMid)
}

// Cache complete results for fallback use. Do not replace a richer cached
// list when LINE returns only the caller.
lc.cacheGroupMemberMIDs(chatMid, mids)

return mids, nil
return mids, hasPendingInvitees, nil
}

// autoRegisterGroupKey fetches group members, then registers a new E2EE group key
// for the chat. This is called when fetchAndUnwrapGroupKey finds no key exists.
func (lc *LineClient) autoRegisterGroupKey(ctx context.Context, chatMid string) error {
members, err := lc.getChatMemberMIDs(ctx, chatMid)
members, hasPendingInvitees, err := lc.getChatMemberMIDs(ctx, chatMid)
if err != nil {
return fmt.Errorf("getChatMemberMIDs: %w", err)
}

// If getChatMemberMIDs returned only ourself, the server likely returned
// an empty MemberMids map (known LINE API issue). Fall back to cached
// member list from CreateGroup or a prior successful fetch.
if len(members) == 1 && members[0] == lc.Mid {
if len(members) == 1 && members[0] == lc.Mid && !hasPendingInvitees {
lc.cacheMu.Lock()
cached, ok := lc.groupMemberCache[chatMid]
cached = append([]string(nil), cached...)
Expand All @@ -285,7 +299,7 @@ func (lc *LineClient) autoRegisterGroupKey(ctx context.Context, chatMid string)
}

// Last resort: query Matrix room members via the bridge API.
if len(members) == 1 && members[0] == lc.Mid {
if len(members) == 1 && members[0] == lc.Mid && !hasPendingInvitees {
matrixMembers, err := lc.getGroupMemberMIDsViaMatrix(ctx, chatMid)
if err != nil {
lc.UserLogin.Bridge.Log.Warn().Err(err).Str("chat_mid", chatMid).
Expand Down
57 changes: 57 additions & 0 deletions pkg/connector/e2ee_keys_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,63 @@ func TestCacheGroupMemberMIDsPreservesRicherList(t *testing.T) {
}
}

func TestGroupKeyFetchErrorAllowsLatestKeyPlaintextFallback(t *testing.T) {
err := groupKeyFetchError(0, errNotMember)

if !errors.Is(err, line.ErrNoUsableE2EEGroupKey) {
t.Fatalf("error = %v, want ErrNoUsableE2EEGroupKey", err)
}
if !errors.Is(err, errNotMember) {
t.Fatalf("error = %v, want not-member error to remain wrapped", err)
}
}

func TestGroupKeyFetchErrorPreservesSpecificKeyMembershipError(t *testing.T) {
err := groupKeyFetchError(123, errNotMember)

if !errors.Is(err, errNotMember) {
t.Fatalf("error = %v, want original not-member error", err)
}
if errors.Is(err, line.ErrNoUsableE2EEGroupKey) {
t.Fatalf("error = %v, specific group key lookup must not allow plaintext fallback", err)
}
}

func TestRegisterGroupKeyAllowsPlaintextFallbackWithoutKnownMembers(t *testing.T) {
lc := newPeerKeyTestClient()
lc.Mid = "U-self"

err := lc.registerGroupKey(context.Background(), "C-group", []string{"U-self"})
if !errors.Is(err, line.ErrNoUsableE2EEGroupKey) {
t.Fatalf("error = %v, want ErrNoUsableE2EEGroupKey", err)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test doesn't exercise the intended path: newPeerKeyTestClient() returns a LineClient with E2EE unset, so registerGroupKey returns via the new lc.E2EE == nil guard on the very first line — the filtered-to-self member handling this PR added is never touched. Consider either (a) renaming this to reflect that it covers the E2EE-nil case, or (b) constructing a client with a real (or stub) E2EE manager and asserting that registerGroupKey(ctx, chatMid, []string{lc.Mid}) now proceeds to a self-only RegisterE2EEGroupKey call instead of erroring — which is the actual behavior change relative to the removed no other members to register group key for error.


func TestJoinedGroupMemberMIDsExcludeInvitees(t *testing.T) {
group := &line.GroupExtra{
MemberMids: line.FlexibleMidMap{
"U-self": true,
"U-member": true,
},
InviteeMids: line.FlexibleMidMap{
"U-invitee": true,
},
}

mids, hasPendingInvitees := joinedGroupMemberMIDs(group, "U-self")
if !hasPendingInvitees {
t.Fatal("hasPendingInvitees = false, want true")
}
if len(mids) != 2 {
t.Fatalf("joined mids = %v, want self and joined member only", mids)
}
for _, mid := range mids {
if mid == "U-invitee" {
t.Fatalf("joined mids unexpectedly contain pending invitee: %v", mids)
}
}
}

func TestResolveGroupMemberPublicKeysFillsPartialBatchResponse(t *testing.T) {
oldGetLast := getLastE2EEPublicKeysWithClient
oldNegotiate := negotiateE2EEPublicKeyWithClient
Expand Down
11 changes: 9 additions & 2 deletions pkg/line/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,16 @@ import "encoding/json"
type FlexibleMidMap map[string]bool

func (f *FlexibleMidMap) UnmarshalJSON(data []byte) error {
var m map[string]bool
var m map[string]json.RawMessage
if err := json.Unmarshal(data, &m); err == nil {
*f = m
if m == nil {
*f = nil
return nil
}
*f = make(map[string]bool, len(m))
for mid := range m {
(*f)[mid] = true
}
return nil
}

Expand Down
47 changes: 47 additions & 0 deletions pkg/line/structs_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package line

import (
"encoding/json"
"testing"
)

func TestFlexibleMidMapUnmarshalJSON(t *testing.T) {
tests := []struct {
name string
data string
want []string
}{
{
name: "string valued object",
data: `{"U-one":"first","U-two":"second"}`,
want: []string{"U-one", "U-two"},
},
{
name: "boolean valued object",
data: `{"U-one":true,"U-two":false}`,
want: []string{"U-one", "U-two"},
},
{
name: "array",
data: `["U-one","U-two"]`,
want: []string{"U-one", "U-two"},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
var got FlexibleMidMap
if err := json.Unmarshal([]byte(test.data), &got); err != nil {
t.Fatalf("Unmarshal returned error: %v", err)
}
if len(got) != len(test.want) {
t.Fatalf("decoded MIDs = %v, want %v", got, test.want)
}
for _, mid := range test.want {
if !got[mid] {
t.Errorf("decoded MIDs = %v, missing %s", got, mid)
}
}
})
}
}
Loading