search

home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.6k stars 30.76k forks source link

Ambient Weather does not provide app keys #19932

Closed sfgabe closed 5 years ago

sfgabe commented 5 years ago

Ambient weather does not want to provide app keys and has told me the developer of this should have hard coded the app key in to this.

I referred them to the HA documentation and the following was the response:

"Unfortunately, he is wrong. It is one app with many users, not many apps with many users. Thus, he needs to supply you with the app key unique to his application."

marchingphoenix commented 5 years ago

Relates to #18551 Affects #19902

bachya commented 5 years ago

19902 will address this. 👍

tmd224 commented 5 years ago

So I brought this up on the discord channel when I was developing this component. I asked how to securely hardcore an APP key into the code given that it’s open source. I was told there wasn’t really a way around this and that I should have users request both keys themselves.

marchingphoenix commented 5 years ago

Merged from #19933 posted by @ohiospyderman

Home Assistant release with the issue:

0.85.0

Last working Home Assistant release (if known): Ambient Weather Support is new to 0.85.0

Operating environment (Hass.io/Docker/Windows/etc.):

I run HA under Windows 10 (64 bit)

Component/platform:

https://www.home-assistant.io/components/sensor.ambient_station/

Description of problem: When I emailed the Ambient Weather support team for an APP key (I already have an API key), I received the following response:

"Its not the way the app key works.

There is one unique app key per app. Each app key has many users (not the other way around).

The author should hard code the app key"

They won't provide me with the APP key. Without it, HA errors out saying its missing. They provided the following link:

https://ambientweather.net/help/i-need-an-application-key-for-home-assistant/

I would love to be able to use the new Ambient Weather support just included in 0.85.0, but I can't get the key from Ambient Weather!

Problem-relevant configuration.yaml entries and (fill out even if it seems unimportant):

None

Traceback (if applicable):

N.A.

Additional information: Feel free to email me at bstirling@yahoo.com if there is anything I can do to help

bachya commented 5 years ago

@tmd224 No worries at all; it's possible that Ambient was confused, changed their policy, etc. You didn't do anything wrong. Now that we know better, we can adjust!

tmd224 commented 5 years ago

To be clear, I’m talking about devs on the HA discord server.

tmd224 commented 5 years ago

So I have the app key for HA, I wonder what the best way to get it to other users is?

What is your plan for integrating the key with hass?

bachya commented 5 years ago

It sounds like they made a call without enough information. Ambient requires an app key and an API key to authenticate. We don't lose any security by hardcoding an app key when the API key needs to be provided by each user.

EDIT:

So I have the app key for HA, I wonder what the best way to get it to other users is?

I have one as well; before I knew your integration existed, I discussed using mine as the end-all-be-all app key in HASS; the Ambient support guy said, "Sounds great." So, I'll just use the one he gave me and you can do whatever you like with yours.

What is your plan for integrating the key with hass?

Take a look at https://github.com/home-assistant/home-assistant/pull/19902; it's in place.

tmd224 commented 5 years ago

Well the issue is that anyone can get an api key but as we are now seeing they will only issue a single app key for a given app.

They throttle api calls to 3 requests / sec per app key. Now anyone can use our app key since it’s on github which could reduce user experience (google why not to hardcore api keys).

bachya commented 5 years ago

Are you certain of this? In my discussion with their support person, he led me to believe that throttling was based on app key/API key combo, not by app key alone.

Now anyone can use our app key since it’s on github which could reduce user experience (google why not to hardcore api keys).

I think you're blending concepts. Once again: an application key is designed for an API to understand what types of applications are pinging it; API keys are designed to understand which users are pinging it. Combining the two allows for more granular rate limiting, logging, reporting, etc.

tmd224 commented 5 years ago

I did not talk to their developer about this but checkout their docs:

https://ambientweather.docs.apiary.io/#introduction/authentication

tmd224 commented 5 years ago

If it is truly an app/api key combo that is throttled then I think this approach is fine. I interpreted the docs to mean they rate limit app keys.

bachya commented 5 years ago

I now see this part:

API requests are capped at 1 request/second for each user's apiKey and 3 requests/second per applicationKey. When this limit is exceeded, the API will return a 429 response code. Please be kind to our servers :)

...which you pointed out; thanks for the clarification.

Unfortunately, this is a quandary: either we do what they've said not to do (per @sfgabe's first comment above) and have everything obtain their own application key (which Ambient won't do), or we design this in such a way that somehow accommodates data for everyone without endless 429s...

We really should be using Ambient's realtime API, but in my testing, it's pretty buggy.

tmd224 commented 5 years ago

This was exactly my conundrum...so I asked more experienced devs on the discord and they said make users get their own app key.

emlove commented 5 years ago

Someone just needs to explain to Ambient that we are an open source project, and don't host services for our users. All of our other OAuth integrations require users to register their own application keys. It's up to Ambient if they want to support this workflow, or provide an alternative way to authenticate.

bachya commented 5 years ago

I'm a member of their org on GitHub; let me post an issue there and see what kind of response I get. Thanks to everyone for helping me to understand the history here.

bachya commented 5 years ago

FYI: https://github.com/ambient-weather/api-docs/issues/14

tmd224 commented 5 years ago

Ok great, I also emailed Ed who I worked with to get the APP key with initially.

bachya commented 5 years ago

Me too! Ed is great; seems super helpful.

bachya commented 5 years ago

While we wait, a quick response from Ed re: throttling: as @tmd224 helped me recognize, there is application key-specific throttling. However, we can request a decrease in the rate limiting, since we are a "larger app."

Before we do that, I've asked him to help me with their realtime API; if we get that figured out (and change our integration to a cloud push), the rate-limiting problem goes away. Assuming they can help, I'm planning on creating a separate, async-focused Ambient package that has support for both REST and realtime APIs.

Just FYI. More to come, hopefully.

bachya commented 5 years ago

Okay – response from Ed re: hardcoding the application key:

The issue is that it is time consuming since we review each app key. I would not worry about posting the app key. I doubt anyone will do anything nefarious with it. Anyone that writes their own application will want their own app key. Thanks Ed

CC: @armills

tmd224 commented 5 years ago

I just got similar response

bachya commented 5 years ago

@armills Thoughts on Ambient's response? They will not be providing unique application keys per user; I think there are two options:

  1. We allow a HASS application key to be hardcoded.
  2. If that's not part of our standards, we remove the platform (since it will not work for new users).
emlove commented 5 years ago

In my opinion, option 1 is unacceptable. Even if Ambient don't care about security, hard-coding those secrets reflects very poorly on home assistant as an organization. We'll still be the ones responsible for releasing keys publicly whether or not one of their reps felt it was OK.

bachya commented 5 years ago

@balloob Need you to weigh in on this one.

balloob commented 5 years ago

If Ambient tells us to hardcode the app key, we should hardcode the app key. Let's add a comment next to it with a link to their comment where they tell us to.

tube0013 commented 5 years ago

they added a comment asking to hold off now... image

OhioSpyderman commented 5 years ago

End user here, we can already create our own API keys, I already did that when I got the weather station.No big news here... Just my .02 Bob. 

On Tuesday, January 15, 2019, 3:57:09 PM EST, tube0013 <notifications@github.com> wrote:

they added a comment asking to hold off now...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

bachya commented 5 years ago

Ambient has responded and is now allowing users to create their own application keys. This should free up the current implementation to start working (and #19902 can simply focus on architecture); closing this.