-
Notifications
You must be signed in to change notification settings - Fork 545
Tokens can be cached beyond the lifetime of the (http) transport. #834
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
namespace ModelContextProtocol.Authentication; | ||
|
||
/// <summary> | ||
/// Allows the client to cache access tokens beyond the lifetime of the transport. | ||
/// </summary> | ||
public interface ITokenCache | ||
{ | ||
/// <summary> | ||
/// Cache the token. After a new access token is acquired, this method is invoked to store it. | ||
/// </summary> | ||
ValueTask StoreTokenAsync(TokenContainerCacheable token, CancellationToken cancellationToken); | ||
|
||
/// <summary> | ||
/// Get the cached token. This method is invoked for every request. | ||
/// </summary> | ||
ValueTask<TokenContainerCacheable?> GetTokenAsync(CancellationToken cancellationToken); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
|
||
namespace ModelContextProtocol.Authentication; | ||
|
||
/// <summary> | ||
/// Caches the token in-memory within this instance. | ||
/// </summary> | ||
internal class InMemoryTokenCache : ITokenCache | ||
{ | ||
private TokenContainerCacheable? _token; | ||
|
||
/// <summary> | ||
/// Cache the token. | ||
/// </summary> | ||
public ValueTask StoreTokenAsync(TokenContainerCacheable token, CancellationToken cancellationToken) | ||
{ | ||
_token = token; | ||
return default; | ||
} | ||
|
||
/// <summary> | ||
/// Get the cached token. | ||
/// </summary> | ||
public ValueTask<TokenContainerCacheable?> GetTokenAsync(CancellationToken cancellationToken) | ||
{ | ||
return new ValueTask<TokenContainerCacheable?>(_token); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,42 @@ | ||||||
namespace ModelContextProtocol.Authentication; | ||||||
|
||||||
/// <summary> | ||||||
/// Represents a cacheable token representation. | ||||||
/// </summary> | ||||||
public class TokenContainerCacheable | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should use the better name for the public API.
Suggested change
Then, we could call the internal type we deserialize the /token response with |
||||||
{ | ||||||
/// <summary> | ||||||
/// Gets or sets the access token. | ||||||
/// </summary> | ||||||
public string AccessToken { get; set; } = string.Empty; | ||||||
|
||||||
/// <summary> | ||||||
/// Gets or sets the refresh token. | ||||||
/// </summary> | ||||||
public string? RefreshToken { get; set; } | ||||||
|
||||||
/// <summary> | ||||||
/// Gets or sets the number of seconds until the access token expires. | ||||||
/// </summary> | ||||||
public int ExpiresIn { get; set; } | ||||||
|
||||||
/// <summary> | ||||||
/// Gets or sets the extended expiration time in seconds. | ||||||
/// </summary> | ||||||
public int ExtExpiresIn { get; set; } | ||||||
|
||||||
/// <summary> | ||||||
/// Gets or sets the token type (typically "Bearer"). | ||||||
/// </summary> | ||||||
public string TokenType { get; set; } = string.Empty; | ||||||
|
||||||
/// <summary> | ||||||
/// Gets or sets the scope of the access token. | ||||||
/// </summary> | ||||||
public string Scope { get; set; } = string.Empty; | ||||||
|
||||||
/// <summary> | ||||||
/// Gets or sets the timestamp when the token was obtained. | ||||||
/// </summary> | ||||||
public DateTimeOffset ObtainedAt { get; set; } | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
namespace ModelContextProtocol.Authentication; | ||
|
||
internal static class TokenContainerConvert | ||
{ | ||
internal static TokenContainer ForUse(this TokenContainerCacheable token) => new() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add a reflection-based test for this and |
||
{ | ||
AccessToken = token.AccessToken, | ||
RefreshToken = token.RefreshToken, | ||
ExpiresIn = token.ExpiresIn, | ||
ExtExpiresIn = token.ExtExpiresIn, | ||
TokenType = token.TokenType, | ||
Scope = token.Scope, | ||
ObtainedAt = token.ObtainedAt, | ||
}; | ||
|
||
internal static TokenContainerCacheable ForCache(this TokenContainer token) => new() | ||
{ | ||
AccessToken = token.AccessToken, | ||
RefreshToken = token.RefreshToken, | ||
ExpiresIn = token.ExpiresIn, | ||
ExtExpiresIn = token.ExtExpiresIn, | ||
TokenType = token.TokenType, | ||
Scope = token.Scope, | ||
ObtainedAt = token.ObtainedAt, | ||
}; | ||
} |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make the methods name plural? I think StoreTokensAsync and GetTokensAsync makes more sense considering we're storing and retrieving both the access token and refresh token if available.