search

gazette / core

Build platforms that flexibly mix SQL, batch, and stream processing paradigms
https://gazette.dev
MIT License
718 stars 52 forks source link

Azure AD: fix my dumb mistake when refreshing user delegation credentials #349

Closed jshearer closed 1 year ago

jshearer commented 1 year ago

We must refresh whenever the credential expires before 1 minute from now, not after.

This change is Reviewable

jshearer commented 1 year ago

Now that I think about it, I don't think the 1 minute buffer is actually useful here in the first place: AFAICT, the SAS url generated by SignWithUserDelegation() can exceed the UDC (user delegation credential)'s expiration, so long as it was created before the UDC expires. Since we always call getUserDelegationCredential to get the UDC... I think we can simplify the check to a.udcExp.Before(time.Now()). The only concern is if we're so close to the expiration that it expires in between the check and the call to SignWithUserDelegation()... and if that happens, then we generate one bad URL, re-try, refresh, and get a good url. @jgraettinger thoughts?

jshearer commented 1 year ago

I take back what I said in my comment. According to the docs I found:

The SAS is invalid after the user delegation key expires

jshearer commented 1 year ago

That's correct: Signed URL Exp <= UDC Exp