-
-
Notifications
You must be signed in to change notification settings - Fork 381
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
add documentation for remote_json authenticator #827
base: master
Are you sure you want to change the base?
Conversation
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.
Thank you for providing the docs for changes you'd like to introduce in oathkeeper :) I went through your code PR and have two open questions :)
docs/oathkeeper/pipeline/authn.md
Outdated
- `preserve_path` (boolean, optional - defaults to `false`) - If set to `true`, any path in `service_url` will be preserved instead of replacing the path with the path of the request being checked. | ||
- `extra_from` (string, optional - defaults to `extra`) - A [GJSON Path](https://github.com/tidwall/gjson/blob/master/SYNTAX.md) pointing to the `extra` field. This defaults to `extra`, but it could also be `@this` (for the root element), `session.foo.bar` for `{ "subject": "...", "session": { "foo": {"bar": "whatever"} } }`, and so on. | ||
- `subject_from` (string, optional - defaults to `subject`) - A [GJSON Path](https://github.com/tidwall/gjson/blob/master/SYNTAX.md) pointing to the `subject` field. This defaults to `subject`. Example: `identity.id` for `{ "identity": { "id": "1234" } }`. | ||
- `method` (string, optional) - The method to pass to the authenticator service. If set, the method of the original request is overwritten with the specified method. If not set, the method of the original request is used. |
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.
Open question : does it make sense to use the original request method as a fallback instead of the default method set to POST? For example, let's say we do a DELETE request and have set this authentication in our rule but haven't overridden the config method propoperty.
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 think it makes sense because it would be the only way to pass the original method. Now it is true that usually an authentication server does not perform the authentication based on the request method, but I believe that it would be useful to cover such a case as well.
Another possibility would be to make the default behaviour to send a POST request to the authn server if no method is specified, and have another config flag that enables the users to simply forward the original method as well, which by default is disabled, so one needs to explicitly enable the current behaviour which preserves the method.
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.
Another possibility would be to make the default behaviour to send a POST request to the authn server if no method is specified, and have another config flag that enables the users to simply forward the original method as well, which by default is disabled, so one needs to explicitly enable the current behaviour which preserves the method.
Makes sense to me
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.
Changed the behaviour of the authenticator and updated the docs
docs/oathkeeper/pipeline/authn.md
Outdated
|
||
## `remote_json` | ||
|
||
The `remote_json` authenticator will forward the HTTP request (method, path, headers and body) to an upstream service. If the service returns `200 OK` and body `{ "subject": "...", "extra": {} }`, then the authenticator will set the subject appropriately. |
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.
Could you extend a bit the description to point out that it aims to send both request body & headers? At least this is how I understood from the root issue description and would be nice to give here also a small note about the difference to bearer_token
authenticator which you have pointed in the issue comment :)
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.
Added a note to point out the difference between the remote_json
& bearer_token
authenticators
added the |
upstream is merged |
Adds documentation for the
remote_json
authenticator.Related Issue or Design Document
ory/oathkeeper#841