Skip to content

Added HttpDigestAuth ability to Telegram when sending photos#7396

Closed
tchellomello wants to merge 2 commits into
home-assistant:devfrom
tchellomello:telegram_url
Closed

Added HttpDigestAuth ability to Telegram when sending photos#7396
tchellomello wants to merge 2 commits into
home-assistant:devfrom
tchellomello:telegram_url

Conversation

@tchellomello
Copy link
Copy Markdown
Contributor

@tchellomello tchellomello commented May 2, 2017

Description:

When creating an automation/script using notify/Telegram with URL photo support, it will not work if the webserver requires an HTTPDigestAuth.

For example, the new Amcrest firmware requires a HttpDigestAuth to download the camera snapshot.

Related issue (if applicable): fixes #7395

Example entry for configuration.yaml (if applicable):

#scripts.yaml
send_snapshot_telegram_driveway:
  alias: 'Send snapshot driveway camera'
  sequence:
    - service: notify.telebot
      data:
        title: "Driveway Motion"
        message: "There is a motion detected on the driveway"
        data:
          photo:
            - url: !secret amcrest_driveway_snapshot_url
              username: !secret amcrest_driveway_username
              password: !secret amcrest_driveway_password

Checklist:

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link
Copy Markdown

@tchellomello, thanks for your PR! By analyzing the history of the files in this pull request, we identified @pvizeli, @fabaff and @icovada to be potential reviewers.

Copy link
Copy Markdown
Member

@pvizeli pvizeli left a comment

Choose a reason for hiding this comment

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

Please do it in same way as we do it inside generic camer

@balloob
Copy link
Copy Markdown
Member

balloob commented May 3, 2017

Let's put this PR on hold for now until #7294 gets merged

 method to use. Currently methods supported are: 'digest' and 'basic'.
@tchellomello
Copy link
Copy Markdown
Contributor Author

@pvizeli since the telegram component can be used for multiple purposes, instead of making setting a self._authentication value globally, I kept the same approach and kept the authentication as an attribute to be used like caption for example.

If you agree with that, I'll go ahead and modify the documentation explaining the new configuration:

send_snapshot:
  alias: 'send snapshot'
  sequence:
    - service: notify.telegramBot
      data:
        title: 'Snapshot Cam'
        message: 'Sending snap from cam'
        data:
          photo:
            - url: !secret amcrest_url
              caption: "Snapshot Amcrest"
              authentication: "digest"
              username: !secret amcrest_username
              password: !secret amcrest_password

If authentication is not present, then it will try basic auth.

@tchellomello
Copy link
Copy Markdown
Contributor Author

@balloob sounds good.

@pvizeli @balloob Thanks for your help.

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented May 3, 2017

Yeah, don't do that global, make it per request and default Basic 👍

@tchellomello
Copy link
Copy Markdown
Contributor Author

@pvizeli yes, so that is the way it is now. As @balloob mention, let me know if that looks good after the PR #7294

Thank you!

@balloob
Copy link
Copy Markdown
Member

balloob commented May 5, 2017

Alright, closing this one for now then

@balloob balloob closed this May 5, 2017
@tchellomello tchellomello deleted the telegram_url branch May 18, 2017 22:22
azogue added a commit to azogue/home-assistant that referenced this pull request May 25, 2017
MartinHjelmare pushed a commit that referenced this pull request May 26, 2017
…as, variable templating, digest auth (#7771)

* fix double template rendering when messages come from notify.telegram

* fix 'chat' information not present in callback queries

* better inline keyboards with yaml

To make a row of InlineKeyboardButtons you pass:
- a list of tuples like: `[(text_b1, data_callback_b1), (text_b2, data_callback_b2), ...]
- a string like: `/cmd1, /cmd2, /cmd3`
- or a string like: `text_b1:/cmd1, text_b2:/cmd2`

Example:
```yaml
data:
   message: 'TV is off'
   disable_notification: true
   inline_keyboard:
     - TV ON:/service_call switch.turn_on switch.tv, Other:/othercmd
     - /help, /init
```

* fix send file to multiple targets

* fix message templating, multiple file targets, HA cameras

- Allow templating for caption, url, file, longitude and latitude fields
- Fix send a file to multiple targets
- Load data with some retrying for HA cameras, which return 500 one or two times sometimes (generic cams, always!).
- Doc in services for new inline keyboards yaml syntax: `Text button:/command`

* HttpDigest authentication as proposed in #7396

* review changes

- Don't use `file` as variable name.
- For loop
- Simplify filter allowed `chat_id`s.

* Don't use `file` as variable name!

* make params outside the while loop

* fix chat_id validation when editing sent messages
@home-assistant home-assistant locked and limited conversation to collaborators Sep 4, 2017
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.

Telegram does not send photo if URL requires HttpDigestAuth

6 participants