aarondcoleman / Fitbit.NET

Fitbit .NET API Client Library
MIT License
197 stars 140 forks source link

SubscriptionManager class is missing from Fitbit.Portable #81

Open mxa0079 opened 8 years ago

mxa0079 commented 8 years ago

Current SubscriptionManager implementation depends on RestSharp. We need to re-implement functionality without dependency on RestSharp.

WestDiscGolf commented 8 years ago

What does the subscription manager do, could never work it out. Also there is heavy reliance on RestSharp and Xml, both of which we don't support any more

WestDiscGolf commented 8 years ago

As far as I can make out all it does is data parsing? @aarondcoleman is a SubscriptionManager still applicable? What is it used for in the new json world? How do you use it at Fitabase?

mxa0079 commented 8 years ago

Shell implementation is in. I think this can be more efficiently fixed by @aarondcoleman since he has full background.

aarondcoleman commented 8 years ago

Yeah, exactly right @mxa0079 @WestDiscGolf -- SubscriptionManager is mostly a deserializer that I had high hopes for. The background on SubscriptionManager is that when you get a PubSub http response, you just need to parse out what's in it. You don't need client keys / secrets, so adding helper methods to FitbitClient didn't seem right.

Open to suggestions for where that should go. Would also love to not ignore the signature header like we've been doing.

WestDiscGolf commented 8 years ago

@aarondcoleman totally agree that it shouldn't be in FitbitClient as it is completely different / separate. What is in the signature header? What type of data and parsing is to be expected? How would you expect to use it?

WestDiscGolf commented 8 years ago

Might need a few more readings, don't quite grok this at the moment - https://dev.fitbit.com/docs/subscriptions/#security

WestDiscGolf commented 8 years ago

For reference: Hostname -> IP - https://msdn.microsoft.com/en-us/library/ms143998(v=vs.110).aspx IP -> host - https://msdn.microsoft.com/en-us/library/ms143997(v=vs.110).aspx

aarondcoleman commented 8 years ago

Hey @WestDiscGolf the signature header is one way that Fitbit sends you a way to validate that some hacker isn't randomly sending you a faked POST that looks like it came from Fitbit. It's a shared key type security model. Basically, any time a Fitbit syncs that you've subscribed for PubSub, you get a ping. That ping might contain multiple UpdatedResources for different users, Fitbit groups those and sends them together to save bandwidth.

A little clearer? Would be awesome if you wanted to take a stab at validating those signatures.

aarondcoleman commented 8 years ago

At the very least though, we need to convert the XML deserialization to JSON.

WestDiscGolf commented 8 years ago

@aarondcoleman do you have any examples of the json as the documentation seems very limited in this section or some pointers as to how to setup / dev test subscriptions to intercept the headers myself?

mxa0079 commented 8 years ago

Did we ever come to an agreement on this?

WestDiscGolf commented 8 years ago

Still a little confused as to how you can dev against subscriptions and the how the data structure for the json representation will look like as I couldn't find it on the documentation site ... unless they've updated it recently? @aarondcoleman any pointers with this as I'm guessing you use subscriptions quite a lot?

aarondcoleman commented 8 years ago

Really all we need is a parser SubscriptionMessageParser sounds good for the class. When you set up an HTTP(s) server endpoint for the subscriptions, Fitbit then sends them batched every few seconds activated when devices sync. So, your web app will get the raw HTTP body and pass it in entirety, and we just need to parse for a list of all like the existing one, but JSON instead of XML. https://github.com/aarondcoleman/Fitbit.NET/blob/master/Fitbit/SubscriptionManager.cs

An example body response is here: https://dev.fitbit.com/docs/subscriptions/#response-body-format

We today are still on the XML one and the way that body comes in is the first characters are the signature, which we ignore, and just start parsing at the opening tag. I imagine it's the same with JSON but starting at the beginning of the JSON array ( "[{..." ) but I or someone could spin up a test MVC site, connect their Fitbit, and receive a few of these to verify and get a full body.

If we are going to implement the signature validation (probably a good idea), just note that it's computed from the ConsumerSecret key, so certainly use a test account and that'll have to be hard coded in to the unite test along with the passed signature hash.

mxa0079 commented 8 years ago

Good news! This is implemented in the issue81 branch without depending on RestSharp... man! it had been a while since I deserialized XML.

This branch is a child of the current pending PR #143 so I will wait until this branch is merge before I submit the PR for this (hint hint @aarondcoleman ;)

WestDiscGolf commented 8 years ago

Bit confused by this, how can the xml parsing work if json is being returned from the end points? Have I missed something here?

WestDiscGolf commented 8 years ago

Had a look at this the other night and noticed when you define the an end point on your application at the Fitbit end there are 3 different options as to what the payload will be (Json, xml and xml file I believe off the top of my head) so would be good to support all 3 at some point. However when I came to setting up a development example end point to see what the values were when they came through I got stuck. I noticed on the Fitbit documentation for subscriptions that Runscope allowed for a free account to aid with development but I've yet to get that plumbed in and working for a development environment to see what the different formats and values are. Slowly but surely will get there :)

WestDiscGolf commented 8 years ago

In addition to this parsing we will need to make sure all the subscription urls are up to date as per the issues already logged :)

WestDiscGolf commented 8 years ago

Struggling with getting a subscription set up and a test notification for debugging purposes. Is the example from the api documention below representative of what is to be expected?

[ { "collectionType":"foods", "date":"2010-03-01", "ownerId":"228S74", "ownerType":"user", "subscriptionId":"1234", }, { "collectionType":"foods", "date":"2010-03-02", "ownerId":"228S74", "ownerType":"user", "subscriptionId":"1234", }, { "collectionType":"activities", "date":"2010-03-01", "ownerId":"184X36", "ownerType":"user", "subscriptionId":"2345", } ]

https://dev.fitbit.com/docs/subscriptions/#example-notifications

WestDiscGolf commented 8 years ago

Investigated the Forward-Confirmed Reverse DNS using nslookup and can't get a reverse lookup ...

C:\Users\Adam> nslookup api.fitbit.com Server: UnKnown Address: fd0c:dfea:79a7:0:7e4c:a5ff:fe95:85b0

Non-authoritative answer: Name: api.fitbit.com.cdn.cloudflare.net Addresses: 104.16.65.50 104.16.66.50 Aliases: api.fitbit.com

C:\Users\Adam> nslookup 104.16.66.50 Server: UnKnown Address: fd0c:dfea:79a7:0:7e4c:a5ff:fe95:85b0

*\ No internal type for both IPv4 and IPv6 Addresses (A+AAAA) records available for 104.16.66.50

WestDiscGolf commented 7 years ago

I've done something recently at work to test where the request has come from so that shouldn't be an issue. If I can get confirmation on the data expected to come in / out then we can move forward with this.