feat: simplify return types

[JustinBeckwith]

BREAKING CHANGE: The return types and callbacks for multiple methods have changed. Multiple methods on the OAuth2 class previously returned a promise that resolved with an object, which contained both the relevant return data and the response object returned from the HTTP request. The data from the HTTP request is not necessary, so the method calls were simplified to only return the relevant data. Affected methods include:

• OAuth2.getToken()
• OAuth2.refreshAccessToken()
• OAuth2.getAccessToken()
• OAuth2.getRequestMetadata()
• OAuth2.getFederatedSignonCerts()
• JWTAccess.getRequestMetadata()

For example:

Old code

const response = await oAuth2Client.getToken(qs.code);
const tokens = response.tokens;
console.log(tokens);

New code

const tokens = await oAuth2Client.getToken(qs.code);
console.log(tokens);

---

A note for the readers: it's possible this is a bad idea. I am trying to do a couple of things here:

• Reduce the number of Axios specific interfaces we export as part of the public facing API. It's unclear what our HTTP request client of choice will be, so I want to slowly quarantine any axios or request specific behaviors.
• Simplify the API surface. Simply put... there's just no reason to return these response objects along with the API. They were like this when we started, so I just never changed them.

I am a little concerned because these changes change the return type with practically no warning, even though this is semver major. An alternative approach could be:

• OAuth2.getToken() - Introduce a new OAuth2.getTokens() method with the new return type, and update the docs and samples. Deprecate the getToken method, and delete it in 3.0. The name getToken is funny, since we return multiple tokens in many cases.
• OAuth2.refreshAccessToken - Deprecate it, and delete it in 3.0. This method is already marked as deprecated in the jsdocs.
• OAuth2.getAccessToken - I got nothin. The object return type as it is today is just weird and unexpected, but I don't know how to nuance a fix.
• OAuth2.getRequestMetadata() - Introduce a getAuthorizationHeader method that returns a string with the Authorization header. Deprecate this method, decom in 3.0. I always thought this method was weird, since all it does is return a map with a single header value set. This forces users to Object.assign against other headers, instead of letting them just say myheaders.Authorization = client.getAuthorizationHeader().
• OAuth2.getFederatedSignonCerts() - I have no idea why in the world this is a public method. We should just deprecate, and make private in 3.0.

I honestly don't know if all the churn above is really worth it, but I want to hear other opinions here :)

[codecov-io]

Codecov Report

Merging #407 into master will increase coverage by 0.06%.
The diff coverage is 94.73%.

@@            Coverage Diff             @@
##           master     #407      +/-   ##
==========================================
+ Coverage   94.89%   94.96%   +0.06%     
==========================================
  Files          15       15              
  Lines         960      953       -7     
  Branches      218      218              
==========================================
- Hits          911      905       -6     
+ Misses         49       48       -1

Impacted Files	Coverage Δ
src/auth/jwtaccess.ts	98.11% <100%> (ø)	⬆️
src/auth/computeclient.ts	94.73% <100%> (ø)	⬆️
src/auth/googleauth.ts	94.66% <100%> (ø)	⬆️
src/auth/jwtclient.ts	94.84% <100%> (ø)	⬆️
src/auth/refreshclient.ts	90.69% <50%> (ø)	⬆️
src/auth/oauth2client.ts	94.57% <93.93%> (+0.2%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba3a57a...49aac8c. Read the comment docs.

[JustinBeckwith]

@stephenplusplus for your thoughts too :)

[kjin]

Possible idea: If we are expecting x today and the return value is switched to x.a, then maybe we can add an unenumerable get property x.a.a which, when gotten, also returns x.a but emits a deprecation warning.

[kjin]

+1 to the suggested changes. The code looks LGTM but it seems like you are awaiting feedback from others as well.

[ofrobots]

Changing the return type is very subtle for users and will be hard for them to debug. I'm -1 on doing this without a rename, but I would be +1 to the suggestion from @kjin. (getter that emits a deprecation warning or maybe throws).