fix(generic): check local token expiry

[becm]

Not checking for expired tokens triggers failures on first fetch/push after expiration.
Many Oauth2 implementations use JWT, where expiration time stamp is stored in structured data.

Fixes #268
Fixes #1408
Fixes #1784

[becm]

I'd very much like to have any approach towards identifying local token expiration.
Alternative idea:

• the username is (for the generic provider, at the moment) basically unused (fixed value).
• we can change the stored value to OAUTH_USER@<expiration>.

The last approach changing the API for the storage format (#1464) is now stale for almost a year.
Neither suggestions here would need additional code to be touched.

The username approach is likely even waaayyy more compact.
For fully transparent handling of Git credential flow using store, the full username must be retained for the auth_token credential.
Due to limitations of Git using a BasicAuth header, a colon is not a valid separator.

[becm]

Some major caveat found while trying to prototype a "username@expiration" approach:
API may only return expires_in for the new access_token, not an update refresh_token!

Some storage back ends may also be thrown off by the changing username…

Relative time in protocol additionally indicates this is to be used as a transient element.
Committing that value to permanent storage (after calculating an absolute date) feels like a data flow violation.

For now I'd definitely favor using structured token detection (transparent, backward-compatible, non-intrusive).

[lostmsu]

@mjcheetham @mpysson can you please take a look at this change? It's already been iterated upon.

[lostmsu]

@mjcheetham @mpysson there've been no changes in this project for 5 months. Can I co-maintain it? Not sure if I can do it long term but I promise to go through the currently open pull requests at the very least. Better than nothing.

[itsjustnickdev]

any updates on this push?

[dscho]

any updates on this push?

@itsjustnickdev cautiously: yes. I opened #2059.

However, during the extended time of ownership limbo, Git Credential Manager's release process has withered to the point where no releases can be made as of time of writing (notarizing, for example, is broken, yet it is required for Git Credential Manager to work on macOS).

Therefore, the primary concern right now is to get that release process into a healthy shape.

If you want to look into #2059 in the meantime, that would be really cool, of course!

[mjcheetham]

What do you think about the merits of using the JsonWebToken type from the https://www.nuget.org/packages/Microsoft.IdentityModel.JsonWebTokens package?

[becm]

@mjcheetham replaced handling of JWT with a generic StructuredToken placeholder.

This should make expanding support (or replace implementation) fairly easily later on.
Kept the minimalist JWT extraction approach (fancy 3-liner plus boilerplate) for now.

[becm]

If I know how this needs to be improved, I may have a chance to get it done in time for the next release. 😅

[becm]

@mjcheetham I'd prefer to apply the (optional) switch to a "proper Identity library" at a later date.
Doing this by hand also has the benefit of only having code for the actually needed operations.

The update to .Net 10 will be a major change in dependencies anyway and we can do it properly (if desired).
This will also be a good chance to replace the local Base64Url code (one centralized switch of dependencies).

In the current (revised) approach, changes to internal code should not require additional adjustments.

[becm]

Latest update in main massively reduced required adjustments to GitCredentialManager.GenericHostProvider as it already implements GetCredentialAsync().

Moved update to Base64UrlConvert into separate commit.
Tuned code flow in StructuredToken.TryCreate() in hope to ease later extension of token types (if required).

[becm]

Revised code flow in GetCredentialAsync to have distinct log output for missing and invalid credential.

[colejohnson66]

What is preventing this from being merged? The issue has existed for FIVE YEARS.

[becm]

Removed the expiration check from the refresh_token, as there is basically no benefit.
The existing code (and request) paths should cope with and recover from using an expired token.

This should never cause issues in the proper single well-behaved client case.

If an update to a refresh_token is missed (bad client behavior or parallel use of same client ID)
the expiry date of the local token becomes invalid without notice
(immediately or after some minutes, depending on server setup).