Open jrschumacher opened 3 days ago
The one thing I have concerns with here is putting public_routes
in the configuration. I feel like that opens up potential for miss configuration.
In the roles
block could we define role: admin
multiple times which gets mapped from different claims?
That would solve this issue #1031
In the roles block could we define role: admin multiple times which gets mapped from different claims?
I like this. I think that's a good solution to #1031.
The one thing I have concerns with here is putting public_routes in the configuration. I feel like that opens up potential for miss configuration.
While misconfiguration is a valid concern, couldn't the WithPublicRoutes
be hijacked by a PEP anyway to short-circuit the casbin policy as it stands today? I am wondering if the potential for issues is greater having four alternate sources of truth (below) than having a single, more powerful source of truth driving everything.
WithPublicRoutes
hook config option func
https://github.com/opentdf/platform/blob/c3828d088a3483b78079cd257b4237291cf7b6f0/service/pkg/server/options.go#L41The one thing I have concerns with here is putting public_routes in the configuration. I feel like that opens up potential for miss configuration.
While misconfiguration is a valid concern, couldn't the
WithPublicRoutes
be hijacked by a PEP anyway to short-circuit the casbin policy as it stands today? I am wondering if the potential for issues is greater having four alternate sources of truth (below) than having a single, more powerful source of truth driving everything.
- the
WithPublicRoutes
hook config option func https://github.com/opentdf/platform/blob/c3828d088a3483b78079cd257b4237291cf7b6f0/service/pkg/server/options.go#L41- casbin defaultPolicy https://github.com/opentdf/platform/blob/c3828d088a3483b78079cd257b4237291cf7b6f0/service/internal/auth/casbin.go#L27
- casbin custom policy in config https://github.com/opentdf/platform/blob/c3828d088a3483b78079cd257b4237291cf7b6f0/opentdf-dev.yaml#L46
- hardcoded allowedPublicEndpoints https://github.com/opentdf/platform/blob/c3828d088a3483b78079cd257b4237291cf7b6f0/service/internal/auth/authn.go#L45
Are you saying a PEP developer could accidentally override the platform's public routes or do it maliciously? Also, it might not be fair to expect someone deploying the platform (who isn’t actively developing it) to know exactly which routes should be public and which shouldn’t.
I think a PEP developer trying to deploy the platform with maliciously overriden public route policy is not something we should necessarily try to control for, but I have had the experience of developing a PEP on the platform, unexpectedly getting a 403 on a route from Casbin, and struggling to find where the route policy is defined (or should defined) within the four places. That said, you're absolutely right that developers will know the APIs and can guard them more effectively than admins.
Casbin is being used to provide AuthZ for our routes. This works well but there is some duplication we need to handle.
This issue is going to capture small tasks to improve this:
Casbin policy could be more readable and declarative instead of comma separated such as: