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

chore: adds initial proposal from the backlog working group #7

Merged
merged 30 commits into from
Jul 8, 2024

Conversation

caubut-charter
Copy link
Contributor

What type of PR is this?

Add one of the following kinds:

  • subproject management

What this PR does / why we need it:

Copies the proposal from the backlog working group into this repo.

Which issue(s) this PR fixes:

Special notes for reviewers:

Leaving this PR open for at least a couple weeks to gather initial comments and to schedule a first meeting.

Changelog input

Adds initial proposal from the backlog working group.

Additional documentation

Copy link

github-actions bot commented Jun 5, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 2 0 0.03s
✅ OPENAPI spectral 1 0 3.01s
✅ REPOSITORY git_diff yes no 0.01s
✅ REPOSITORY secretlint yes no 0.73s
✅ YAML yamllint 1 0 0.69s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

Copy link
Contributor

@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.

A few comments regarding oauth.

code/API_definitions/network_access_management.yaml Outdated Show resolved Hide resolved
code/API_definitions/network_access_management.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@hdamker hdamker left a comment

Choose a reason for hiding this comment

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

@caubut-charter @RandyLevensalor Please refer to the work of Identity & Consent Management - there shouldn't be a need to have a detailed discussion here, especially if 3-legged is your default use case.

Regarding the definition of securitySchemes and security: https://github.com/camaraproject/IdentityAndConsentManagement/blob/main/documentation/CAMARA-API-access-and-user-consent.md#camara-api-specification---authorization-and-authentication-common-guidelines

And regarding the details of the flows, e.g. PKCE, see https://github.com/camaraproject/IdentityAndConsentManagement/blob/main/documentation/CAMARA-Security-Interoperability.md

@hdamker
Copy link
Contributor

hdamker commented Jun 11, 2024

The "approval" was accidentally done ... I wanted mainly to comment on the security discussion. Please consider #7 (review) as a comment. Maybe it is fine to commit the initial content, but with Version: wip and have a separat discussion about the security topics.

code/API_definitions/network_access_management.yaml Outdated Show resolved Hide resolved
Copy link

@jpengar jpengar left a comment

Choose a reason for hiding this comment

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

I've made some very general initial comments. I understand that it's fine if you want to add this as a "wip" first version to iterate on, and you don't need to fix everything in this PR. And I will give a second thought to what refers to the functionality this API provides and how it relates to the Home Devices QoD. At the moment I'm not really sure if we need to align anything in the definitions of the two APIs.

code/API_definitions/network_access_management.yaml Outdated Show resolved Hide resolved
code/API_definitions/network_access_management.yaml Outdated Show resolved Hide resolved
code/API_definitions/network_access_management.yaml Outdated Show resolved Hide resolved
code/API_definitions/network_access_management.yaml Outdated Show resolved Hide resolved
code/API_definitions/network_access_management.yaml Outdated Show resolved Hide resolved
@RandyLevensalor RandyLevensalor force-pushed the feat/initial-proposal branch from ee3b87f to 95b5a28 Compare June 28, 2024 16:18
@caubut-charter
Copy link
Contributor Author

Mostly done. Two tasks left:

  1. (wip) Finish adding minimal scopes in the security prop to every endpoint.
  2. Align on CAMARA terminology, such as replace remaining CPE references with Network Access Management in the description text and review the other terms used in https://github.com/camaraproject/Commonalities/blob/main/documentation/Glossary.md.

Opened #12 and #13 as follow up items.

@caubut-charter
Copy link
Contributor Author

Randy opened #14 to go over the glossary and remove CPE.

Finished adding sec defs.

@caubut-charter caubut-charter marked this pull request as ready for review July 8, 2024 19:19
@caubut-charter caubut-charter requested a review from mayur007 as a code owner July 8, 2024 19:19
Copy link
Contributor

@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.

This is a good initial commit. As mentioned there are additional items before the fist beta release.

@caubut-charter caubut-charter dismissed hdamker’s stale review July 8, 2024 20:11

Requested changes made or split out into different issues.

@caubut-charter caubut-charter merged commit 1c804a5 into main Jul 8, 2024
1 check passed
@caubut-charter caubut-charter deleted the feat/initial-proposal branch July 26, 2024 19:33
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.

5 participants