Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: add security at the operation level #584
feat: add security at the operation level #584
Changes from all commits
9cc3d98
d4ac103
f353b1c
a93ec64
9b8dff9
bc68645
4e7a033
975fc86
3616061
52d01b4
521fd58
bf3961e
3480206
7c44dd4
8e81651
5e3763a
4739be1
c675782
b373afd
fb3b9bf
3460840
d1e8017
4a964f7
4c3d916
7a6200f
f8f9fcb
9ecf2a9
2874072
a57fb91
514b5fe
0d2a2d4
9e9463a
b221433
5a6faeb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sekharbans-ebay Would it make sense to link the channel to a server(s)?
You can do this by adding
servers
field. For example:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sekharbans-ebay I just noticed there is a typo here. It should be:
test_oauth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I'm writing the release notes, this comment seems confusing to me.
The spec says:
However, by reading this comment, I understand that only one (the server or rather the operation) security requirement should be satisfied.
Would you mind clarifying this @sekharbans-ebay? Also pinging Code Owners @derberg @fmvilas @dalelane
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example text is poorly worded - will reword this. My intention was to state that you could still operate on the channel using security credentials supported at the server level perhaps thru a different operation. Is this okay:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind creating a PR against
2022-04-release
branch with such a fix (and the others I mentioned in other comments) @sekharbans-ebay ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Please review:
#768