fido2-net-lib icon indicating copy to clipboard operation
fido2-net-lib copied to clipboard

Use DateTimeOffset

Open vbjay opened this issue 3 years ago • 4 comments

https://github.com/passwordless-lib/fido2-net-lib/blob/f25c7e120b1d6b9ec6a1b7b4ca0310bd6d351ae9/Src/Fido2/DevelopmentInMemoryStore.cs#L75

Storing the full date and timezone allows us to support web farms and region servers and translation to show the user if desired in browser's timezone will just work. I know we can force utc time but with DatetimeOffset, it will automatically translate no matter how it was stored and sent.

vbjay avatar Dec 06 '21 01:12 vbjay

Hello @vbjay and thank you for the proposal. I've used DateTimeOffset with success in other large apps, but if I recall correctly, DateTimeOffset comes with tradeoffs. For example I think storing DateTimeOffset can be a bit cumbersome compared to a normal date? Running into conversion issues in popular ORM's or when storing that information in a sql datebase with type "Datetime". Or am I misstaken?

If we enforce UTC in all our usecases, we would not (?) introduce problems for servers in different timezones and we would not add Offset-related complexity to developers. For developer interested in localising the datetime it would still be possible to convert it outside the library. Correct?

abergs avatar Dec 06 '21 09:12 abergs

Hello @vbjay and thank you for the proposal. I've used DateTimeOffset with success in other large apps, but if I recall correctly, DateTimeOffset comes with tradeoffs. For example I think storing DateTimeOffset can be a bit cumbersome compared to a normal date? Running into conversion issues in popular ORM's or when storing that information in a sql datebase with type "Datetime". Or am I misstaken?

SQL Server natively supports DateTimeOffsets, but other ORM's do not.

If we enforce UTC in all our usecases, we would not (?) introduce problems for servers in different timezones and we would not add Offset-related complexity to developers. For developer interested in localising the datetime it would still be possible to convert it outside the library. Correct?

DateTimeOffsets can be converted to UTC DateTimes with the UtcDateTime property, so it's really a toss-up. If you really want to stick with DateTimes in UTC format, I'd recommend making that explicitly clear by renaming the properties to have a postfix of UTC - e.g. RegDateUTC.

Sora2455 avatar Dec 07 '21 23:12 Sora2455

If you are using EF for stuff it automatically stores it correctly. Example create a model with a datetimeoffset and use sqlite.

@Sora2455 Thanks That is the confusion and issue I am trying to solve. Adding UTC to the end would be another solution to hint to the user to use an UTC date and not just any date but would not handle it automatically. Why I suggested the offset usage. Along with https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/issues/819#issuecomment-531132122 showing it is possible. Yes I know EF is not the only orm but I am showing that it is not just sql server with ef that can support it.

So this library is geared to communication to servers that will most likely cross timezones. Why make it hard? If I get a 12/5/2021 12:30:00 -07:00 date stored in db I can always translate it to utc, other offset... The web browser can take the value and know how to translate to client time automatically like shown below.

image No matter if I use a cloud solution that is region specific and get registered time as EST or PST or... I don't have to do the "oh is this datetime utc or not " work. All I do is get DateTimeOffset.Now and don't care. It will store correctly in the db and will translate correctly everywhere it travels whether api, on server, on client or anywhere else.

Sql server even has AT TIME ZONE https://docs.microsoft.com/en-us/sql/t-sql/queries/at-time-zone-transact-sql

vbjay avatar Dec 12 '21 18:12 vbjay

I'm onboard for DatetimeOffset. Let's make it happen.

abergs avatar Dec 27 '21 22:12 abergs