search

Azure / data-api-builder

Data API builder provides modern REST and GraphQL endpoints to your Azure Databases and on-prem stores.
https://aka.ms/dab/docs
MIT License
903 stars 182 forks source link

Make X-MS-API-ROLE optional #1240

Open thomasgauvin opened 1 year ago

thomasgauvin commented 1 year ago

DAB currently requires developers to provide 'X-MS-API-ROLE' : 'admin' as a header when making their requests. However, this is not intuitive for new users, and this is inconvenient for all developers. They expect the role to be used to be assumed as the highest priority, especially when they only consider RBAC (for ex, if I have both authenticated & admin, I want DAB to use admin assuming it has higher permissions).

RBAC should not require the specification of 'X-MS-API-ROLE' : 'admin'.

I understand that the explanation provided to me was that, while we can determine the 'most permissive role' for RBAC that the user has (according to the permissions in the config), we cannot determine the most permissive policy in case a policy is applied to a role.

One method I propose is that, if policies apply to the specific query the customer made, we respect the order of the permissions as specified in the config file & match the first policy, with the option to override with the X-MS-API-ROLE header.

This solution would not remove any current functionality, while leaving the X-MS-API-ROLE header reserved for more advanced use cases (policy matching).

yorek commented 1 year ago

I also got the feedback that this aspect of role management should be improved. There are serious security implications here, and by automatically using a role, as it will lead to non-deterministic behaviors, since the authentication is done using an external provider. (Something that was working yesterday, for example, could stop working as someone added me to role "X" today) One option to allow the developer to choose the best approach for their use case, could be to have a configuration setting in the config file, something like role-assignement-rule that can be set to auto or explicit. If set to auto, we'll use the role we find in the claims, as long there is only one (aside the "anonymous" or "authenticated" roles). If there are more than one role, we can use the "first match win policy" as suggested above. If set to explicit the behavior will be like the one we have today. Thoughts?

yorek commented 1 year ago

Adding also @PaoloPia as he provided similar feeback

PaoloPia commented 1 year ago

Hi, I think that the suggested option role-assignement-rule could work. Ideally, the auto behavior should merge all the permissions associated with the role claims of the user, but if that is too complex a "first match win policy" can be fine, too. Personally, I would set it to auto by default and keep the explicit mode for really specific scenarios. Thanks.

itpropro commented 7 months ago

Hey @PaoloPia, did this plan actually land on some roadmap? I had some issues with the role header and I realized it’s really uncomfortable to implement a complex authorization model, as you would need to replicate the whole logic of your permission model on the client in addition to the server which adds a lot of friction.

itpropro commented 6 months ago

Hey @yorek, did you already had some time to discuss this internally? Especially in the combination with SWA, this poses a lot of problems in actual production deployments. If there are some additional information you need to go forward with this, I am happy to help out!

yorek commented 6 months ago

Hi @itpropro. It was discussed but the work in order to do it was postponed as other request had higher priority. Let me add @JerryNixon to this thread so he can follow up in the next months.

itpropro commented 5 months ago

Hi @itpropro. It was discussed but the work in order to do it was postponed as other request had higher priority. Let me add @JerryNixon to this thread so he can follow up in the next months.

Could you add this item to the roadmap, at least in the backlog so it won't get lost? It is quite essential, as the current state doesn't allow for any kind of real world authentication with Static Web Apps. It would already be enough to be able to hand over multiple/all roles that the user currently has to "unblock" most scenarios.

itpropro commented 1 month ago

@yorek any updates on this? This is still blocking the usage if data-api-builder directly as well as in Azure Static Web Apps for any application that needs more than one or two roles.