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

Add HTTP auth and SSL verification to REST notify #22016

Merged
merged 4 commits into from Mar 15, 2019

Conversation

Projects
None yet
5 participants
@escoand
Copy link
Contributor

commented Mar 13, 2019

Breaking Change:

nothing

Description:

add username, password, authentication and verify_ssl to rest notification

Related issue (if applicable): fixes #

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#8935

Example entry for configuration.yaml (if applicable):

- name: nextcloud
  platform: rest
  resource: https://my-nextcloud/ocs/v2.php/apps/admin_notifications/api/v2/notifications/my-user
  method: POST
  authentication: basic
  username: my-user
  password: 1234567890
  verify_ssl: false
  headers:
    OCS-APIREQUEST: 'true'
  title_param_name: shortMessage
  message_param_name: longMessage

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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

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

  • [ ] New dependencies have been added to the REQUIREMENTS variable (example).
  • [ ] New dependencies are only imported inside functions that use them (example).
  • [ ] New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • [ ] New files were added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@awarecan awarecan changed the title add HTTP auth and SSL verification Add HTTP auth and SSL verification to REST notify Mar 13, 2019

@awarecan
Copy link
Contributor

left a comment

You already have the capability to set the header of request, add username/password/auth seems do not have much value add, but I think it won't hurt.

verify_ssl is a good enhancement.

Show resolved Hide resolved homeassistant/components/notify/rest.py Outdated
@escoand

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

Off course but generating the base64 combination and the digest is a unneeded hurdle. We have computers to do that.

@escoand

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

With this it's now on par with the rest sensor.

@awarecan
Copy link
Contributor

left a comment

Can be merged after build done.

@escoand escoand referenced this pull request Mar 14, 2019

Merged

describe new attributes for rest notify #8935

2 of 2 tasks complete
@fabaff

fabaff approved these changes Mar 15, 2019

@fabaff fabaff merged commit 3b34594 into home-assistant:dev Mar 15, 2019

4 checks passed

Hound No violations found. Woof!
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.06%) to 92.768%
Details

@ghost ghost removed the in progress label Mar 15, 2019

@escoand escoand deleted the escoand:patch-1 branch Mar 15, 2019

mxworm added a commit to mxworm/home-assistant that referenced this pull request Mar 15, 2019

Merge branch 'dev' into current
* dev: (177 commits)
  Bump androidtv to 0.0.12 (home-assistant#22072)
  Fix func
  Updated frontend to 20190315.0
  Binary Sensor for Remote UI & Fix timezone (home-assistant#22076)
  Update rest.py (home-assistant#22077)
  Correct context (home-assistant#22061)
  Upgrade ruamel.yaml to 0.15.89 (home-assistant#22064)
  Fix more test
  Fix test
  Return config entry ID after creation (home-assistant#22060)
  Add HTTP auth and SSL verification to REST notify (home-assistant#22016)
  Mobile App: Expose Cloud Remote UI FQDN in registration response (home-assistant#22055)
  Cleanup remaining constants stuff in ZHA (home-assistant#22050)
  Fix cloud services.yaml
  Minor ZHA consts fixes (home-assistant#22049)
  Mobile App: Sensors (home-assistant#21854)
  Fix Google Assistant User with Cloud (home-assistant#22042)
  Mobile App: Register devices into the registry (home-assistant#21856)
  Z-Wave usb_path in configuration.yaml overrides config entry usb_path (home-assistant#22038)
  Update additional platforms to use new MQTT message callback (home-assistant#22030)
  ...

@balloob balloob referenced this pull request Apr 3, 2019

Merged

0.91.0 #22688

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.