camaraproject / Commonalities

Repository to describe, develop, document and test the common guidelines and assets for CAMARA APIs
Apache License 2.0
9 stars 24 forks source link

Clear documentation on the use of wildcard scope in Camara #184

Closed shilpa-padgaonkar closed 1 month ago

shilpa-padgaonkar commented 3 months ago

Problem description The current API design guidelines in section 11.6 points to CAMARA-API-access-and-user-consent.md doc in ICM WG to follow security guidelines and then goes in detail to describe scope-naming in section 11.6.1.

We do not explicitly document in design guidelines if Camara supports the use of wildcard scope (to represent "all scopes included case") in addition to the usual granular scopes. This is only documented in the applying-purpose-concept-section of the CAMARA-API-access-and-user-consent.md document.

We need to clearly document if we support the use of wildcard scope in Camara and add this in the design guidelines and if there is a decision to support wildcard scope, the naming convention for such a wildcard scope has to be documented in design guidelines. Every subproject needs to also add this within the spec file so that we do not have a situation where some providers support it and some not.

Expected action Get a consensus in commonalities on the support of wildcard scopes in Camara and if we agree to support this, it should be clearly documented in design guidelines and the individual API spec files.

Additional context

shilpa-padgaonkar commented 3 months ago

@bigludo7 @PedroDiez @hdamker @rartych As discussed in the subscription discussion call, created the issue

AxelNennker commented 3 months ago

I think wildcard-scopes are in conflict with security principles Least privilege https://github.com/camaraproject/Commonalities/blob/main/documentation/API-design-guidelines.md#101-api-rest-security

One of the original use cases of oauth2 is the "valet key" - As a user you want to give the valet exactly just enough control over your resources that is needed.

A mother-of-all-access-token is too dangerous. Especially because Camara does not have sender-constraint access tokens.

shilpa-padgaonkar commented 3 months ago

Tagging some of the ICM WG participants @jpengar @Elisabeth-Ericsson @palmerabollo for feedback.

diegogonmar commented 2 months ago

Hi @shilpa-padgaonkar , The information in ICM is the one that the group agreed for release v0.1, including DT approval. Actually ICM is somehow a part of commonalities, separated for operative reasons. So first comment its that we really don’t need to find a consensus of what was approved in ICM, it was already approved. What we may discuss is whether that existing agreement should be reflected or have impact in commonalities guidelines, i.e.: document that the .yamls have to include the scope value referring to all the API endpoints (what we called wildcard, which is probably a little bit misleading by the way).

Focusing into that point, not pretty sure but I’d say that we should not mandate to include the scope referring to all the API endpoints in the yaml. For two reasons:

1)

2) We have the agreement to review the purpose solution, so maybe things will change. In that point we should identify what to reflect in design guide, if any.

Alternatively the discussion could then be whether it is interesting to have a scope shared by every endpoint of each API, and to document that in guidelines. Regarding this, I think it’s an interesting approach but maybe only makes sense in certain APIs. Also we may find scenarios where it is interesting to have a scope shared by a sub-set of endpoints.

Regarding @AxelNennker comments:

shilpa-padgaonkar commented 2 months ago

@diegogonmar There was no intention to undermine any work/agreement from ICM, but to document things clearly in design guidelines and remove any ambiguity. IMHO wildcard scope since documented only in context of the purpose topic in icm document, gave an impression that it was only allowed/agreed in that context. For me personally, it is fine to use such an alias in general.

I would propose to document the wildcard scope and its naming convention in the design guideline. Let's also see what feedback we get from other participants regarding adding it in the API specs.

BTW, if you have a better word for wildcard scope, please feel free to suggest.

Also, if you could explain your below text with some example, would be very helpful

Alternatively, the discussion could then be whether it is interesting to have a scope shared by every endpoint of each API, and to document that in guidelines. Regarding this, I think it’s an interesting approach, but maybe only makes sense in certain APIs. Also we may find scenarios where it is interesting to have a scope shared by a sub-set of endpoints.

AxelNennker commented 2 months ago

@diegogonmar the Least Privilege is in danger because I fear that clients are going to always request all scopes even when they need only one of the scopes.

Elisabeth already mentioned the Android guideline that Android applications should request the permission they need in that moment and explain it then to the user and gracefully handle consent being denied. Google/Android has been there were we are now with designing the Permission/Scope system, and during the evolution of the system arrived at the current guideline and an implementation that app permissions were not granted at application installation time but requested when needed.

Many IT security system went through the same evolution of their permission system. Today it is frowned upon if you remotely login as root. The same with Windows, today an admin uses their personal account and then get admin privileges when they need them. Superuser/admin access still exists but the credentials are usually kept in a safe.

If a Camara client really wants to request all scopes they can do that.

I think it is a good security practice to avoid handing out master keys.

If the Camara API is simple e.g. two scopes and four endpoints, and all endpoints are equally in the sense of privacy and security, then scope = API-Name is harmless. But even for simple APIs one endpoint might reveal too much user info. Especially for simple APIs there appears to be no need for wildcards because the client developer knows which scope is needed for each endpoint. For complex APIs (which Camara does not want anyway "atomic API") the temptation of just requesting the mother-of-all-scopes-access-token with scope=API-Name might be too big to resist for client developers.

I think we should not make it too easy to request too many scopes.

Having said the above, it makes sense to "group" some scopes in the sense that if the client requests one scope the AZ grants two or more. I would not be happy with scope but I think some grouping makes sense.

diegogonmar commented 2 months ago

@diegogonmar There was no intention to undermine any work/agreement from ICM, but to document things clearly in design guidelines and remove any ambiguity. IMHO wildcard scope since documented only in context of the purpose topic in icm document, gave an impression that it was only allowed/agreed in that context. For me personally, it is fine to use such an alias in general.

I would propose to document the wildcard scope and its naming convention in the design guideline. Let's also see what feedback we get from other participants regarding adding it in the API specs.

BTW, if you have a better word for wildcard scope, please feel free to suggest.

Also, if you could explain your below text with some example, would be very helpful

Alternatively, the discussion could then be whether it is interesting to have a scope shared by every endpoint of each API, and to document that in guidelines. Regarding this, I think it’s an interesting approach, but maybe only makes sense in certain APIs. Also we may find scenarios where it is interesting to have a scope shared by a sub-set of endpoints.

Thanks, my comment was expressing that if thinking about purpose, there was no special need to reflect in commonalities as inclusion in .yaml or not of the api-name as scope value was not so relevant with the existing purpose approach.

But, speaking in general, having the rule to 'name' an scope giving access to every endpoint by using the api-name as naming rule, could be fine. Maybe not mention wildcard scope but have a section in commonalities saying that in addition to having an scope protecting an individual operation (endpoint + HTTP verb), an scope protecting a group of endpoints could be defined. And when the scope covers every operation in an api, it should be named using the api-name as convention. I'd not set this as mandatory for every API but only to those that makes sense because operations are intended to be granted together.

Regarding the example you request: in QoD API there are endpoints to handle QoD sessions (read,create,get,delete), then an endpoint to retrieve existing profiles. I'd say that it would make sense to group CRUD operations in a single scope manage-sessions, but maybe retrieve profiles is a different thing. So in this API it makes sense to have an scope to manage-sessions covering every endpoint except the one to retrieve the profiles. Maybe this is not the best example because API is now being splitted :-). Maybe these scenarios are only theoretical and when this happens means that API has to be splitted, so I'd go case by case.

diegogonmar commented 2 months ago

@diegogonmar the Least Privilege is in danger because I fear that clients are going to always request all scopes even when they need only one of the scopes.

Elisabeth already mentioned the Android guideline that Android applications should request the permission they need in that moment and explain it then to the user and gracefully handle consent being denied. Google/Android has been there were we are now with designing the Permission/Scope system, and during the evolution of the system arrived at the current guideline and an implementation that app permissions were not granted at application installation time but requested when needed.

Many IT security system went through the same evolution of their permission system. Today it is frowned upon if you remotely login as root. The same with Windows, today an admin uses their personal account and then get admin privileges when they need them. Superuser/admin access still exists but the credentials are usually kept in a safe.

If a Camara client really wants to request all scopes they can do that.

I think it is a good security practice to avoid handing out master keys.

If the Camara API is simple e.g. two scopes and four endpoints, and all endpoints are equally in the sense of privacy and security, then scope = API-Name is harmless. But even for simple APIs one endpoint might reveal too much user info. Especially for simple APIs there appears to be no need for wildcards because the client developer knows which scope is needed for each endpoint. For complex APIs (which Camara does not want anyway "atomic API") the temptation of just requesting the mother-of-all-scopes-access-token with scope=API-Name might be too big to resist for client developers.

I think we should not make it too easy to request too many scopes.

Having said the above, it makes sense to "group" some scopes in the sense that if the client requests one scope the AZ grants two or more. I would not be happy with scope but I think some grouping makes sense.

Clients will have to request those permissions they can be granted, if they request others those will be rejected. The situation does not change whether we simplify life to developers defining scopes that group access to several endpoints (when it makes sense), or we don't do that.

bigludo7 commented 2 months ago

Hello

As you know we have dedicated yaml to manage subscription request for a set of 'close' event (in sim swap, device location, etc.) The scope is useful there to ask the subscriber if it's ok to let the app the get these notifications.

For example for roaming api we'll have following scope regarding the event subscription - these ones target the line subscriber

device-status-roaming-subscriptions.roaming-status,
device-status-roaming-subscriptions.roaming-on,
device-status-roaming-subscriptions.roaming-off,
device-status-roaming-subscriptions.roaming-change-country

but we have also scope for managing the subscription itself management for DELETE and GET - these ones target the subscription owners

device-status-roaming-subscriptions:read
device-status-roaming-subscriptions:delete

Do you think the wildcard scope is also relevant for notification subscription API? I found this a bit 'odd' to mix all these in one wildcard request.

@diegogonmar @AxelNennker @shilpa-padgaonkar WDYT?

hdamker commented 2 months ago

After reading through the comments I see three points:

1) We discuss here Commonalities v0.4.0 which should fit to the ICM v0.2.0. So we can reconsider the "wildcard scope" defined by the API name.

2) We seem to have a consensus that that an implicit "all" scope for an API would not fit for all APIs, but we shall have it explicit within the API specification if it is applicable for an API.

3) If there is an "all" scope defined for an API, the next question is the naming convention for such scope. The agreement in ICM v0.1 was to interpret the API name as such "all" scope. But that can create the impression that every API will have such scope. And maybe it would be anyway better to have an explicit name which is indicating the intended scope? @diegogonmar brought a good example:

QoD API there are endpoints to handle QoD sessions (read,create,get,delete), then an endpoint to retrieve existing profiles. I'd say that it would make sense to group CRUD operations in a single scope manage-sessions.

After the split of the API into quality-on-demand and qos-profiles, quality-on-demand:manage-sessions could be exactly the "all" scope for the quality-on-demand API. Such an explicit name would make authorization policies much more readable than an implicit "wildcard scope" of quality-on-demand.

diegogonmar commented 2 months ago

Hello

As you know we have dedicated yaml to manage subscription request for a set of 'close' event (in sim swap, device location, etc.) The scope is useful there to ask the subscriber if it's ok to let the app the get these notifications.

For example for roaming api we'll have following scope regarding the event subscription - these ones target the line subscriber

device-status-roaming-subscriptions.roaming-status,
device-status-roaming-subscriptions.roaming-on,
device-status-roaming-subscriptions.roaming-off,
device-status-roaming-subscriptions.roaming-change-country

but we have also scope for managing the subscription itself management for DELETE and GET - these ones target the subscription owners

device-status-roaming-subscriptions:read
device-status-roaming-subscriptions:delete

Do you think the wildcard scope is also relevant for notification subscription API? I found this a bit 'odd' to mix all these in one wildcard request.

@diegogonmar @AxelNennker @shilpa-padgaonkar WDYT?

Yes, for me it would make sense to have a single scope to delete or read any of your owned subscriptions, that could be of different events, depending on the scopes you have to create subscriptions, e.g. roaming-status, roaming-on...

diegogonmar commented 2 months ago

After reading through the comments I see three points:

  1. We discuss here Commonalities v0.4.0 which should fit to the ICM v0.2.0. So we can reconsider the "wildcard scope" defined by the API name.
  2. We seem to have a consensus that that an implicit "all" scope for an API would not fit for all APIs, but we shall have it explicit within the API specification if it is applicable for an API.
  • The main reason is that for some APIs the implications of an "all" scope might be not clearly defined or would include very broad access rights which were never intended to grant together (e.g. if they are used typically by different roles which should not be mixed).
  • The comment from @bigludo7 on subscription is another point which speaks against a general "all" scope across every API.
  1. If there is an "all" scope defined for an API, the next question is the naming convention for such scope. The agreement in ICM v0.1 was to interpret the API name as such "all" scope. But that can create the impression that every API will have such scope. And maybe it would be anyway better to have an explicit name which is indicating the intended scope? @diegogonmar brought a good example:

QoD API there are endpoints to handle QoD sessions (read,create,get,delete), then an endpoint to retrieve existing profiles. I'd say that it would make sense to group CRUD operations in a single scope manage-sessions.

After the split of the API into quality-on-demand and qos-profiles, quality-on-demand:manage-sessions could be exactly the "all" scope for the quality-on-demand API. Such an explicit name would make authorization policies much more readable than an implicit "wildcard scope" of quality-on-demand.

There are two aspects actually, the use of wildcard-scope as defined in ICM (wildcard is not part of .yaml, it's a concept related with purpose handling) and the inclusion of such scope in the .yaml, being discussed here. I suggest waiting for final decision in ICM, as there are some options being discussed for v0.2 and maybe the one using the wildcard-scope (the one in ICM v0.1.0) is not the chosen one. If so, we can just define here the rules and naming for scopes covering more than one endpoint, including scope covering all. I think ICM decision may be taken next week.

AxelNennker commented 2 months ago

I think wildcard scopes aka <API-Name> and purpose are unrelated.

I think what if a API subproject wants to support <API-Name> as a scope then it should define that scope in its yaml file.

paths:
  /retrieve-date:
    post:
      security:
        - openId:
          - sim-swap
        - openId:
          - sim-swap:retrieve-date
...
  /check:
    post:
      security:
        - openId:
          - sim-swap
        - openId:
          - sim-swap:check

A client can now request a authorization code for the scope sim-swap and get access to both endpoints.

I think the above is valid openapi yaml and the meaning is that one of the security objects must be satisfied. The security object is openid in both cases and in the first the required scope is sim-swap and in the second case the required scope is sim-swap:retrieve-date.

This scheme is not limited to <API-Name>. If there is a group of paths that the API subgroup want to group together under one scope then the subgroup can add the alternative security object to those paths.

The OAI spec states:

When a list of Security Requirement Objects is defined on the OpenAPI Object or Operation Object, only one of the Security Requirement Objects in the list needs to be satisfied to authorize the request.

So, maybe Camara does not need wildcard scopes because this can be specified in the API's openapi yaml file.