feat: add ExecutableSource credentials #525
Conversation
bshaffer
force-pushed
the
add-executable-credentialsource
branch
from
46fca69
to
024c8ce
Compare
| ) { | ||
| $json = json_decode($outputFileContents, true); | ||
| if (isset($json['expiration_time']) && time() < $json['expiration_time']) { | ||
| return $this->parseTokenFromResponse($outputFileContents); |
There was a problem hiding this comment.
nit: If there is an error here, the messages won't specify that it comes from reading an output file vs running the executable directly right? Might be helpful to differentiate that for the user.
There was a problem hiding this comment.
Also, if the file response is valid Json but unsuccessful what happens here? In that case we want to re-run the executable.
There was a problem hiding this comment.
If there is an error here, the messages won't specify that it comes from reading an output file vs running the executable directly right?
That's correct. It should be easy to add that
if the file response is valid Json but unsuccessful what happens here?
Good question, I believe it throws an error same as if it's the response. Are you suggesting that if the file JSON contains success: false, we should silently retry the executable?
Is there a good document that lists the expected logic for the outputFile, because it's not very intuitive. I followed the NodeJS implementation, although I may have missed some things.
There was a problem hiding this comment.
go/pluggable-auth-design but it doesn't look like that has a great flow diagram. But yes, the expected behavior is that if the output file is parseable (I.e. good JSON) but success:false, then we would re-run the executable instead of just returning the error again
There was a problem hiding this comment.
I have manually run the tests in go/pluggable-auth-sdk-testing and they are now passing
Co-authored-by: aeitzman <12433791+aeitzman@users.noreply.github.com>
…is/google-auth-library-php into add-executable-credentialsource
addresses #523