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.

Sign up for GitHub

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

Jump to bottom

Joe/session issued time #1297

Merged
merged 4 commits into from
May 24, 2023
Merged

Joe/session issued time #1297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d56d312
Add test for server side session issued time bug
josephdecock May 22, 2023
740dfd9
Fix server side session issued time bug
josephdecock May 22, 2023
c780a1f
Remove dead code
josephdecock May 23, 2023
c617cd3
Fix creation and renewal times in ServerSideTicketStore
josephdecock May 23, 2023
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
4 changes: 4 additions & 0 deletions src/IdentityServer/Extensions/AuthenticationTicketExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ public static AuthenticationTicket Deserialize(this ServerSideSession session, I
properties.ExpiresUtc = null;
}

// similar to the expires update above, this is needed so that the ticket will have the
// right issued timestamp when the cookie handler performs its sliding logic
properties.IssuedUtc = new DateTimeOffset(session.Renewed, TimeSpan.Zero);

josephdecock marked this conversation as resolved.
Show resolved Hide resolved
return new AuthenticationTicket(user, properties, ticket.Scheme);
}
catch (Exception ex)
Expand Down
50 changes: 11 additions & 39 deletions src/IdentityServer/Stores/Default/ServerSideTicketStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,24 +177,8 @@ public async Task<IReadOnlyCollection<UserSession>> GetSessionsAsync(SessionFilt

var sessions = await _store.GetSessionsAsync(filter, cancellationToken);

var results = sessions
.Select(x => new { x.Renewed, Ticket = x.Deserialize(_protector, _logger)! })
.Where(x => x != null && x.Ticket != null)
.Select(item => new UserSession
{
SubjectId = item.Ticket.GetSubjectId(),
SessionId = item.Ticket.GetSessionId(),
DisplayName = item.Ticket.GetDisplayName(_options.ServerSideSessions.UserDisplayNameClaimType),
Created = item.Ticket.GetIssued(),
Renewed = item.Renewed,
Expires = item.Ticket.GetExpiration(),
Issuer = item.Ticket.GetIssuer(),
ClientIds = item.Ticket.Properties.GetClientList().ToList().AsReadOnly(),
AuthenticationTicket = item.Ticket
})
.ToArray();
return AsUserSessions(sessions);

return results;
}

/// <inheritdoc />
Expand All @@ -204,22 +188,7 @@ public async Task<QueryResult<UserSession>> QuerySessionsAsync(SessionQuery filt

var results = await _store.QuerySessionsAsync(filter, cancellationToken);

var tickets = results.Results
.Select(x => new { x.Renewed, Ticket = x.Deserialize(_protector, _logger)! })
.Where(x => x != null && x.Ticket != null)
.Select(item => new UserSession
{
SubjectId = item.Ticket.GetSubjectId(),
SessionId = item.Ticket.GetSessionId(),
DisplayName = item.Ticket.GetDisplayName(_options.ServerSideSessions.UserDisplayNameClaimType),
Created = item.Ticket.GetIssued(),
Renewed = item.Renewed,
Expires = item.Ticket.GetExpiration(),
Issuer = item.Ticket.GetIssuer(),
ClientIds = item.Ticket.Properties.GetClientList().ToList().AsReadOnly(),
AuthenticationTicket = item.Ticket
})
.ToArray();
var tickets = AsUserSessions(results.Results);

var result = new QueryResult<UserSession>
{
Expand All @@ -242,23 +211,26 @@ public async Task<IReadOnlyCollection<UserSession>> GetAndRemoveExpiredSessionsA

var sessions = await _store.GetAndRemoveExpiredSessionsAsync(count, cancellationToken);

var results = sessions
.Select(x => new { x.Renewed, Ticket = x.Deserialize(_protector, _logger)! })
return AsUserSessions(sessions);
}

private UserSession[] AsUserSessions(IEnumerable<ServerSideSession> sessions)
{
return sessions
.Select(x => new { x.Created, Ticket = x.Deserialize(_protector, _logger)! })
.Where(x => x != null && x.Ticket != null)
.Select(item => new UserSession
{
SubjectId = item.Ticket.GetSubjectId(),
SessionId = item.Ticket.GetSessionId(),
DisplayName = item.Ticket.GetDisplayName(_options.ServerSideSessions.UserDisplayNameClaimType),
Created = item.Ticket.GetIssued(),
Renewed = item.Renewed,
Created = item.Created,
Renewed = item.Ticket.GetIssued(),
Expires = item.Ticket.GetExpiration(),
Issuer = item.Ticket.GetIssuer(),
ClientIds = item.Ticket.Properties.GetClientList().ToList().AsReadOnly(),
AuthenticationTicket = item.Ticket
})
.ToArray();

return results;
}
}
21 changes: 17 additions & 4 deletions test/IdentityServer.IntegrationTests/Hosting/ServerSideSessionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
using Microsoft.AspNetCore.Session;
using FluentAssertions.Common;
using Duende.IdentityServer;
using Microsoft.AspNetCore.DataProtection;
using Duende.IdentityServer.Extensions;

namespace IntegrationTests.Hosting;

Expand All @@ -34,6 +36,7 @@ public class ServerSideSessionTests
private ISessionManagementService _sessionMgmt;
private IPersistedGrantStore _grantStore;
private IRefreshTokenStore _refreshTokenStore;
private IDataProtector _protector;

private MockServerUrls _urls = new MockServerUrls();

Expand Down Expand Up @@ -122,6 +125,7 @@ public ServerSideSessionTests()
_sessionMgmt = _pipeline.Resolve<ISessionManagementService>();
_grantStore = _pipeline.Resolve<IPersistedGrantStore>();
_refreshTokenStore = _pipeline.Resolve<IRefreshTokenStore>();
_protector = _pipeline.Resolve<IDataProtectionProvider>().CreateProtector("Duende.SessionManagement.ServerSideTicketStore");
}

async Task<bool> IsLoggedIn()
Expand Down Expand Up @@ -542,17 +546,26 @@ public async Task using_refresh_token_should_extend_session()
RedirectUri = "https://client/callback"
});

var expiration1 = (await _sessionStore.GetSessionsAsync(new SessionFilter { SubjectId = "alice" })).Single().Expires.Value;
var ticket1 = (await _sessionStore.GetSessionsAsync(new SessionFilter { SubjectId = "alice" })).Single()
.Deserialize(_protector, null);
var expiration1 = ticket1.GetExpiration();
var issued1 = ticket1.GetIssued();

await Task.Delay(1000);

await _pipeline.BackChannelClient.RequestRefreshTokenAsync(new RefreshTokenRequest {
Address = IdentityServerPipeline.TokenEndpoint,
ClientId = "client",
RefreshToken = tokenResponse.RefreshToken
});

var expiration2 = (await _sessionStore.GetSessionsAsync(new SessionFilter { SubjectId = "alice" })).Single().Expires.Value;

expiration2.Should().BeAfter(expiration1);
var ticket2 = (await _sessionStore.GetSessionsAsync(new SessionFilter { SubjectId = "alice" })).Single()
.Deserialize(_protector, null);
var expiration2 = ticket2.GetExpiration();
var issued2 = ticket2.GetIssued();

issued2.Should().BeAfter(issued1);
expiration2.Value.Should().BeAfter(expiration1.Value);
}

[Fact]
Expand Down