Skip to content
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

Allow prompt parameter with PAR #1563

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public async Task ConsumeByHashAsync(string referenceValueHash)
using var activity = Tracing.StoreActivitySource.StartActivity("PushedAuthorizationRequestStore.Get");

var par = (await Context.PushedAuthorizationRequests
.AsNoTracking().Where(x => x.ReferenceValueHash == referenceValueHash)
.Where(x => x.ReferenceValueHash == referenceValueHash)
.ToArrayAsync(CancellationTokenProvider.CancellationToken))
.SingleOrDefault(x => x.ReferenceValueHash == referenceValueHash);
var model = par?.ToModel();
Expand All @@ -79,14 +79,28 @@ public async Task ConsumeByHashAsync(string referenceValueHash)
public virtual async Task StoreAsync(Models.PushedAuthorizationRequest par)
{
using var activity = Tracing.StoreActivitySource.StartActivity("PushedAuthorizationStore.Store");

Context.PushedAuthorizationRequests.Add(par.ToEntity());

// If we're already tracking the PAR request, then we will update it.
//
// This is an optimization that allows us to use Store to add or update
// without needing extra queries to determine if we need an add or
// insert. It is relying on the fact that when we are updating, we will
// have previously retrieved the PAR and it will be tracked in the context.
var entry = Context.PushedAuthorizationRequests.Local.FindEntry("ReferenceValueHash", par.ReferenceValueHash);
if(entry?.State is EntityState.Unchanged)
{
entry.CurrentValues.SetValues(par); // Not calling ToEntityHere so that we don't try to overwrite the id
}
// Otherwise, we will add it
else
{
Context.PushedAuthorizationRequests.Add(par.ToEntity());
}

try
{
await Context.SaveChangesAsync(CancellationTokenProvider.CancellationToken);
}
// REVIEW - Is this exception possible, since we don't try to load (and then update) an existing entity?
// I think it isn't, but what happens if we somehow two calls to StoreAsync with the same PAR are made?
catch (DbUpdateConcurrencyException ex)
{
Logger.LogWarning("exception updating {referenceValueHash} pushed authorization in database: {error}", par.ReferenceValueHash, ex.Message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ namespace Duende.IdentityServer.Validation;

public static class ValidatedAuthorizeRequestExtensions
{
/// <summary>
/// Removes the prompt parameter from an authorize request. Use this method
/// when processing an authorize request to indicate that the prompt parameter
/// has been taken into account. Typically the prompt parameter will cause
/// UI to be displayed, such as the login screen, that should only be displayed
/// once.
/// </summary>
// TODO - This method cannot easily interact with the PAR store, because it is an
// extension method. This means that whenever this method is called, the PAR store also
// needs to be updated, with e.g, AuthorizeInteractionResponseGenerator.ProcessPromptInParAsync.
// Ideally, we should introduce a new abstraction that does both, and possibly deprecate
// RemovePrompt in favor of it.
public static void RemovePrompt(this ValidatedAuthorizeRequest request)
{
var suppress = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ public class AuthorizeInteractionResponseGenerator : IAuthorizeInteractionRespon
/// The profile service.
/// </summary>
protected readonly IProfileService Profile;

/// <summary>
/// The pushed authorization service.
/// </summary>
protected readonly IPushedAuthorizationService PushedAuthorization;

/// <summary>
/// The clock
Expand All @@ -54,18 +59,21 @@ public class AuthorizeInteractionResponseGenerator : IAuthorizeInteractionRespon
/// <param name="logger">The logger.</param>
/// <param name="consent">The consent.</param>
/// <param name="profile">The profile.</param>
/// <param name="pushedAuthorization"></param>
public AuthorizeInteractionResponseGenerator(
IdentityServerOptions options,
IClock clock,
ILogger<AuthorizeInteractionResponseGenerator> logger,
IConsentService consent,
IProfileService profile)
IProfileService profile,
IPushedAuthorizationService pushedAuthorization)
{
Options = options;
Clock = clock;
Logger = logger;
Consent = consent;
Profile = profile;
PushedAuthorization = pushedAuthorization;
}

/// <summary>
Expand Down Expand Up @@ -138,7 +146,7 @@ public virtual async Task<InteractionResponse> ProcessInteractionAsync(Validated
/// </summary>
/// <param name="request">The request.</param>
/// <returns></returns>
protected internal virtual Task<InteractionResponse> ProcessCreateAccountAsync(ValidatedAuthorizeRequest request)
protected internal virtual async Task<InteractionResponse> ProcessCreateAccountAsync(ValidatedAuthorizeRequest request)
{
InteractionResponse result;

Expand All @@ -147,6 +155,7 @@ protected internal virtual Task<InteractionResponse> ProcessCreateAccountAsync(V
{
Logger.LogInformation("Showing create account: request contains prompt=create");
request.RemovePrompt();
await ProcessPromptInParAsync(request);
result = new InteractionResponse
{
IsCreateAccount = true
Expand All @@ -157,7 +166,7 @@ protected internal virtual Task<InteractionResponse> ProcessCreateAccountAsync(V
result = new InteractionResponse();
}

return Task.FromResult(result);
return result;
}

/// <summary>
Expand All @@ -177,7 +186,8 @@ protected internal virtual async Task<InteractionResponse> ProcessLoginAsync(Val
// remove prompt so when we redirect back in from login page
// we won't think we need to force a prompt again
request.RemovePrompt();

await ProcessPromptInParAsync(request);

return new InteractionResponse { IsLogin = true };
}

Expand Down Expand Up @@ -285,6 +295,29 @@ protected internal virtual async Task<InteractionResponse> ProcessLoginAsync(Val
return new InteractionResponse();
}

/// <summary>
/// Updates a pushed authorization request in the store to reflect that the
/// prompt parameter has been used.
/// </summary>
// TODO - In a future release, we should add a service that processes prompts to consolidate this code
// and the existing RemovePrompt extension method on ValidatedAuthorizeRequest.
private async Task ProcessPromptInParAsync(ValidatedAuthorizeRequest request)
{
if (request.AuthorizeRequestType == AuthorizeRequestType.AuthorizeWithPushedParameters)
{
var par = new DeserializedPushedAuthorizationRequest
{
ReferenceValue = request.PushedAuthorizationReferenceValue,
ExpiresAtUtc = request.PushedAuthorizationExpiresAtUtc
?? throw new InvalidOperationException("Missing ExpiresAtUtc for pushed authorization request"),
PushedParameters = request.Raw
};

par.PushedParameters.Remove("prompt");
await PushedAuthorization.StoreAsync(par);
}
}

/// <summary>
/// Processes the consent logic.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ private static bool IsParRequestUri(string requestUri)
description: "expired pushed authorization request");
}
}
authorizeRequest.PushedAuthorizationExpiresAtUtc = pushedAuthorizationRequest.ExpiresAtUtc;
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

using Duende.IdentityServer.Extensions;
using IdentityModel;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Security.Claims;
Expand Down Expand Up @@ -260,6 +261,11 @@ public class ValidatedAuthorizeRequest : ValidatedRequest
/// </summary>
public string? PushedAuthorizationReferenceValue { get; set; }

/// <summary>
/// The expiration time of the pushed authorization request, if one was used.
/// </summary>
public DateTime? PushedAuthorizationExpiresAtUtc { get; set; }

/// <summary>
/// Gets or sets a value indicating the context in which authorization
/// validation is occurring (the PAR endpoint or the authorize endpoint with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ public AuthorizeInteractionResponseGeneratorTests()
_clock,
TestLogger.Create<Duende.IdentityServer.ResponseHandling.AuthorizeInteractionResponseGenerator>(),
_mockConsentService,
new MockProfileService());
new MockProfileService(),
null);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ public AuthorizeInteractionResponseGeneratorTests_Consent()
new StubClock(),
TestLogger.Create<Duende.IdentityServer.ResponseHandling.AuthorizeInteractionResponseGenerator>(),
_mockConsent,
_fakeUserService);
_fakeUserService,
null);
}

private static ResourceValidationResult GetValidatedResources(params string[] scopes)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public CustomAuthorizeInteractionResponseGenerator(
IClock clock,
ILogger<Duende.IdentityServer.ResponseHandling.AuthorizeInteractionResponseGenerator> logger,
IConsentService consent,
IProfileService profile) : base(options, clock, logger, consent, profile)
IProfileService profile) : base(options, clock, logger, consent, profile, null)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public AuthorizeInteractionResponseGeneratorTests_Login()
_clock,
TestLogger.Create<Duende.IdentityServer.ResponseHandling.AuthorizeInteractionResponseGenerator>(),
_mockConsentService,
new MockProfileService());
new MockProfileService(), null);
}

[Fact]
Expand Down
Loading