diff --git a/daemon/src/account.cpp b/daemon/src/account.cpp
index 51c150dc4a2da564fe9181fce0a00024cc82d199..454bdbf0ca34b2d59abbc39a206fd14254e75c10 100644
--- a/daemon/src/account.cpp
+++ b/daemon/src/account.cpp
@@ -83,6 +83,7 @@ const char * const Account::DEFAULT_USER_AGENT = PACKAGE_NAME "/" PACKA
const char * const Account::USER_AGENT_KEY = "useragent";
const char * const Account::HAS_CUSTOM_USER_AGENT_KEY = "hasCustomUserAgent";
const char * const Account::PRESENCE_MODULE_ENABLED_KEY = "presenceModuleEnabled";
+const char * const Account::UPNP_ENABLED_KEY = "upnpEnabled";
using std::map;
using std::string;
@@ -106,6 +107,7 @@ Account::Account(const string &accountID)
, userAgent_(DEFAULT_USER_AGENT)
, hasCustomUserAgent_(false)
, mailBox_()
+ , upnpEnabled_(false)
{
std::random_device rdev;
std::seed_seq seed {rdev(), rdev()};
@@ -194,6 +196,7 @@ void Account::serialize(YAML::Emitter &out)
out << YAML::Key << USERNAME_KEY << YAML::Value << username_;
out << YAML::Key << DISPLAY_NAME_KEY << YAML::Value << displayName_;
out << YAML::Key << HOSTNAME_KEY << YAML::Value << hostname_;
+ out << YAML::Key << UPNP_ENABLED_KEY << YAML::Value << upnpEnabled_;
}
void Account::unserialize(const YAML::Node &node)
@@ -218,6 +221,8 @@ void Account::unserialize(const YAML::Node &node)
parseValue(node, USER_AGENT_KEY, userAgent_);
parseValue(node, RINGTONE_PATH_KEY, ringtonePath_);
parseValue(node, RINGTONE_ENABLED_KEY, ringtoneEnabled_);
+
+ parseValue(node, UPNP_ENABLED_KEY, upnpEnabled_);
}
void Account::setAccountDetails(const std::map<std::string, std::string> &details)
@@ -237,6 +242,7 @@ void Account::setAccountDetails(const std::map<std::string, std::string> &detail
parseString(details, Conf::CONFIG_ACCOUNT_USERAGENT, userAgent_);
else
userAgent_ = DEFAULT_USER_AGENT;
+ parseBool(details, Conf::CONFIG_UPNP_ENABLED, upnpEnabled_);
}
std::map<std::string, std::string> Account::getAccountDetails() const
@@ -260,6 +266,7 @@ std::map<std::string, std::string> Account::getAccountDetails() const
a[Conf::CONFIG_ACCOUNT_AUTOANSWER] = autoAnswerEnabled_ ? TRUE_STR : FALSE_STR;
a[Conf::CONFIG_RINGTONE_ENABLED] = ringtoneEnabled_ ? TRUE_STR : FALSE_STR;
a[Conf::CONFIG_RINGTONE_PATH] = ringtonePath_;
+ a[Conf::CONFIG_UPNP_ENABLED] = upnpEnabled_ ? TRUE_STR : FALSE_STR;
return a;
}
@@ -459,26 +466,41 @@ Account::parseBool(const std::map<std::string, std::string> &details, const char
#undef find_iter
-void
-Account::setUseUPnP(bool useUPnP)
+/**
+ * Checks whether the upnp settings is enabled in the account.
+ * If so, tries to get a controller with a valid IGD.
+ * If not, destroys the controller.
+ *
+ * Returns whether or not there is a controller with a valid IGD to use.
+ */
+bool
+Account::checkUPnP()
{
std::unique_lock<std::mutex> lk(upnp_mtx);
- if (useUPnP == static_cast<bool>(upnp_))
- return;
+ if (upnpEnabled_ != (bool)upnp_) {
+ if (upnpEnabled_){
+ upnp_.reset(new upnp::Controller());
+ if (upnp_->hasValidIGD())
+ upnpIp_ = upnp_->getExternalIP();
+ } else {
+ upnp_.reset();
+ upnpIp_ = IpAddr{};
+ }
+ }
- if (useUPnP){
- upnp_.reset(new upnp::Controller());
- upnpIp_ = upnp_->getExternalIP();
- } else
- upnp_.reset();
+ return upnp_ and upnp_->hasValidIGD();
}
+/**
+ * Returns whether or not the upnp controller has a valid IGD
+ * with which to make port mappings
+ */
bool
Account::getUseUPnP() const
{
std::unique_lock<std::mutex> lk(upnp_mtx);
- return static_cast<bool>(upnp_);
+ return upnp_ and upnp_->hasValidIGD();
}
} // namespace ring
diff --git a/daemon/src/account.h b/daemon/src/account.h
index 15590e4762a482c55b8a2eff8593f73406b36ada..9f5075ff203caeba9f003169f0819e572a9ac4a3 100644
--- a/daemon/src/account.h
+++ b/daemon/src/account.h
@@ -255,12 +255,6 @@ class Account : public Serializable, public std::enable_shared_from_this<Account
static const char * const VIDEO_CODEC_BITRATE;
/**
- * Set whether or not to use UPnP
- */
- void setUseUPnP(bool useUPnP);
-
- /**
- * Get whether UPnP is used.
* @return bool Flag which determines if UPnP is used or not.
*/
bool getUseUPnP() const;
@@ -313,6 +307,7 @@ class Account : public Serializable, public std::enable_shared_from_this<Account
static const char * const HAS_CUSTOM_USER_AGENT_KEY;
static const char * const DEFAULT_USER_AGENT;
static const char * const PRESENCE_MODULE_ENABLED_KEY;
+ static const char * const UPNP_ENABLED_KEY;
static std::string mapStateNumberToString(RegistrationState state);
@@ -414,6 +409,23 @@ class Account : public Serializable, public std::enable_shared_from_this<Account
IpAddr upnpIp_ {};
std::unique_ptr<ring::upnp::Controller> upnp_;
mutable std::mutex upnp_mtx {};
+
+ /**
+ * Whether or not UPnP is enabled for this account
+ * Note that this is only used to store the account setting,
+ * getUseUPnP() should be used to check if UPnP is being use
+ * ie: if the UPnP controller has been initialized
+ */
+ bool upnpEnabled_;
+
+ /**
+ * Checks whether the upnp settings is enabled in the account.
+ * If so, tries to get a controller with a valid IGD.
+ * If not, destroys the controller.
+ *
+ * Returns whether or not there is a controller with a valid IGD to use.
+ */
+ bool checkUPnP();
};
} // namespace ring
diff --git a/daemon/src/account_schema.h b/daemon/src/account_schema.h
index 4c2607809ca2a4c8ebc0e3f0ee322c52df9a5d20..495b617073e9b1a517a0ab4b189134d04297dea5 100644
--- a/daemon/src/account_schema.h
+++ b/daemon/src/account_schema.h
@@ -74,6 +74,7 @@ static const char *const CONFIG_PUBLISHED_SAMEAS_LOCAL = "Account.publi
static const char *const CONFIG_LOCAL_PORT = "Account.localPort";
static const char *const CONFIG_PUBLISHED_PORT = "Account.publishedPort";
static const char *const CONFIG_PUBLISHED_ADDRESS = "Account.publishedAddress";
+static const char *const CONFIG_UPNP_ENABLED = "Account.upnpEnabled";
// SIP specific parameters
static const char *const CONFIG_STUN_SERVER = "STUN.server";
diff --git a/daemon/src/managerimpl.cpp b/daemon/src/managerimpl.cpp
index cb4586f80b87b1571cadc688d71de86bf976457f..c9d01821955678bfb31e0d3df23b28bbaf4d73bd 100644
--- a/daemon/src/managerimpl.cpp
+++ b/daemon/src/managerimpl.cpp
@@ -243,10 +243,10 @@ ManagerImpl::init(const std::string &config_file)
history_.load(preferences.getHistoryLimit());
- if (preferences.getUseUPnP()) {
- RING_DBG("Remove any old UPnP mappings for RING mapped to this IP");
- upnp::Controller().removeMappingsByLocalIPAndDescription();
- }
+ /* TODO: Is this necessary? This will require looking for a UPnP enabled IGD at each start
+ * of the daemon... */
+ RING_DBG("Remove any old UPnP mappings for RING mapped to this IP");
+ upnp::Controller().removeMappingsByLocalIPAndDescription();
registerAccounts();
}
@@ -295,10 +295,9 @@ ManagerImpl::finish()
RING_ERR("%s", err.what());
}
- if (preferences.getUseUPnP()) {
- RING_DBG("Remove any remaning ports mapped to this client for RING");
- upnp::Controller().removeMappingsByLocalIPAndDescription();
- }
+
+ RING_DBG("Remove any remaning ports mapped to this client for RING");
+ upnp::Controller().removeMappingsByLocalIPAndDescription();
}
bool
@@ -2407,7 +2406,6 @@ ManagerImpl::setAccountDetails(const std::string& accountID,
saveConfig();
if (account->isEnabled()) {
- account->setUseUPnP(preferences.getUseUPnP());
account->doRegister();
} else
account->doUnregister();
@@ -2454,8 +2452,6 @@ ManagerImpl::addAccount(const std::map<std::string, std::string>& details)
preferences.addAccount(newAccountID);
- newAccount->setUseUPnP(preferences.getUseUPnP());
-
newAccount->doRegister();
saveConfig();
@@ -2715,7 +2711,6 @@ ManagerImpl::registerAccounts()
a->loadConfig();
if (a->isEnabled()) {
- a->setUseUPnP(preferences.getUseUPnP());
a->doRegister();
}
}
@@ -2743,7 +2738,6 @@ ManagerImpl::sendRegister(const std::string& accountID, bool enable)
Manager::instance().saveConfig();
if (acc->isEnabled()) {
- acc->setUseUPnP(preferences.getUseUPnP());
acc->doRegister();
} else
acc->doUnregister();
diff --git a/daemon/src/preferences.cpp b/daemon/src/preferences.cpp
index 57c81f6ca0cb6dd0dc2dec7628fa6781a64d53f4..504c34fbc3e7c22631ebf3652f7473b70bc29bea 100644
--- a/daemon/src/preferences.cpp
+++ b/daemon/src/preferences.cpp
@@ -77,7 +77,6 @@ static const char * const ZONE_TONE_CHOICE_KEY = "zoneToneChoice";
static const char * const PORT_NUM_KEY = "portNum";
static const char * const SEARCH_BAR_DISPLAY_KEY = "searchBarDisplay";
static const char * const MD5_HASH_KEY = "md5Hash";
-static const char * const USE_UPNP = "useUPnP";
// voip preferences
constexpr const char * const VoipPreference::CONFIG_LABEL;
@@ -138,7 +137,6 @@ Preferences::Preferences() :
, portNum_(5060)
, searchBarDisplay_(true)
, md5Hash_(false)
- , useUPnP_(false)
{}
void Preferences::verifyAccountOrder(const std::vector<std::string> &accountIDs)
@@ -197,7 +195,6 @@ void Preferences::serialize(YAML::Emitter &out)
out << YAML::Key << REGISTRATION_EXPIRE_KEY << YAML::Value << registrationExpire_;
out << YAML::Key << SEARCH_BAR_DISPLAY_KEY << YAML::Value << searchBarDisplay_;
out << YAML::Key << ZONE_TONE_CHOICE_KEY << YAML::Value << zoneToneChoice_;
- out << YAML::Key << USE_UPNP << YAML::Value << useUPnP_;
out << YAML::EndMap;
}
@@ -213,7 +210,6 @@ void Preferences::unserialize(const YAML::Node &in)
parseValue(node, PORT_NUM_KEY, portNum_);
parseValue(node, SEARCH_BAR_DISPLAY_KEY, searchBarDisplay_);
parseValue(node, MD5_HASH_KEY, md5Hash_);
- parseValue(node, USE_UPNP, useUPnP_);
}
VoipPreference::VoipPreference() :
diff --git a/daemon/src/preferences.h b/daemon/src/preferences.h
index f601e7c616afd012122e600db6180983c1c3aca2..54887d326565a5c89f54106e3ae45e11215dd549 100644
--- a/daemon/src/preferences.h
+++ b/daemon/src/preferences.h
@@ -126,13 +126,6 @@ class Preferences : public Serializable {
md5Hash_ = md5;
}
- bool getUseUPnP() const {
- return useUPnP_;
- }
- void setUseUPnP(bool upnp) {
- useUPnP_ = upnp;
- }
-
private:
std::string accountOrder_;
int historyLimit_;
@@ -142,7 +135,6 @@ class Preferences : public Serializable {
int portNum_;
bool searchBarDisplay_;
bool md5Hash_;
- bool useUPnP_;
constexpr static const char * const CONFIG_LABEL = "preferences";
};
diff --git a/daemon/src/ringdht/ringaccount.cpp b/daemon/src/ringdht/ringaccount.cpp
index f52e29856d261293b271d21e69c44a487b2acced..c07cd925cd38355e006cffe9cf1a60a80825dfa0 100644
--- a/daemon/src/ringdht/ringaccount.cpp
+++ b/daemon/src/ringdht/ringaccount.cpp
@@ -603,7 +603,9 @@ void RingAccount::doRegister()
return;
}
- if (not mapPortUPnP())
+ /* check UPnP to see if the account setting is enabled and try to get a
+ * UPnP controller; if this succeeds then try to map the account port */
+ if (not (checkUPnP() and mapPortUPnP()) )
RING_WARN("Could not successfully map DHT port with UPnP, continuing with account registration anyways.");
try {
diff --git a/daemon/src/sip/sipaccount.cpp b/daemon/src/sip/sipaccount.cpp
index 2cbd92b08af26588e2b138da54a1568bb9a4e147..125c2c173414024ce1f15e575ab0a955015b2f24 100644
--- a/daemon/src/sip/sipaccount.cpp
+++ b/daemon/src/sip/sipaccount.cpp
@@ -744,7 +744,9 @@ void SIPAccount::doRegister()
RING_DBG("doRegister %s", hostname_.c_str());
- if (not mapPortUPnP())
+ /* check UPnP to see if the account setting is enabled and try to get a
+ * UPnP controller; if this succeeds then try to map the account port */
+ if (not (checkUPnP() and mapPortUPnP()) )
RING_WARN("Could not successfully map SIP port with UPnP, continuing with account registration anyways.");
if (hostname_.empty() || isIP2IP()) {
diff --git a/daemon/src/upnp/upnp.cpp b/daemon/src/upnp/upnp.cpp
index ac2ff91c48abbb946f2f4ca875330fb38f639b57..70e246e4923b0eb8954a4c35a09cb39539285be4 100644
--- a/daemon/src/upnp/upnp.cpp
+++ b/daemon/src/upnp/upnp.cpp
@@ -88,7 +88,6 @@ IGD& IGD::operator=(IGD&& other)
IGD::~IGD()
{
/* free the URLs */
- RING_WARN("UPnP : deleting IGD");
FreeUPNPUrls(&urls_);
}
@@ -145,6 +144,19 @@ Controller::~Controller()
removeMappings();
}
+/**
+ * Return whether or not this controller has a valid IGD,
+ * if 'flase' then all requests will fail
+ */
+bool
+Controller::hasValidIGD()
+{
+#if HAVE_UPNP
+ return not defaultIGD_->isEmpty();
+#endif
+ return false;
+}
+
/**
* tries to add mapping
*/
diff --git a/daemon/src/upnp/upnp.h b/daemon/src/upnp/upnp.h
index 86902edbf3c934ad27f457db9cce032d3022d9ee..dfd9d9259dcff5fa6e6d1f877b1450e36318ac93 100644
--- a/daemon/src/upnp/upnp.h
+++ b/daemon/src/upnp/upnp.h
@@ -162,6 +162,12 @@ public:
/* destructor */
~Controller();
+ /**
+ * Return whether or not this controller has a valid IGD,
+ * if 'flase' then all requests will fail
+ */
+ bool hasValidIGD();
+
/**
* tries to add mapping from and to the port_desired
* if unique == true, makes sure the client is not using this port already