From f6b88c2cc86084d4c47995edac475b919f6eb58a Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Thu, 21 Mar 2024 21:44:17 -0500 Subject: [PATCH 1/4] Work in progress on fix for DPoP nonces received from the AS --- .../ConfigureOpenIdConnectOptions.cs | 14 +++-- .../DPoPProofTokenHandler.cs | 52 ++++++++++++------- .../DPoPExtensions.cs | 2 +- 3 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs b/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs index a4419f6..15c7e01 100644 --- a/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs +++ b/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Authentication.OpenIdConnect; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using System; using System.Net.Http; @@ -22,6 +23,9 @@ public class ConfigureOpenIdConnectOptions : IConfigureNamedOptions _userAccessTokenManagementOptions; + + private readonly ILoggerFactory _loggerFactory; + private readonly string? _configScheme; private readonly string _clientName; @@ -33,13 +37,14 @@ public ConfigureOpenIdConnectOptions( IDPoPProofService dPoPProofService, IHttpContextAccessor httpContextAccessor, IOptions userAccessTokenManagementOptions, - IAuthenticationSchemeProvider schemeProvider) + IAuthenticationSchemeProvider schemeProvider, + ILoggerFactory loggerFactory) { _dPoPNonceStore = dPoPNonceStore; _dPoPProofService = dPoPProofService; _httpContextAccessor = httpContextAccessor; _userAccessTokenManagementOptions = userAccessTokenManagementOptions; - + _configScheme = _userAccessTokenManagementOptions.Value.ChallengeScheme; if (string.IsNullOrWhiteSpace(_configScheme)) { @@ -55,6 +60,7 @@ public ConfigureOpenIdConnectOptions( } _clientName = OpenIdConnectTokenManagementDefaults.ClientCredentialsClientNamePrefix + _configScheme; + _loggerFactory = loggerFactory; } /// @@ -72,7 +78,9 @@ public void Configure(string? name, OpenIdConnectOptions options) options.Events.OnAuthorizationCodeReceived = CreateCallback(options.Events.OnAuthorizationCodeReceived); options.Events.OnTokenValidated = CreateCallback(options.Events.OnTokenValidated); - options.BackchannelHttpHandler = new DPoPProofTokenHandler(_dPoPProofService, _dPoPNonceStore, _httpContextAccessor) + var logger = _loggerFactory.CreateLogger(); + + options.BackchannelHttpHandler = new DPoPProofTokenHandler(_dPoPProofService, _dPoPNonceStore, _httpContextAccessor, logger) { InnerHandler = options.BackchannelHttpHandler ?? new HttpClientHandler() }; diff --git a/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs b/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs index ddcaa78..ae5adf9 100644 --- a/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs +++ b/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs @@ -3,6 +3,7 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -10,28 +11,26 @@ namespace Duende.AccessTokenManagement.OpenIdConnect; /// -/// Delegating handler that injects the DPoP proof token from the OIDC handler workflow +/// Delegating handler that injects the DPoP proof token. This is intended to be +/// used on the OIDC authentication handler's backchannel http client. /// -class DPoPProofTokenHandler : DelegatingHandler +public class DPoPProofTokenHandler : DelegatingHandler { private readonly IDPoPProofService _dPoPProofService; private readonly IDPoPNonceStore _dPoPNonceStore; private readonly IHttpContextAccessor _httpContextAccessor; + private readonly ILogger _logger; - /// - /// ctor - /// - /// - /// - /// - public DPoPProofTokenHandler( + internal DPoPProofTokenHandler( IDPoPProofService dPoPProofService, IDPoPNonceStore dPoPNonceStore, - IHttpContextAccessor httpContextAccessor) + IHttpContextAccessor httpContextAccessor, + ILogger logger) { _dPoPProofService = dPoPProofService; _dPoPNonceStore = dPoPNonceStore; _httpContextAccessor = httpContextAccessor; + _logger = logger; } /// @@ -40,23 +39,33 @@ protected override async Task SendAsync(HttpRequestMessage await SetDPoPProofTokenAsync(request, cancellationToken).ConfigureAwait(false); var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + // The authorization server might send us a new nonce in a successful + // request, indicating that we should use the new nonce in future. var dPoPNonce = response.GetDPoPNonce(); - // retry if 401 - if (response.StatusCode == System.Net.HttpStatusCode.Unauthorized && response.IsDPoPError()) + if (dPoPNonce != null) { - response.Dispose(); + _logger.LogDebug("The authorization server has supplied a new nonce"); - await SetDPoPProofTokenAsync(request, cancellationToken, dPoPNonce).ConfigureAwait(false); - return await base.SendAsync(request, cancellationToken).ConfigureAwait(false); - } - else if (dPoPNonce != null) - { await _dPoPNonceStore.StoreNonceAsync(new DPoPNonceContext { Url = request.GetDPoPUrl(), Method = request.Method.ToString(), }, dPoPNonce); + + // But the authorization server might also send a failure response, and expect us to retry + if (response.StatusCode == System.Net.HttpStatusCode.BadRequest) + { + + // REVIEW: Is it good enough to check the status code and + // existence of the new nonce? Should we parse the response, and + // look for the "use_dpop_nonce" value in the error property? + + _logger.LogDebug("Request failed (bad request). Retrying request with new DPoP proof token that includes the new nonce"); + response.Dispose(); + await SetDPoPProofTokenAsync(request, cancellationToken, dPoPNonce).ConfigureAwait(false); + return await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + } } return response; @@ -85,8 +94,15 @@ protected virtual async Task SetDPoPProofTokenAsync(HttpRequestMessage request, if (proofToken != null) { + _logger.LogDebug("Sending DPoP proof token in request to endpoint: {url}", + request.RequestUri?.GetLeftPart(System.UriPartial.Path)); request.SetDPoPProofToken(proofToken.ProofToken); } + else + { + _logger.LogDebug("No DPoP proof token in request to endpoint: {url}", + request.RequestUri?.GetLeftPart(System.UriPartial.Path)); + } } } } diff --git a/src/Duende.AccessTokenManagement/DPoPExtensions.cs b/src/Duende.AccessTokenManagement/DPoPExtensions.cs index 6090b2a..c433060 100644 --- a/src/Duende.AccessTokenManagement/DPoPExtensions.cs +++ b/src/Duende.AccessTokenManagement/DPoPExtensions.cs @@ -43,7 +43,7 @@ public static void SetDPoPProofToken(this HttpRequestMessage request, string? pr } /// - /// Reads the WWW-Authenticate response header to determine if the respone is in error due to DPoP + /// Reads the WWW-Authenticate response header to determine if the response is in error due to DPoP /// public static bool IsDPoPError(this HttpResponseMessage response) { From 69f07eaf60f67ca51897f89c60be94fb763ae283 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Fri, 22 Mar 2024 13:54:35 -0500 Subject: [PATCH 2/4] DPoP nonce updates from review discussion --- .../ConfigureOpenIdConnectOptions.cs | 4 +- .../DPoPProofTokenHandler.cs | 61 +++++++++++-------- 2 files changed, 37 insertions(+), 28 deletions(-) diff --git a/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs b/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs index 15c7e01..b93aa79 100644 --- a/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs +++ b/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs @@ -78,9 +78,7 @@ public void Configure(string? name, OpenIdConnectOptions options) options.Events.OnAuthorizationCodeReceived = CreateCallback(options.Events.OnAuthorizationCodeReceived); options.Events.OnTokenValidated = CreateCallback(options.Events.OnTokenValidated); - var logger = _loggerFactory.CreateLogger(); - - options.BackchannelHttpHandler = new DPoPProofTokenHandler(_dPoPProofService, _dPoPNonceStore, _httpContextAccessor, logger) + options.BackchannelHttpHandler = new AuthorizationServerDPoPHandler(_dPoPProofService, _dPoPNonceStore, _httpContextAccessor, _loggerFactory) { InnerHandler = options.BackchannelHttpHandler ?? new HttpClientHandler() }; diff --git a/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs b/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs index ae5adf9..20aab2f 100644 --- a/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs +++ b/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs @@ -1,9 +1,10 @@ -// Copyright (c) Brock Allen & Dominick Baier. All rights reserved. +// Copyright (c) Brock Allen & Dominick Baier. All rights reserved. // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; +using System.Net; using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -11,26 +12,30 @@ namespace Duende.AccessTokenManagement.OpenIdConnect; /// -/// Delegating handler that injects the DPoP proof token. This is intended to be -/// used on the OIDC authentication handler's backchannel http client. +/// Delegating handler that injects the DPoP proof token into requests that are +/// made to the authorization server. This is intended to be used on the OIDC +/// authentication handler's backchannel http client. /// -public class DPoPProofTokenHandler : DelegatingHandler +internal class AuthorizationServerDPoPHandler : DelegatingHandler { private readonly IDPoPProofService _dPoPProofService; private readonly IDPoPNonceStore _dPoPNonceStore; private readonly IHttpContextAccessor _httpContextAccessor; - private readonly ILogger _logger; + private readonly ILogger _logger; - internal DPoPProofTokenHandler( + internal AuthorizationServerDPoPHandler( IDPoPProofService dPoPProofService, IDPoPNonceStore dPoPNonceStore, IHttpContextAccessor httpContextAccessor, - ILogger logger) + ILoggerFactory loggerFactory) { _dPoPProofService = dPoPProofService; _dPoPNonceStore = dPoPNonceStore; _httpContextAccessor = httpContextAccessor; - _logger = logger; + // We depend on the logger factory, rather than the logger itself, since + // the type parameter of the logger (referencing this class) will not + // always be accessible. + _logger = loggerFactory.CreateLogger(); } /// @@ -39,32 +44,38 @@ protected override async Task SendAsync(HttpRequestMessage await SetDPoPProofTokenAsync(request, cancellationToken).ConfigureAwait(false); var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); - // The authorization server might send us a new nonce in a successful - // request, indicating that we should use the new nonce in future. + // The authorization server might send us a new nonce on either a success or failure var dPoPNonce = response.GetDPoPNonce(); if (dPoPNonce != null) { - _logger.LogDebug("The authorization server has supplied a new nonce"); - - await _dPoPNonceStore.StoreNonceAsync(new DPoPNonceContext - { - Url = request.GetDPoPUrl(), - Method = request.Method.ToString(), - }, dPoPNonce); - - // But the authorization server might also send a failure response, and expect us to retry - if (response.StatusCode == System.Net.HttpStatusCode.BadRequest) + // If the response code is a bad request, we can infer that we + // should retry with the new nonce. The server should have also set + // the error: use_dpop_nonce, but there's no need to incur the cost + // of parsing the json and checking for that, as we would only + // receive the nonce http header when that error was set. + // Authorization servers might preemptively send a new nonce, but + // the spec specifically says to do that on a success (and we handle + // that case in the else block) + // + // TL;DR - presence of nonce and 400 response code is enough to + // trigger a retry + if (response.StatusCode == HttpStatusCode.BadRequest) { - - // REVIEW: Is it good enough to check the status code and - // existence of the new nonce? Should we parse the response, and - // look for the "use_dpop_nonce" value in the error property? - _logger.LogDebug("Request failed (bad request). Retrying request with new DPoP proof token that includes the new nonce"); response.Dispose(); await SetDPoPProofTokenAsync(request, cancellationToken, dPoPNonce).ConfigureAwait(false); return await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + } + else if (response.StatusCode == HttpStatusCode.OK) + { + _logger.LogDebug("The authorization server has supplied a new nonce on a successful response, which will be stored and used in future requests to the authorization server"); + + await _dPoPNonceStore.StoreNonceAsync(new DPoPNonceContext + { + Url = request.GetDPoPUrl(), + Method = request.Method.ToString(), + }, dPoPNonce); } } From ccefcfd6a4535e9744b8530113d8f2d96f07b376 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Fri, 29 Mar 2024 09:23:47 -0500 Subject: [PATCH 3/4] Reworked DPoP server nonce implementation Also renamed some internals for clarity --- .../AuthorizationServerDPoPHandler.cs | 149 ++++++++++++++++++ ...reOpenIdConnectClientCredentialsOptions.cs | 2 +- .../ConfigureOpenIdConnectOptions.cs | 7 +- .../DPoPProofTokenHandler.cs | 119 -------------- .../TokenManagementHttpContextExtensions.cs | 5 +- .../UserTokenEndpointService.cs | 2 + test/Tests/Framework/AppHost.cs | 34 ++-- test/Tests/Framework/IdentityServerHost.cs | 4 + test/Tests/Framework/IntegrationTestBase.cs | 28 +++- test/Tests/UserTokenManagementTests.cs | 4 +- .../Tests/UserTokenManagementWithDPoPTests.cs | 118 ++++++++++++++ 11 files changed, 330 insertions(+), 142 deletions(-) create mode 100644 src/Duende.AccessTokenManagement.OpenIdConnect/AuthorizationServerDPoPHandler.cs delete mode 100644 src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs create mode 100644 test/Tests/UserTokenManagementWithDPoPTests.cs diff --git a/src/Duende.AccessTokenManagement.OpenIdConnect/AuthorizationServerDPoPHandler.cs b/src/Duende.AccessTokenManagement.OpenIdConnect/AuthorizationServerDPoPHandler.cs new file mode 100644 index 0000000..cbb1781 --- /dev/null +++ b/src/Duende.AccessTokenManagement.OpenIdConnect/AuthorizationServerDPoPHandler.cs @@ -0,0 +1,149 @@ +// Copyright (c) Brock Allen & Dominick Baier. All rights reserved. +// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. + +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; +using System.Net; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; + +namespace Duende.AccessTokenManagement.OpenIdConnect; + +/// +/// Delegating handler that adds behavior needed for DPoP to the backchannel +/// http client of the OIDC authentication handler. +/// +/// This handler has two main jobs: +/// +/// 1. Store new nonces from successful responses from the authorization server. +/// +/// 2. Attach proof tokens to token requests in the code flow. +/// +/// On the authorize request, we will have sent a dpop_jkt parameter with a +/// key thumbprint. The AS expects that we will use the corresponding key to +/// create our proof, and we track that key in the http context. This handler +/// retrieves that key and uses it to create proof tokens for use in the code +/// flow. +/// +/// Additionally, the token endpoint might respond to a token exchange +/// request with a request to retry with a nonce that it supplies via http +/// header. When it does, this handler retries those code exchange requests. +/// +/// +internal class AuthorizationServerDPoPHandler : DelegatingHandler +{ + private readonly IDPoPProofService _dPoPProofService; + private readonly IDPoPNonceStore _dPoPNonceStore; + private readonly IHttpContextAccessor _httpContextAccessor; + private readonly ILogger _logger; + + internal AuthorizationServerDPoPHandler( + IDPoPProofService dPoPProofService, + IDPoPNonceStore dPoPNonceStore, + IHttpContextAccessor httpContextAccessor, + ILoggerFactory loggerFactory) + { + _dPoPProofService = dPoPProofService; + _dPoPNonceStore = dPoPNonceStore; + _httpContextAccessor = httpContextAccessor; + // We depend on the logger factory, rather than the logger itself, since + // the type parameter of the logger (referencing this class) will not + // always be accessible. + _logger = loggerFactory.CreateLogger(); + } + + /// + protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + var codeExchangeJwk = _httpContextAccessor.HttpContext?.GetCodeExchangeDPoPKey(); + if (codeExchangeJwk != null) + { + await SetDPoPProofTokenForCodeExchangeAsync(request, jwk: codeExchangeJwk).ConfigureAwait(false); + } + + var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + + // The authorization server might send us a new nonce on either a success or failure + var dPoPNonce = response.GetDPoPNonce(); + + if (dPoPNonce != null) + { + // This handler contains specialized logic to create the new proof + // token using the proof key that was associated with a code flow + // using a dpop_jkt parameter on the authorize call. Other flows + // (such as refresh), are separately responsible for retrying with a + // server-issued nonce. So, we ONLY do the retry logic when we have + // the dpop_jkt's jwk + if (codeExchangeJwk != null) + { + // If the http response code indicates a bad request, we can infer + // that we should retry with the new nonce. + // + // The server should have also set the error: use_dpop_nonce, but + // there's no need to incur the cost of parsing the json and + // checking for that, as we would only receive the nonce http header + // when that error was set. Authorization servers might preemptively + // send a new nonce, but the spec specifically says to do that on a + // success (and we handle that case in the else block). + // + // TL;DR - presence of nonce and 400 response code is enough to + // trigger a retry during code exchange + if (response.StatusCode == HttpStatusCode.BadRequest) + { + _logger.LogDebug("Token request failed with DPoP nonce error. Retrying with new nonce."); + response.Dispose(); + await SetDPoPProofTokenForCodeExchangeAsync(request, dPoPNonce, codeExchangeJwk).ConfigureAwait(false); + return await base.SendAsync(request, cancellationToken).ConfigureAwait(false); + } + } + + if (response.StatusCode == HttpStatusCode.OK) + { + _logger.LogDebug("The authorization server has supplied a new nonce on a successful response, which will be stored and used in future requests to the authorization server"); + + await _dPoPNonceStore.StoreNonceAsync(new DPoPNonceContext + { + Url = request.GetDPoPUrl(), + Method = request.Method.ToString(), + }, dPoPNonce); + } + } + + return response; + } + + /// + /// Creates a DPoP proof token and attaches it to a request. + /// + internal async Task SetDPoPProofTokenForCodeExchangeAsync(HttpRequestMessage request, string? dpopNonce = null, string? jwk = null) + { + if (!string.IsNullOrEmpty(jwk)) + { + // remove any old headers + request.ClearDPoPProofToken(); + + // create proof + var proofToken = await _dPoPProofService.CreateProofTokenAsync(new DPoPProofRequest + { + Url = request.GetDPoPUrl(), + Method = request.Method.ToString(), + DPoPJsonWebKey = jwk, + DPoPNonce = dpopNonce, + }); + + if (proofToken != null) + { + _logger.LogDebug("Sending DPoP proof token in request to endpoint: {url}", + request.RequestUri?.GetLeftPart(System.UriPartial.Path)); + request.SetDPoPProofToken(proofToken.ProofToken); + } + else + { + _logger.LogDebug("No DPoP proof token in request to endpoint: {url}", + request.RequestUri?.GetLeftPart(System.UriPartial.Path)); + } + } + } +} diff --git a/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectClientCredentialsOptions.cs b/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectClientCredentialsOptions.cs index e724e7a..019d94f 100644 --- a/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectClientCredentialsOptions.cs +++ b/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectClientCredentialsOptions.cs @@ -7,7 +7,7 @@ namespace Duende.AccessTokenManagement.OpenIdConnect; /// -/// Named options to synthetize client credentials based on OIDC handler configuration +/// Named options to synthesize client credentials based on OIDC handler configuration /// public class ConfigureOpenIdConnectClientCredentialsOptions : IConfigureNamedOptions { diff --git a/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs b/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs index b93aa79..9c61eec 100644 --- a/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs +++ b/src/Duende.AccessTokenManagement.OpenIdConnect/ConfigureOpenIdConnectOptions.cs @@ -109,7 +109,10 @@ async Task Callback(RedirectContext context) // checking for null allows for opt-out from using DPoP if (jkt != null) { - // we store the proof key here to associate it with the access token returned + // we store the proof key here to associate it with the + // authorization code that will be returned. Ultimately we + // use this to provide proof of possession during code + // exchange. context.Properties.SetProofKey(key.JsonWebKey); // pass jkt to authorize endpoint @@ -132,7 +135,7 @@ Task Callback(AuthorizationCodeReceivedContext context) if (jwk != null) { // set it so the OIDC message handler can find it - context.HttpContext.SetOutboundProofKey(jwk); + context.HttpContext.SetCodeExchangeDPoPKey(jwk); } return result; diff --git a/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs b/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs deleted file mode 100644 index 20aab2f..0000000 --- a/src/Duende.AccessTokenManagement.OpenIdConnect/DPoPProofTokenHandler.cs +++ /dev/null @@ -1,119 +0,0 @@ -// Copyright (c) Brock Allen & Dominick Baier. All rights reserved. -// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. - -using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.Logging; -using System.Net; -using System.Net.Http; -using System.Threading; -using System.Threading.Tasks; - -namespace Duende.AccessTokenManagement.OpenIdConnect; - -/// -/// Delegating handler that injects the DPoP proof token into requests that are -/// made to the authorization server. This is intended to be used on the OIDC -/// authentication handler's backchannel http client. -/// -internal class AuthorizationServerDPoPHandler : DelegatingHandler -{ - private readonly IDPoPProofService _dPoPProofService; - private readonly IDPoPNonceStore _dPoPNonceStore; - private readonly IHttpContextAccessor _httpContextAccessor; - private readonly ILogger _logger; - - internal AuthorizationServerDPoPHandler( - IDPoPProofService dPoPProofService, - IDPoPNonceStore dPoPNonceStore, - IHttpContextAccessor httpContextAccessor, - ILoggerFactory loggerFactory) - { - _dPoPProofService = dPoPProofService; - _dPoPNonceStore = dPoPNonceStore; - _httpContextAccessor = httpContextAccessor; - // We depend on the logger factory, rather than the logger itself, since - // the type parameter of the logger (referencing this class) will not - // always be accessible. - _logger = loggerFactory.CreateLogger(); - } - - /// - protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { - await SetDPoPProofTokenAsync(request, cancellationToken).ConfigureAwait(false); - var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); - - // The authorization server might send us a new nonce on either a success or failure - var dPoPNonce = response.GetDPoPNonce(); - - if (dPoPNonce != null) - { - // If the response code is a bad request, we can infer that we - // should retry with the new nonce. The server should have also set - // the error: use_dpop_nonce, but there's no need to incur the cost - // of parsing the json and checking for that, as we would only - // receive the nonce http header when that error was set. - // Authorization servers might preemptively send a new nonce, but - // the spec specifically says to do that on a success (and we handle - // that case in the else block) - // - // TL;DR - presence of nonce and 400 response code is enough to - // trigger a retry - if (response.StatusCode == HttpStatusCode.BadRequest) - { - _logger.LogDebug("Request failed (bad request). Retrying request with new DPoP proof token that includes the new nonce"); - response.Dispose(); - await SetDPoPProofTokenAsync(request, cancellationToken, dPoPNonce).ConfigureAwait(false); - return await base.SendAsync(request, cancellationToken).ConfigureAwait(false); - } - else if (response.StatusCode == HttpStatusCode.OK) - { - _logger.LogDebug("The authorization server has supplied a new nonce on a successful response, which will be stored and used in future requests to the authorization server"); - - await _dPoPNonceStore.StoreNonceAsync(new DPoPNonceContext - { - Url = request.GetDPoPUrl(), - Method = request.Method.ToString(), - }, dPoPNonce); - } - } - - return response; - } - - /// - /// Creates a DPoP proof token and attaches it to the request. - /// - protected virtual async Task SetDPoPProofTokenAsync(HttpRequestMessage request, CancellationToken cancellationToken, string? dpopNonce = null) - { - var jwk = _httpContextAccessor.HttpContext?.GetOutboundProofKey(); - - if (!string.IsNullOrEmpty(jwk)) - { - // remove any old headers - request.ClearDPoPProofToken(); - - // create proof - var proofToken = await _dPoPProofService.CreateProofTokenAsync(new DPoPProofRequest - { - Url = request.GetDPoPUrl(), - Method = request.Method.ToString(), - DPoPJsonWebKey = jwk, - DPoPNonce = dpopNonce, - }); - - if (proofToken != null) - { - _logger.LogDebug("Sending DPoP proof token in request to endpoint: {url}", - request.RequestUri?.GetLeftPart(System.UriPartial.Path)); - request.SetDPoPProofToken(proofToken.ProofToken); - } - else - { - _logger.LogDebug("No DPoP proof token in request to endpoint: {url}", - request.RequestUri?.GetLeftPart(System.UriPartial.Path)); - } - } - } -} diff --git a/src/Duende.AccessTokenManagement.OpenIdConnect/TokenManagementHttpContextExtensions.cs b/src/Duende.AccessTokenManagement.OpenIdConnect/TokenManagementHttpContextExtensions.cs index c10da10..2284563 100755 --- a/src/Duende.AccessTokenManagement.OpenIdConnect/TokenManagementHttpContextExtensions.cs +++ b/src/Duende.AccessTokenManagement.OpenIdConnect/TokenManagementHttpContextExtensions.cs @@ -9,6 +9,7 @@ using Duende.AccessTokenManagement; using Duende.AccessTokenManagement.OpenIdConnect; using Microsoft.Extensions.Options; +using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Authentication; @@ -105,11 +106,11 @@ internal static void RemoveProofKey(this AuthenticationProperties properties) } const string HttpContextDPoPKey = "dpop_proof_key"; - internal static void SetOutboundProofKey(this HttpContext context, string key) + internal static void SetCodeExchangeDPoPKey(this HttpContext context, string key) { context.Items[HttpContextDPoPKey] = key; } - internal static string? GetOutboundProofKey(this HttpContext context) + internal static string? GetCodeExchangeDPoPKey(this HttpContext context) { if (context.Items.ContainsKey(HttpContextDPoPKey)) { diff --git a/src/Duende.AccessTokenManagement.OpenIdConnect/UserTokenEndpointService.cs b/src/Duende.AccessTokenManagement.OpenIdConnect/UserTokenEndpointService.cs index f0173b4..0681da6 100755 --- a/src/Duende.AccessTokenManagement.OpenIdConnect/UserTokenEndpointService.cs +++ b/src/Duende.AccessTokenManagement.OpenIdConnect/UserTokenEndpointService.cs @@ -115,6 +115,8 @@ public async Task RefreshAccessTokenAsync( dPoPJsonWebKey != null && response.DPoPNonce != null) { + _logger.LogDebug("DPoP error during token refresh. Retrying with server nonce"); + var proof = await _dPoPProofService.CreateProofTokenAsync(new DPoPProofRequest { Url = request.Address!, diff --git a/test/Tests/Framework/AppHost.cs b/test/Tests/Framework/AppHost.cs index 5300c11..2923294 100644 --- a/test/Tests/Framework/AppHost.cs +++ b/test/Tests/Framework/AppHost.cs @@ -1,14 +1,15 @@ // Copyright (c) Duende Software. All rights reserved. // See LICENSE in the project root for license information. +using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using System.Net; -using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Http; -using RichardSzalay.MockHttp; +using System.Web; +using IdentityModel; using Duende.AccessTokenManagement.OpenIdConnect; -using Microsoft.AspNetCore.Mvc; +using RichardSzalay.MockHttp; namespace Duende.AccessTokenManagement.Tests; @@ -17,18 +18,20 @@ public class AppHost : GenericHost private readonly IdentityServerHost _identityServerHost; private readonly ApiHost _apiHost; private readonly string _clientId; - + private readonly Action? _configureUserTokenManagementOptions; + public AppHost( IdentityServerHost identityServerHost, ApiHost apiHost, string clientId, - string baseAddress = "https://app") + string baseAddress = "https://app", + Action? configureUserTokenManagementOptions = default) : base(baseAddress) { _identityServerHost = identityServerHost; _apiHost = apiHost; _clientId = clientId; - + _configureUserTokenManagementOptions = configureUserTokenManagementOptions; OnConfigureServices += ConfigureServices; OnConfigure += Configure; } @@ -97,6 +100,11 @@ private void ConfigureServices(IServiceCollection services) services.AddOpenIdConnectAccessTokenManagement(opt => { opt.UseChallengeSchemeScopedTokens = true; + + if (_configureUserTokenManagementOptions != null) + { + _configureUserTokenManagementOptions(opt); + } }); } @@ -145,18 +153,24 @@ await context.ChallengeAsync(new AuthenticationProperties }); } - public async Task LoginAsync(string sub, string? sid = null) + public async Task LoginAsync(string sub, string? sid = null, bool verifyDpopThumbprintSent = false) { await _identityServerHost.CreateIdentityServerSessionCookieAsync(sub, sid); - return await OidcLoginAsync(); + return await OidcLoginAsync(verifyDpopThumbprintSent); } - public async Task OidcLoginAsync() + public async Task OidcLoginAsync(bool verifyDpopThumbprintSent) { var response = await BrowserClient.GetAsync(Url("/login")); response.StatusCode.ShouldBe((HttpStatusCode)302); // authorize response.Headers.Location!.ToString().ToLowerInvariant().ShouldStartWith(_identityServerHost.Url("/connect/authorize")); + if (verifyDpopThumbprintSent) + { + var queryParams = HttpUtility.ParseQueryString(response.Headers.Location.Query); + queryParams.AllKeys.ShouldContain(OidcConstants.AuthorizeRequest.DPoPKeyThumbprint); + } + response = await _identityServerHost.BrowserClient.GetAsync(response.Headers.Location.ToString()); response.StatusCode.ShouldBe((HttpStatusCode)302); // client callback response.Headers.Location!.ToString().ToLowerInvariant().ShouldStartWith(Url("/signin-oidc")); diff --git a/test/Tests/Framework/IdentityServerHost.cs b/test/Tests/Framework/IdentityServerHost.cs index c547708..f03fbd5 100644 --- a/test/Tests/Framework/IdentityServerHost.cs +++ b/test/Tests/Framework/IdentityServerHost.cs @@ -49,6 +49,10 @@ private void ConfigureServices(IServiceCollection services) services.AddIdentityServer(options=> { options.EmitStaticAudienceClaim = true; + + // Artificially low durations to force retries + options.DPoP.ServerClockSkew = TimeSpan.Zero; + options.DPoP.ProofTokenValidityDuration = TimeSpan.FromSeconds(1); }) .AddInMemoryClients(Clients) .AddInMemoryIdentityResources(IdentityResources) diff --git a/test/Tests/Framework/IntegrationTestBase.cs b/test/Tests/Framework/IntegrationTestBase.cs index 9549c7c..8ded95f 100644 --- a/test/Tests/Framework/IntegrationTestBase.cs +++ b/test/Tests/Framework/IntegrationTestBase.cs @@ -1,10 +1,9 @@ // Copyright (c) Duende Software. All rights reserved. // See LICENSE in the project root for license information. +using Duende.AccessTokenManagement.OpenIdConnect; using Duende.IdentityServer.Models; -using Duende.IdentityServer.Services; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; +using IdentityModel; using System.Security.Claims; namespace Duende.AccessTokenManagement.Tests; @@ -15,7 +14,7 @@ public class IntegrationTestBase protected ApiHost ApiHost; protected AppHost AppHost; - public IntegrationTestBase(string clientId = "web") + public IntegrationTestBase(string clientId = "web", Action? configureUserTokenManagementOptions = null) { IdentityServerHost = new IdentityServerHost(); @@ -50,13 +49,30 @@ public IntegrationTestBase(string clientId = "web") AccessTokenLifetime = 10 }); - + + IdentityServerHost.Clients.Add(new Client + { + ClientId = "dpop", + ClientSecrets = { new Secret("secret".ToSha256()) }, + AllowedGrantTypes = GrantTypes.CodeAndClientCredentials, + RedirectUris = { "https://app/signin-oidc" }, + PostLogoutRedirectUris = { "https://app/signout-callback-oidc" }, + AllowOfflineAccess = true, + AllowedScopes = { "openid", "profile", "scope1" }, + + RequireDPoP = true, + DPoPValidationMode = DPoPTokenExpirationValidationMode.Nonce, + DPoPClockSkew = TimeSpan.FromMilliseconds(10), + + AccessTokenLifetime = 10 + }); + IdentityServerHost.InitializeAsync().Wait(); ApiHost = new ApiHost(IdentityServerHost, "scope1"); ApiHost.InitializeAsync().Wait(); - AppHost = new AppHost(IdentityServerHost, ApiHost, clientId); + AppHost = new AppHost(IdentityServerHost, ApiHost, clientId, configureUserTokenManagementOptions: configureUserTokenManagementOptions); AppHost.InitializeAsync().Wait(); } diff --git a/test/Tests/UserTokenManagementTests.cs b/test/Tests/UserTokenManagementTests.cs index 0d797d8..971e237 100644 --- a/test/Tests/UserTokenManagementTests.cs +++ b/test/Tests/UserTokenManagementTests.cs @@ -216,7 +216,7 @@ public async Task Short_token_lifetime_should_trigger_refresh() .WithFormData("refresh_token", "initial_refresh_token") .Respond("application/json", JsonSerializer.Serialize(refreshTokenResponse)); - // short token lifetime should trigger refresh on 2st use + // short token lifetime should trigger refresh on 2nd use var refreshTokenResponse2 = new { access_token = "refreshed2_access_token", @@ -349,4 +349,4 @@ public async Task Resources_get_distinct_tokens() token.RefreshToken.ShouldBe("initial_refresh_token"); token.Expiration.ShouldNotBe(DateTimeOffset.MaxValue); } -} \ No newline at end of file +} diff --git a/test/Tests/UserTokenManagementWithDPoPTests.cs b/test/Tests/UserTokenManagementWithDPoPTests.cs new file mode 100644 index 0000000..056f434 --- /dev/null +++ b/test/Tests/UserTokenManagementWithDPoPTests.cs @@ -0,0 +1,118 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using Duende.AccessTokenManagement.OpenIdConnect; +using IdentityModel; +using Microsoft.IdentityModel.Tokens; +using RichardSzalay.MockHttp; + +namespace Duende.AccessTokenManagement.Tests; + +public class UserTokenManagementWithDPoPTests : IntegrationTestBase +{ + // (An example jwk from RFC7517) + const string _privateJWK = "{\"kty\":\"RSA\",\"n\":\"0vx7agoebGcQSuuPiLJXZptN9nndrQmbXEps2aiAFbWhM78LhWx4cbbfAAtVT86zwu1RK7aPFFxuhDR1L6tSoc_BJECPebWKRXjBZCiFV4n3oknjhMstn64tZ_2W-5JsGY4Hc5n9yBXArwl93lqt7_RN5w6Cf0h4QyQ5v-65YGjQR0_FDW2QvzqY368QQMicAtaSqzs8KJZgnYb9c7d0zgdAZHzu6qMQvRL5hajrn1n91CbOpbISD08qNLyrdkt-bFTWhAI4vMQFh6WeZu0fM4lFd2NcRwr3XPksINHaQ-G_xBniIqbw0Ls1jF44-csFCur-kEgU8awapJzKnqDKgw\",\"e\":\"AQAB\",\"d\":\"X4cTteJY_gn4FYPsXB8rdXix5vwsg1FLN5E3EaG6RJoVH-HLLKD9M7dx5oo7GURknchnrRweUkC7hT5fJLM0WbFAKNLWY2vv7B6NqXSzUvxT0_YSfqijwp3RTzlBaCxWp4doFk5N2o8Gy_nHNKroADIkJ46pRUohsXywbReAdYaMwFs9tv8d_cPVY3i07a3t8MN6TNwm0dSawm9v47UiCl3Sk5ZiG7xojPLu4sbg1U2jx4IBTNBznbJSzFHK66jT8bgkuqsk0GjskDJk19Z4qwjwbsnn4j2WBii3RL-Us2lGVkY8fkFzme1z0HbIkfz0Y6mqnOYtqc0X4jfcKoAC8Q\",\"p\":\"83i-7IvMGXoMXCskv73TKr8637FiO7Z27zv8oj6pbWUQyLPQBQxtPVnwD20R-60eTDmD2ujnMt5PoqMrm8RfmNhVWDtjjMmCMjOpSXicFHj7XOuVIYQyqVWlWEh6dN36GVZYk93N8Bc9vY41xy8B9RzzOGVQzXvNEvn7O0nVbfs\",\"q\":\"3dfOR9cuYq-0S-mkFLzgItgMEfFzB2q3hWehMuG0oCuqnb3vobLyumqjVZQO1dIrdwgTnCdpYzBcOfW5r370AFXjiWft_NGEiovonizhKpo9VVS78TzFgxkIdrecRezsZ-1kYd_s1qDbxtkDEgfAITAG9LUnADun4vIcb6yelxk\",\"dp\":\"G4sPXkc6Ya9y8oJW9_ILj4xuppu0lzi_H7VTkS8xj5SdX3coE0oimYwxIi2emTAue0UOa5dpgFGyBJ4c8tQ2VF402XRugKDTP8akYhFo5tAA77Qe_NmtuYZc3C3m3I24G2GvR5sSDxUyAN2zq8Lfn9EUms6rY3Ob8YeiKkTiBj0\",\"dq\":\"s9lAH9fggBsoFR8Oac2R_E2gw282rT2kGOAhvIllETE1efrA6huUUvMfBcMpn8lqeW6vzznYY5SSQF7pMdC_agI3nG8Ibp1BUb0JUiraRNqUfLhcQb_d9GF4Dh7e74WbRsobRonujTYN1xCaP6TO61jvWrX-L18txXw494Q_cgk\",\"qi\":\"GyM_p6JrXySiz1toFgKbWV-JdI3jQ4ypu9rbMWx3rQJBfmt0FoYzgUIZEVFEcOqwemRN81zoDAaa-Bk0KWNGDjJHZDdDmFhW3AN7lI-puxk_mHZGJ11rxyR8O55XLSe3SPmRfKwZI6yU24ZxvQKFYItdldUKGzO6Ia6zTKhAVRU\",\"alg\":\"RS256\",\"kid\":\"2011-04-29\"}"; + + public UserTokenManagementWithDPoPTests() : base("dpop", opt => + { + opt.DPoPJsonWebKey = _privateJWK; + }){} + + [Fact] + public async Task dpop_jtk_is_attached_to_authorize_requests() + { + await AppHost.InitializeAsync(); + await AppHost.LoginAsync("alice", verifyDpopThumbprintSent: true); + } + + [Fact] + public async Task dpop_token_refresh_should_succeed() + { + await AppHost.InitializeAsync(); + await AppHost.LoginAsync("alice"); + + // The DPoP proof token is valid for 1 second, and that validity is checked with the server nonce. + // We have to wait 2 seconds to make sure our previous (from the initial login) nonce is no longer + // valid. Ideally we would verify that we actually retried, but in this test we aren't mocking + // the http client so there isn't an obvious way to do that. However, the next test + // (dpop_nonce_is_respected_during_code_exchange) does exactly that. + Thread.Sleep(2000); + + // This API call should trigger a refresh, and that refresh request must use a nonce from the server (because the client is configured that way) + var response = await AppHost.BrowserClient.GetAsync(AppHost.Url("/user_token")); + var token = await response.Content.ReadFromJsonAsync(); + + token.ShouldNotBeNull(); + token.IsError.ShouldBeFalse(); + token.AccessTokenType.ShouldBe("DPoP"); + } + + [Fact] + public async Task dpop_nonce_is_respected_during_code_exchange() + { + var mockHttp = new MockHttpMessageHandler(BackendDefinitionBehavior.Always); + AppHost.IdentityServerHttpHandler = mockHttp; + + // Initial login request + var initialTokenResponse = new + { + id_token = IdentityServerHost.CreateIdToken("1", "dpop"), + access_token = "initial_access_token", + expires_in = 10, + refresh_token = "initial_refresh_token", + }; + mockHttp.When("/connect/token") + .WithFormData("grant_type", "authorization_code") + .Respond("application/json", JsonSerializer.Serialize(initialTokenResponse)); + + // First refresh token request - no nonce + var nonceResponse = new + { + error = "invalid_dpop_proof", + error_description = "Invalid 'nonce' value.", + }; + var nonce = "server-provided-nonce"; + mockHttp.Expect("/connect/token") + .WithFormData("grant_type", "refresh_token") + .Respond(HttpStatusCode.BadRequest, headers: new Dictionary + { + { OidcConstants.HttpHeaders.DPoPNonce, nonce } + }, + "application/json", JsonSerializer.Serialize(nonceResponse)); + + // Second refresh request + var tokenResponse = new + { + id_token = IdentityServerHost.CreateIdToken("1", "dpop"), + access_token = "access_token", + token_type = "DPoP", + expires_in = 3600, + refresh_token = "refresh_token", + }; + mockHttp.Expect("/connect/token") + .WithFormData("grant_type", "refresh_token") + .With(request => + { + var dpopProof = request.Headers.GetValues("DPoP").SingleOrDefault(); + var payload = dpopProof?.Split('.')[1]; + var decodedPayload = Base64UrlEncoder.Decode(payload); + return decodedPayload.Contains($"\"nonce\":\"{nonce}\""); + }) + .Respond("application/json", JsonSerializer.Serialize(tokenResponse)); + + + await AppHost.InitializeAsync(); + await AppHost.LoginAsync("alice"); + + // This API call triggers a refresh + var response = await AppHost.BrowserClient.GetAsync(AppHost.Url("/user_token")); + var token = await response.Content.ReadFromJsonAsync(); + token.ShouldNotBeNull(); + token.IsError.ShouldBeFalse(); + token.AccessTokenType.ShouldBe("DPoP"); + mockHttp.VerifyNoOutstandingExpectation(); + } +} \ No newline at end of file From ad953290d70fcc1d494c6c9084599e02fc16ed32 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 3 Apr 2024 20:05:26 -0500 Subject: [PATCH 4/4] Add a title to the sample console --- samples/Web/Program.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/samples/Web/Program.cs b/samples/Web/Program.cs index ca4b55a..2f53d26 100644 --- a/samples/Web/Program.cs +++ b/samples/Web/Program.cs @@ -14,6 +14,8 @@ Log.Information("Host.Main Starting up"); +Console.Title = "Web (Sample)"; + try { var builder = WebApplication.CreateBuilder(args);