Skip to content

Add verify_ssl option for connection setup to synology_dsm #42539

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.

Sign up for GitHub

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

Merged
Quentame merged 1 commit into from
Nov 2, 2020
Merged

Add verify_ssl option for connection setup to synology_dsm #42539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions homeassistant/components/synology_dsm/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
CONF_SSL,
CONF_TIMEOUT,
CONF_USERNAME,
CONF_VERIFY_SSL,
)
from homeassistant.core import callback
from homeassistant.exceptions import ConfigEntryNotReady
Expand All @@ -43,7 +44,8 @@
from .const import (
CONF_VOLUMES,
DEFAULT_SCAN_INTERVAL,
DEFAULT_SSL,
DEFAULT_USE_SSL,
DEFAULT_VERIFY_SSL,
DOMAIN,
ENTITY_CLASS,
ENTITY_ENABLE,
Expand All @@ -64,7 +66,8 @@
{
vol.Required(CONF_HOST): cv.string,
vol.Optional(CONF_PORT): cv.port,
vol.Optional(CONF_SSL, default=DEFAULT_SSL): cv.boolean,
vol.Optional(CONF_SSL, default=DEFAULT_USE_SSL): cv.boolean,
vol.Optional(CONF_VERIFY_SSL, default=DEFAULT_VERIFY_SSL): cv.boolean,
Comment thread
springstan marked this conversation as resolved.
Copy link
Copy Markdown
Member

@frenck frenck Nov 2, 2020

Choose a reason for hiding this comment

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

Even though this integration does support the configuration flow, we still do not accept changes to the YAML configuration.

This PR, therefore, violates ADR-0010: https://github.com/home-assistant/architecture/blob/master/adr/0010-integration-configuration.md#decision

Copy link
Copy Markdown
Member

@frenck frenck Nov 2, 2020

Choose a reason for hiding this comment

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

@MartinHjelmare Should we revert?

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare Nov 2, 2020

Choose a reason for hiding this comment

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

I think we should just remove the change to the config yaml schema. The rest is ok I think.

Copy link
Copy Markdown
Member

@Quentame Quentame Nov 2, 2020

Choose a reason for hiding this comment

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

Oh wait, we can drop yaml support 💁‍♂️

Copy link
Copy Markdown
Member

@Quentame Quentame Nov 2, 2020

Choose a reason for hiding this comment

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

Ok, I've seen that I'm on "Watch release" on the architecture repo, that explain things 😅

Do you think it's possible to make a release when a new ADR comes out ?
So it won't spam but let followers up to date.

Copy link
Copy Markdown
Member

@frenck frenck Nov 2, 2020

Choose a reason for hiding this comment

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

@Quentame Well... you have been notified of this 4 days ago as well:

See: #42539 (comment)

Copy link
Copy Markdown
Member

@Quentame Quentame Nov 2, 2020

Choose a reason for hiding this comment

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

Indeed 😞
Was too quick on reading apparently.

To forget me, let me remove YAML support.

Copy link
Copy Markdown
Member

@Quentame Quentame Nov 2, 2020

Choose a reason for hiding this comment

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

Proposed #42777, in draft because of a question.

vol.Required(CONF_USERNAME): cv.string,
vol.Required(CONF_PASSWORD): cv.string,
vol.Optional(CONF_DISKS): cv.ensure_list,
Expand Down Expand Up @@ -260,6 +263,7 @@ async def async_setup(self):
self._entry.data[CONF_USERNAME],
self._entry.data[CONF_PASSWORD],
self._entry.data[CONF_SSL],
self._entry.data[CONF_VERIFY_SSL],
timeout=self._entry.options.get(CONF_TIMEOUT),
)
await self._hass.async_add_executor_job(
Expand Down
20 changes: 16 additions & 4 deletions homeassistant/components/synology_dsm/config_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
CONF_SSL,
CONF_TIMEOUT,
CONF_USERNAME,
CONF_VERIFY_SSL,
)
from homeassistant.core import callback
import homeassistant.helpers.config_validation as cv
Expand All @@ -34,8 +35,9 @@
DEFAULT_PORT,
DEFAULT_PORT_SSL,
DEFAULT_SCAN_INTERVAL,
DEFAULT_SSL,
DEFAULT_TIMEOUT,
DEFAULT_USE_SSL,
DEFAULT_VERIFY_SSL,
)
from .const import DOMAIN # pylint: disable=unused-import

Expand All @@ -62,7 +64,13 @@ def _ordered_shared_schema(schema_input):
vol.Required(CONF_USERNAME, default=schema_input.get(CONF_USERNAME, "")): str,
vol.Required(CONF_PASSWORD, default=schema_input.get(CONF_PASSWORD, "")): str,
vol.Optional(CONF_PORT, default=schema_input.get(CONF_PORT, "")): str,
vol.Optional(CONF_SSL, default=schema_input.get(CONF_SSL, DEFAULT_SSL)): bool,
vol.Optional(
CONF_SSL, default=schema_input.get(CONF_SSL, DEFAULT_USE_SSL)
): bool,
vol.Optional(
CONF_VERIFY_SSL,
default=schema_input.get(CONF_VERIFY_SSL, DEFAULT_VERIFY_SSL),
): bool,
}


Expand Down Expand Up @@ -117,7 +125,8 @@ async def async_step_user(self, user_input=None):
port = user_input.get(CONF_PORT)
username = user_input[CONF_USERNAME]
password = user_input[CONF_PASSWORD]
use_ssl = user_input.get(CONF_SSL, DEFAULT_SSL)
use_ssl = user_input.get(CONF_SSL, DEFAULT_USE_SSL)
verify_ssl = user_input.get(CONF_VERIFY_SSL, DEFAULT_VERIFY_SSL)
otp_code = user_input.get(CONF_OTP_CODE)

if not port:
Expand All @@ -126,7 +135,9 @@ async def async_step_user(self, user_input=None):
else:
port = DEFAULT_PORT

api = SynologyDSM(host, port, username, password, use_ssl, timeout=30)
api = SynologyDSM(
host, port, username, password, use_ssl, verify_ssl, timeout=30
)

try:
serial = await self.hass.async_add_executor_job(
Expand Down Expand Up @@ -161,6 +172,7 @@ async def async_step_user(self, user_input=None):
CONF_HOST: host,
CONF_PORT: port,
CONF_SSL: use_ssl,
CONF_VERIFY_SSL: verify_ssl,
CONF_USERNAME: username,
CONF_PASSWORD: password,
CONF_MAC: api.network.macs,
Expand Down
3 changes: 2 additions & 1 deletion homeassistant/components/synology_dsm/const.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
# Configuration
CONF_VOLUMES = "volumes"

DEFAULT_SSL = True
DEFAULT_USE_SSL = True
DEFAULT_VERIFY_SSL = False
Comment thread
MartinHjelmare marked this conversation as resolved.
DEFAULT_PORT = 5000
DEFAULT_PORT_SSL = 5001
# Options
Expand Down
2 changes: 2 additions & 0 deletions homeassistant/components/synology_dsm/strings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"host": "[%key:common::config_flow::data::host%]",
"port": "[%key:common::config_flow::data::port%]",
"ssl": "[%key:common::config_flow::data::ssl%]",
"verify_ssl": "[%key:common::config_flow::data::verify_ssl%]",
"username": "[%key:common::config_flow::data::username%]",
"password": "[%key:common::config_flow::data::password%]"
}
Expand All @@ -23,6 +24,7 @@
"description": "Do you want to setup {name} ({host})?",
"data": {
"ssl": "[%key:common::config_flow::data::ssl%]",
"verify_ssl": "[%key:common::config_flow::data::verify_ssl%]",
"username": "[%key:common::config_flow::data::username%]",
"password": "[%key:common::config_flow::data::password%]",
"port": "[%key:common::config_flow::data::port%]"
Expand Down
8 changes: 4 additions & 4 deletions homeassistant/components/synology_dsm/translations/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
},
"error": {
"cannot_connect": "Failed to connect",
"connection": "Connection error: please check your host, port & ssl",
"invalid_auth": "Invalid authentication",
"login": "Login error: please check your username & password",
"missing_data": "Missing data: please retry later or an other configuration",
"otp_failed": "Two-step authentication failed, retry with a new pass code",
"unknown": "Unexpected error"
Expand All @@ -25,7 +23,8 @@
"password": "Password",
"port": "Port",
"ssl": "Uses an SSL certificate",
"username": "Username"
"username": "Username",
"verify_ssl": "Verify SSL certificate"
},
"description": "Do you want to setup {name} ({host})?",
"title": "Synology DSM"
Expand All @@ -36,7 +35,8 @@
"password": "Password",
"port": "Port",
"ssl": "Uses an SSL certificate",
"username": "Username"
"username": "Username",
"verify_ssl": "Verify SSL certificate"
},
"title": "Synology DSM"
}
Expand Down
36 changes: 25 additions & 11 deletions tests/components/synology_dsm/test_config_flow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@
DEFAULT_PORT,
DEFAULT_PORT_SSL,
DEFAULT_SCAN_INTERVAL,
DEFAULT_SSL,
DEFAULT_TIMEOUT,
DEFAULT_USE_SSL,
DEFAULT_VERIFY_SSL,
DOMAIN,
)
from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_SSDP, SOURCE_USER
Expand All @@ -31,6 +32,7 @@
CONF_SSL,
CONF_TIMEOUT,
CONF_USERNAME,
CONF_VERIFY_SSL,
)
from homeassistant.helpers.typing import HomeAssistantType

Expand All @@ -42,7 +44,8 @@
HOST_2 = "nas.worldwide.me"
SERIAL_2 = "mySerial2"
PORT = 1234
SSL = True
USE_SSL = True
VERIFY_SSL = False
USERNAME = "Home_Assistant"
PASSWORD = "password"
DEVICE_TOKEN = "Dév!cè_T0k€ñ"
Expand Down Expand Up @@ -124,7 +127,8 @@ async def test_user(hass: HomeAssistantType, service: MagicMock):
data={
CONF_HOST: HOST,
CONF_PORT: PORT,
CONF_SSL: SSL,
CONF_SSL: USE_SSL,
CONF_VERIFY_SSL: VERIFY_SSL,
CONF_USERNAME: USERNAME,
CONF_PASSWORD: PASSWORD,
},
Expand All @@ -134,7 +138,8 @@ async def test_user(hass: HomeAssistantType, service: MagicMock):
assert result["title"] == HOST
assert result["data"][CONF_HOST] == HOST
assert result["data"][CONF_PORT] == PORT
assert result["data"][CONF_SSL] == SSL
assert result["data"][CONF_SSL] == USE_SSL
assert result["data"][CONF_VERIFY_SSL] == VERIFY_SSL
assert result["data"][CONF_USERNAME] == USERNAME
assert result["data"][CONF_PASSWORD] == PASSWORD
assert result["data"][CONF_MAC] == MACS
Expand All @@ -150,6 +155,7 @@ async def test_user(hass: HomeAssistantType, service: MagicMock):
data={
CONF_HOST: HOST,
CONF_SSL: False,
CONF_VERIFY_SSL: VERIFY_SSL,
CONF_USERNAME: USERNAME,
CONF_PASSWORD: PASSWORD,
},
Expand All @@ -160,6 +166,7 @@ async def test_user(hass: HomeAssistantType, service: MagicMock):
assert result["data"][CONF_HOST] == HOST
assert result["data"][CONF_PORT] == DEFAULT_PORT
assert not result["data"][CONF_SSL]
assert result["data"][CONF_VERIFY_SSL] == VERIFY_SSL
assert result["data"][CONF_USERNAME] == USERNAME
assert result["data"][CONF_PASSWORD] == PASSWORD
assert result["data"][CONF_MAC] == MACS
Expand Down Expand Up @@ -201,7 +208,8 @@ async def test_user_2sa(hass: HomeAssistantType, service_2sa: MagicMock):
assert result["title"] == HOST
assert result["data"][CONF_HOST] == HOST
assert result["data"][CONF_PORT] == DEFAULT_PORT_SSL
assert result["data"][CONF_SSL] == DEFAULT_SSL
assert result["data"][CONF_SSL] == DEFAULT_USE_SSL
assert result["data"][CONF_VERIFY_SSL] == DEFAULT_VERIFY_SSL
assert result["data"][CONF_USERNAME] == USERNAME
assert result["data"][CONF_PASSWORD] == PASSWORD
assert result["data"][CONF_MAC] == MACS
Expand All @@ -225,7 +233,8 @@ async def test_user_vdsm(hass: HomeAssistantType, service_vdsm: MagicMock):
data={
CONF_HOST: HOST,
CONF_PORT: PORT,
CONF_SSL: SSL,
CONF_SSL: USE_SSL,
CONF_VERIFY_SSL: VERIFY_SSL,
CONF_USERNAME: USERNAME,
CONF_PASSWORD: PASSWORD,
},
Expand All @@ -235,7 +244,8 @@ async def test_user_vdsm(hass: HomeAssistantType, service_vdsm: MagicMock):
assert result["title"] == HOST
assert result["data"][CONF_HOST] == HOST
assert result["data"][CONF_PORT] == PORT
assert result["data"][CONF_SSL] == SSL
assert result["data"][CONF_SSL] == USE_SSL
assert result["data"][CONF_VERIFY_SSL] == VERIFY_SSL
assert result["data"][CONF_USERNAME] == USERNAME
assert result["data"][CONF_PASSWORD] == PASSWORD
assert result["data"][CONF_MAC] == MACS
Expand All @@ -257,7 +267,8 @@ async def test_import(hass: HomeAssistantType, service: MagicMock):
assert result["title"] == HOST
assert result["data"][CONF_HOST] == HOST
assert result["data"][CONF_PORT] == DEFAULT_PORT_SSL
assert result["data"][CONF_SSL] == DEFAULT_SSL
assert result["data"][CONF_SSL] == DEFAULT_USE_SSL
assert result["data"][CONF_VERIFY_SSL] == DEFAULT_VERIFY_SSL
assert result["data"][CONF_USERNAME] == USERNAME
assert result["data"][CONF_PASSWORD] == PASSWORD
assert result["data"][CONF_MAC] == MACS
Expand All @@ -273,7 +284,8 @@ async def test_import(hass: HomeAssistantType, service: MagicMock):
data={
CONF_HOST: HOST_2,
CONF_PORT: PORT,
CONF_SSL: SSL,
CONF_SSL: USE_SSL,
CONF_VERIFY_SSL: VERIFY_SSL,
CONF_USERNAME: USERNAME,
CONF_PASSWORD: PASSWORD,
CONF_DISKS: ["sda", "sdb", "sdc"],
Expand All @@ -285,7 +297,8 @@ async def test_import(hass: HomeAssistantType, service: MagicMock):
assert result["title"] == HOST_2
assert result["data"][CONF_HOST] == HOST_2
assert result["data"][CONF_PORT] == PORT
assert result["data"][CONF_SSL] == SSL
assert result["data"][CONF_SSL] == USE_SSL
assert result["data"][CONF_VERIFY_SSL] == VERIFY_SSL
assert result["data"][CONF_USERNAME] == USERNAME
assert result["data"][CONF_PASSWORD] == PASSWORD
assert result["data"][CONF_MAC] == MACS
Expand Down Expand Up @@ -434,7 +447,8 @@ async def test_form_ssdp(hass: HomeAssistantType, service: MagicMock):
assert result["title"] == "192.168.1.5"
assert result["data"][CONF_HOST] == "192.168.1.5"
assert result["data"][CONF_PORT] == 5001
assert result["data"][CONF_SSL] == DEFAULT_SSL
assert result["data"][CONF_SSL] == DEFAULT_USE_SSL
assert result["data"][CONF_VERIFY_SSL] == DEFAULT_VERIFY_SSL
assert result["data"][CONF_USERNAME] == USERNAME
assert result["data"][CONF_PASSWORD] == PASSWORD
assert result["data"][CONF_MAC] == MACS
Expand Down