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

Add required security scheme and scopes #295

Merged
merged 10 commits into from
Jul 11, 2024
Merged

Conversation

eric-murray
Copy link
Collaborator

What type of PR is this?

  • correction

What this PR does / why we need it:

API security schemes and scopes must comply with agreed API design guidelines and ICM guidelines

Which issue(s) this PR fixes:

Fixes #244

Special notes for reviewers:

None

Changelog input

 release-note
 - Add security scheme and scopes for each endpoint / method

Additional documentation

None

Copy link
Collaborator

@RandyLevensalor RandyLevensalor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though I'm not an expert in this area.

Copy link
Collaborator

@jlurien jlurien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

:create and :update could be merged in a common scope :write, if there is no use case to differentiate them

Copy link
Collaborator

@sfnuser sfnuser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. You may want to remove lines 22-23 in quality-on-demand.yaml for completeness.

Sigh! Finally, QoD is also swallowed by this vague security schema change :-(. Apologies - nothing towards your change - I keep recording my displeasure on this change hoping that someday ICM may take this into account (i.e.) it is better to describe all applicable security schemas in the spec and MNO pick and choose schemas based on the their jurisdiction/contract.

@eric-murray
Copy link
Collaborator Author

@sfnuser
I expect documentation updates following from this proposed change will be discussed in today's meeting

Regarding the openId scheme itself, I think the point frequently overlooked is that the openIdConnectUrl has to be API specific to be useful. A common openid-configuration for all APIs will likely list all the three possible grant types, leaving the API consumer none the wiser as to which apply to which scopes.

CAMARA have only defined three grant types and, in principle, a 3-legged token could be used with any endpoint (maybe with a change in behaviour over a 2-legged token, but still used nonetheless). So the question in a developer's mind should be "which endpoints will accept an 2-legged token?". I think possibly the way to go is to try to split APIs into separate YAMLs such that all endpoints in the YAML can be accessed using the same token, as we have done here.

That can be documented, but whether the token must be a 3-legged token or could be a 2-legged token in practice, the developer will only find out when they read the openid-configuration for that YAML.

@RandyLevensalor
Copy link
Collaborator

FYI: I've created camaraproject/IdentityAndConsentManagement#178 to address the wording in the mandatory text.

@hdamker
Copy link
Collaborator

hdamker commented Jul 10, 2024

@AxelNennker @sfnuser can you please check if your comments are addressed (based on ICM v0.2.0)? I would like to merge this one, as we already end of June decided that it is ready for final review.

@sfnuser
Copy link
Collaborator

sfnuser commented Jul 10, 2024

@AxelNennker @sfnuser can you please check if your comments are addressed (based on ICM v0.2.0)? I would like to merge this one, as we already end of June decided that it is ready for final review.

@hdamker: Please proceed.

@hdamker hdamker merged commit 4afb178 into camaraproject:main Jul 11, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Align securitySchemes and security of QOD API spec with IdentityAndConsentManagement
6 participants