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

Resolution on where the documentation of ICM AuthN/AuthZ common guidelines for API specs must be located #160

Closed jpengar closed 3 weeks ago

jpengar commented 1 month ago

Problem description

Current status: The ICM AuthN/AuthZ common guidelines for the CAMARA API specifications are currently documented in CAMARA-API-access-and-user-consent.md. So far it standardises the specification of securitySchemes and security openAPI fields across all CAMARA API subprojects with common mandatory guidelines as agreed by the TSC and the participants of this working group (see #57 and #93 for further details on the process resulting in this documentation).

An issue has been opened in Commonalities https://github.com/camaraproject/Commonalities/issues/207 to reflect the decisions of Camara Security and Interoperablity Profile in Camara API Design Guidelines. It would be similar to the work done in ICM in #154 and #155 but with CAMARA-API-access-and-user-consent.md document.

A new PR https://github.com/camaraproject/Commonalities/pull/208 has also been created to address issue https://github.com/camaraproject/Commonalities/issues/207. However, this PR not only includes changes to the new ICM profile, but also moves without notice in ICM the information related to the securitySchemes and security guidelines specified in the ICM documentation, which are not part of the ICM profile document. This PR did not even include in the description that the PR was moving this piece of information as well (this was later corrected by @AxelNennker after it was raised in the PR comments). And the PR also changed the source information in the ICM documentation. This PR changes even the content of such a sensitive part as the info.description template, which needed a long review and approval by the ICM working group participants. Changing this information should be reviewed and approved by ICM.

After discussion during the ICM meeting on 22 May, it was agreed to create a new issue on the ICM working group github in order to make this issue visible to the ICM working group participants, let them express their opinions, and eventually agree on a resolution on where the documentation of the ICM AuthN/AuthZ common guidelines for API specs should be located: ICM doc vs. Commonalities doc.

CC @Elisabeth-Ericsson @tanjadegroot @diegogonmar @AxelNennker

Expected action

Additional context

You can find additional context in the comments of:

jpengar commented 1 month ago

Now the current situation is described and the issue raised. From my point of view:

shilpa-padgaonkar commented 1 month ago

Thanks to ICM for defining the openid security scheme for all APIs to use.

However, I also see that the PR https://github.com/camaraproject/Commonalities/pull/208, makes the design guidelines doc look more complete. I see a similar example from the release management wg. The API versioning section, which was worked on and agreed in the release management working group also makes its way now into the design guidelines with the PR https://github.com/camaraproject/Commonalities/pull/215 to ensure completeness of the design guidelines doc. If changes are needed to the API versioning section in the future, they are going to happen in alignment with release management WG by creating an issue in this WG. The same is true for the alignment between Commonalities and ICM, if changes are needed in the future then they are going to happen in alignment with ICM by working on the issue in ICM.

IMHO I see benefit to the readers if all OpenAPI defintions are in one place in the API design guidelines doc.

jpengar commented 4 weeks ago

If changes are needed to the API versioning section in the future, they are going to happen in alignment with release management WG by creating an issue in this WG. The same is true for the alignment between Commonalities and ICM, if changes are needed in the future then they are going to happen in alignment with ICM by working on the issue in ICM.

IMHO I see benefit to the readers if all OpenAPI defintions are in one place in the API design guidelines doc.

This has been a very polarising issue (the one related to securitySchemes), and it is already happening (with all the overhead and confusion that entails) that there are more than one open issue in Commonalities, API subprojects, Slack messages, and even multiple issues in the ICM just related to securitySchemes. Having this information in the ICM would allow us to have better control over it, and also guide CAMARA participants to the decisions made by the ICM and TSC, avoiding overhead and confusion. This is a BIG benefit IMHO to keep the information where it is now. As I said, in general I tend to agree with you that it is better to have everything in one place if possible, but for this matter I believe it will bring as more problems than benefits. From a document reader's perspective, it is just one click to browse the github documentation in ICM, which means nothing. There are plenty of other references like this in the API design guidelines or the ICM profile itself and it is not a problem. It's like browsing through the Confluence pages... I see a clear benefit, and I don't see a problem with keeping it the way it is.

AxelNennker commented 4 weeks ago

Copied from https://github.com/camaraproject/Commonalities/pull/208#issuecomment-2149489854 My paraphrasing of the arguments presented - with edits by @jpengar in the cons section

Main question is whether or not moving OpenAPI security schemes and info.description to OpenAPI definitions

Pro :

Contra moving arguments:

Options:

jpengar commented 3 weeks ago

I vote in favour of keeping From my point of view, I would keep the information in ICM doc for the reasons I've given above.

AxelNennker commented 3 weeks ago

Please avoid the word "vote" :-)

Elisabeth-Ericsson commented 3 weeks ago

Proposal is to keep the document https://github.com/camaraproject/IdentityAndConsentManagement/blob/main/documentation[/CAMARA-API-access-and-user-consent.md intact. I made a proposal in https://github.com/camaraproject/Commonalities/pull/208/files#top to shorten the text, which is proposed there and instead point to the relevant sections in the CAMARA-API-access-and-user-consent.md#use-of-openidconnect-for-securityschemes. I only concentrated on chapter 10.6. I hope that this is a middle way, we can agree on.

jpengar commented 3 weeks ago

Proposal is to keep the document [https://github.com/camaraproject/IdentityAndConsentManagement/blob/main/documentation[/CAMARA-API-access-and-user-consent.md](https://github.com/camaraproject/IdentityAndConsentManagement/blob/main/documentation%5B/CAMARA-API-access-and-user-consent.md) intact. I made a proposal in https://github.com/camaraproject/Commonalities/pull/208/files#top to shorten the text, which is proposed there and instead point to the relevant sections in the CAMARA-API-access-and-user-consent.md#use-of-openidconnect-for-securityschemes. I only concentrated on chapter 10.6. I hope that this is a middle way, we can agree on.

For some reason, some of your suggestions in https://github.com/camaraproject/Commonalities/pull/208/files#top don't seem to be formatted correctly by github. But If I understand your suggestions correctly, it would be fine with me. Maybe @AxelNennker could try to apply your suggestions to confirm it.

jpengar commented 3 weeks ago

For some reason, some of your suggestions in https://github.com/camaraproject/Commonalities/pull/208/files#top don't seem to be formatted correctly by github. But If I understand your suggestions correctly, it would be fine with me. Maybe @AxelNennker could try to apply your suggestions to confirm it.

@AxelNennker has already done this. I've just added a few suggestions. So hopefully we can all agree to close this. CC @Elisabeth-Ericsson @shilpa-padgaonkar

jpengar commented 3 weeks ago

Commonalities PR https://github.com/camaraproject/Commonalities/pull/208 is MERGED (with changes agreed after ICM 5 June meeting discussions) and issue https://github.com/camaraproject/Commonalities/issues/207 is CLOSED. So, I'm going to close this issue after agree on a resolution on where and how to document the ICM AuthN/AuthZ common guidelines for API specs.