Inconsistent Timezones

[mike-capyh]

Preflight checklist

• [X] I could not find a solution in the existing issues, docs, nor discussions.
• [X] I agree to follow this project's Code of Conduct.
• [X] I have read and am following this repository's Contribution Guidelines.
• [ ] This issue affects my Ory Cloud project.
• [ ] I have joined the Ory Community Slack.
• [ ] I am signed up to the Ory Security Patch Newsletter.

Describe the bug

Kratos sometimes uses UTC and sometimes uses system time for times that probably should be in the same timezone. This can cause immediately-stale flows when system time has a sufficiently negative UTC offset (eg bare-metal installation of Kratos on OS set to "America/Los_Angeles (PDT, -0700)" and might cause unintentionally-long-lived flows when the OS has a positive UTC offset.

An example of this is in the selfservice registration flow, note the Now().UTC() vs naked Now() below:

https://github.com/ory/kratos/blob/master/selfservice/flow/registration/flow.go#L88

func NewFlow(conf *config.Config, exp time.Duration, csrf string, r *http.Request, ft flow.Type) (*Flow, error) {
    now := time.Now().UTC()

https://github.com/ory/kratos/blob/master/selfservice/flow/registration/handler.go#L342

    if ar.ExpiresAt.Before(time.Now()) {

On Linux, timedatectl set-timezone UTC can be used to work around the issue, and timedatectl set-timezone America/Los_Angeles can be used to reproduce the stale registration flow.

Reproducing the bug

1. Set system time to America/Los_Angeles (or any other sufficiently negative UTC offset timezone).
2. Initiate and attempt to complete registration browser flow.
3. Kratos errors with 410 Gone and a message to the effect that the flow is expired. For the example LA timezone, the flow will be about 6 hours expired (if PDT) due to the -7 offset and the apparent 1 hour default expiration time.

Relevant log output

No response

Relevant configuration

No response

Version

v0.10.1 (build commit: ab16580b4326250885b920198b280456eb873a6b)

On which operating system are you observing this issue?

Linux

In which environment are you deploying?

Binary

Additional Context

No response

[brahmlower]

I had a few thoughts about a long term solution to this yesterday. While doing some testing for my other branch, I replaced every instance of time.Now() with time.Now().UTC() (even in cases where it wasn't technically necessary). It did seem to resolve some timezone issues, but it left a few questions:

1. How should accidental usage of the naked time.Now() be prevented in the future? I considered a custom lint, but that might be more energy than it's worth?
2. Should we create a wrapper function in x/time.go that just returns time.Now().UTC() and just use that everywhere instead of always having to call UTC. There are a lot of usages of time.Now, and abstracting our "custom" definition of now (Now+UTC) out might be useful in case it needs to be tweaked again in the future
3. Default values for timestamps in structs (like Identity CreatedAt) are still using system time, and continue to be a problem after changing all usages of time.Now(). I didn't do enough digging, but I'm not sure if those values are being set by the database or by the ORM. But the database columns are timestamp and the cockroach docs say that type defaults to UTC timestamps, making me think it's the ORM that's creating those default values when creating new struct instances

[aeneasr]

I believe this is since fixed.