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

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

Closed
shilpa-padgaonkar opened this issue Apr 16, 2024 · 13 comments · Fixed by #221
Closed

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

shilpa-padgaonkar opened this issue Apr 16, 2024 · 13 comments · Fixed by #221
Labels
documentation Improvements or additions to documentation

Comments

@shilpa-padgaonkar
Copy link
Collaborator

shilpa-padgaonkar commented Apr 16, 2024

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 shilpa-padgaonkar added the documentation Improvements or additions to documentation label Apr 16, 2024
@shilpa-padgaonkar
Copy link
Collaborator Author

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

@AxelNennker
Copy link
Contributor

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
Copy link
Collaborator Author

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

@diegogonmar
Copy link

diegogonmar commented Apr 17, 2024

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:

  • Current approach in ICM v0.1 indicates that (to cope with the purpose handling need) scope value in OAuth request (CIBA or AuthCode) has to follow this pattern: dpv:<dpvValue>#<technicalParameter>. Being value set to either an endpoint scope (existing in the .yaml currently) or set to an api-name.

  • It seems clear that when value is set to an endpoint scope, that one will be in the yaml. But in any use case, including when is set to an endpoint scope value, when requesting access to the API the scope in the yaml cannot be used aone to cope with purpose handling need.

  • So it’s not so relevant to define an scope matching the api-name also in the yaml. Because anyway you need to build the structure dpv:<dpvValue>#<technicalParameter>

  1. 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:

  • The so-called wildcard scope, which notice is actually a scope giving access to every API endpoint, has no issue with the Least Privilege guideline. Nowhere it is stated that access to all the API has to be granted if not authorized (which will break the least privilege guideline); its the other way round, if you have the permission to access every endpoint in an API, you don’t have to request the AuthServer that you want scope=read-list,write,delete,modify,read-id, instead you request AuthServer you want scope:manage. Many APIs work this way, it makes sense and it's simple. And is not in conflict with having an API Client that can only e.g.: request scope=read but not scope=write.
  • I don’t get what you mean with the mother of all tokens, it’s a token to access every endpoint of an API - and APIs in CAMARA are designed as to provide little limited functionality (we don’t have a single Location API but several, for example)

@shilpa-padgaonkar
Copy link
Collaborator Author

@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
Copy link
Contributor

@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
Copy link

@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
Copy link

@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
Copy link
Collaborator

bigludo7 commented Apr 30, 2024

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
Copy link
Collaborator

hdamker commented May 3, 2024

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.

@diegogonmar
Copy link

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
Copy link

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants