Oauth error gets buried

[stt]

Issue Strategy's loadUserProfile reports a generic error without context to what happened.

Cause It appears that the first callback argument to node-oauth's getOAuthAccessToken isn't intended to report back oauth errors

So when for example oauth server returns "error=bad_verification_code&error_description=The+code+passed+is+incorrect+or+expired.&error_uri=https%3A%2F%2Fdeveloper.github.com%2Fv3%2Foauth%2F%23bad-verification-code"

1. the passport callback is called with null as error and undefined as access_token and refresh_token.
2. "if(err)" is skipped and self._loadUserProfile is called with undefined accessToken
3. a generic error is returned ('failed to fetch user profile' in this case) and the original cause is buried.

Solution? Shouldn't getOAuthAccessToken callback do "if (err || params['error'])" and pass "err || params" to _createOAuthError?

[tjmehta]

+1, I would love to create a PR for this but I am unsure how to solve it exactly.

params.error exists in the case for github but may not for other auth strategies.

The problem lies here: https://github.com/jaredhanson/passport-oauth2/blob/master/lib/strategy.js#L173 this._oauth2.getOAuthAccessToken is assumed to return an error or accessToken but this is not the case. In the github example above, github responds with a 200 but no access_token in the body.

Good thing is that oauth.getOAuthAccessToken also calls back with params (response body). So we could check params for an error when it calls back with no error AND no accessToken.

@jaredhanson how would you like this solved? Should we create an error parse method that can be overridden by strategies that inherit from passport-oauth2?