opentdf / platform

Persistent data centric security that extends owner control wherever data travels
BSD 3-Clause Clear License
19 stars 10 forks source link

Implement access token refresh in the SDK #375

Closed mkleene closed 6 months ago

mkleene commented 8 months ago

There are two possibilities, after a token has expired as defined by expires_in or when a token has been involved in a request that the server says is unauthorized.

Token Expiry

We won't actually need to worry about clock skew (only drift, which is much less of an issue) since expires_in is specified as an interval, not an absolute time. This is what x/oauth2 uses and it seems like the intent of the spec. It's marked as should and if we come across servers that don't provide this value we can add a parameter that allows customers to specify how this should work.

Unauthorized Requests

Has the potential to be more correct (exact expiration) but we have the burden of making sure that every server we use this with replies in a way that we understand.

mkleene commented 8 months ago

@biscoe916 @strantalis @dmihalcik-virtru

strantalis commented 8 months ago

I think along with the first option we can check the exp claim as well in the token. Every token should have this claim. With the first option we could maybe do something clever and proactively refresh in the background by taking the current time and exp to calculate the difference.

Tried to visualize what I am thinking.

image

In my opinion the second option leads to us having to deal with retry logic and potentially masking the wrong failure to the user.

mkleene commented 8 months ago

exp is also optional but yeah, I'd expect we have one of those values and if not, and they are doing a non-interactive use case with a weird IDP we can let them specify something reasonable. The algorithm you've described is pretty much what's in x/oauth2

jrschumacher commented 8 months ago

From a downstream perspective will admins expect to control this via the platform or via the IdP? @cassandrabailey293 @pnancarrow

mkleene commented 8 months ago

From a downstream perspective will admins expect to control this via the platform or via the IdP? @cassandrabailey293 @pnancarrow

It seems like it's a decent fit for inclusion in the config service but I think that we should ignore it for as long as possible since it's recommended by the spec and most reasonable IdPs will send us a value here.

jrschumacher commented 8 months ago

Yea, I'd be onboard with that perspective. As I understand your point, we'd inform customers that they can manage this control via the IdP token expiration?

mkleene commented 8 months ago

Yea, I'd be onboard with that perspective. As I understand your point, we'd inform customers that they can manage this control via the IdP token expiration?

In the case that the IDP provides expires_in or exp then yes, this should all be dealt with by controlling the IdP token expiration.

jrschumacher commented 8 months ago

Mar 21, 2024: Arch hours notes

open to edits

Notes

References

jrschumacher commented 7 months ago

@mkleene is there anything else we need to do here?