Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Token lifetime handling #118

Closed
WolfspiritM opened this issue Mar 14, 2017 · 16 comments
Closed

Token lifetime handling #118

WolfspiritM opened this issue Mar 14, 2017 · 16 comments

Comments

@WolfspiritM
Copy link

Thanks for this great example repository!

I've set up a project similar to this one.
I have an MVC Application calling a API microservice.

I tried to use nearly the same settings as in this example and it works quiet well.

I can log in and then call the API Services via HttpClient without issues.
However after running the app for some time until the identityserver token expires the frontend still keeps me signed in but the Controller calling the API Service ends up with 500 as the api request returns a 401 not authorized.

What is the best way to handle that?
Maybe I missed something while trying to replicate this example, but my OpenIdConnectOptions, CookieAuthenticationOptions and the IdentityServerAuthenticationOptions are identical.
Is it possible there is still an issue with that?

Is setting "UseTokenLifetime" for OpenIdConnectOptions the best option here?

@CESARDELATORRE
Copy link
Collaborator

CESARDELATORRE commented Mar 14, 2017

Agreed. There a few scenarios where the authentication system we have in place needs to be fixed. It happens something similar if the IdentityServer container was re-started.
It is indeed related to specific IdentityServer4 configuration and usage.
We'll keep this issue open and will research it, eventually.
But, due to many other priorities with other features we're developing, we cannot research it in the short term.
We'll track it down, eventually.
If you want to research this issue with IdentityServer4 folks (as this is not an MSFT asset, actually) https://github.com/IdentityServer/IdentityServer4 , and improve it, it would be great if you do a pull request with the improvements.
Thanks for the feedback! :)

@WolfspiritM
Copy link
Author

Thanks!

I noticed that the same thing happens when the IdentityServer container is restarted.
The reason for that seems to be the "AddTemporarySigningCredential()" for the IdentityServer which creates new Signatures on every start causing the old tokens to be invalid as they can't be validated.

So I think the best place would be the Database to store currently valid signing Credentials in production?!

However: I might be wrong but to me it seems like UseOpenIdConnectAuthentication is not validating the token anymore after the initial Cookie creation and I'm not sure what the best approach is to get this working. I'd expect the Controller to redirect me back to Identity Server if the token is invalid or expired.

I will open an issue in the Samples repository of IdentityServer4 as it seems like their example is also doing nothing different.

@eiximenis
Copy link
Contributor

Hi @WolfspiritM
Yes, we are currently using temporary signing credentials for IdSvr. In a real scenario you must use a signing certificate installed on IdSvr machine. Refer to http://docs.identityserver.io/en/release/topics/crypto.html#refcrypto for more information on how setup IdSvr to use a certificate.

Usually dealing with a invalid/expired token is handled at application level. Authentication on your app is set by the cookie, so once you have the cookie you are authenticated from the point of view of your application. Token you have is for authenticating you to some other external scopes (another APIs/microservices used). Note than in a pure SPA application maybe you have only one request to the "web server", the first one. Other requests could be to other servers (other APIs or other microservices), so validating token in a middleware won't help you in these scenarios.
When you have the token, you can know its expiration date and if it is about to expire you can use the refresh token to get a new fresh token. But this need to be handled from the client side. Another option is not to deal with expiration date, try to call service and if 401 is returned use refresh token and retry the call again. It's up to you how you want to deal with possible expired tokens.

Check out at http://docs.identityserver.io/en/release/topics/grant_types.html (Refresh Tokens section) for more info.

@CESARDELATORRE
Copy link
Collaborator

Btw, when migrating from Indentity Server 4 RC3 to its release version, issue #116 we should also improve the token's timeouts and life management so it doesn't throw errors in special cases discussed in this thread.

@CESARDELATORRE
Copy link
Collaborator

CESARDELATORRE commented Mar 25, 2017

Move to Priority 1 issue to be fixed as it is very common to get these errors when working with the app or debugging starting the app multiple times and if the browser was already opened before, you get to this issue. Let's fix this in the next "fixing issues focus day".

It is very common to get the following error and you need to close the browser, all the tabs, and start over..:

An unhandled exception occurred while processing the request.
HttpRequestException: Response status code does not indicate success: 401 (Unauthorized).
System.Net.Http.HttpResponseMessage.EnsureSuccessStatusCode()

dsrodenas added a commit that referenced this issue Mar 29, 2017
@dsrodenas
Copy link
Collaborator

Fixed in commit ba71b19

I have added a certificate for the Identity Server. When resetting the server the credentials are not lost and the tokens are still valid. The certificate is only for test applications, in a real environemnt we should create one in the proper way. But for download, compile and run is the best option.

@Franklin89
Copy link

@dsrodenas This only fixes one issue of token life time management. It will only make sure that after the identity server restarts the tokens are still valid. But what if the token really does expire? By that i mean the access_token used to call the api's from the controller in the mvc frontend for example.

public async Task<Basket> GetBasket(ApplicationUser user)
        {
            var context = _httpContextAccesor.HttpContext;
            var token = await context.Authentication.GetTokenAsync("access_token");

            _apiClient.Inst.DefaultRequestHeaders.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", token);

            ...
        }

The call above would then still fail. I am working on a small project where I have that exact problem and I solved it by using the refresh_token to aquire a new access_token, as suggested by @eiximenis further up. Would be nice to see something like that in the sample.

var client = await CreateSecureClient();
var response = await client.GetAsync(url);

if (response.StatusCode == System.Net.HttpStatusCode.Unauthorized)
{
    // renew tokens and then try again
    await RenewTokens();
    ...Retry...
}

response.EnsureSuccessStatusCode();
dataString = await response.Content.ReadAsStringAsync();

@dsrodenas
Copy link
Collaborator

@Franklin89 you are completely right. The intention of the fix was to solve only the reset IdSvr issue, which was very annoying when debugging and a bit urgent to be solved. We are adding functionality and solving issues by priority, and refresh token it is not so urgent by the moment. Pull requests are more than welcome if you want to participate :)

@Franklin89
Copy link

@dsrodenas will see what i can do...

@WolfspiritM
Copy link
Author

WolfspiritM commented Mar 29, 2017

Would be nice to see an example for that.
I tried to implement it but I'm a bit struggling with how to do it "right".
Sure...it's possible to use a refresh token to get a new access token but that would mean the application needs to request the offline access scope afaik.

Won't be a big deal but I'm not sure if that's the best way.

I noticed that for normal GET requests it seems to work to just redirect the 401 to the browser (exit the Controller with a this.Unauthorized()).
The Middleware then seems to send the user back to the identityserver and as the cookie for the identityserver is still valid the user will be redirected back to the application. This looks like a more "general" way how to deal with something like that but it would prevent a POST request from being send again as far as I can see. So in some cases it would cause data to get lost if the user submits something.

@andrelmp
Copy link
Contributor

andrelmp commented Apr 4, 2017

i will try to do some work on this, it's really not a big deal

@lleslie84
Copy link

I encountered the same issue. Sadly I wasn't able to get the github IdentityService4 solution to compile and I feel a little lazy right now. However below is a copy of the solution I used to resolved the issue. I'll try to create a pull request later

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Threading.Tasks;
using IdentityModel.Client;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Authentication;
using Microsoft.AspNetCore.Http.Features.Authentication;

namespace GoLib.Common.Extensions {
public static class AuthenticationManagerExtensions {

  public static async Task<bool> RenewTokens(this HttpContext context, string identityUrl, string client, string secret, string authenticationScheme, bool automaticallyIssueChallenge = false) {
     //Step 1: Get the refresh token so that we can use it later to retrieve the access token
     var refreshToken = await context.Authentication.GetTokenAsync("refresh_token");

     if (string.IsNullOrWhiteSpace(refreshToken)) {
        return await AuthenticationFailed(context, automaticallyIssueChallenge);
     }

     //Step 2: Retrieve the authenticated users token info and user info
     var authContext = new AuthenticateContext(authenticationScheme);
     await context.Authentication.AuthenticateAsync(authContext);

     if (authContext.Principal == null || authContext.Properties == null) {
        return await AuthenticationFailed(context, automaticallyIssueChallenge);
     }

     var user = authContext.Principal;
     var authProperties = new AuthenticationProperties(authContext.Properties);

     // Deny anonymous request beyond this point.
     if (user == null || !user.Identities.Any(identity => identity.IsAuthenticated)) {
        return await AuthenticationFailed(context, automaticallyIssueChallenge);
     }

     //Step 3: Request for access token
     var pairs = new Dictionary<string, string>()  {
        { "client_id", client },
        { "client_secret", secret },
        { "grant_type", "refresh_token" },
        { "refresh_token", refreshToken }
     };

     var disco = await DiscoveryClient.GetAsync(identityUrl);
     var tokenClient = new TokenClient(disco.TokenEndpoint, client, secret);
     var refreshResponse = await tokenClient.RequestAsync(pairs, context.RequestAborted);

     var httpMessage = new HttpResponseMessage(refreshResponse.HttpStatusCode);
     if (httpMessage.IsSuccessStatusCode) {
        // Persist the new acess token
        authProperties.UpdateTokenValue("access_token", refreshResponse.AccessToken);
        refreshToken = refreshResponse.RefreshToken;
        if (!string.IsNullOrEmpty(refreshToken)) {
           authProperties.UpdateTokenValue("refresh_token", refreshToken);
        }

        if (refreshResponse.ExpiresIn > 0) {
           var expiresAt = DateTimeOffset.UtcNow + TimeSpan.FromSeconds(refreshResponse.ExpiresIn);
           authProperties.UpdateTokenValue("expires_at", expiresAt.ToString("o", CultureInfo.InvariantCulture));
        }

        await context.Authentication.SignInAsync(authenticationScheme, user, authProperties);
        return true;
     }

     return false;
  }

  private static async Task<bool> AuthenticationFailed(HttpContext context, bool automaticallyIssueChallenge) {
     if (automaticallyIssueChallenge) {
        // The cookie middleware will handle this and redirect to /login
        await context.Authentication.ChallengeAsync();
        return false;
     }

     throw new InvalidOperationException("The request is not authenticated.");
  }

}
}

@mvelosop
Copy link
Collaborator

Closed as it was included in the Roadmap (vNext).

@CESARDELATORRE
Copy link
Collaborator

I'm re-opening this issue as we need to fix it when possible.
Basically, after some time (let's say 1h) after you logged in and authenticated in the app, if you leave the browser open and come back later, when you try to use the app, you will get an error because the token has expired:
Response status code does not indicate success: 401 (Unauthorized)

image

This needs to be fixed, either by setting a longer expiration time or any other approach.

Also, related to this bug, we need to be able to really LOG OUT and LOG IN providing user/pwd again, instead of automatically reusing the original token/cookie... This is however a different issue but related to authentication, too.

ramon-tomas-c added a commit that referenced this issue Nov 14, 2018
onixus74 added a commit to onixus74/eShopOnContainers that referenced this issue Apr 1, 2019
* Updated Microservices eBook for eReaders (.MOBI and .EPUB) to v2.1.02

* Migrate WebMVC app to bootstrap 4

* update swagger nuget to 2.4.0 from 3.0.0

* Removed bower from WebStatus app
Migrated WebStatus app to Bootstrap 4

* Migrated Identity.api to Bootstrap 4

* Fixed issue header not included in requests
Fixed issue httpclient returned response

* MediatorR: Migration Guide 4.x to 5.0

* Fixed broken UI in SPA after migrating to Bootstrap 4

* Delete eShopOnContainers.TestRunner.iOS.csproj.bak

* Fixed issue webmvc signalr not notifying submitting state

* ✔ Typo  fix  "suittable" => "suitable"

* Initial approach

* Updated OrderingIntegrationEvent svc

* Add private setters so deserializing on integration event handler works as expected.

* Send commands for modifying state in IntegrationEventHandlers

* [BUG] When subscribing more than 1 different event handlers for the same event type dotnet-architecture#645

dotnet-architecture#645

* [Bug] Deserialization of IntegrationEvent dotnet-architecture#667

dotnet-architecture#667

* Corrected the grammatical and spelling mistakes on Readme.md

* [BUG] After explicit logout, hit on login, then it is automatically recognized instead of showing the login page dotnet-architecture#626

dotnet-architecture#626

* Token lifetime handling dotnet-architecture#118

dotnet-architecture#118

* Send IntegrationEvents after committing transactions

* Refactoring IntegrationEventLog service

* Fix method name in OnDisconnectAsync

* Improves README

* Added Architecture and presentation decks

* Fixed typo.

* Fixed ordering unit test

* change netcore 2.1 to 2.2 and update nugets

* local k8s docker support

* Updated Ocelot version

* update dockerfiles

* add features api

* standar names and fixed response for swagger apis

* Refactoring Buyer aggregate

* add test to CI

* Fixed hardcoded redis connection in FunctionalTests

* get rid of flatmap-stream

* udpated npm-watch

* update test deploy

* Updated eBook (pdf) to v2.1.03

* Integrate HealthCheck to all eShop services and apps
Integrate WebStatus with Healthcheck

* Updated Helm scripts to include healthcheck url env vars
Updated Webstatus docker-compose to include HealthCheck url env vars

* Updated Webstatus startup
Ignore temp healthchecksdb

* Fixed rxjs version in package.json

* Updated tests

* Migrated to netcore 2.2

* Added serilog

* fix error on  aks deployment

* Fixed missing env variables in marketing.api in k8s scripts
Added Resource path in webstatus UI when deploying with K8S
Added Liveness healthChecks
Updated k8s healthcheck configuration

* fixed merge

* Fixed netcore2.2 CORS issue not allowing wildcard origins

* Updated ReadMe to include .NET 2.2 updates

* Updated eBook to 2.2

* Fixed type in HttpGlobalExceptionFilter

Changed meesage to message

* Fixed a minor Typo in the eBook.

* Update README.md

* Update README.md

* deleted global.json

* Webhooks API: WIP - Initial commit

* Publish eReader Formats

* Updated v2.2 book eReader formats

* WIP Webhooks

* WIP

* comment updated in compose

* healtchecks

* badge for buid status

* added port configuration for liveness/readiness probes in helm charts

* updated helm charts for final probes config on k8s

* webhooks API & client

* Add Serilog and Seq working in Docker

* Add integration event traces

* Remove MachineName from logger configuration, not really useful for eShopOnContainers

* Use structured logging for exceptions

* Add Seq as sink and some integration event traces

* Standarize log message

* new helm charts for webhooks

* Add Seq to Ordering.API

* Expore Program.AppName/AppShortName for logging

* Added traces for UserCheckoutAcceptedIntegrationEvent

* OrderController Recommendation

It's good idea to take advantage of GetOrderByIdQuery and handle by GetCustomerByIdQueryHandler

* Add log traces for integration event handling

* webhooks flow finished. Only missing bug in api that don't show the hooks

* Add logging to CreateOrderCommandHandler

* Remove specific version for Microsoft.AspNetCore.App

* Added ApiGateways to solution mimicking eShopOnContainers-ServicesAndWebApps solution structure

* AllowedCorsOrigins disabled for Xamarin client. Fixes dotnet-architecture#905

* webhooks finished

* forcing granturl to be in same origin as hook url

* typo

typo

* .gitignore site.*.js files, as the are created on opening the Project

* Rename BuyerEntityTYpeConfiguration.cs to BuyerEntityTypeConfiguration.cs

* Added Seq to Identity.API

* Added Seq to Locations.API

* Added Seq to Marketing.API

* Add Seq to Payment.API

* Add Seq to WebStatus

* Add Seq to ApiGatewayAggregators

* Ass Seq to SignalR.Hub

* Add LogContext to IntegrationEventHandlers

* Revert "Add Seq to ApiGatewayAggregators"

This reverts commit 21427eb.

* Adding logger to unit test

* Fix IntegrationEventIdContext logging property name

* Exclude generated files in solution .gtignore

* Add Seq to WebMVC

* Refactor LogContext for IntegrationEvent Handlers to include AppName

* Add log traces for commands

* Add LogContext for behaviour transacction

* Ensure transaction is committed in the correct context, when handling chained or nested commands

* Add Seq to ordering.backgroundtasks

* Add publishing integration events traces

* Fix naming inconsistency

* Include files rebuilt by bundling and minification upon opening VS

* Fix non-structured traces

* Update traces templates

* minor packages update

replace preview apis with production
replace EnableKubernetes with AddApplicationInsightsKubernetesEnricher
replace  loggerFactory.AddAzureWebAppDiagnostics(); with  loggerBuilder.AddAzureWebAppDiagnostics();

* replace  loggerFactory.AddConsole(); with  loggerBuilder.AddConsole();

* General refactoring for documentation

* Standarize ApplicationContext logging property

* runfix

* Fix css 404 on logging in

* Remove unused external logins classes

* Refactor ModelDTOs for a more consistent folder structure

* Add logging of subscription events

* Fix duplicate x-requestid headers

* Fix JSON structure error.

* Fix bad previous fix

* Restored original name

* Updated file names to match class and interface names.

* Rename orderStatusChangedToAwaitingValidationIntegrationEventHandler.cs to OrderStatusChangedToAwaitingValidationIntegrationEventHandler.cs

* Enabling istio on local kubernetes and first pod configuration, needs more work, unstable

* Enabling istio on local kubernetes and first pod configuration, needs more work, unstable

* Added support to istio with nginx-ingress

* Fix errors on las commit

* Added bash version of the Helm deploy script

* Build correct solution

* Fix path error in docker-compose command

* host don't need anymore

* Fix initialization of the use_custom_registry boolean

* Critical vulnerabilities solved

* Update JS packages to new versions without know vulnerabilities;
Add HealthChecks to appsettings so SPA can run from VS using services in containers

* Correctly set the backing field for the use-local-k8s parameter

* Tested on AKS

* Fixed changes on deploy-all.ps1 on AKS

* Fixed documentation errors

* Commented out local configuration, to avoid exception

* Split RabbitMQ channel create and consumer create.

Because when channel create the Subscribe is not register, so the previously stored message may lose. This fix split the channel create and consumer create, when Subscribe registered then call the consumer create function.

* Update HealthChecks UI libraries to properly handle missing services.

* scripts fot catalog

* updated build catalog

* First version with Kibana/ElasticSearch & Logstash, needs refactor

* updated build catalog 2

* new yaml files

* builds readme

* Update azure-pipelines.yml for Azure Pipelines

* Update azure-pipelines.yml for Azure Pipelines

* builds updated

* Finished

* updated builds to use git branch name

* Added Documentation

* Updated readme.md for builds info

* Corrected a Typo

* webhooks.client rebranded

* adding triggers to yaml builds

* Fixed NPM Issues and nuget packages

* Done Updating versions

* Update README add note for EventBus

* Updated pointing to ELK external service for testing and demo purposes

* Updated pointing to ELK external service for testing and demo purposes, documentation update

* Update EventBus notes

* Changed default logstashUrl to null

* Updated Bootstrap to 4.3.1 to fix security issues

* Fixed some security issues with npm audit

* infrastructure build

* Simplify Enumeration and CardType classes as proposed in PR dotnet-architecture#704

* Remove test class for MVC project

* removing uneeded files

* compose for put linux and win in tag

* PS1 for creating multiarch manifests

* fix markdown headers
@mvelosop
Copy link
Collaborator

mvelosop commented May 3, 2019

Closing this issue now as it seems to solved now.

Session cookie is configurable now in WebMVC as well as TokeLifetime in Identity.API

As tested now:

  • When session cookie expires you have to login again

    • If token hasn't expired yet, the login is automatic when clicking login (no need to enter user/passord).
    • If token has expired, user/password are mandatory.
  • When user logs out then user/password are mandatory for next login.

@mvelosop mvelosop closed this as completed May 3, 2019
@thutson
Copy link

thutson commented Jan 18, 2020

Same problem but in the Xamarin space, What's the best practice to handle an expired access token in the Xamarin Forms platform?

The example app saves the refresh token but does nothing with it once the access token has expired (default token lifetime is 1 hour).

I've read a few ideas (longer token lifetime, auto logout after token expire, etc...) but I am not sure what is the best way to handle this.

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

No branches or pull requests

9 participants