From fe7863f2ccd133c1629a01d605d37cb588c2a3fb Mon Sep 17 00:00:00 2001 From: Boshi Lian Date: Fri, 21 Feb 2025 01:26:59 -0800 Subject: [PATCH 1/4] Refactor OidcTokenProvider to remove dependency on IdentityModel and improve token handling --- .../Authentication/OidcTokenProvider.cs | 94 +++++++++++++------ src/KubernetesClient/KubernetesClient.csproj | 2 - 2 files changed, 66 insertions(+), 30 deletions(-) diff --git a/src/KubernetesClient/Authentication/OidcTokenProvider.cs b/src/KubernetesClient/Authentication/OidcTokenProvider.cs index ef9c35403..0b8aa404b 100644 --- a/src/KubernetesClient/Authentication/OidcTokenProvider.cs +++ b/src/KubernetesClient/Authentication/OidcTokenProvider.cs @@ -1,23 +1,32 @@ -using IdentityModel.OidcClient; using k8s.Exceptions; -using System.IdentityModel.Tokens.Jwt; +using System.Net.Http; using System.Net.Http.Headers; +using System.Text; namespace k8s.Authentication { public class OidcTokenProvider : ITokenProvider { - private readonly OidcClient _oidcClient; + private readonly string _clientId; + private readonly string _clientSecret; + private readonly string _idpIssuerUrl; + private string _idToken; private string _refreshToken; private DateTimeOffset _expiry; public OidcTokenProvider(string clientId, string clientSecret, string idpIssuerUrl, string idToken, string refreshToken) { + _clientId = clientId; + _clientSecret = clientSecret; + _idpIssuerUrl = idpIssuerUrl; _idToken = idToken; _refreshToken = refreshToken; - _oidcClient = getClient(clientId, clientSecret, idpIssuerUrl); - _expiry = getExpiryFromToken(); + + if (!string.IsNullOrEmpty(_idToken)) + { + _expiry = GetExpiryFromToken(); + } } public async Task GetAuthenticationHeaderAsync(CancellationToken cancellationToken) @@ -30,49 +39,78 @@ public async Task GetAuthenticationHeaderAsync(Cancel return new AuthenticationHeaderValue("Bearer", _idToken); } - private DateTime getExpiryFromToken() + private DateTimeOffset GetExpiryFromToken() { - long expiry; - var handler = new JwtSecurityTokenHandler(); - try + var parts = _idToken.Split('.'); + if (parts.Length != 3) { - var token = handler.ReadJwtToken(_idToken); - expiry = token.Payload.Expiration ?? 0; + throw new ArgumentException("Invalid JWT token format."); } - catch + + var payload = parts[1]; + var jsonBytes = Base64UrlDecode(payload); + var json = Encoding.UTF8.GetString(jsonBytes); + + using var document = JsonDocument.Parse(json); + if (document.RootElement.TryGetProperty("exp", out var expElement)) { - expiry = 0; + var exp = expElement.GetInt64(); + var expiryDateTime = DateTimeOffset.FromUnixTimeSeconds(exp); + return expiryDateTime; + } + else + { + throw new ArgumentException("JWT token does not contain 'exp' claim."); } - - return DateTimeOffset.FromUnixTimeSeconds(expiry).UtcDateTime; } - private OidcClient getClient(string clientId, string clientSecret, string idpIssuerUrl) + private static byte[] Base64UrlDecode(string input) { - OidcClientOptions options = new OidcClientOptions + var output = input.Replace('-', '+').Replace('_', '/'); + switch (output.Length % 4) { - ClientId = clientId, - ClientSecret = clientSecret ?? "", - Authority = idpIssuerUrl, - }; + case 2: output += "=="; break; + case 3: output += "="; break; + } - return new OidcClient(options); + return Convert.FromBase64String(output); } private async Task RefreshToken() { try { - var result = await _oidcClient.RefreshTokenAsync(_refreshToken).ConfigureAwait(false); + using var httpClient = new HttpClient(); + var request = new HttpRequestMessage(HttpMethod.Post, _idpIssuerUrl); + request.Content = new FormUrlEncodedContent(new Dictionary + { + { "grant_type", "refresh_token" }, + { "client_id", _clientId }, + { "client_secret", _clientSecret }, + { "refresh_token", _refreshToken }, + }); + + var response = await httpClient.SendAsync(request).ConfigureAwait(false); + response.EnsureSuccessStatusCode(); - if (result.IsError) + var responseContent = await response.Content.ReadAsStringAsync().ConfigureAwait(false); + var jsonDocument = JsonDocument.Parse(responseContent); + + if (jsonDocument.RootElement.TryGetProperty("id_token", out var idTokenElement)) { - throw new Exception(result.Error); + _idToken = idTokenElement.GetString(); } - _idToken = result.IdentityToken; - _refreshToken = result.RefreshToken; - _expiry = result.AccessTokenExpiration; + if (jsonDocument.RootElement.TryGetProperty("refresh_token", out var refreshTokenElement)) + { + _refreshToken = refreshTokenElement.GetString(); + } + + if (jsonDocument.RootElement.TryGetProperty("expires_in", out var expiresInElement)) + { + var expiresIn = expiresInElement.GetInt32(); + _expiry = DateTimeOffset.UtcNow.AddSeconds(expiresIn); + } } catch (Exception e) { diff --git a/src/KubernetesClient/KubernetesClient.csproj b/src/KubernetesClient/KubernetesClient.csproj index fc8ea8ac5..dba319136 100644 --- a/src/KubernetesClient/KubernetesClient.csproj +++ b/src/KubernetesClient/KubernetesClient.csproj @@ -7,8 +7,6 @@ - - From dffc59e5cefa691b146c5780874358ad85495e2e Mon Sep 17 00:00:00 2001 From: Boshi Lian Date: Fri, 21 Feb 2025 01:48:20 -0800 Subject: [PATCH 2/4] Improve OidcTokenProvider error handling and expiry setting The constructor `OidcTokenProvider` now always sets the `_expiry` field by calling `GetExpiryFromToken()`, regardless of whether `_idToken` is null or empty, removing the previous check for a non-empty `_idToken`. The `GetExpiryFromToken` method has been updated to handle invalid JWT token formats more gracefully. Instead of throwing an `ArgumentException` when the token format is invalid or when the 'exp' claim is missing, the method now returns a default value. The logic for parsing the JWT token and extracting the 'exp' claim has been wrapped in a try-catch block. If any exception occurs during this process, it is caught, and the method returns a default value instead of throwing an exception. --- .../Authentication/OidcTokenProvider.cs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/KubernetesClient/Authentication/OidcTokenProvider.cs b/src/KubernetesClient/Authentication/OidcTokenProvider.cs index 0b8aa404b..1826928e6 100644 --- a/src/KubernetesClient/Authentication/OidcTokenProvider.cs +++ b/src/KubernetesClient/Authentication/OidcTokenProvider.cs @@ -22,11 +22,7 @@ public OidcTokenProvider(string clientId, string clientSecret, string idpIssuerU _idpIssuerUrl = idpIssuerUrl; _idToken = idToken; _refreshToken = refreshToken; - - if (!string.IsNullOrEmpty(_idToken)) - { - _expiry = GetExpiryFromToken(); - } + _expiry = GetExpiryFromToken(); } public async Task GetAuthenticationHeaderAsync(CancellationToken cancellationToken) @@ -44,24 +40,28 @@ private DateTimeOffset GetExpiryFromToken() var parts = _idToken.Split('.'); if (parts.Length != 3) { - throw new ArgumentException("Invalid JWT token format."); + return default; } - var payload = parts[1]; - var jsonBytes = Base64UrlDecode(payload); - var json = Encoding.UTF8.GetString(jsonBytes); - - using var document = JsonDocument.Parse(json); - if (document.RootElement.TryGetProperty("exp", out var expElement)) + try { - var exp = expElement.GetInt64(); - var expiryDateTime = DateTimeOffset.FromUnixTimeSeconds(exp); - return expiryDateTime; + var payload = parts[1]; + var jsonBytes = Base64UrlDecode(payload); + var json = Encoding.UTF8.GetString(jsonBytes); + + using var document = JsonDocument.Parse(json); + if (document.RootElement.TryGetProperty("exp", out var expElement)) + { + var exp = expElement.GetInt64(); + return DateTimeOffset.FromUnixTimeSeconds(exp); + } } - else + catch { - throw new ArgumentException("JWT token does not contain 'exp' claim."); + // ignore to default } + + return default; } private static byte[] Base64UrlDecode(string input) From c253b82b158fa3bd4f578558f446bb8b477ffba6 Mon Sep 17 00:00:00 2001 From: Boshi Lian Date: Fri, 21 Feb 2025 02:15:25 -0800 Subject: [PATCH 3/4] Refactor parts initialization inside try block Moved the initialization of the `parts` variable, which splits the `_idToken` string, inside the `try` block. Removed the previous check for exactly three elements in the `parts` array and the default return value if the check failed. --- src/KubernetesClient/Authentication/OidcTokenProvider.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/KubernetesClient/Authentication/OidcTokenProvider.cs b/src/KubernetesClient/Authentication/OidcTokenProvider.cs index 1826928e6..912ea0fde 100644 --- a/src/KubernetesClient/Authentication/OidcTokenProvider.cs +++ b/src/KubernetesClient/Authentication/OidcTokenProvider.cs @@ -37,14 +37,9 @@ public async Task GetAuthenticationHeaderAsync(Cancel private DateTimeOffset GetExpiryFromToken() { - var parts = _idToken.Split('.'); - if (parts.Length != 3) - { - return default; - } - try { + var parts = _idToken.Split('.'); var payload = parts[1]; var jsonBytes = Base64UrlDecode(payload); var json = Encoding.UTF8.GetString(jsonBytes); From 389b0bdb2db44ad2e14a0ceadedcd0e7b13ea7a7 Mon Sep 17 00:00:00 2001 From: Brendan Burns <5751682+brendandburns@users.noreply.github.com> Date: Fri, 11 Apr 2025 23:30:30 +0000 Subject: [PATCH 4/4] Add tests. --- Directory.Packages.props | 107 +++++++++--------- .../KubernetesClient.Tests.csproj | 1 + tests/KubernetesClient.Tests/OidcAuthTests.cs | 87 ++++++++++++++ 3 files changed, 142 insertions(+), 53 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index f1bd2cccc..c224b2acb 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -1,54 +1,55 @@ - - - true - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + true + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj b/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj index a87beb7b6..fade8e304 100644 --- a/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj +++ b/tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj @@ -13,6 +13,7 @@ + diff --git a/tests/KubernetesClient.Tests/OidcAuthTests.cs b/tests/KubernetesClient.Tests/OidcAuthTests.cs index 23eb7292d..c05a52f10 100644 --- a/tests/KubernetesClient.Tests/OidcAuthTests.cs +++ b/tests/KubernetesClient.Tests/OidcAuthTests.cs @@ -1,8 +1,13 @@ using FluentAssertions; using k8s.Authentication; using k8s.Exceptions; +using System; +using System.Net; using System.Threading; using System.Threading.Tasks; +using WireMock.Server; +using WireMock.RequestBuilders; +using WireMock.ResponseBuilders; using Xunit; namespace k8s.Tests @@ -53,5 +58,87 @@ public async Task TestOidcAuth() Assert.StartsWith("Unable to refresh OIDC token.", e.Message); } } + + [Fact] + public async Task TestOidcAuthWithWireMock() + { + // Arrange + var server = WireMockServer.Start(); + var idpIssuerUrl = server.Url + "/token"; + var clientId = "CLIENT_ID"; + var clientSecret = "CLIENT_SECRET"; + var expiredIdToken = "eyJhbGciOiJIUzI1NiJ9.eyJleHAiOjB9.f37LFpIw_XIS5TZt3wdtEjjyCNshYy03lOWpyDViRM0"; + var refreshToken = "REFRESH_TOKEN"; + var newIdToken = "NEW_ID_TOKEN"; + var expiresIn = 3600; + + // Simulate a successful token refresh response + server + .Given(Request.Create().WithPath("/token").UsingPost()) + .RespondWith(Response.Create() + .WithStatusCode(HttpStatusCode.OK) + .WithBody($@"{{ + ""id_token"": ""{newIdToken}"", + ""refresh_token"": ""{refreshToken}"", + ""expires_in"": {expiresIn} + }}")); + + var auth = new OidcTokenProvider(clientId, clientSecret, idpIssuerUrl, expiredIdToken, refreshToken); + + // Act + var result = await auth.GetAuthenticationHeaderAsync(CancellationToken.None); + + // Assert + result.Scheme.Should().Be("Bearer"); + result.Parameter.Should().Be(newIdToken); + + // Verify that the expiry is set correctly + var expectedExpiry = DateTimeOffset.UtcNow.AddSeconds(expiresIn); + var actualExpiry = typeof(OidcTokenProvider) + .GetField("_expiry", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance) + ?.GetValue(auth) as DateTimeOffset?; + actualExpiry.Should().NotBeNull(); + actualExpiry.Value.Should().BeCloseTo(expectedExpiry, precision: TimeSpan.FromSeconds(5)); + + // Verify that the refresh token is set correctly + var actualRefreshToken = typeof(OidcTokenProvider) + .GetField("_refreshToken", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance) + ?.GetValue(auth) as string; + actualRefreshToken.Should().NotBeNull(); + actualRefreshToken.Should().Be(refreshToken); + + // Stop the server + server.Stop(); + } + + [Fact] + public async Task TestOidcAuthWithServerError() + { + // Arrange + var server = WireMockServer.Start(); + var idpIssuerUrl = server.Url + "/token"; + var clientId = "CLIENT_ID"; + var clientSecret = "CLIENT_SECRET"; + var expiredIdToken = "eyJhbGciOiJIUzI1NiJ9.eyJleHAiOjB9.f37LFpIw_XIS5TZt3wdtEjjyCNshYy03lOWpyDViRM0"; + var refreshToken = "REFRESH_TOKEN"; + + // Simulate a server error response + server + .Given(Request.Create().WithPath("/token").UsingPost()) + .RespondWith(Response.Create() + .WithStatusCode(HttpStatusCode.InternalServerError) + .WithBody(@"{ ""error"": ""server_error"" }")); + + var auth = new OidcTokenProvider(clientId, clientSecret, idpIssuerUrl, expiredIdToken, refreshToken); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => auth.GetAuthenticationHeaderAsync(CancellationToken.None)); + exception.Message.Should().StartWith("Unable to refresh OIDC token."); + exception.InnerException.Message.Should().Contain("500"); + + // Stop the server + server.Stop(); + } } }