Missing foreign keys in entities

[darianferrer]

I'm developing an admin app for managment of clients, scopes, etc. using angular2 and breezejs in the frontend and asp.net webapi in the backend. I can create a client with it's scopes, secrets and other entities' collections, but breezejs needs the foreign keys to mantain each entities' associations (http://breeze.github.io/doc-js/navigation-properties.html). I think it would be easy to just add the foreign keys as properties in each entity.

[ralfwaters]

I also noticed when clients are deleted that Tokens for these clients are still present in the DB. And when clients with the same client Id are created later on (in my case during automated tests), then sometimes I get an error on the login page. I have not yet found a rule when these left over tokens hurt and when not.

[darianferrer]

Hi @ralfwaters I think if you put this code new TokenCleanup(efOptions).Start(); in your configuration, then tokens are cleared, by default each hour, but you can change that.

[brockallen]

Does the TokenCleanup solve your issue?

[ralfwaters]

Hi, thanks for the idea to use the TokenCleanup, but because the issue happens during automated tests which might run frequently I would need an option to clean the tokens now, not based on a schedule and I did not see this option on the TokenCleanup class.

Unfortunately have I not yet understood when the issue actually happens. It seems sporadic. But sometimes after deleting a client (through the Entity Framework classes) when trying to access the permissions web page I get an error on that page. And the only thing I noticed as odd is that there are tokens in the Tokens table on the DB for a client although I deleted that client.

The log shows this: 2016-07-14 13:12:23.7474 WebApi Diagnostics DEBUG 2524/03/4 [2016-07-14T11:12:23.7554372Z] Level=Info, Kind=Begin, Category='System.Web.Http.Action', Id=d2fd1e2c-a2ed-40bf-b3d3-05cb225ab098, Message='Action='ShowPermissions()'', Operation=ApiControllerActionInvoker.InvokeActionAsync 2016-07-14 13:12:23.7474 WebApi Diagnostics DEBUG 2524/03/4 [2016-07-14T11:12:23.7574380Z] Level=Info, Kind=Begin, Category='System.Web.Http.Action', Id=d2fd1e2c-a2ed-40bf-b3d3-05cb225ab098, Message='Invoking action 'ShowPermissions()'', Operation=ReflectedHttpActionDescriptor.ExecuteAsync 2016-07-14 13:12:23.7664 ClientPermissionsController INFO 2524/03/4 Permissions page requested 2016-07-14 13:12:23.7664 ClientPermissionsController INFO 2524/03/4 Rendering permissions page 2016-07-14 13:12:24.1365 WebApi Diagnostics DEBUG 2524/03/4 [2016-07-14T11:12:24.1235844Z] Level=Error, Kind=End, Category='System.Web.Http.Action', Id=d2fd1e2c-a2ed-40bf-b3d3-05cb225ab098, Operation=ReflectedHttpActionDescriptor.ExecuteAsync, Exception=System.NullReferenceException: Object reference not set to an instance of an object. at IdentityServer3.Core.Models.Token.get_ClientId() in c:\local\identity\server3\Core\source\Core\Models\Token.cs:line 149 at IdentityServer3.Core.Services.Default.TokenMetadataPermissionsStoreAdapter.<LoadAllAsync>b__1(ITokenMetadata token) in c:\local\identity\server3\Core\source\Core\Services\Default\TokenMetadataPermissionsStoreAdapter.cs:line 47 at System.Linq.Enumerable.WhereSelectArrayIterator2.MoveNext() at System.Linq.Buffer1..ctor(IEnumerable1 source) at System.Linq.Enumerable.ToArray[TSource](IEnumerable1 source) at IdentityServer3.Core.Services.Default.TokenMetadataPermissionsStoreAdapter.<LoadAllAsync>d__3.MoveNext() in c:\local\identity\server3\Core\source\Core\Services\Default\TokenMetadataPermissionsStoreAdapter.cs:line 54 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at IdentityServer3.Core.Services.Default.AggregatePermissionsStore.<<LoadAllAsync>b__3>d__10.MoveNext() in c:\local\identity\server3\Core\source\Core\Services\Default\AggregateConsentStore.cs:line 0 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at IdentityServer3.Core.Services.Default.AggregatePermissionsStore.<LoadAllAsync>d__13.MoveNext() in c:\local\identity\server3\Core\source\Core\Services\Default\AggregateConsentStore.cs:line 38 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at IdentityServer3.Core.Services.Default.DefaultClientPermissionsService.<GetClientPermissionsAsync>d__11.MoveNext() in c:\local\identity\server3\Core\source\Core\Services\Default\DefaultClientPermissionsService.cs:line 80 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at IdentityServer3.Core.Endpoints.ClientPermissionsController.<RenderPermissionsPage>d__e.MoveNext() in c:\local\identity\server3\Core\source\Core\Endpoints\ClientPermissionsController.cs:line 135 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at IdentityServer3.Core.Endpoints.ClientPermissionsController.<ShowPermissions>d__0.MoveNext() in c:\local\identity\server3\Core\source\Core\Endpoints\ClientPermissionsController.cs:line 80 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Threading.Tasks.System.Web.Http902394.TaskHelpersExtensions.<CastToObject>d__31.MoveNext() in c:\local\identity\server3\Core\source\Core\Validation\HashedSharedSecretValidator.cs:line 0 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Web.Http.Tracing.ITraceWriterExtensions.d__181.MoveNext() in c:\local\identity\server3\Core\source\Core\Validation\HashedSharedSecretValidator.cs:line 0

[brockallen]

So if this were public you'd be all set?

https://github.com/IdentityServer/IdentityServer3.EntityFramework/blob/master/Source/Core.EntityFramework/TokenCleanup.cs#L95

[ralfwaters]

Hi, With this I would be able to solve my problem for my automated tests. But someone deleting clients through either the Admin UI or some other code will not run the token cleanup code and might then have a similar problem that I run in from time to time in my automated tests. Wouldn't it be a better solution to add the foreign key relation between token.clientid to client.clientid so that when a client is deleted also all tokens for that client are deleted as well? Or is there a need to keep these tokens?

[brockallen]

I suggested it because you said this:

but because the issue happens during automated tests which might run frequently I would need an option to clean the tokens now, not based on a schedule and I did not see this option on the TokenCleanup class.

[brockallen]

As for the parent FKs on the entities, can't you teach breeze to walk the parent object model (e.g. ClientClaim.Client.ClientId property)?

[darianferrer]

Hi @brockallen let me show the breeze scenario that I have:

A form for edit a Client, and a list for each of it's dependent entities (ClientScope, ClientSecret, etc). All these entities are fetched by breeze, and are stored in it's entity manager. Now when I create or modify a dependent entity(ies) if I save, breeze only include in the request the affected entities, and because of these entities do not have a foreign key field, when breeze deserialize from json in the server to it's entity type, there is no client info, so the Client property in these entities is null. I manage to solve this limitation, it's easy, I send always the client id each time a save operation happens, and in the server I fetch the client from database and assign to the deserialized entity, looking the admin sample application in the repo I see you do the same, but it would be better to include the foreign keys, I think there wouldn't be any problem, if I get a chance I might made a pull request with this change.

[ralfwaters]

Hi, I just looked at the TokenCleanup.ClearTokens method and found that it does only look for expired tokens, but does not remove tokens where the clientid is orphaned. Calling this would therefore not help.

[brockallen]

@ralfwaters So then in a unit test can't you just instantiate the DbContext and delete them all manually?

[ralfwaters]

Hi, Yes, I could, but can't this also happen in production? People might be deleting and recreating clients. If the Permission page does then sometimes not work would be a bit annoying. Maybe one step back: I have not understood why the permissions page does sometimes(!) load with an error when there are orphaned tokens. Maybe there is a workaround that could be added somewhere.

[brockallen]

@ralfwaters Ok, I see what you're saying. Well, the design of these 3 pieces were meant to be separate meaning you don't need to use them all together (Clients, Scopes, Operational). For example, some people only use the Client stores, but not the other two.

So as of now, no, they don't have keys across those three boundaries. If you want that, then you'd have to build your own implementation.