nextcloud / twofactor_gateway

🔑 Second factor provider using an external messaging gateway (SMS, Telegram, Signal)
GNU Affero General Public License v3.0
109 stars 63 forks source link

Not compatible with v2 of the Signal REST API #573

Open ostasevych opened 1 year ago

ostasevych commented 1 year ago

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 ?

rotdrop commented 1 year ago

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 commented 1 year ago

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);

/**

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;

/**

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

ostasevych commented 1 year ago

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?