costastf / locationsharinglib

A library to retrieve coordinates from an google account that has been shared locations of other accounts.
MIT License
170 stars 29 forks source link

Why call MyAccount twice or even at all? #74

Closed AlejandroRivera closed 4 years ago

AlejandroRivera commented 5 years ago

Hello,

While trying to troubleshoot a problem similar to Issue #71, I started exploring the code. I realized that there are TWO invocations of session.get(ACCOUNT_URL) in:

    def _validate_cookie(self, cookies_file):
        session = self._get_authenticated_session(cookies_file)
        response = session.get(ACCOUNT_URL)
        self._logger.debug('Getting personal account page and its cookies...\n %s', response.content)
        response = session.get(ACCOUNT_URL)

(see source)

Why is this done twice or even at all? I presume that the intention was not only to validate the cookies, but to also inject some new cookie into the session from having called MyAccount; however, during my own tests (in a non-2FA enabled account), I realize that calling directly https://www.google.com/maps/preview/locationsharing/read with the right params and cookies, is enough to get the full result.

The query params being: ?authuser=0&hl=en&gl=es&pb= and the cookies being:

1P_JAR
APISID
CONSENT
HSID
NID         
SAPISID
SID     
SIDCC           
SSID

(and all these cookies can be extracted from the browser and hardcoded in a cookies file)

Can you help me understand things better?

costastf commented 5 years ago

Hi Alejandro,

I needed a way to validate the cookies provided since the library does not handle the log in anymore. So looking at different ways that would work across locales (maps does not behave the same in all countries) I ended up using the myaccount endpoint. That endpoint though does behave a bit differently between browsers and OSes where in some there is a redirect that sets up some extra cookies. So in order to catch as many options as possible I decided to visit the url twice. I hope that helps with your wondering.

kind regards Costas

On Tue, Oct 8, 2019 at 2:31 PM Alejandro Rivera notifications@github.com wrote:

Hello,

While trying to troubleshoot a problem similar to Issue #71 https://github.com/costastf/locationsharinglib/issues/71, I started exploring the code. I realized that there are TWO invocations of session.get(ACCOUNT_URL) in:

def _validate_cookie(self, cookies_file):
    session = self._get_authenticated_session(cookies_file)
    response = session.get(ACCOUNT_URL)
    self._logger.debug('Getting personal account page and its cookies...\n %s', response.content)
    response = session.get(ACCOUNT_URL)

(see source https://github.com/costastf/locationsharinglib/blob/master/locationsharinglib/locationsharinglib.py#L105-L107 )

Why is this done twice or even at all? I presume that the intention was not only to validate the cookies, but to also inject some new cookie into the session from having called MyAccount; however, during my own tests (in a non-2FA enabled account), I realize that calling directly https://www.google.com/maps/preview/locationsharing/read with the right params and cookies, is enough to get the full result.

The query params being: ?authuser=0&hl=en&gl=es&pb= and the cookies being:

1P_JAR APISID CONSENT HSID NID
SAPISID SID
SIDCC
SSID

(and all these cookies can be extracted from the browser and hardcoded in a cookies file)

Can you help me understand things better?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/costastf/locationsharinglib/issues/74?email_source=notifications&email_token=ABDMK22QQOMCEDSFSF6OX2LQNR4R5A5CNFSM4I6RC6L2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HQKVC3A, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDMK24VHS3A6L52UHK4EF3QNR4R5ANCNFSM4I6RC6LQ .

AlejandroRivera commented 5 years ago

Hi Costas,

Thanks for the prompt response. The only remaining question is: if you were to call https://www.google.com/maps/preview/locationsharing/read directly without having made the calls to "MyAccount", would the response not be enough to determine that the cookies are invalid? I just tried it myself with no cookies whatsoever, and I got this response back:

)]}'
[null,null,"0ahUKEwjAtr6O6I7lAhWcAWMBHSzK4Q8ZABCAE","hKGdXcDkHJyDjL6z8Ao",null,null,"GgA\u003d",1800,157612365053]

and then using valid cookies with an account that has no shared locations with anyone, the response is:

)]}'
[null,null,"0ahUKEwifxtyA647lAhUJnxQKH0Q8ZABCAE","jKSdXZ_QN4m-Uv_xregE",null,null,"GiwAHrX0pmts8Uk8/5SBjmCAHNjWKlSllT5PwCJ2Ychl3tAfuGzVKuTZCUw\u003d\u003d",1800,157612365053]

Given this, I imagine you want to be able to distinguish between a valid request (with valid credentials) and having no shared locations, vs an "empty" response from not having valid credentials, right?

However, given the cost of doing double call to MyAccount and the sharedlocation, do you think it'd make sense to give the option to the developers to ignore validating cookies and just have them interpret an "empty" response in whatever way they want?

costastf commented 5 years ago

Hi Alejandro,

As you can see the differences on the payload are not easily distinguishable and honestly I was just trying to find a way to deterministically validate the cookies and I thought that that would work across locals. Sadly this is not working over a documented api but it is reverse engineered. If you can provide a better way to validate the cookies that would work across locales I would be happy to accept an MR.

As for your second comment, this is supposed to be a library to easily provide access to shared location info to be used as a location tracker for Home assistant and was written to solve a specific personal itch. It is not meant to be a "framework" of any kind so as far as the project is concerned there are no developers that will interpret anything, but users that will read the info provided. It might not be obvious but the support required on this even with this extremely limited functionality provided is quite high, never mind how high it was when it was handling the login on the service automatically and populated its own cookies. That being said if you have a specific usage case in mind and don't mind explaining what that is I can try to help you get the functionality you need. Lastly , as for the cost of the double call to myaccount, taking in mind that this project was designed to be a long running process that would poll that endpoint with a caching of 30 seconds continuously, making two extra calls in the beginning of the process to validate the posture of the authentication and save time if that failed should be considered negligible i my opinion.

AlejandroRivera commented 5 years ago

Hi Costas,

It might not be obvious but the support required on this even with this extremely limited functionality provided is quite high, never mind how high it was when it was handling the login on the service automatically and populated its own cookies.

Yes, i can imagine that and that's why i was seeking your input to see if an idea like mine would be worth pursuing so i could submit a PR for you to review.

As for your second comment, this is supposed to be a library to easily provide access to shared location info to be used as a location tracker for Home assistant and was written to solve a specific personal itch

Oh great! I also came here because I'm a Home Assistant user myself! I thought the "google_maps" component was simply taking advantage of your library and that the main/other use-cases were simply others.

This bring me nicely to the next point. The way i see it, when Home Assistant invokes this library, the library validates the cookies (which are many and few used for MyAccount, others for Maps, etc.) and if they're invalid, some exception with an appropriate message is shown. The only way the end-user sees the msg is by looking at the Home Assistant logs. In this case they will know for sure, the cookies are invalid for sure.

With my suggestion, instead of being so forthcoming to the user, the cookies aren't validated ahead of the call to the Maps Location Sharing API. The cookies are taken as they are, and the output from the call might be "empty" because either the cookies are invalid or there's no location sharing configured in that account. The error message that would be printed to the logs would simply be: "No location sharing content found in response. Cookies might be invalid.". The end-user, when reading this msg will know for sure if the cookies are invalid because they know if the account has shared locations with others.

With this approach, you could drop entirely support for validating cookies and double calling my account (which is also an undocumented API that might change, right?). It also simplifies things for people having to provide cookies because they ONLY need the cookies specific to the Maps Location Sharing API (9 cookies total, which they can manually extract from the Network call instead of having to use extensions to dump cookies in a specific format); and their google_maps component in Home Assistant will work much faster. This would work nicely

Well, all of the above might be correct if you don't care breaking backwards compatibility with a new major version that removes all the code for cookie validation. If you want to preserve it (which was my original thinking), I was recommending using a constructor flag to indicate if validation should be done, and place the responsibility on the codebase using this library to use this approach.

Anyway, I just wanted to explain my rationale for you to decide if there's any value in this idea, or if it's just too much hassle. Either way, i'm fine with your decision and i'll remain forever thankful for your contribution to Home Assistant by making possible to use GMaps loc. sharing to the world 😃

costastf commented 5 years ago

Hi @AlejandroRivera , apologies for not answering earlier, I thought I did but it was drafted :( .

I get your point but I do think it is a bit biased :)

In my experience there are two categories of people using this. People that know python and can navigate the code and can actually fix things (which I guess are the people you would be targeting to handle the empty answer case) and people that cannot even install a python tool coming from a windows background with not even having computer experience.

I am afraid that the second category are the vast majority of users and what happens is that they go to HA forums and make a comment like "maps do not work, whats wrong?" in which case I need to guide them to the documentation of the library on how to install the library by itself and test it out. 100% of the time it has been an authentication issue so basically a cookie issue. If I can save myself and everyone else time by making it very explicit and clear that that is the case I think its a good thing :) even if it means that there are some false positives on the way because standard gmail accounts do not expire so if someone makes it work once he is set for life. If i remove that check there i am very very afraid that the confusion of people will rise and it will also make it extremely more difficult for me to troubleshoot anything for them because that discussion will go like, "please send me your output to check", "yeah, your authentication is not correct", "it does not matter that you get a response, the response is not authenticated", "oops you actually pasted a full authenticated response for the whole world to see, that's not a good thing for you". So for those people i tried to make the solution as easy and straight forward as possible. I think that installing a browser addon and saving the cookies is easier for everyone than making the cookie file manually by reading the browser settings (i would definitely would not want to be the one troubleshooting that manual cookie file for anyone)

That being said if it would help your usage case to implement a feature flag to actually disable the cookie validation if set by the user of course I do not mind at all, as long as the default action is to validate and bail as soon as possible and as loud as possible.

So long story short, I would happily accept an MR that implements a feature flag to bypass the cookie validity check. :)