Addresses issue #283 by adding oauth2 security scopes to operation docs #287
Conversation
There was a problem hiding this comment.
@jimmyjames thanks for kicking it off and sorry for long reply! This looks good!
In order to support HTML display in the tooltip, changes dependency on hint.css to html-hint.css (based on hint.css).
I have a concern here as the html-hint seems to be not maintained heavily: last changes a year ago comparing to 6 days ago by hint.css
This pass only supports OAuth2 security definitions per operation as a first-pass - it does not consider api_keys. OK for a first pass?
absolutelly OK!
This implementation does not attempt to handle any $ref to security definitions per-operation.
I will test these changes on some major documentations powered by ReDoc and most likely will request some more changes.
Regarding padlock, just use svg. So replace ! by this:
<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
width="20px" height="20px" viewBox="0 0 512 512" enable-background="new 0 0 512 512" xml:space="preserve">
<path d="M384,224v-96C384,57.438,326.594,0,256,0c-70.578,0-128,57.438-128,128v96c-35.344,0-64,28.656-64,64v160c0,35.344,28.656,64,64,64h256c35.344,0,64-28.656,64-64V288C448,252.656,419.344,224,384,224z M272,379.094V432c0,8.844-7.156,16-16,16s-16-7.156-16-16v-52.906c-9.391-5.563-16-15.375-16-27.094c0-17.688,14.328-32,32-32s32,14.313,32,32C288,363.719,281.391,373.531,272,379.094z M320,224H192v-96c0-35.313,28.703-64,64-64c35.281,0,64,28.688,64,64V224z"/>
</svg>| @@ -137,6 +137,42 @@ export class SpecManager { | |||
| return obj; | |||
| } | |||
|
|
|||
| getOperationScopes(operationPtr:string) { | |||
| let that = this; | |||
| function getSecurityDefinition(name:string, scope:string) { | |||
There was a problem hiding this comment.
Transform this function to fat-arrow function and remove let that = this trick:
const getSecurityDefinition = (name: string, scope: string) => {
// just use `this` here
}
|
@jimmyjames are you planning on picking up the feedback changes here? I need these for a project so would be happy to pick up if you aren't planning to |
|
Hey @wilsonge and @RomanGotsiy sorry for the late reply. @wilsonge if you have some bandwidth and can incorporate the feedback changes that would be great - it may be a little bit until I can get back to this. Thanks!
@RomanGotsiy did you get a chance to do this? |
|
I'm not able to pull the changes and test in the next couple days; @wilsonge can you include a screenshot of how it will look? Given the desire to not use html-hint, I'm wondering using tooltip is not the right approach at all (unless another solution using hints that supports html content is included), and instead just include the scopes as a subsection under the operation description. It definitely makes the OAuth2 scopes more prominent in the docs instead of tucked away, for better or worse. @RomanGotsiy curious for your feedback, as well as if you had a chance to test further. |
|
Hey @jimmyjames, sorry for delaying this for so long time. I was on vacation. Today I will release a new version of ReDoc (as there are a few fixes already merged) and then I will start testing this.
Maybe we'll have to go back to |
|
Hey @jimmyjames, would you mind if I introduce this feature a bit later together with the rewrite to React (#327)? |
|
@RomanGotsiy sorry I missed this until now. I understand wanting to minimize the amount of migration done. In our case, our API has many different scopes, and our most of our operations require different scopes. Not having the scopes required per operation shown in the docs is not acceptable for us, but we can fork/modify/host ourselves if we need to until migration is done and the feature is added. |
|
@jimmyjames sorry for not answering for so long... So basic version of this feature is already implemented there. Closing this PR in favor of React version. |
This PR attempts to address issue discussed in issue #283
Operations often have a specific security scope associated with them, and currently that information is not available via ReDoc. This PR:
Some things to consider/review from those with more knowledge:
$refto security definitions per-operation. I don't know if this is supported/common so worth considering (perhaps at least needs to be handled for thesecurityDefinitionslookup of the descriptions?).!styled similar to other tips like Parameters. Might be better to display as some sort of padlock icon (like Swagger UI) to be clearer, but per issue comments starting with the!