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

Support for Plex servers with enforced SSL #8341

Merged
merged 5 commits into from
Jul 12, 2017
Merged

Support for Plex servers with enforced SSL #8341

merged 5 commits into from
Jul 12, 2017

Conversation

nmaggioni
Copy link
Contributor

@nmaggioni nmaggioni commented Jul 4, 2017

Description:

This PR introduces support for Plex servers that enforce SSL. Such servers may not have signed SSL certificates available for their local domain, causing connection failures for API requests.

The plex.conf file format was updated to enable SSL connections and invalid certificate credentials, toggleable by the user. Safe defaults are in place: no SSL, and certificate validation enabled.

@Hellowlol may want to take a look at this PR.

Related issue: fixes #7602

Pull request in home-assistant.github.io with documentation: home-assistant/home-assistant.io#2932

Example entry for plex.conf:

From this:

{"x.x.x.x:32400": {"token": "..."}}

To this:

{"x.x.x.x:32400": {"token": "...", "ssl": true, "verify": false}}

Checklist:

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully.
  • New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

@homeassistant
Copy link
Contributor

Hi @nmaggioni,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

'name': 'Use SSL',
'type': ''
},
{

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented

@@ -230,6 +253,16 @@ def plex_configuration_callback(data):
'id': 'token',
'name': 'X-Plex-Token',
'type': ''
},
{

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented

try:
plexserver = plexapi.server.PlexServer('http://%s' % host, token)
plexserver = plexapi.server.PlexServer('%s://%s' % (http_prefix, host), token, cert_session)

Choose a reason for hiding this comment

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

line too long (100 > 79 characters)



def setup_plexserver(host, token, hass, config, add_devices_callback):
def setup_plexserver(host, token, has_ssl, verify_ssl, hass, config, add_devices_callback):

Choose a reason for hiding this comment

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

line too long (91 > 79 characters)

else:
return

setup_plexserver(host, token, hass, config, add_devices_callback)
setup_plexserver(host, token, has_ssl, verify_ssl, hass, config, add_devices_callback)

Choose a reason for hiding this comment

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

line too long (90 > 79 characters)

'id': 'has_ssl',
'name': 'Use SSL',
'type': ''
},{

Choose a reason for hiding this comment

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

missing whitespace after ','

@@ -230,6 +260,14 @@ def plex_configuration_callback(data):
'id': 'token',
'name': 'X-Plex-Token',
'type': ''
},{

Choose a reason for hiding this comment

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

missing whitespace after ','

@@ -217,7 +243,11 @@ def request_configuration(host, hass, config, add_devices_callback):
def plex_configuration_callback(data):
"""Handle configuration changes."""
setup_plexserver(
host, data.get('token'), hass, config, add_devices_callback)
host, data.get('token'),
True if data.get('has_ssl') else False,
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to validate the data. Just presence of it is not enough. Use homeassistant.helpers.config_validation.boolean(value) to convert the values.

Better would be if you implement checkbox support in the frontend https://github.com/home-assistant/home-assistant-polymer/blob/master/src/more-infos/more-info-configurator.html#L66-L73

Copy link
Contributor Author

@nmaggioni nmaggioni Jul 7, 2017

Choose a reason for hiding this comment

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

I set up data validation; an update to the documentation PR is on its way.

Checkboxes would have been a much better alternative, but unfortunately I don't have enough experience in Polymer and the whole HA developing ecosystem to develop that feature at its best.

Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

validate / process input from configurator.

@balloob
Copy link
Member

balloob commented Jul 7, 2017

Have a look at the Travis build results. There are still some lint issues. The snips lint issue can be ignored (or fixed with a rebase)

Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

Once linting issues resolved.

@nmaggioni
Copy link
Contributor Author

@balloob Anything else that needs improving?

@balloob balloob merged commit 229000b into home-assistant:dev Jul 12, 2017
@balloob
Copy link
Member

balloob commented Jul 12, 2017

My bad. Was busy. 👍 all good

@nmaggioni
Copy link
Contributor Author

nmaggioni commented Jul 12, 2017

No problem at all, life goes on for everyone :)

What about the docs PR?

@balloob balloob mentioned this pull request Jul 13, 2017
Landrash pushed a commit to home-assistant/home-assistant.io that referenced this pull request Jul 17, 2017
* Update Plex docs for SSL connections (home-assistant/core#8341)

* Stricter values allowed in the configurator

home-assistant/core@f8808c6
dethpickle pushed a commit to dethpickle/home-assistant that referenced this pull request Aug 18, 2017
* Support for Plex servers with enforced SSL

* Fixed HoundCI warnings

* Fixed HoundCI warnings (2nd)

* Configurator data validation

* Travis linting
@home-assistant home-assistant locked and limited conversation to collaborators Oct 20, 2017
@ghost ghost added the integration: plex label Mar 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SSL support for Plex Server
5 participants