fix(generic): save new refresh_token value

[becm]

Missing the update of single-use-tokens will lead to continuous re-registration requirement for GCM.

Check is done correctly for new value.
Just the storage step used the old value instead of the new one.

[dscho]

Thank you for bearing with us for such a long time in limbo. It's not quite over yet; @mjcheetham is busy right now with the huge task to re-establish a working release process (currently all signing/notarizing is missing all prerequisite secrets, and as a consequence no binaries would be signed, which would break Git Credential Manager on macOS). So please bear with us a bit longer.

Just a quick note why this has not yet been merged: While on the face of it, it should be a no-brainer to store any new refresh token (at least it was me, until I asked Matthew about it and he patiently explained to me that I need to consider more scenarios than the obvious one), a new refresh token can be provided in circumstances where we would need to ask the user for confirmation (for example if the account name was changed, in which case the account name also would have to be updated). Another plausible situation where this would happen is when there is a malicious actor who uses the same account elsewhere, in which case Git Credential Manager should probably ring the alarm bells.

Also, there are now finally credential protocol enhancements in Git itself that would allow refresh tokens to be processed in a stateful manner, with enough context to avoid, say, storing them for the wrong account. For more details see #2059.

In short: More things need to be considered before this Pull Request can go forward. And the release process needs to be fixed before it would be well-spent time to enhance GCM... I therefore have to ask for even more patience...

[becm]

This update happens only in the context of an already completely resolved account.
The degrees of freedom at this point are limited to bound token values and their expiration info.

At this point we:

• have resolved a certain fixed account scope
• successfully used the previous refresh_token (so the account was correctly identified) and
• the server told us explicitly to use a different value next time we come back in this context.

Assuming we are dealing with a server that is already set up according to current recommendations
→ single use refresh_token:

• failing to update the token value at this stage will trigger auth issues on the next request
• using outdated credentials should also trigger invalidation of the new value.

The current behavior is unlikely to have an effect on most aspects of MITM attacks.
It even prevents use of proper server settings to have at least a chance to detect token leaks.

This update is actually the only operation that MUST be performed (immediately) at this point.
We MAY NOT wait until git store/erase comes back with a new refresh_token at some later time.
In case of a crash or when getting interrupted it may never come back at all.

If the superseded request_token is used during a parallel git credential operation, this would already trigger credential invalidation of the new in-flight value:

• correctly updating the refresh_token value is MANDATORY, any reuse of old value breaks client registration
• the time between API reply and value update MUST be kept minimal to avoid conflicting use.

Also; the comment above it says the code should be this way! ☺

[dscho]

I have to admit that I lack understanding about the logic. For one, I do not even understand how the current code works, and when I look at the OAuth2TokenResult class I see that there is more information that should be stored. But I do not have the big picture (is it really appropriate to store the refresh token if it turns out that the password does not work, e.g. due to missing scopes?), therefore I'll defer to @mjcheetham (who is still quite occupied with repairing processes that fell into disarray).

[becm]

Only the AccessToken member of the OAuth2TokenResult is handled (correctly) at the moment.

This PR is to address a bug in processing the RefreshToken value
(as required by spec and in accordance with @mjcheetham's comment in the original code).
The refresh_token value (if supplied) MUST be stored, the old value may NEVER be reused.

Even a later git credential erase should NOT touch a stored refresh_token → different auth level
(will become a headache if stored together, since this somehow MUST clear the access_token).

The members ExpiresIn and TokenType (both metadata for AccessToken) are currently discarded.

Use and persistent storage of TokenType will be required for effective support of #2058.

The ExpiresIn:

• can be passed on to git only in the initial reply,
• should be stored (if desired) as calculated absolute value and
• must be recalculated on reuse by a later git credential get request (or directly trigger refresh on expiry)