Skip to content
This repository has been archived by the owner on Jul 30, 2021. It is now read-only.

New auth mechanism #115

Merged
merged 2 commits into from
Dec 6, 2020
Merged

New auth mechanism #115

merged 2 commits into from
Dec 6, 2020

Conversation

yoooou
Copy link
Contributor

@yoooou yoooou commented Mar 22, 2020

Inspired by updates from hass-aarlo repo. Just trying to make Arlo login work by making minimal changes.

@yoooou yoooou mentioned this pull request Mar 22, 2020
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 79.434% when pulling 710870b on yoooou:new_auth into 1a8f39d on tchellomello:master.

@mepster
Copy link

mepster commented Apr 14, 2020

Thanks @yoooou, this works great for me, you are awesome.

@tchellomello you can probably merge, thanks for all your work. I run pyarlo every day.

@dscoular
Copy link

dscoular commented Apr 18, 2020

Hi @yoooou ,
I found I had to add a mechanism to bypass cloudflare. I added clouscraper to the requirements and edited the pyarlo/__init__.py as follows:

$ diff -u __init__.py~ __init__.py
--- __init__.py~        2020-04-17 18:41:56.000000000 +1000
+++ __init__.py 2020-04-18 10:51:17.687413694 +1000
@@ -2,6 +2,7 @@
 """Base Python Class file for Netgear Arlo camera module."""
 import logging
 import requests
+import cloudscraper

 from pyarlo.base_station import ArloBaseStation
 from pyarlo.camera import ArloCamera
@@ -41,7 +42,7 @@
         # set username and password
         self.__password = password
         self.__username = username
-        self.session = requests.Session()
+        self.session = cloudscraper.CloudScraper() # requests.Session()

         # login user
         self.login()

After this your fix worked perfectly for me!

@fermulator
Copy link

fermulator commented Aug 15, 2020

What are next steps to get this merged in?

EDIT: fwiw, I pulled down these diffs, shoved them into /srv/homeassistant/lib/python3.7/site-packages/pyarlo/, and restarted home assistant; WORKS!

@fermulator
Copy link

@tchellomello ❗ ?

@tchellomello
Copy link
Owner

@yoooou Thanks for your contribution. Are you planning to add the suggestion from @dscoular above?

@tchellomello tchellomello self-assigned this Sep 4, 2020
@yoooou
Copy link
Contributor Author

yoooou commented Sep 6, 2020

@yoooou Thanks for your contribution. Are you planning to add the suggestion from @dscoular above?

TBH I don't have enough knowledge to tell whether @dscoular 's change should be included or not. It doesn't look like something neccesary at least for some of us it seems. Some clarifications/suggestions would be helpful.

@fermulator
Copy link

fermulator commented Sep 7, 2020

of note, i did not require #115 (comment)

EDIT: I vote we merge w/out it, and see if it fixes for people; if there is still an issue and/or some corner case for some users, then we can subsequently do it

@fermulator
Copy link

@tchellomello mergie mergie time? (let me know how I can help)

@TakesTheBiscuit
Copy link
Collaborator

👀

@tchellomello tchellomello merged commit c1703d8 into tchellomello:master Dec 6, 2020
@tchellomello
Copy link
Owner

tchellomello commented Dec 6, 2020

Sorry guys for the long wait. @yoooou thanks for your contribution.
'm looking for some maintainers to help to keep this code. If you are interested, please let me know.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants