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

Not compatible with v2 of the Signal REST API #573

Open
ostasevych opened this issue Jun 10, 2023 · 9 comments · May be fixed by #579
Open

Not compatible with v2 of the Signal REST API #573

ostasevych opened this issue Jun 10, 2023 · 9 comments · May be fixed by #579

Comments

@ostasevych
Copy link

ostasevych commented Jun 10, 2023

Hi! I am migrating to the updated version of the Signal CLI REST API by bbernhard, which has deprecated the v1/send option.

Would you be so kind to upgrade the code according to the swagger provided in https://bbernhard.github.io/signal-cli-rest-api/? See also the examples at https://github.com/bbernhard/signal-cli-rest-api/blob/master/doc/EXAMPLES.md.

In particular, to rework
curl -d '{"message": "foo"}' http://127.0.0.1/v1/send/<recipient_number>
into
curl -X POST -H "Content-Type: application/json" -d '{"message": "<message>", "number": "<my_number>", "recipients": ["<recipient_number>"]}' 'http://127.0.0.1/v2/send.

Or advise how to patch it myself.

UPD:

What I've managed to do quickly:

        public function send(IUser $user, string $identifier, string $message) {
                $client = $this->clientService->newClient();
                // determine type of gateway
                $response = $client->get($this->config->getUrl() . '/v1/about');
                if ($response->getStatusCode() === 200) {
                        // New v2 style gateway https://github.com/bbernhard/signal-cli-rest-api
                        $recip = [$identifier];
                        $response = $client->post(
                                $this->config->getUrl() . '/v2/send',
                                [
                                        'json' => [
                                                'message' => $message,
                                                'number' => '<my_number>',
                                                'recipients' => $recip
                                                 ],

                                ]
                        );
                        $body = $response->getBody();
                        $json = json_decode($body, true);
                        if ($response->getStatusCode() !== 201 || is_null($json) || !is_array($json) || !isset($json['timestamp'])) {
                                $status = $response->getStatusCode();
                                throw new SmsTransmissionException("error reported by Signal gateway, status=$status, body=$body}");
                        }
                }
                else {
                        // Try old deprecated gateway https://gitlab.com/morph027/signal-web-gateway

The question, how to reuse the environmental of exported in docker phone number SIGNAL_CLI_DBUS_REST_API_ACCOUNT to use it instead of hardcoded <my_number>?

@rotdrop
Copy link
Contributor

rotdrop commented Jun 16, 2023

Isn't this a different thing? The original signal stuff is built around

https://gitlab.com/morph027/signal-cli-dbus-rest-api

which has been super-seeded by

https://gitlab.com/morph027/python-signal-cli-rest-api

where we read ;)

This project is not actively maintained anymore as signal-cli is now offering a native JSON-RPC api via HTTP.

Still, this docker stuff: is that built around this JSON-RPC of signal-cli, or this an (then perhaps obsolete) addon?

Currently I am using that python-signal-cli-rest-api via the patch contained here: #574

I am not the maintainer of this package, but using Signal with the twofactor gateway.

@ostasevych
Copy link
Author

ostasevych commented Jun 17, 2023

At https://gitlab.com/morph027/python-signal-cli-rest-api it is written:
This project is not actively maintained anymore as signal-cli is now offering a native JSON-RPC api via HTTP.

So, my attempts are to use bbernhard's signal-cli API implementation https://github.com/bbernhard/signal-cli-rest-api.
What I've done:

  1. Added a new system variable SIGNAL_CLI_DBUS_REST_API_ACCOUNT in the config.php
  2. Modified public function send() to be compatible with the API v2, which is the only in the bberhnard's code (API v1 has been completely removed) at lib/Service/Gateway/Gateway.php:
<?php

declare(strict_types=1);

/**
 * @author Christoph Wurst <christoph@winzerhof-wurst.at>
 *
 * @license GNU AGPL version 3 or any later version
 *
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU Affero General Public License as
 * published by the Free Software Foundation, either version 3 of the
 * License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU Affero General Public License for more details.
 *
 * You should have received a copy of the GNU Affero General Public License
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.
 *
 */

namespace OCA\TwoFactorGateway\Service\Gateway\Signal;

use OCA\TwoFactorGateway\Exception\SmsTransmissionException;
use OCA\TwoFactorGateway\Service\Gateway\IGateway;
use OCA\TwoFactorGateway\Service\Gateway\IGatewayConfig;
use OCP\Http\Client\IClientService;
use OCP\ILogger;
use OCP\IUser;

// added to get SystemValue:
use OCP\IConfig;

/**
 * An integration of https://github.com/bbernhard/signal-cli-rest-api with back deprecated compatibility of https://gitlab.com/morph027/signal-web-gateway
 */
class Gateway implements IGateway {

        /** @var IClientService */
        private $clientService;

        /** @var GatewayConfig */
        private $config;

        /** @var ILogger */
        private $logger;

        public function __construct(IClientService $clientService,
                                                                GatewayConfig $config,
                                                                ILogger $logger,
// added to get SystemValue:
                                                                IConfig $config2) {
                $this->clientService = $clientService;
                $this->config = $config;
                $this->logger = $logger;
// added to get SystemValue:
                $this->config2 = $config2;
        }
        /**
         * @param IUser $user
         * @param string $identifier
         * @param string $message
         *
         * @throws SmsTransmissionException
         * 2023-06-18 patched by OS: added the system variable of the registered number/account SIGNAL_CLI_DBUS_REST_API_ACCOUNT, modified send function
         */
        public function send(IUser $user, string $identifier, string $message) {
                $client = $this->clientService->newClient();
                // determine type of gateway
                $response = $client->get($this->config->getUrl() . '/v1/about');
                if ($response->getStatusCode() === 200) {
                // new v2 style gateway https://github.com/bbernhard/signal-cli-rest-api
                        $recipient_acct = [$identifier];
                        $registered_acct = $this->config2->getSystemValue('SIGNAL_CLI_DBUS_REST_API_ACCOUNT','');
                        $response = $client->post(
                                $this->config->getUrl() . '/v2/send',
                                [
                                        'json' => [
                                                'message' => $message,
                                                'number' => $registered_acct,
                                                'recipients' => $recipient_acct
                                                 ],

                                ]
                        );
                        $body = $response->getBody();
                        $json = json_decode($body, true);
                        if ($response->getStatusCode() !== 201 || is_null($json) || !is_array($json) || !isset($json['timestamp'])) {
                                $status = $response->getStatusCode();
                                throw new SmsTransmissionException("error reported by Signal gateway, status=$status, body=$body}");
                        }
                }
                else {
                        // Try old deprecated gateway https://gitlab.com/morph027/signal-web-gateway
                        $response = $client->post(
                                $this->config->getUrl() . '/v1/send/' . $identifier,
                                [
                                        'body' => [
                                                'to' => $identifier,
                                                'message' => $message,
                                        ],
                                        'json' => [ 'message' => $message ],
                                ]
                        );
                        $body = $response->getBody();
                        $json = json_decode($body, true);

                        if ($response->getStatusCode() !== 200 || is_null($json) || !is_array($json) || !isset($json['success']) || $json['success'] !== true) {
                                $status = $response->getStatusCode();
                                throw new SmsTransmissionException("error reported by Signal gateway, status=$status, body=$body}");
                        }
                }

        }

        /**
         * Get the gateway-specific configuration
         *
         * @return IGatewayConfig
         */
        public function getConfig(): IGatewayConfig {
                return $this->config;
        }
}

So, it takes the variable of the registered account from config.php, eg
SIGNAL_CLI_DBUS_REST_API_ACCOUNT = '+380941112233' and uses to send the auth messages.
A bit dirty hack, but works for me. Please, correct where needed.

@rotdrop
Copy link
Contributor

rotdrop commented Jun 19, 2023

Could you please provide your code as a pull-request? Otherwise it is difficult to see the differences.

Once comment at this point in time: hard-coding the number is not necessary (but see my pull request #575, the current code base is unfortunately broken).

Still: you are trying to use a package that has nothing to do with the REST-API the signal 2fa gateway has been written for. So this issue is more about supporting another REST API provider, and not about changing the version of the current API.

Also, as morph027 states, it is no longer necessary to use an additional REST API wrapper, as signal-cli can do this by itself.

So me feeling here is that one should rewrite the Signal tfa provider to talk to signal-cli directly. BTW, morph027 provides a Python client which can be used for testing here:

https://gitlab.com/morph027/pysignalclijsonrpc

@rotdrop
Copy link
Contributor

rotdrop commented Jun 19, 2023

Also, as morph027 states, it is no longer necessary to use an additional REST API wrapper, as signal-cli can do this by itself.

So me feeling here is that one should rewrite the Signal tfa provider to talk to signal-cli directly. BTW, morph027 provides a Python client which can be used for testing here:

https://gitlab.com/morph027/pysignalclijsonrpc

Actually, that morh027 is not quite correct here. signal-cli itself provides a non-HTTP JSON RPC daemon mode via stdin/stdout or network sockets.

@rotdrop
Copy link
Contributor

rotdrop commented Jun 19, 2023

Ooopps. YES: signal-cli itself DOES provide an HTTP transport for its JSON RPC stuff:

signal-cli 0.11.5+

So my suggestion would then to support that, and do not rely an yet another third party library.

I probably will provide a pull request later.

@rotdrop rotdrop linked a pull request Jun 19, 2023 that will close this issue
@rotdrop
Copy link
Contributor

rotdrop commented Jun 19, 2023

I hava placed a PR which by default uses the native Signal-Cli HTTP endpoint and falls back to morph027 / bbernhard if that is not available.

FYI, the needed sending account is added to the Signal gateway configuration and NOT hard-coded as a constant.

I did not test that bbernhard Docker thing, could you please test it? Thanks!

@ostasevych
Copy link
Author

ostasevych commented Jun 23, 2023

I hava placed a PR which by default uses the native Signal-Cli HTTP endpoint and falls back to morph027 / bbernhard if that is not available.

FYI, the needed sending account is added to the Signal gateway configuration and NOT hard-coded as a constant.

I did not test that bbernhard Docker thing, could you please test it? Thanks!

Hi! Please, advise how to test it? Please, send the instructions.

@rotdrop
Copy link
Contributor

rotdrop commented Aug 18, 2023

Well, you need to checkout the app from git, apply my PR, build the app ... all this inside a Nextcloud installation.

@ostasevych
Copy link
Author

ostasevych commented Nov 18, 2023

Well, you need to checkout the app from git, apply my PR, build the app ... all this inside a Nextcloud installation.

Hi! Sorry for so late question. Have you already applied your PR or are you still waiting for testing from my side?
Would you be so kind to send the link to your PR?

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 a pull request may close this issue.

2 participants