camaraproject / IdentityAndConsentManagement

Repository to describe, develop, document and test the Identity And Consent Management for CAMARA APIs
Apache License 2.0
18 stars 30 forks source link

Define a CIBA OpenAPI security scheme #157

Open AxelNennker opened 1 month ago

AxelNennker commented 1 month ago

Problem description In Camara guidelines on how an API is described in OpenAPI are documented in the Commonalities' API guidelines document.

Currently there is no guideline on how an API can express that it supports or requires CIBA.

Possible evolution

ICM should define a common definition of a CIBA security scheme and propose a guideline for API designers to use that security scheme to Commonalities

Suggestion:


An Camara API using CIBA should use the following security scheme definition.

components:
  securitySchemes:
    ciba:
      type: oauth2
      flows:
        x-ciba:
          authorizationUrl: https://az.example.com/bc-authorize
          tokenUrl: https://az.example.com/token
          x-cibaDeliveryModes: ["poll"]

The ciba flow is going to be defined in OpenAPI version 3.2. For now Camara is using the in OpenAPI extension mechanism using the x- prefix because Camara is using OpenAPI version 3.0.3.


Additional context

OpenAPI tools and especially code generators are currently ignoring OpenAPI security. E.g. if you specify the openIdConfigurationUrl or not, the generated Java client code does not change.

Specifying security in OpenAPI is valuable information for developers though and should be integrated into Camara OpenAPI files.

OpenAPI is viewing OIDC as an evolution of OAuth2 and API designers can only specify the openIdConfigurationUrl to an security object of type openIdConnect. As such CIBA is also an evalution of OAuth2 and OpenAPI is going to define the CIBA flow in the security object of type oauth2.

jpengar commented 1 month ago

This issue has been discussed in the past on https://github.com/camaraproject/IdentityAndConsentManagement/issues/57. It was escalated to the TSC and the TSC made a final decision after a very long discussion in the WG. Which is documented in https://github.com/camaraproject/IdentityAndConsentManagement/blob/main/documentation/CAMARA-API-access-and-user-consent.md#camara-api-specification---authorization-and-authentication-common-guidelines

From my point of view, I don't see the point in re-opening this issue after the TSC decision. At least in the short term, until we get some experience from the first estable APIs launched and so on... Then of course, in the mid-term we could iterate if it is needed.

AxelNennker commented 1 month ago

Could you please help me by providing a quote of what was decided in TSC? There is talk about openid configuration and CIBA and harmonizing the openIdConnect security scheme https://github.com/camaraproject/Commonalities/issues/53 https://github.com/camaraproject/IdentityAndConsentManagement/issues/57

jpengar commented 1 month ago

@AxelNennker Here you have PR #93 (Nov last year) where it was included in the WG documentation the final agreement reached by the TSC and the WG participants on the securitySchemes and security of the CAMARA API specification, and the mandatory template for info.description, explaining to API customers why the API specification does not list specific grant types, and how to find out what authorization flows they can use.

That PR was reviewed and approved by you.

Also in issue #57 (fixed by #93) there are references to TSC meeting notes and agreements. Such as https://github.com/camaraproject/IdentityAndConsentManagement/issues/57#issuecomment-1795205396 or https://github.com/camaraproject/IdentityAndConsentManagement/issues/57#issuecomment-1813106499

Agreement to use openId connect discovery clause in the Open API Specification (OAS) and provided API scheme for the API in another form (either in seperate file or as documentation section within the OAS). Ref: https://github.com/camaraproject/Governance/blob/main/documentation/MeetingMinutes/TSC/TSC-2023-10-19-Meeting-Minutes.md

Eventually it was documentation within the OAS as you can see in #93 (info.description template) pointing to a set of authorization flows defined in CAMARA-API-access-and-user-consent.md

https://github.com/camaraproject/Commonalities/issues/53 was closed by #57

AxelNennker commented 1 month ago

During ICM's meeting on May 22th I understood that we keep this issue and add the label ICM-Backlog. I did this.

AxelNennker commented 1 month ago

@jpengar Please detail what was escalated to TSC and which decision was the outcome. Maybe it is time to revisit that decision.

I think CIBA is used in some Camara protocols and there should be a way to express that in OpenAPI.

Axel

ps: Is there a list of those protocols? I would like to discuss with subgroups why they are using with flow.

jpengar commented 1 month ago

During ICM's meeting on May 22th I understood that we keep this issue and add the label ICM-Backlog. I did this.

During the ICM meeting on 22 May, it was pointed out that this very issue has been discussed for weeks in the past (issue #57), eventually escalated to the TSC, which ultimately made a final decision on the security scheme policies to be followed. And it was properly documented in CAMARA-API-access-and-user-consent.md (PR #93). It was also mentioned that this issue shouldn't be discussed againnow again. I gave my personal opinion that I would close this issueissue for now. But I was happy not to re-open it, but to keep it in the backlog "for the record". I've added this to the meeting notes to make it clear.

jpengar commented 1 month ago

@jpengar Please detail what was escalated to TSC and which decision was the outcome. Maybe it is time to revisit that decision.

I think CIBA is used in some Camara protocols and there should be a way to express that in OpenAPI.

Axel

ps: Is there a list of those protocols? I would like to discuss with subgroups why they are using with flow.

This is the opposite of what I understand as not re-opening an issue. I've already given my opinion, which was also discussed in yesterday's meeting, and I have nothing more to say. I will allow other WG participants to express their views as well. And I will forward this directly to the TSC.

It is really time consuming to keep bringing up the same issues, especially when it is something that has been discussed over such a long period of time and when there is a TSC resolution. So there is nothing more to say from my side

@jpengar Please detail what was escalated to the TSC and what the decision was. Perhaps it is time to review that decision.

I've already provided the information in previous comments and pointed you to the issues and PRs where you can find further details.

ps: Is there a list of those protocols? I would like to discuss with subgroups why they are using with flow.

IMHO, this will create more overhead and noise in API subprojects, considering that they were already aware that a securityScheme guideline exists from ICM and they have to apply it for the next meta-release. There are API subprojects that are already using this guideline, and others are on their way, like Quality on Demand: https://github.com/camaraproject/QualityOnDemand/pull/295

eric-murray commented 1 month ago

Discussions, and even decisions, within the TSC do not preclude proposals for alternative "new" ways of doing things being discussed with a working group. It is for the proponent (in this case, @AxelNennker) to make the case for the advantages of their proposal over the currently agreed scheme, and for the working group as a whole to discuss it within the issue itself or at meetings. For sure, it is harder to justify "breaking" changes, but they are not prohibited.

Hopefully the working group can reach a consensus on any given given proposal. But if not, then it can be escalated to the TSC for a decision by anyone who feels their strong arguments are being ignored. So the TSC just provides a mechanism to break such deadlocks if required. The TSC can also intervene if it thinks a working group has overlooked factors that affect other working groups or sub-projects, which is why each working group provides an update at TSC meetings.

Meeting agenda items and time management within meetings are for the working group moderator to manage.

jpengar commented 1 month ago

Discussions, and even decisions, within the TSC do not preclude proposals for alternative "new" ways of doing things being discussed with a working group. It is for the proponent (in this case, @AxelNennker) to make the case for the advantages of their proposal over the currently agreed scheme, and for the working group as a whole to discuss it within the issue itself or at meetings. For sure, it is harder to justify "breaking" changes, but they are not prohibited.

Hopefully the working group can reach a consensus on any given given proposal. But if not, then it can be escalated to the TSC for a decision by anyone who feels their strong arguments are being ignored. So the TSC just provides a mechanism to break such deadlocks if required. The TSC can also intervene if it thinks a working group has overlooked factors that affect other working groups or sub-projects, which is why each working group provides an update at TSC meetings.

I totally agree.

But just one question, if there is a TSC resolution on a particular issue and a particular solution is agreed, how long does that resolution last so that the same issue is not reopened if I do not fully agree with the final TSC resolution? Do we always have to start from scratch? Should at least be considered if the context of the first decision has somehow changed. I'm just giving my point of view and I think that the issue that is being raised here has already been discussed and I don't see any significant change in the context of the first decision to re-open this issue now. And that, of course, is my opinion. Other WG participants can of course express theirs. And if the WG or the TSC decides otherwise, I will accept that.

Of course, any decision can be revisited. Iteration is positive so that we can make improvements and changes. But we have to find the right balance in the process. Otherwise, even the release process would be much harder.

But in this case I would prefer to escalate it to the TSC, so that they can at least decide whether we should revisit their existing resolution now or not.

Meeting agenda items and time management within meetings are for the working group moderator to manage.

I completely agree, but I don't know why you're saying it now in the context of this discussion.

AxelNennker commented 1 month ago

This is the TSC text:

Agreement to use openId connect discovery clause in the Open API Specification (OAS) and provided API scheme for the API in another form (either in seperate file or as documentation section within the OAS). Documentation must also list possible security schema for the APIs.

I do not read that CIBA is forbidden by this TSC agreement. Actually Documentation must also list possible security schema reads like "a CIBA security scheme is possible and if an API supports/requires CIBA then the documentation must list it"

I agree that if personal information is involved and authentication and consumption device are the same then OIDC authorization code flow MUST be used if e.g. GDPR requires consent etc. If authentication device and consumption device are different then CIBA can also be used to authenticate the user and collect consent when personal information is involved and GDPR is involved.

IF a Camara API offers/demands CIBA then there should be a guideline specifying a CIBA security scheme.

So, I think, client might what to know upfront whether an API call might result in an SMS might be sent.

We/Camara do not want API designers to define there own security scheme and if an API wants to express CIBA then that should follow a CIBA guideline we define.

I think that OIDC security scheme is the most important for most Camara API and I am happy we have a guideline for API subprojects how to use it.

AxelNennker commented 1 month ago

ICM please also see https://github.com/camaraproject/PopulationDensityData/issues/24#issuecomment-2137902260