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

Extends security scheme with ciba flow #3609

Conversation

shilpa-padgaonkar
Copy link

Fixes #3587

Uses https://github.com/OAI/OpenAPI-Specification/blob/v3.0.4-dev/versions/3.0.4.md#specification-extensions for ciba flow so that OAS is not overburdened with CIBA flow specific additional fields in oauth flow object.

@handrews
Copy link
Member

@shilpa-padgaonkar thanks! For extensions, we don't put them directly in the specification. instead, there is a registry on the gh-pages branch (which has web site content rather than spec content, because of how GitHub pages is configured).

You'll notice the registry is mostly empty- I don't think we've figured out the criteria for listing extensions (e.g. whether they have to be implemented first or not), and it looks like the linked issue on the registry page for figuring it out is still open. So we might need to discuss this in a Thursday TCD call - feel free to add your issue to the agenda by adding a comment to that agenda issue.

I think @miqui is also working on things for extension registries, including #3598.

I'm going to close this for now, but that's not a rejection of this solution. It's just that this is not where it goes if we want to do an extension.

@handrews handrews closed this Feb 24, 2024
@handrews
Copy link
Member

@shilpa-padgaonkar BTW, all are welcome to join the weekly calls. If that time does not work for your time zone but you would like to join a call, please let us know- we are trying to figure out if we should have some calls at alternate times, and knowing who would benefit and what time zones they are in will help us with that planning.

@shilpa-padgaonkar
Copy link
Author

@handrews I have made some updates to my branch where a fixed field (ciba) gets added to oauth flows https://github.com/shilpa-padgaonkar/OpenAPI-Specification/blob/shilpa-padgaonkar-ciba-flow/versions/3.0.4.md#oauth-flows-object to accomodate the grant type ciba and

the oauth flow object https://github.com/shilpa-padgaonkar/OpenAPI-Specification/blob/shilpa-padgaonkar-ciba-flow/versions/3.0.4.md#oauth-flow-object will have extensions for ciba request parameters.

As you have now explained that extensions don't get added to this doc, I can carve out the extension piece and (this can be discussed as you said in one of the calls) and for the fixed field related changes, I can open a new PR. WDYT?

@handrews
Copy link
Member

@shilpa-padgaonkar Security is really not my area of expertise – looking at the linked issue, I can't really tell what the appropriate way to add this to the spec really is. It looks like there's some confusion or disagreement among folks who have commented, and I don't see anyone from our Technical Steering Committee (TSC) weighing in.

I would recommend discussing your proposal in more detail in the issue first, and/or joining a Thursday call if that's convenient for you. If that call is not convenient, we can put the issue on the agenda and get some TSC guidance on your behalf (although it might take another week or two before we get to it- we have a large backlog right now).

It's generally best to have clear agreement on the solution in an issue before opening a PR. So your PR #3607 is great as the solution was already clear fro the issue (and thank you for figuring out the right branch and file! I'm sure we'll merge that as soon as we get someone from the TSC to approve it). But with this issue, it's not yet clear that everyone agrees on the problem and solution.

We try to avoid having PRs open when there's not yet agreement, because they tend to just get stuck - we've been trying to clear out the old PRs where that's happened and not add new ones. So I'd recommend continuing the issue discussion until there's more clarity and agreement.

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.

2 participants