patternpythonMinor
Python auth using requests
Viewed 0 times
usingauthpythonrequests
Problem
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
Can you check if the code looks OK, and if not what kind of changes would you recommend?
```
#!/usr/bin/env python
import requests
import json
import os
import sys
def 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 = '1@1.hu'
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 + '/
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 authenticationCan you check if the code looks OK, and if not what kind of changes would you recommend?
```
#!/usr/bin/env python
import requests
import json
import os
import sys
def 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 = '1@1.hu'
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 + '/
Solution
I don't have time to do a thorough review, but here are some comments based on a casual reading.
-
In
will make your life easier later.
Also, is this meant to be
-
New-style classes should inherit from object: that is, use
instead of
-
Don't copy and paste the API URL throughout your code. Make it a variable, and then pass
-
Don't hard code username, email and password in the
-
The
and then later:
-
In
read_file_contents, the function returns None 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 Nonewill make your life easier later.
Also, is this meant to be
open(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 pass
api_url as an input to ApiClient(). 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 the
login function. It makes it harder to change them later, and it's a security risk. Consider using something like the keyring 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 used keyring.) Instead, consider passing token 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'tand 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()Code Snippets
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 Noneclass ApiClient(object):class ApiClient():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'tif __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()Context
StackExchange Code Review Q#46652, answer score: 3
Revisions (0)
No revisions yet.