Skip to content

Commit

Permalink
Fix bug with attributing change to a guide to the repo owner not author
Browse files Browse the repository at this point in the history
- This was a problem with caching a user object with the wrong email address.
- This fixes a bug that could cause github commits to be attributed to the
  wrong user. Github tracks commits with the email address, not username.
- The following scenario would lead to this bug:
    - User x logs in leading to their username and email being cached
    - Cache times out
    - Another visitor browses to a guide written by user x
    - find_user() incorrectly reads username of user x and email address of
      logged in user, which there isn't one on this machine. So, the github
      token defaults to the repo owner.
    - User x saved to cache with wrong email
    - User x commits with application and commit attributed to repo owner
      instead of real author
  • Loading branch information
durden committed Mar 14, 2016
1 parent 4ad8c14 commit 495efee
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
17 changes: 10 additions & 7 deletions pskb_website/models/user.py
Expand Up @@ -18,19 +18,22 @@ def find_user(username=None):
:returns: User object
"""

user_info = cache.read_user(username)
if user_info is not None:
return User.from_json(user_info)
if username is not None:
user_info = cache.read_user(username)
if user_info is not None:
return User.from_json(user_info)

user_info = remote.read_user_from_github(username)
if not user_info:
return None

# This doesn't take a username b/c it's only accessible via the logged in
# user, which the remote layer can tell from the session.
email = remote.primary_github_email_of_logged_in()
user = User(user_info['name'], user_info['login'])
user.email = email

# Request is for logged in user only
if username is None:
email = remote.primary_github_email_of_logged_in()
user.email = email

user.avatar_url = user_info['avatar_url']
user.location = user_info['location']
user.blog = user_info['blog']
Expand Down
6 changes: 6 additions & 0 deletions pskb_website/views.py
Expand Up @@ -518,6 +518,12 @@ def save():
flash('Cannot save unless logged in', category='error')
return render_template('index.html'), 404

# User could have been cached previously without an email
if user.email is None:
user.email = remote.primary_github_email_of_logged_in()
if user.email is None:
flash('Unable to read email address from Github API to properly attribute your commit to your account. Please make sure you have authorized the application to access your email.', category='warning')

content = request.form['content']
path = request.form['path']
title = request.form['title']
Expand Down

0 comments on commit 495efee

Please sign in to comment.