tiled icon indicating copy to clipboard operation
tiled copied to clipboard

Feat: client driven timeout

Open maffettone opened this issue 1 year ago • 3 comments

Give the client a chance to manually reduce their refresh token lifetime. Example being, I want to auth but know I will only be active for 2 hours before leaving the beamline; this provides a secondary safety mechanism should I forget to logout.

Closes #819

Checklist

  • [x] Add a Changelog entry
  • [x] Add the ticket number which this PR closes to the comment section

  • To see the specific tasks where the Asana app for GitHub is being used, see below:
    • https://app.asana.com/0/0/1209761536371702

maffettone avatar Dec 02 '24 16:12 maffettone

@Kezzsim Would you review and test-drive this?

danielballan avatar Dec 07 '24 00:12 danielballan

This works with a few caveats:

  1. I've had to increase the wait time in the tests in commit 149efe2 because they fail when run together on CI
  2. Using the toy_authentication config with an ipython environment, this is not intuitive to test. Unlike in the Tiled example docs, from_uri prompts the user for username and password synchronously instead of being able to invoke c.login(username, password, refresh_token_max_age) after declaration. There is no way that I know of to pass the refresh_token_max_age to from_uri and test it.
  3. Might it be more useful to also have this set some sort of timeout server side to match? I can still use
curl -H "Content-type: multipart/form-data" \                                  
     -F username=alice \
     -F password=secret \
     -F refresh_token_max_age="1" \                                                
     -X POST \
     http://localhost:8000/api/v1/auth/provider/toy/token

to log in and then

curl -i http://localhost:8000/api/v1/metadata/ \                               
  -H "Authorization: Basic eyJhbGc..."

to get data in perpituity.

Kezzsim avatar Mar 24 '25 20:03 Kezzsim

That makes sense regarding (1). I'm cautious about tests with sleeps in them, but I'll stick to the status quo for now.

For 2, we can definitely just extend the signature of tiled.client.constructors functions. I'm not sure if these are meant to be convenience methods that expose all functionality or just some basic functionality. But an easy fix if it's the right thing to do.

For 3, I think that only works because the access token hasn't yet expired. When that happens, the refresh flow would fail. Are you suggesting we raise an auth error if the refresh max age is less than the access token age (which is set server side)?

maffettone avatar Mar 31 '25 16:03 maffettone