Skip to content
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

Better reporting when using keto_engine_acp_ory #614

Closed
jdnurmi opened this issue Dec 10, 2020 · 3 comments
Closed

Better reporting when using keto_engine_acp_ory #614

jdnurmi opened this issue Dec 10, 2020 · 3 comments
Labels
stale Feedback from one or more authors is required to proceed.

Comments

@jdnurmi
Copy link

jdnurmi commented Dec 10, 2020

Frequently when using generated policies and roles, it can be difficult when only reading source and configs to see the difference in "user:" and "users:", or more complex formats like "org:foo:bar:baz" and "org:foo:baz:bar". This is made even harder when working with an existing authorization system, keto, and a transition to oathkeeper.

Compounding this, neither oathkeeper nor keto log the authorization variables (resource, subject, action, context) at any log level; While implementing in keto seems made difficult due to the rego implementation (at least as far as my hackery got me), implementing at oathkeeper makes perfect sense to me at least optionally, for both approved and denied requests.

Describe the solution you'd like

I would like both of the following:

  • An ability to log the ACP parameters (and decision result) going outbound so that they can be traced and verified in an audit
  • An ability to customize an error message so that I could return to a user a comprehensible object such as:
    { "error": { "code":403, "message": "{{ .Subject }} is not permitted to call {{ .Action }} on {{.Resource}}", "details": "{{ .Context }}"}}

The first, to better aid my own debugging, and the second to aid debugging by my API consumers, though I understand that both should be optional as in some situations it could expose sensitive data in the first case, and enumeration issues in the second.

Describe alternatives you've considered

My original Keto integration was built into code instead of using keto_engine_acpi_ory; This allowed me to verify policies and roles were working as expected, customize errors, etc. But this adds a lot of code bloat to the application that really should be able to live solely in the Authorization and Authentication sections.

Adding logging to Keto or Oathkeeper both seem viable, but the logging code in Keto seems to be a bit set back at least at the last docker release, and accessing the request data/processing seems deep into rego, a bit more than I'm up for. I've added hacks to OK to make it work for debugging, but it would be nice to have a "blessed" solution from upstream.

A function head from my legacy, but friendlier error generator:

func newForbiddenError(code int, err error, action, resource, subject string, ctx map[string]interface{}) (E models.Error) {
       E = models.NewError(code, err)
        E.Status = "ForbiddenByPolicy"
        E.Reason = "Action rejected by policy"
        E.Message = fmt.Sprintf("Principal %q is not permitted to call %q on %q", subject, action, resource)

In the errors case, inserting ACP data into the AuthenticationSession (or perhaps allowing a distinct AuthorizationSession object for authorization errors), and templating of the errors generated would be nice.

In the logging case, perhaps an option like log.authorization.keto_engine_acp_ory.debug existing to log request context, and the various authorizers supporting deeper logging on demand would be helpful.

I'll file a slightly simpler version of this request in the ory/keto project, but as an implementer/administrator, both would be super helpful in adopting the Ory ecosystem.

Additional context

Add any other context or screenshots about the feature request here.

@aeneasr
Copy link
Member

aeneasr commented Dec 11, 2020

Thank you for these ideas and suggestions, I also think that this needs improvement. Customized error messages is on the roadmap for ORY Oathkeeper next (#441).� I think what we could do here is also use OpenTracing to understand what is going on. It is security best practice to not expose this information to the caller as it aids in advesary osint. What do you think about this?

@jdnurmi
Copy link
Author

jdnurmi commented Dec 17, 2020

I'm not super familiar with OpenTracing, but on a quick glance it seems viable. I think anything that can give insight to the administrators/developers on what's being requested/denied is going to be helpful in setup and troubleshooting.

Thanks for the link to #441, I didn't realize keto_engine_acp was on its way out (even if still usable)

I 100% agree that by default, you wouldn't want to expose this information by default, but being able to configure an error message with decision details would be helpful for my use-case, though there's definitely cases where the security concern would outweigh the convenience/usability question.

Thanks for the great ory ecosystem!

@github-actions
Copy link

I am marking this issue as stale as it has not received any engagement from the community or maintainers in over half a year. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas how you could contribute towards resolving it;
  • open a new issue with updated details and a plan on resolving the issue.

We are cleaning up issues every now and then, primarily to keep the 4000+ issues in our backlog in check and to prevent maintainer burnout. Burnout in open source maintainership is a widespread and serious issue. It can lead to severe personal and health issues as well as enabling catastrophic attack vectors.

Thank you for your understanding and to anyone who participated in the issue! 🙏✌️

If you feel strongly about this issues and have ideas on resolving it, please comment. Otherwise it will be closed in 30 days!

@github-actions github-actions bot added the stale Feedback from one or more authors is required to proceed. label Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Feedback from one or more authors is required to proceed.
Projects
None yet
Development

No branches or pull requests

2 participants