985007 - Add configurable parameter to (stage/test!) tokenserver to allow client to set token expiry length

Status: RESOLVED FIXED

(Cloud Services Graveyard :: Server: Token, defect)

Description

[Toby Elliott [:telliott]]

Nick was looking for a way to test sending expired tokens to storage servers. It might be good to allow (by configuration) a backdoor to have the client able to specify in a request param "give me a token that expires in 3 seconds"

Comment 1

[Ryan Kelly [:rfkelly]]

Requesting shorter durations sounds safe in general, perhaps we can have an optional "duration" parameter in the request, and we set the token duration to min(requested_duration, default_duration).

Comment 2

[Ryan Kelly [:rfkelly]]

Attachment: ts-client-specified-duration.diff

Here's a quick take on this - you can add "duration=X" in the query string, and you'll get min(duration, server_default).

Frankly this seems safe and useful to enable even in production, but I can hide it behind a flag if you prefer.

Comment 3

[Toby Elliott [:telliott]]

Comment on attachment 8393927 [details] [diff] [review]
ts-client-specified-duration.diff

Review of attachment 8393927 [details] [diff] [review]:
-----------------------------------------------------------------

I wouldn't worry about hiding it. I'm not sure we should document it yet, though, in case we end up pulling it.

Comment 4

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8393927 [details] [diff] [review]
ts-client-specified-duration.diff

Review of attachment 8393927 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

::: tokenserver/tests/test_service.py
@@ +187,5 @@
> +        # But you can't exceed the server's default value.
> +        res = self.app.get('/1.0/aitc/1.0?duration=4000', headers=headers)
> +        self.assertEquals(res.json['duration'], 3600)
> +        # And nonsense values are ignored.
> +        res = self.app.get('/1.0/aitc/1.0?duration=lolwut', headers=headers)

Wouldn't mind getting a 400 bad request back in these cases, but not too important.

Comment 5

[Ryan Kelly [:rfkelly]]

Committed in https://github.com/mozilla-services/tokenserver/commit/88d2a4ef71565b16a6d4fa81ae5e9467fce0f918

Let's keep this bug open while we try it out, and consider whether to make it an official part of the protocol.

Comment 6

[Ryan Kelly [:rfkelly]]

I think what we have is good enough, we can leave it as a testing-only feature and not document it as part of the protocol.

Comment 7

[Rémy Hubscher (:natim)]

I am taking back this bug since I am interested by the feature for Syncto.

We want to be able to configure the surface attack of the credentials we get.
The syncto server will ask new credentials for each transaction that should take less than 5 minutes. We will cache this credentials so it is important for us to reduce the 1 hour duration.

Is it reasonable to use this parameter?

Comment 8

[Ryan Kelly [:rfkelly]]

> Is it reasonable to use this parameter?

Yes, if you need it, I think it's reasonable to use and to make it a more formal (and documented) part of the protocol.