ncw / swift

Go language interface to Swift / Openstack Object Storage / Rackspace cloud files (golang)
MIT License
313 stars 107 forks source link

Time difference between server and client #166

Closed xaduha closed 2 years ago

xaduha commented 3 years ago

Hi, I've noticed that when I have a wrong timezone on the client (e.g. when booted with live CD in a VM) I get this error

"Response didn't have storage url and auth token"

from https://github.com/ncw/swift/blob/v1/swift.go#L535, but that's not what actually happens there, I checked and compared the responses and if I correct the time everything works just fine, so at the very least the error message isn't accurate if that's intended behavior. Is it though? Is there a requirement that time is synced between client and server? Seems strange. Seems to me that the token or something gets expired right after it gets assigned an expiration date in the same function call.

ncw commented 3 years ago

Is there a requirement that time is synced between client and server? Seems strange. Seems to me that the token or something gets expired right after it gets assigned an expiration date in the same function call.

Are you using oauth? That needs accurate times.

It will depend on the server most likely.

xaduha commented 3 years ago

No, just Keystone v3.

ncw commented 3 years ago

The tokens do have an expiry time in them don't they...

So if your time is wrong then the token will have appeared to have expired instantly.

xaduha commented 3 years ago

Well, the client can check the time difference and if not do something about by setting an offset, then at the very minimum it should say so with a correct error message. Basically it's using two sources of time, server one vs client one and then comparing them. If that seems right to you, then there's indeed a requirement to have synced time.

ncw commented 3 years ago

There is no way the client can know about a time difference from the server just looking at the expires header.

It could read the time returned from the server in the Date header.

So, yes, as it stands there is a requirement when using auth v2 and v3 to have a synced time if the server returns an expiry for the token.

If you'd like to have a go at fixing that requirement then you could patch the auth v2 and auth v3 Expires method to apply an offset.

xaduha commented 3 years ago

If I modify this bit

    if do, ok := c.Auth.(Expireser); ok {
        c.Expires = do.Expires()
    } else {
        c.Expires = time.Time{}
    }

with

    if do, ok := c.Auth.(Expireser); ok {
        c.Expires = do.Expires()
                c.Expires = time.Time{}
    } else {
        c.Expires = time.Time{}
    }

for instance, then it all works just fine. I haven't dived into the code, but it stands to reason that the server should decide what has expired and what hasn't, not the client.

ncw commented 3 years ago

I haven't dived into the code, but it stands to reason that the server should decide what has expired and what hasn't, not the client.

Your change does exactly that. However getting the client to do it is much more efficient and saves the client making lots of requests to have them rejected and have to retry them.

Perhaps an option to ignore expiry times on tokens might be the best way forward?

xaduha commented 3 years ago

I don't know, I just don't think a token should expire just as it was assigned on a first login attempt. Surely you have server time and client time right there? This whole goto thing is questionable.

ncw commented 3 years ago

If you'd like to submit a PR to parse the token time relative to the server time instead of the using the client time then I would merge it.