Python auth using requests

Question 1

I am trying to implement a custom ApiClient in Python, using requests.The way it does authentication is by:

login(username, password) -> get back a token if valid, http error code if not
set the token in{'Authorization': token} header and use that header for all endpoints which need authentication

Can you check if the code looks OK, and if not what kind of changes would you recommend?

#!/usr/bin/env pythonimport requestsimport jsonimport osimport sysdef read_file_contents(path):    if os.path.exists(path):        with open(path) as infile:            return infile.read().strip()class ApiClient():    token = None    api_url = 'http://10.0.1.194:1234'    session = requests.Session()    def __init__(self):        self.token = self.load_token()        if self.token:            self.session.headers.update({'Authorization': self.token})        else:            # if no token, do login            if not self.token:                try:                    self.login()                except requests.HTTPError:                    sys.exit('Username-password invalid')    def load_token(self):        return read_file_contents('token')    def save_token(self, str):        with open('token', 'w') as outfile:            outfile.write(str)    def login(self):        email = '[email protected]'        password = 'passwd'        headers = {'content-type': 'application/json'}        payload = {            'email': email,            'password': password        }        r = requests.post(self.api_url + '/auth',                          data=json.dumps(payload),                          headers=headers)        # raise exception if cannot login        r.raise_for_status()        # save token and update session        self.token = r.json()['session']['token']        self.save_token(self.token)        self.session.headers.update({'Authorization': self.token})    def test_auth(self):        r = self.session.get(self.api_url + '/auth/is-authenticated')        return r.okif __name__ == '__main__':    api = ApiClient()    print api.test_auth()

Question 2

the email-password will obviously not be hard coded

Question 3

I don't have time to do a thorough review, but here are some comments based on a casual reading.

Inread_file_contents, the function returnsNone if the file doesn’t exist. I’d suggest modifying this to print or return an explicit message to that effect, to make debugging easier later. For example, something like:
```
def read_file_contents(path):    try:        with open(path) as infile:            return infile.read().strip()    except FileNotFoundError:        print "Couldn't find the token file!"        return None
```
will make your life easier later.
Also, is this meant to beopen(path, 'r')?
New-style classes should inherit from object: that is, use
```
class ApiClient(object):
```
instead of
```
class ApiClient():
```
Don't copy and paste the API URL throughout your code. Make it a variable, and then passapi_url as an input toApiClient(). That way, you can reuse this code for other APIs, or make changes to the URL in a single place. (If, for example, the API changes.)
Don't hard code username, email and password in thelogin function. It makes it harder to change them later, and it's a security risk. Consider using something like thekeyring module for storing sensitive information. (Should the token be stored securely as well?)

The__init__ function uses the token, and also retrieves it from the file. This can cause problems if you later get the token from somewhere else. (For example, if you usedkeyring.) Instead, consider passingtoken as an argument to__init__, and having the code to obtain the token somewhere else. Something like:

def __init__(self, api_url, token=None):    self.api_url = api_url    self.token = token    if self.token is not None:        # if you get a token    else:        # if you don't

and then later:

if __name__ == '__main__':    api_token = read_file_contents('token')    api_url = 'http://10.0.1.194:1234'    api = ApiClient(api_token, api_url)    print api.test_auth()

Question 4

Even better than returningNone if the file is unreadable, just let the exception propagate, and catch it in__init__().

Question 5

open() mode is'r' by default.

Question 6

@200_success While that is true, it could be argued that this qualifies as "explicit is better than implicit".

alexwlchan 8,6751 gold badge23 silver badges55 bronze badges · Accepted Answer · 2014-04-08 16:43:21Z

I don't have time to do a thorough review, but here are some comments based on a casual reading.

Inread_file_contents, the function returnsNone if the file doesn’t exist. I’d suggest modifying this to print or return an explicit message to that effect, to make debugging easier later. For example, something like:
```
def read_file_contents(path):    try:        with open(path) as infile:            return infile.read().strip()    except FileNotFoundError:        print "Couldn't find the token file!"        return None
```
will make your life easier later.
Also, is this meant to beopen(path, 'r')?
New-style classes should inherit from object: that is, use
```
class ApiClient(object):
```
instead of
```
class ApiClient():
```
Don't copy and paste the API URL throughout your code. Make it a variable, and then passapi_url as an input toApiClient(). That way, you can reuse this code for other APIs, or make changes to the URL in a single place. (If, for example, the API changes.)
Don't hard code username, email and password in thelogin function. It makes it harder to change them later, and it's a security risk. Consider using something like thekeyring module for storing sensitive information. (Should the token be stored securely as well?)

The__init__ function uses the token, and also retrieves it from the file. This can cause problems if you later get the token from somewhere else. (For example, if you usedkeyring.) Instead, consider passingtoken as an argument to__init__, and having the code to obtain the token somewhere else. Something like:

def __init__(self, api_url, token=None):    self.api_url = api_url    self.token = token    if self.token is not None:        # if you get a token    else:        # if you don't

and then later:

if __name__ == '__main__':    api_token = read_file_contents('token')    api_url = 'http://10.0.1.194:1234'    api = ApiClient(api_token, api_url)    print api.test_auth()

Even better than returningNone if the file is unreadable, just let the exception propagate, and catch it in__init__().
@200_success While that is true, it could be argued that this qualifies as "explicit is better than implicit".

Movatterモバイル変換

Stack Exchange Network

Python auth using requests

1 Answer1

You mustlog in to answer this question.

Related

Hot Network Questions

Subscribe to RSS