search

tdex-network / tdex-daemon

Go implementation of the TDEX Beta Daemon
https://tdex.network
MIT License
11 stars 13 forks source link

Tdexconnect add proto & host to the url #625

Closed sekulicd closed 2 years ago

sekulicd commented 2 years ago

This adds protocol and host to the tdex connect url.

This closes #622

@tiero please review.

sekulicd commented 2 years ago

since the daemon's operator interface can now be served with macaroon but without tls, the --insecure flag of the tdexdconnect service must be replaced with 2 flags --no-tls and --no-macaroon to have proper control on what to include in the connect URL that is gonna be generated.

Done

sekulicd commented 2 years ago

@tiero @altafan please review again.

altafan commented 2 years ago

@sekulicd @tiero I think in this PR we're forgetting about the case where the Operator service has TLS enabled with self-signed cert and extra_ip/extra_domain set. As already discussed with dusan, the simplest solution is to override host in cmd/tdexd/main.go in case an operator extra domain or IP is set (by giving precedence to domain since both can be set). At the same time, in config we should enforce that connectUrl host and operator extra ip/domain are mutually exclusive since the first means using a CA cert while the second ones mean using a self-signed cert for the operator interface.

sekulicd commented 2 years ago

@sekulicd @tiero I think in this PR we're forgetting about the case where the Operator service has TLS enabled with self-signed cert and extra_ip/extra_domain set. As already discussed with dusan, the simplest solution is to override host in cmd/tdexd/main.go in case an operator extra domain or IP is set (by giving precedence to domain since both can be set). At the same time, in config we should enforce that connectUrl host and operator extra ip/domain are mutually exclusive since the first means using a CA cert while the second ones mean using a self-signed cert for the operator interface.

@tiero @altafan please check if this is ok.

tiero commented 2 years ago

At the same time, in config we should enforce that connectUrl host and operator extra ip/domain are mutually exclusive since the first means using a CA cert while the second ones mean using a self-signed cert for the operator interface.

Don't think so, they are not mutually exclusive, but it's correct, if both given, they can't be different otherwise won't work.

What I would do is instead return error when given if they are different, in way that at least either IP or DOMAIN must be euqal to HOST.

extra_ip/extra_domain should only affect behvior of generation of self signed cert, and CONNECT_URL_HOST should only affect what is shown in the HTML Page.

We should also return error if IP/DOMAIN is given and NO_OPERATOR_TLS is given together