fix: RBAC bypass vulnerabilities in model access

[derekhiggins]

Closes security gaps where RBAC checks could be bypassed:

o Inference router: Added RBAC enforcement in the fallback
path to ensure access control is applied consistently.

o Model listing: Dynamic models fetched via provider_data were returned
without RBAC checks. Added filtering to ensure users only see models
they have permission to access.

Both fixes create temporary ModelWithOwner objects for RBAC validation, maintaining security through consistent access control enforcement.

Closes: #4269

[cdoern]

lgtm, one or two nits.

[bbrowning]

With no owner and empty metadata, does that have an impact on our RBAC policies? Will this cause checks to pass that should otherwise fail? Or checks to fail that should otherwise pass?

[cdoern]

I think this is the temporary ModelWithOwner objects for RBAC validation, meaning is_action_allowed(self.routing_table.policy, "read", temp_model, user): below fails if the user does not have access to the model? so for temp_model the owner doesn't matter, its just for comparison, I think?

[derekhiggins]

yes, its a tmp model for policy check, with no owner (by design)

if auth hasn't been enabled then user==None and is_action_allowed will pass, the default doesn't change all models are still available

if auth is enabled then the default policy allows access

if auth is enabled and using a custom policy its upto the user to make sure they allow access to the model with their policy either by name/resource_id or by using other claims in the token (this is the only behaviour that should be changing in this PR, previously a custom policy wasn't restricting anything.

[bbrowning]

Does this change the default visibility of models in our starter distro and similar setups that don't use a customized RBAC policy? Are all models visible to every user when authentication is disabled? The changes look reasonable as far as enforcement, but it wasn't immediately obvious whether this is also adding additional restrictions in our default case.

[derekhiggins]

Does this change the default visibility of models in our starter distro and similar setups that don't use a customized RBAC policy? Are all models visible to every user when authentication is disabled? The changes look reasonable as far as enforcement, but it wasn't immediately obvious whether this is also adding additional restrictions in our default case.

Yup, if not using Authentication nothing should change, the first check in is_action_allowed returns True if auth isn't enabled (user == None)

[bbrowning]

This looks reasonable and my questions have been addressed. Thanks!

[cdoern]

@Mergifyio backport release-0.3.x

[mergify]

backport release-0.3.x

✅ Backports have been created

• #4285 fix: RBAC bypass vulnerabilities in model access (backport #4270) has been created for branch release-0.3.x but encountered conflicts

[ashwinb]

this reads awkward, maybe we need to think about the RBAC API a bit so this "temp model" creation is not necessary

[derekhiggins]

Yup, creating that seemed a little excessive but I couldn't see any way around it

[mattf]

@derekhiggins we ask providers to return Model, but we always need ModelWithOwner. can we move the ResourceWithOwner up a level in the class hierarcy?

[derekhiggins]

We'd have to look into how difficult it would be but wouldn't be a bad idea, I'd like to add a few simple RBAC policies into the integration auth tests next. Then would be more comfortable with refactors like this.