Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

refactor accounts elsewhere #1369

Merged
merged 110 commits into from
Feb 26, 2014
Merged

refactor accounts elsewhere #1369

merged 110 commits into from
Feb 26, 2014

Conversation

chadwhitacre
Copy link
Contributor

Currently we have four accounts elsewhere, three useful for authentication (Bitbucket, GitHub, Twitter) and one that can be connected but not used for authentication (Bountysource). These are implemented in a very WET way and we need to DRY it up. This is an old pull request from @lyndsysimon that I brought over from his fork into a branch on the main repo so we can bring more hands on deck.

  • review OO model
  • review simplates
  • review CSRF/security

Bitbucket

  • sign in
  • connect to existing
  • reconnect (merge) to existing
  • view elsewhere
  • pledge
  • lock - success
  • lock - failure
  • unlock - success
  • unlock - failure
  • view group elsewhere
  • pledge via group elsewhere (Note from Changaco: this is not currently implemented)

Bountysource

  • connect to existing
  • reconnect (merge) to existing

GitHub

  • sign in
  • connect to existing
  • reconnect (merge) to existing
  • view elsewhere
  • pledge
  • lock - success
  • lock - failure
  • unlock - success
  • unlock - failure
  • view group elsewhere
  • pledge via group elsewhere (Note from Changaco: this is not currently implemented)

OpenStreetMap

  • sign in
  • connect to existing
  • reconnect (merge) to existing

Edit by Changaco: the following aren't possible for this plaftorm, see #1369 (comment)

  • view elsewhere
  • pledge
  • lock - success
  • lock - failure
  • unlock - success
  • unlock - failure

Twitter

  • sign in
  • connect to existing
  • reconnect (merge) to existing
  • view elsewhere
  • pledge
  • lock - success
  • lock - failure
  • unlock - success
  • unlock - failure

Venmo

  • connect to existing
  • reconnect (merge) to existing

Conflicts:
	configure-aspen.py
	gittip/models/participant.py
	gittip/wireup.py
I should've done this on the last commit but neglected to.
Again, should have been done in the merge.
When LyndsySimon started on the accounts elsewhere refactor (#1369) he
also started adding support for Google accounts as well. I've made a
"google" branch to keep that code around, I'm ripping it out of the
elsewhere branch because our first goal needs to be to refactor accounts
elsewhere without any user-visible changes.
HTML-escape some things and fix up API for oauth URLs.
@chadwhitacre
Copy link
Contributor Author

IRC

  • the bitbucket page is missing gravatar
  • there is no sign-in link on bitbucket
  • there is no lock button on bitbucket

Actually, these are bugs with the current implementation (IRC). Let's pick them off with this PR if we can.

@chadwhitacre
Copy link
Contributor Author

  • unknown platform should be 404 (not 500)

We were using "Service" but in the `elsewhere` table we use `platform`.
@chadwhitacre
Copy link
Contributor Author

So it turns out that this branch still contains the /on/{bitbucket,github,twitter}/ directories, meaning that the /on/%platform/ directory is never actually dispatched to. I'm going to start with Twitter and fold that into the refactored version, because Bitbucket and GitHub both have a group/organization concept that make them more complex.

This branch started under an old version of Aspen that didn't require
the .spt extension.
This adds a PlatformRegistry to website, which provides access to
instances of Platform subclasses, which have a load method that takes a
username on the given platform and returns an AccountElsewhere
orm.Model. This is all barely working with one test. Surely other tests
are broken now, I haven't even checked. Gonna start building back up and
out.
@chadwhitacre
Copy link
Contributor Author

I get the following traceback when connecting a Bountysource account:

Traceback (most recent call last):
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/algorithm.py", line 288, in run
    new_state = function(**deps.as_kwargs)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/algorithms/website.py", line 88, in get_response_for_resource
    return {'response': resource.respond(request)}
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/resources/dynamic_resource.py", line 52, in respond
    exec self.pages[1] in context
  File "/Users/whit537/personal/gittip/www.gittip.com/www/on/%platform/associate.spt", line 29, in 
    account = platform.upsert(platform.get_user_self_info(sess))
  File "/Users/whit537/personal/gittip/www.gittip.com/gittip/elsewhere/bountysource.py", line 69, in get_user_self_info
    return self.extract_user_info(info)
  File "/Users/whit537/personal/gittip/www.gittip.com/gittip/elsewhere/__init__.py", line 181, in extract_user_info
    gravatar_id = hashlib.md5(email.strip().lower()).hexdigest()
NameError: global name 'email' is not defined

DEBUG is so generic, it's impossible to know what it's for without a
comment.
@chadwhitacre
Copy link
Contributor Author

Per IRC, I'm hitting this problem with Venmo:

pid-78415 thread-4399927296 (CP Server Thread-10) Unable to find key "username" in venmo API response:
pid-78415 thread-4399927296 (CP Server Thread-10) {
pid-78415 thread-4399927296 (CP Server Thread-10)     "data": {
pid-78415 thread-4399927296 (CP Server Thread-10)         "balance": null,
pid-78415 thread-4399927296 (CP Server Thread-10)         "user": {
pid-78415 thread-4399927296 (CP Server Thread-10)             "username": "whit537",
pid-78415 thread-4399927296 (CP Server Thread-10)             "first_name": "Chad",
pid-78415 thread-4399927296 (CP Server Thread-10)             "last_name": "Whitacre",
pid-78415 thread-4399927296 (CP Server Thread-10)             "display_name": "Chad Whitacre",
pid-78415 thread-4399927296 (CP Server Thread-10)             "is_friend": false,
pid-78415 thread-4399927296 (CP Server Thread-10)             "friends_count": 5,
pid-78415 thread-4399927296 (CP Server Thread-10)             "about": " ",
pid-78415 thread-4399927296 (CP Server Thread-10)             "id": "909648748085248861",
pid-78415 thread-4399927296 (CP Server Thread-10)             "phone": null,
pid-78415 thread-4399927296 (CP Server Thread-10)             "profile_picture_url": "https://s3.amazonaws.com/venmo/no-image.gif",
pid-78415 thread-4399927296 (CP Server Thread-10)             "email": null,
pid-78415 thread-4399927296 (CP Server Thread-10)             "date_joined": "2012-06-09T01:49:41"
pid-78415 thread-4399927296 (CP Server Thread-10)         }
pid-78415 thread-4399927296 (CP Server Thread-10)     }
pid-78415 thread-4399927296 (CP Server Thread-10) }

That's after I switched to using access_profile scope to get around this problem:

venmo api responded with 403:
{"error": {"message": "Invalid Scope.", "code": 246}}
venmo lookup failed with 403

@chadwhitacre
Copy link
Contributor Author

Next:

Traceback (most recent call last):
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/algorithm.py", line 288, in run
    new_state = function(**deps.as_kwargs)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/algorithms/website.py", line 88, in get_response_for_resource
    return {'response': resource.respond(request)}
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/resources/dynamic_resource.py", line 52, in respond
    exec self.pages[1] in context
  File "/Users/whit537/personal/gittip/www.gittip.com/www/on/%platform/associate.spt", line 29, in 
    account = platform.upsert(platform.get_user_self_info(sess))
  File "/Users/whit537/personal/gittip/www.gittip.com/gittip/elsewhere/__init__.py", line 255, in get_user_self_info
    return self.extract_user_info(self.api_parser(r))
  File "/Users/whit537/personal/gittip/www.gittip.com/gittip/elsewhere/__init__.py", line 169, in extract_user_info
    r.user_name = self.x_user_name(info)
  File "/Users/whit537/personal/gittip/www.gittip.com/gittip/elsewhere/_extractors.py", line 14, in f
    v = info.pop(k, *default)
KeyError: u'username'
pid-78686 thread-4383150080 (CP Server Thread-10) Unable to find key "username" in venmo API response:
pid-78686 thread-4383150080 (CP Server Thread-10) {
pid-78686 thread-4383150080 (CP Server Thread-10)     "balance": null,
pid-78686 thread-4383150080 (CP Server Thread-10)     "user": {
pid-78686 thread-4383150080 (CP Server Thread-10)         "username": "whit537",
pid-78686 thread-4383150080 (CP Server Thread-10)         "first_name": "Chad",
pid-78686 thread-4383150080 (CP Server Thread-10)         "last_name": "Whitacre", 
pid-78686 thread-4383150080 (CP Server Thread-10)         "display_name": "Chad Whitacre", 
pid-78686 thread-4383150080 (CP Server Thread-10)         "is_friend": false, 
pid-78686 thread-4383150080 (CP Server Thread-10)         "friends_count": 5, 
pid-78686 thread-4383150080 (CP Server Thread-10)         "about": " ", 
pid-78686 thread-4383150080 (CP Server Thread-10)         "id": "909648748085248861", 
pid-78686 thread-4383150080 (CP Server Thread-10)         "phone": null, 
pid-78686 thread-4383150080 (CP Server Thread-10)         "profile_picture_url": "https://s3.amazonaws.com/venmo/no-image.gif", 
pid-78686 thread-4383150080 (CP Server Thread-10)         "email": null, 
pid-78686 thread-4383150080 (CP Server Thread-10)         "date_joined": "2012-06-09T01:49:41"
pid-78686 thread-4383150080 (CP Server Thread-10)     }
pid-78686 thread-4383150080 (CP Server Thread-10) }

@@ -26,7 +26,7 @@ if POST:
<h1>Are you sure you wish to sign out?</h1>
<form id="sign-out" method="POST">
<input name="back_to" type="hidden" value="/" />
<input name="csrf_token" type="hidden" value="{{ cookie['csrf_token'].value }}" />
<input name="csrf_token" type="hidden" value="{{ csrf_token }}" />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Changaco Why did we change where csrf_token is taken from?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@whit537 Because: why not ? It's shorter, and now we always get it the same way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I flip-flopped the diff in my mind. Thought we were pulling from cookie instead of the other way around.

chadwhitacre added a commit that referenced this pull request Feb 26, 2014
refactor accounts elsewhere
@chadwhitacre chadwhitacre merged commit 7290e0d into master Feb 26, 2014
@chadwhitacre chadwhitacre deleted the elsewhere branch February 26, 2014 22:15
chadwhitacre added a commit that referenced this pull request Feb 26, 2014
chadwhitacre added a commit that referenced this pull request Feb 26, 2014
seanlinsley added a commit that referenced this pull request Feb 27, 2014
chadwhitacre added a commit that referenced this pull request Feb 27, 2014
The need for this import entered with #1369.
@chadwhitacre chadwhitacre mentioned this pull request Apr 14, 2014
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.

9 participants