Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rudamentary support for signald #1433

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

petermolnar
Copy link

Signald https://gitlab.com/signald/signald is a standalone Signal client written in Java that can be interacted with via Unix socket and newline separated JSON strings.

These commits establish a crude and basic interaction with the signald daemon where by giving a group ID, one can relay messages from and to a Signal group.

Anything beyond this functionality - including, but not limited to registration, device linking, joining a group, etc. - is missing.

To test the functionality one has to set up signald with linking or registration in advance.

Peter Molnar added 5 commits March 23, 2021 09:24
can receive from signald groupv2, but nothing else
I relied on learning from https://gitlab.com/signald/signald-go but because that project is under GPL and there are no plans on re-licensing (see <https://gitlab.com/signald/signald-go/-/issues/7>) it can't be directly used in matterbridge, which is Apache 2.0

Anyway: as it currently is, this module is crude, probably extremely prone to errors, and nowhere remotely close to stable or production use.
Regardless of that, any help with it would be welcome.
- try to re-subscribe on fail
- list contacts
- use nice contact names instead of phone numbers
@42wim
Copy link
Owner

42wim commented Apr 3, 2021

@petermolnar thank you!
Could you fix the linting errors? If anything is not clear, let me know

@codeclimate
Copy link

codeclimate bot commented Apr 4, 2021

Code Climate has analyzed commit 6dc84ca and detected 0 issues on this pull request.

View more on Code Climate.

Copy link

@halms halms left a comment

Choose a reason for hiding this comment

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

First: I very much look forward to this getting merged. I have a specific use case where bridging a WhatsApp with a Signal group is really useful.

It seems your latest changes fixed the linting errors.

When trying this out though, I run into some very weird behavior (my Signal groups got mixed up, ...) and I think I tracked it with the suggested changes.

Let me know, what you think (this is my first time touching Go code...).

socket net.Conn
subscribed bool
reader *bufio.Scanner
groupid string
Copy link

Choose a reason for hiding this comment

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

Making this a single string leads to the problem that the bridge can only work with one Signal group.
(And has weird side effects when configuring more than one group)

I think a map-as-hashset construct could work here.

Suggested change
groupid string
groupids map[string]bool

}

func (b *Bsignald) JoinChannel(channel config.ChannelInfo) error {
b.groupid = channel.Name
Copy link

Choose a reason for hiding this comment

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

Add the groupid to the bridge

Suggested change
b.groupid = channel.Name
b.groupids[channel.Name] = true

socketpath: socketpath,
subscribed: false,
contacts: make(map[string]signaldContact),
}
Copy link

Choose a reason for hiding this comment

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

Suggested change
}
groupids: make(map[string]bool),
}

Comment on lines +233 to +247
groupMatched := false
if response.Data.DataMessage.GroupV2 != nil {
if b.groupid == response.Data.DataMessage.GroupV2.ID {
groupMatched = true
}
} else if response.Data.DataMessage.Group != nil {
if b.groupid == response.Data.DataMessage.Group.ID {
groupMatched = true
}
}

if !groupMatched {
b.Log.Debugln("skipping non-group message")
return
}
Copy link

Choose a reason for hiding this comment

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

Checking can be done against the hashset.
Additionally, we now need the current groupid, because b.groupid is not unique anymore.

Suggested change
groupMatched := false
if response.Data.DataMessage.GroupV2 != nil {
if b.groupid == response.Data.DataMessage.GroupV2.ID {
groupMatched = true
}
} else if response.Data.DataMessage.Group != nil {
if b.groupid == response.Data.DataMessage.Group.ID {
groupMatched = true
}
}
if !groupMatched {
b.Log.Debugln("skipping non-group message")
return
}
groupid := ""
if response.Data.DataMessage.GroupV2 != nil {
groupid = response.Data.DataMessage.GroupV2.ID
} else if response.Data.DataMessage.Group != nil {
groupid = response.Data.DataMessage.Group.ID
}
groupMatched := b.groupids[groupid]
if !groupMatched {
b.Log.Debugln("skipping not addressed to one of the configured groups")
return
}


username := b.GetUsername(response.Data.Source.UUID)
if username == "" {
username = response.Data.Source.Number
Copy link

Choose a reason for hiding this comment

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

As in the WhatsApp-Bridge, I would suggest not putting the Number in the username.

Suggested change
username = response.Data.Source.Number
username = "Someone"

}

rmsg := config.Message{
UserID: response.Data.Source.UUID,
Copy link

Choose a reason for hiding this comment

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

I would prefer the Users Number as ID instead of the UUID, because this for me seems like a more usable identifier (and I have a specific use case that works better this way).

Suggested change
UserID: response.Data.Source.UUID,
UserID: response.Data.Source.Number,

UserID: response.Data.Source.UUID,
Username: username,
Text: response.Data.DataMessage.Body,
Channel: b.groupid,
Copy link

Choose a reason for hiding this comment

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

Here we need the current groupid

Suggested change
Channel: b.groupid,
Channel: groupid,

msgJSON := signaldSendMessage{
Type: "send",
Username: b.GetString(cfgNumber),
RecipientGroupID: b.groupid,
Copy link

Choose a reason for hiding this comment

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

The groupid to send to now depends on the channel of the message, not the bridge configuration.

Suggested change
RecipientGroupID: b.groupid,
RecipientGroupID: msg.Channel,

@halms
Copy link

halms commented May 12, 2021

Is there any update on the status of this PR?
It seems to need a rebase now.

I have a version of this (with my described changes) running since a month or so on a local instance and have not run into any big issues.

@micw
Copy link

micw commented Jun 8, 2021

@42wim What changes (besides a rebase) are required to bring this in a state that you'd merge?
Kind regards,
Michael.

@42wim
Copy link
Owner

42wim commented Jul 31, 2021

@petermolnar looks like @halms suggestions are useful, are you willing to rebase with his proposed changes?

To merge this I need someone who wants to take ownership of this specific bridge as I don't use signal/can't test it.

@halms
Copy link

halms commented Apr 27, 2022

Since there has not been any further activity by @petermolnar, I might take another look at this and use it as a chance to get up to speed in Go.

For this, I would be thinking, though, to use the quasi-official [signald-go](https://gitlab.com/signald/signald-go library. As this is GPLv3, it would be a situation like with the whatsappmulti bridge, where it cannot be distributed as a binary.

I have been using a version based on my proposed changes for quite some time now in production, and it has been working rather okay. For my specific use case, I will probably switch away from Matterbridge at some point and use whatsmeow (or Baileys) and signald directly, but I would very much like to see Matterbridge support Signal anyway.

Once I have something working, I will open a new pull request.

@42wim
Copy link
Owner

42wim commented May 1, 2022

@halms sounds good, thanks!

@z8512
Copy link

z8512 commented May 23, 2022

@halms waiting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants