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

Validate Asp.Net Identity Security Stamp #1203

Closed
MH61Aus opened this issue Apr 5, 2024 · 17 comments
Closed

Validate Asp.Net Identity Security Stamp #1203

MH61Aus opened this issue Apr 5, 2024 · 17 comments

Comments

@MH61Aus
Copy link

MH61Aus commented Apr 5, 2024

Which version of Duende IdentityServer are you using?
7
Which version of .NET are you using?
8
Describe the bug

Question - I've got an implementation using Asp.Net Identity Integration.
Asp.Net Identity uses a SecurityStamp in its store. when changing something like your password, the security stamp is updated. if your session has an old security stamp, it'll be terminated.

I'm not really seeing any support for this in Duende clients. but I feel like its something we'd want.

To Reproduce
In samples repo, open IdentityServer\v7\AspNetIdentity\IdentityServerAspNetIdentity
in the startup on line 34, after DefaultIdentity has been added, add the following line:

services.Configure<SecurityStampValidatorOptions>(o => o.ValidationInterval = TimeSpan.FromSeconds(30));

Login in and navigate to the user profile pages in identity.

Throw a breakpoint on Microsoft.AspNetCore.Identity.SecurityStampValidator.ValidateAsync

Open a second window in incognito - Login with the same user, change password.

In the original window, click any link or part of the profile page. the request will be intercepted, and the validator will run (it might skip the validate check if 30 seconds hasn't passed). The validator will determine that the security stamp is invalid and log the user out.

However, doing anything in the client isn't going to trigger this validation. if instead of clicking around the profile page, you'd clicked around the client instead, you'd just stay logged in, and it there's nothing checking that your security stamp is invalid.

We're looking at doing something like the following on each client:
Add a cookie event handler that hits the User Info endpoint. The SecurityStamp would have to be part of the claims. somehow here we'd have to check if the value has changed, and the cookie handler would sign the user out.

So I guess the quesiton is - is there a recommended way to do this? any gotchas or specific reasons you haven't implemented it already?

related - we never got back channel logout working, but if we could get this working, I figure we could just update the security stamp on logout.

@AndersAbel
Copy link
Member

Asp.Net (both Core and earlier versions) has always relied on encrypted cookies as the default session management mechanism. The advantages is that it requires no state on the server and works seamlessly across different nodes in a web farm. The disadvantage is that without any records on the server side for what sessions are active, there is no easy way to invalidate sessions if needed. The SecurityStamp was added as a way to solve this and works by tapping into the session cookie validation and renewal.

OpenID Connect contains a session management feature relying on iframes that allows a client application to detect when the parent SSO session on the OIDC Provider is ended. With recent security best practices that technique should no longer be used however. The advice currently is to use server side session and OIDC handling for all web clients and rely on refresh tokens for token renewal. We have added features in IdentityServer to handle session lifetime synchronization in these cases. When session lifetime coordination is enabled, any use of a refresh token will renew the corresponding server side session. When the server side session ends, the refresh token is revoked. The client application can then detect that token renewal fails and logout the local session.

These two features were developed independently and their feature set partly overlaps. The security stamp feature of Asp.Net Identity was intended for handling local cookie-based sessions and to avoid the need to set up server side sessions. For an OpenID Provider such as IdentityServer this however not enough. That is why we have implemented server side sessions for these kind of scenarios.

Once server side sessions are available, there is no longer any need to use the security stamp feature. If there is a need to invalidate a session it is much easier to just revoke the session on the server side.

In your scenario, I would recommend:

  • Use Server side sessions on IdentityServer
  • Enable session and refresh token lifetime coordination with CoordinateLifetimeWithUserSession.
  • Add code on your password change page that revokes all existing sessions except the current one when changing password.
  • Possibly enable back channel logout to get immediate logout.

@AndersAbel AndersAbel self-assigned this Apr 9, 2024
@MH61Aus
Copy link
Author

MH61Aus commented Apr 11, 2024

I'm trying to make a start on this, but there's a few concepts I still find confusing.

I'll start by noting that I was looking at Server Side Sessions before, and had an issue logged here #778 but we never really resolved it - we've had them enabled, along with refresh tokens, but we don't have the cookie handler checking for terminated sessions on the clients.

  1. when looking at just Refresh tokens - its talked about how the Access Token should be short lived, whereas the Refresh token should be long lived - but "short" and "long" are pretty vague/relative. The default lifetime of an access token is 1 hour, which is much shorter than the 1 month of a refresh token, but seems extremely generous in situations where a users access may change (and I understand that the end goal is to get the cookie handler working) but for the most part if a user walks away form their machine for more than 5 minutes I'd think we'd want their session to end.
    So what's the best way to handle this? Is setting the AccessTokenLifetime to a couple of minutes an acceptable practice?

  2. I'm confused with SlidingExpiration. If I set AccessTokenLifetime = 60, and SlidingRefreshTokenLifetime = 180 and leave the AbsoluteRefreshTokenLifetime as the default of 30 days its not doing what I thought would happen. What I thought would happen: My access token would expire after 60 seconds, after which if I try to access the API, the refresh token gets another access token for me. But if my access token is expired, and I don't hit try to hit the API for another 3 minutes, then my refresh token will expire and I won't be able to hit the API. but if i were to keep hitting the API every 3 minutes and getting new access tokens, I'd be able to keep using the refresh token for 30 days..... It doesn't appear to work like this at all - have I misunderstood it? or does enabling CoordinateLifetimeWithUserSession override these settings?

  3. Refresh tokens get a new access token every time the access token is expired and I try to hit API. however in one of web projects, the dev team have opted not to use an API. so the access token is really just being used by the client to validate call to its own controllers. the devs set it up with refresh tokens. but from what I can tell a lot of their configuration is redundant - it seems a user can just log in, and leave their session idle, and the access token will expire, and the app doesn't care. what's the best way to handle this use case? if the access token is expired, I'd want to use a refresh token to get a new one, and if it can't, and controller marked my an Authorize attribute should require the user to sign in again.

  4. In my previous ticket about Server side sessions I was really confused by the guidance given, and led to believe that I'd have to set up a sessions table on every client. So I just want a bit of clarity on this again - I have the duende EF stores set up, so I've got an IdentityServer database. When my client hits its CookieEventHandler, I feel like it should be able to just check whether that session is still alive in the IdentityServer database? Am I not understanding something here?

@AndersAbel
Copy link
Member

  1. Yes, the access token lifetime should be set according to your requirements. Many applications want to allow users to stay logged in even if they are inactive for half an hour and then 60 minutes access tokens are fine. My general advice is to keep the lifetime somewhere in the range 5-60 minutes. Going below 5 minutes would require more in depth investigations on how long before expiry that the client libraries fetch a new token.
  2. It should behave the way you describe. If you look in the IdentityServer logs, can you see the calls to the token endpoint to refresh the token? CoordinateLifetimeWithUserSessions doesn't interfer with the refresh token lifetimes. It works the other way around by revoking the refresh token if the session is expired/logged out.
  3. In server side web applications that doesn't call APIs there is as you state no need to use a refresh token flow at all. If you want such an application to become logged out when the IdentityServer session expires you would have to use back-channel logout. You probably also need to implement the refresh token flow just to keep IdentityServer session sliding.
  4. The IdentityServer and each client have their own distinct sessions. The lifetime of those sessions are coordinated using the OpenID Connect Protocol. Each of these clients and the IdentityServer can choose their session storage mechanism independently. A common setup is to use simple cookie based sessions on the clients but do server side sessions on the IdentityServer. How each client and the IdentityServer handles their sessions is an implementation detail to each of them. The data bases/stores are not expected to be shared or reachable. The IdentityServer database should be considered to be a particularly security sensitive part of your setup and should not be accessible by any other applications than the IdentityServer.

@MH61Aus
Copy link
Author

MH61Aus commented Apr 12, 2024

Thanks for the responses. I got the sliding expiration working - what I had done wrong was that I had set the SlidingRefreshTokenLifetime, but I'd never set RefreshTokenExpiration = TokenExpiration.Sliding, meaning the lifetime i set was ignored.

For 3 and 4... so I understand that the clients shouldn't have direct access to the Identity DB, but if the Identity Server is using server side sessions in the SQL Server store, and the client is using cookies, shouldn't there be a way to force it to check if the access token/refresh tokens have expired?

i.e. I was thinking something like this:

public class CookieEventHandler : CookieAuthenticationEvents
{
    public override async Task ValidatePrincipal(CookieValidatePrincipalContext context)
    {
        if (context.Principal.Identity.IsAuthenticated)
        {
            var service = context.HttpContext.RequestServices.GetRequiredService<IUserTokenManagementService>();
            // check if the user has an access token
            var accessToken = await service.GetAccessTokenAsync(context.Principal).ConfigureAwait(false);
            if(accessToken == null)
            {
                    // no access token - user needs to login again
                    context.RejectPrincipal();
                    await context.HttpContext.SignOutAsync();
            }
        }
    }
}

This doesn't work because GetAccessTokenAsync hits the cookie validation, so its just an infinte loop, but hopefully my intent is made clear - basically, if I've got a page that doesn't hit the API but has an [Authorize] attribute, I'd like to check if the user has a valid access token (or gain new one via refresh token), and make them sign in again if they don't. I feel like this should be doable and that I'm just missing something simple.

@AndersAbel
Copy link
Member

That is an interesting approach and it can be done. However as you have found out the helper libraries do call back into the cookie handler so they cannot be used directly.

Before going into details on the solution I want to point out that this can be achieved by implementing single logout (possibly via back channel). Single Logout is a push model for expired/logged out sessions while checking for a valid access token is a pull model.

I think that the easiest way to implement this check in the ValidatePrincipal event is to dig into the authentication properties directly. First check the expires_at value to see if the current access token is valid. If not, I would use IdentityModel directly to call invoke the refresh token flow. If you never use the access token you could even discard it to reduce the size of the session cookie and just update the expires_at and store the new refresh token (if refresh token rotation is enabled).

Storing is done by updating the properties in CookieValidatePrincipalContext.Properties and setting CookieValidatePrincipalContext.ShouldRenew = true

@AndersAbel
Copy link
Member

I added DuendeSoftware/foss#52 as a feature idea to our access token management library.

@MH61Aus
Copy link
Author

MH61Aus commented Apr 12, 2024

Thanks again - I'll take a look at this Monday and get back to you. My other dev is working on Back Channel Logout, but figuring this out is important for me in fully understand the different concepts and seeing what their limitations may be.

I guess my question for now.. continuing with the scenario where I've got a page that doesn't hit an API , but instead hits a local controller with authorize attribute... In this proposed solution, if the auth token is expired and so has the refresh token due to sliding expiration, when we try to get a new token we'd find out we cant and invalidate the session... But if all we do is implement back channel logout and the tokens are expired and we arent hitting an api, what would trigger the back channel logout? I thought backchannel logout would only trigger on user initiated logout, ending a session from session management or when hitting an api with an expired token.

@MH61Aus
Copy link
Author

MH61Aus commented Apr 15, 2024

Still having no luck with this - I was looking at calling the endpoint manually, but when i noticed how much validation the AccessTokenManagement library was doing I was starting to think this wasn't a good idea.
So i started extracting these pieces from the AccessTokenManagement library into private methods on the CookieHandler, and removing the bits that called Authenticate.

this started getting messy as I had my own copies of GetAccessTokenAsync, RefreshUserAccessTokenAsync, GetTokenAsync, GetTokenValue, and a bunch of helper methods,, and I couldn't quite get it to work anyways.

I went back to looking at just calling the library directly and just using refelction so that those methods don't create an infinite loop, i.e.

        if (context.Principal.Identity.IsAuthenticated)
        {
            // check if the user has an access token

            //var userName = context.Principal.FindFirst(JwtClaimTypes.Name)?.Value ??
            //    context.Principal.FindFirst(JwtClaimTypes.Subject)?.Value ?? "unknown";

            var stackTrace = new StackTrace();
            var stackFrames = stackTrace.GetFrames();

            if (stackFrames == null || stackFrames.Length < 3)
            {
                // Not enough frames to determine caller
                return;
            }

            foreach (var frame in stackFrames.Skip(2))
            {
                var callerMethod = frame.GetMethod();
                var callerType = callerMethod?.DeclaringType;

                if (callerType != null && callerType.Namespace != null && callerType.Namespace.StartsWith("Duende.AccessTokenManagement.OpenIdConnect"))
                {
                    return;
                }
            }

            var accessToken = await userTokenManagementService.GetAccessTokenAsync(context.Principal).ConfigureAwait(false);
            //var accessToken = await GetAccessTokenAsync(context.Principal, context).ConfigureAwait(false);
            if (accessToken.IsError)
            {
                // no access token - user needs to login again
                context.RejectPrincipal();
                await context.HttpContext.SignOutAsync();
            }

        }

This works nicely while the token is valid, but when the token needs refreshing, it never returns from UserTokenRequestSynchronization.SynchronizeAsync, and I can't figure out why.

@AndersAbel
Copy link
Member

I don't think that using Duende.AccessTokenManagement as it currently is implemented from the ValidatePrincipal event will work. Using reflection to detect the callstack also feels very much like a hack.

If you want to implement something in the ValidatePrincipal event now I think that you would need to use the lower level IdentityModel primitives to make the refresh token call yourself.

I also opened the mentioned issue in the Duende.AccessTokenManagement repository to discuss if this is a feature we want to add to that library.

@carnahanliam
Copy link

I've managed to get back-channel logout working, and in the process I wanted to test the ExpiredSessionsTriggerBackchannelLogout feature which prompted me to think about the length of our server-side sessions. I noticed that the session length defaulted to 14 days, and so to test the session expiration I set the CookieAuthenticationOptions.ExpireTimeSpan to a shorter length of time and it appears to have triggered a back-channel logout after this time elapsed and the expired sessions were cleaned up.

But that got me thinking about our client application which doesn't use an API, and another client app on which some pages make API calls and others are just marked with the Authorize attribute. Say this app has access token lifetime of 5 minutes, sliding refresh token lifetime of 10 minutes, absolute refresh token lifetime of a few days, and the default authentication ticket lifetime of 14 days. If my access and refresh token lifetimes lapse and I no longer have access to the API, I could still have access to pages which are not API-protected for a few days, which is probably not what we would want. We've discussed here already adding our own cookie validation event handler which we could use in this case to sign the user out if they don't have valid tokens, but it made me wonder:

  • is there an assumption that when using IdentityServer all your clients' pages hit an API and require an access token - and anything different from that requires a more custom solution like the ones discussed here?
  • If not, should one be setting the initial authentication ticket lifetime to be the same length as our access token lifetime?

@MH61Aus
Copy link
Author

MH61Aus commented Apr 17, 2024

I started to call the endpoint manually, so I've ended up with this for now:

public class CookieEventHandler(
    IOptions<UserTokenManagementOptions> options,
    TimeProvider clock,
    IHttpClientFactory httpClientFactory) : CookieAuthenticationEvents
{
    private readonly UserTokenManagementOptions _options = options.Value;

    public override async Task ValidatePrincipal(CookieValidatePrincipalContext context)
    {
        if (context.Principal.Identity.IsAuthenticated)
        {
            var client = httpClientFactory.CreateClient("IDPClient");

            var accessToken = context.Properties.GetTokenValue(OpenIdConnectParameterNames.AccessToken);
            //No access token - user needs to login again
            if (accessToken == null)
            {
                EndSession(context);
                return;
            }

            var utcNow = clock.GetUtcNow();

            var expiresAt = context.Properties.GetTokenValue("expires_at");

            //No expires at - user needs to login again
            if (expiresAt == null)
            {
                EndSession(context);
                return;
            }

            var dtExpires = DateTimeOffset.Parse(expiresAt, CultureInfo.InvariantCulture);

            var dtRefresh = dtExpires.Subtract(_options.RefreshBeforeExpiration);

            if (dtRefresh < utcNow) //|| parameters.ForceRenewal || needsRenewal)
            {
                var discoveryDocumentResponse = await client.GetDiscoveryDocumentAsync();
                if (discoveryDocumentResponse.IsError)
                {
                    throw new Exception(discoveryDocumentResponse.Error);
                }

                var refreshToken = context.Properties.GetTokenValue(OpenIdConnectParameterNames.RefreshToken);

                //No refresh token - user needs to login again
                if (refreshToken == null)
                {
                    EndSession(context);
                    return;
                }

                var accessTokenRevocationResponse = await client
                    .RequestRefreshTokenAsync(new RefreshTokenRequest()
                    {
                        Address = discoveryDocumentResponse.TokenEndpoint,
                        ClientId = "client",
                        ClientSecret = "secret",
                        RefreshToken = refreshToken
                    });

                if (accessTokenRevocationResponse.IsError)
                {
                    EndSession(context);
                }
            }
        }
    }

    private async void EndSession(CookieValidatePrincipalContext context) {
        // no access token - user needs to login again
        context.RejectPrincipal();
        await context.HttpContext.SignOutAsync();
    }

It obviously doesn't have all the checks that access token management has, but I'm unsure if it needs to.
Right now the big issue is that the signout isn't actually working. It actually signs me back in, and I end up with an extra refresh token. is this because my session isn't being killed server side?

If I'm satisfied with the solution @carnahanliam puts in place for back channel I may be able to scrap this - but from what I could see when he was showing me his work in progress today, any pages that didn't hit the API would not extend their session, meaning the expired sessions would trigger back channel logout and the users would lose their work, so I feel i may have to still do this even with back channel logout.

@AndersAbel
Copy link
Member

  • Is there an assumption that when using IdentityServer all your clients' pages hit an API and require an access token - and anything different from that requires a more custom solution like the ones discussed here?

No, there is no assumption, but regularly refreshing access tokens have the side effect to keep the sessions in sync.

  • If not, should one be setting the initial authentication ticket lifetime to be the same length as our access token lifetime?

In most scenarios I would recommend to have the refresh token working for the duration of the application sessions, i.e. the refresh token should not expire before the application session expires.

@MH61Aus Yes, if you are signed right back in through SSO it's probably because your IdentityServer session is still working. If you follow the flow in browser development tools you should be able to see what's happening.

@carnahanliam
Copy link

Things are coming together and starting to make sense to me now. Thank you @AndersAbel!

@MH61Aus
Copy link
Author

MH61Aus commented Apr 23, 2024

I just wanted to continue this, as we still are trying to figure out our solution. We've just left the ticket idle for days as we've gone back through documentation/pluralsight courses/etc as a refresher on all the concepts as we've started second guessing ourselves a little.

I think we're on the right track, but I think there's still a few bits where I get confused, or where the documentation can be confusing.

One doc I've been leaning on here is the Inactivity Timeout doc at https://docs.duendesoftware.com/identityserver/v7/ui/server_side_sessions/inactivity_timeout/ - we're in healthcare, which is why this requirement is pertinent to us.

I think one of the biggest points of confusion for us was the absolute lifetime of the refresh token vs the cookie/session lifetime , which I think we've mostly figured out now after a heap of trial and error.

So to have an inactivity timeout when screen is idle between 5-10 mins, we're finding that things work when with a configuration something like:
Identity Token Lifetime - 5 minutes (i.e. the default)
Access Token Lifetime - 5 minutes
Sliding Refresh Token Lifetime - 10 minutes (so that any activity after 5 mins will require a new access token)
Aspnet Identity Application Cookie ExpireTimeSpan - 10 minutes
Absolute Refresh Token Lifetime - 30 days

From my experiments/understanding this would log the user out if they are idle for 10 mins.

One thing I'm confused about is with the CoordinateLifetimeWithUserSession. I've that enabled along with RemoveExpiredSessions.
What I've noticed is that if I try to do something on the identity server in between the session expiring and the RemoveExpiredSessions job running, the cookie handler on the Identity Server (I assume) will log me out. That will delete the Session but not the Refresh Token.

So then when the job rans and hits GetAndRemoveExpiredSessionsAsyc, there's no expired session for it to find, meaning it doesn't know to delete the refresh token from the store. I don't think this is necessarily a problem since the session is ended and we'll be forcing signout, but I thought I'd mention it.

@AndersAbel AndersAbel reopened this Apr 23, 2024
@MH61Aus
Copy link
Author

MH61Aus commented Apr 23, 2024

Upon a little further investigation, I think the bigger issue than the lingering refresh tokens is that the RemoveExpiredSessions job won't trigger back channel logout in this scenario.

I'm catching all this behaviour in this stack:
image

so basically, the cookie is read, its expired, so the session is removed.
there's nothing in the ServerSideTicketStore that checks for CoordinateLifetimeWithUserSession, so only the session is deleted - the persisted grants remain and backchannel logout doesn't get triggered.

So, I guess this is a bug?

@AndersAbel
Copy link
Member

I will have to investigate this, will get back to you once I've looked into it.

@AndersAbel
Copy link
Member

This one was certainly not trivial, but it looks like there is a bug in IdentityServer.

With "normal" production-level timeouts the expired sessions job will be the one removing the session in almost all cases. In your testing setup with shorter lifetimes the cookie handler detects and handles the timedout session in most cases. It doesn't handle the revocation correctly. It should of course work even with short lifetimes so I've filed a bug at DuendeSoftware/IdentityServer#1552.

I'm closing this issue now and we're tracking the bug fix in the linked issue in the IdentityServer repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants