Minor logic issues in EntityFramework DeviceFlowStore
Which version of Duende IdentityServer are you using?
6.0.4
Which version of .NET are you using?
.NET 6
Describe the bug
Something I noticed while trying to use the EntityFramework storage objects as an example so I can create a backing data store in Cosmos DB, both of which are in DeviceFlowStore:
- When the
ToEntitymethod converts aDeviceCodeto aDeviceFlowCodesentity, it does not set theDeviceFlowCodes.SessionIdproperty. I'm not sure if this is intentional or an oversight. - In
UpdateByUserCodeAsyncthe incomingDeviceCodegets converted to an entity but the converted entity isn't really used; instead, it seems like an end-around to avoid calling theIPersistentGrantSerializer.Serializemethod directly. Again, unclear if this is intentional or an oversight.
I don't actually use Entity Framework so while I recognize there's some magic going on with entities getting changed and marked dirty and database contexts determining what changes to save and so on, I'm not sure if that's the reason this stuff is happening.
If it turns out these are oversights, I'd be happy to submit a PR to fix it.
Hi, thanks for opening this issue. Just now had time to investigate.
1: Yep, you're right. Also, Description is not being set either. But re-examining the code, ToModel is only ever used on creation and those values won't be present since that's during a backchannel call from the device client.
2: Yes, also a good observation. This is where the SessionId and Description should be set, and I agree the call to ToModel is unnecessary. So a better approach would be to call the serializer directly. The only concern here is that ToModel is virtual, so this would be a breaking change. I know we have people using device flow, but I don't know if they override this API.
I think I will do a small fix now to update the SessionId and Description. And then for v7.0 re-think the ToModel API.
Looks like ToModel was used elsewhere to, so I just removed the unused call to it and did the serialization call directly.