-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
auth: support deferring authentication to a third party script #132
Conversation
I think I should mention here that I'd first like to get feedback on the proposed idea (and code). If the outcome is positive, I would move forward with documenting how to set up in more depth and provide example code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @pyr this looks awesome! I like the approach and would be happy to merge this feature. I also considered adding something like https://github.com/traefik/yaegi to support custom functionality/plugins but your approach is more generic and would let people write them in e.g. Javascript or Python or something else if they prefer. And the two solutions are not mutually exclusive anyway.
_, err = stdin.Write(requestBytes) | ||
if err != nil { | ||
return err | ||
} | ||
stdin.Close() | ||
outBytes, err := cmd.Output() | ||
if err != nil { | ||
return err | ||
} | ||
if len(outBytes) <= 0 { | ||
return nil | ||
} | ||
var requestUpdates Request | ||
err = json.Unmarshal(outBytes, &requestUpdates) | ||
if err != nil { | ||
return err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not super familiar with this, but do you think there are potential buffer problems with very large requests? I see the examples (e.g. this one) using streaming decoders or goroutines and a cmd.Wait()
to handle this sort of thing. May need to dig into what gotchas there are here before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @danielgtaylor, thanks for the review. The current assumes that for most APIs inputs have somewhat moderate sizes thus optimizes for this.
Most commonly available APIs (excluding S3 which is the odd one out, but doesn't expose OpenAPI so less relevant here) which allow large body inputs do not include the body in their signature mechanisms.
If the above seems sensible to you, I can adapt the patch to allow controlling what the auth mechanism hands over to the sidecar script (allowing to exclude the body when unnecessary).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I adapted as discussed, adding an omitbody
parameter which when preset does not buffer the input body in memory and forwards it down untouched
Some OpenAPI backed APIs implement a custom API signature scheme to provide stronger guarantees against replay attacks than those provided by OAuth. Generally speaking, these APIs tend to either wire a computed signature in an HTTP header or in a query argument in the URL. This patch implements a new `external-tool` auth method which executes a configured program against a JSON representation of the request built by restish. If the program returns any output it is deserialized from JSON, allowing for new headers to be set, or for the URL to be modified. This then allows small script to be used alongside restish to implement any sort of API signature mechanism.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Do you want to merge as-is or also add some documentation to the docs site?
It would be great if this could go in and I will start working on docs |
Some OpenAPI backed APIs implement a custom API signature scheme to provide stronger guarantees against replay attacks than those provided by OAuth.
Generally speaking, these APIs tend to either wire a computed signature in an HTTP header or in a query argument in the URL.
This patch implements a new
external-tool
auth method which executes a configured program against a JSON representation of the request built by restish. If the program returns any output it is deserialized from JSON, allowing for new headers to be set, or for the URL to be modified. This then allows small script to be used alongside restish to implement any sort of API signature mechanism.