swagger-ui icon indicating copy to clipboard operation
swagger-ui copied to clipboard

Authorization (lock symbol) is rendered incorrectly

Open m-mohr opened this issue 6 years ago • 25 comments

I have endpoints that either have a required authorization or an optional authorization (see example). I think the lock symbols are rendered incorrectly. It shows a black locked symbol for optional authorization (/public) and and a gray unlocked symbol for required authorization (/private).

Q A
Bug or feature request? Bug
Which Swagger/OpenAPI version? 3.0.0 and 2.0
Which Swagger-UI version? 3.x currently used by hosted Swagger Editor + master branch (03.04.2018 15:36)
How did you install Swagger-UI? Hosted Swagger Editor + locally using master branch (03.04.2018 15:36)
Which browser & version? Chrome 65.0.3325.181
Which operating system? Windows 10

Demonstration API definition

openapi: 3.0.0
servers:
  - url: 'https://localhost/api/'
info:
  title: OpenEO API
  version: 0.3.0
paths:
  /public:
    get:
      summary: This endpoint allows users to access it with AND without authentication.
      security:
        - {}
        - Bearer: []
      responses:
        '200':
          description: ...
  /private:
    get:
      summary: This endpoint allows users to access it only with authentication.
      security:
        - Bearer: []
      responses:
        '200':
          description: ...
components:
  securitySchemes:
    Bearer:
      type: http
      scheme: bearer

Expected Behavior

It shows a gray unlocked symbol for optional authorization (/public) and and a black locked symbol for required authorization (/private).

Current Behavior

It shows a black locked symbol for optional authorization (/public) and and a gray unlocked symbol for required authorization (/private).

m-mohr avatar Apr 03 '18 13:04 m-mohr

I understand the confusion, but it's actually working as expected.

When a user fills the authorization, the lock becomes closed and black - that indicates that there's security information provided. An unlocked lock, means that the user has not provided the information. We've had discussions in the past about how some people expect it to be one way and some the other. We'll consider changing it altogether to make it clearer.

In your case, it behaves as expected (in our intent) - since you allow a no-security option, meaning the user can use the call without providing credentials, the lock is black and locked indicating you can execute the call.

webron avatar Apr 03 '18 14:04 webron

Thats more than confusing.

First a better example:

openapi: 3.0.0
servers:
  - url: 'https://localhost/api/'
info:
  title: OpenEO API
  version: 0.3.0
paths:
  /public:
    get:
      summary: This endpoint allows users to access it only without authentication.
      responses:
        '200':
          description: ...
  /misc:
    get:
      summary: This endpoint allows users to access it with AND without authentication.
      security:
        - {}
        - Bearer: []
      responses:
        '200':
          description: ...
  /private:
    get:
      summary: This endpoint allows users to access it only with authentication.
      security:
        - Bearer: []
      responses:
        '200':
          description: ...
  
components:
  securitySchemes:
    Bearer:
      type: http
      scheme: bearer

Three states exist and the natural interpretation of those is easy:

state interpretation
none No authentication required.
open Optional authenticaton, thats why its already open, because you can access it even without.
locked You need an authorization to open it and only with you can access it.

To signalize provided security information, mark the corresponding locks green or so.

IceflowRE avatar Sep 25 '18 09:09 IceflowRE

I found the current implementation very confusing. Just take a real-life scenario - if something is locked (in this case, the black lock symbol), then it generally means it cannot be accessed without a key (e.g. an api key). The key "unlocks" the service, and grants access to it. Thus, the open lock symbol would be shown after credentials were entered.

bmbell avatar Jan 16 '19 00:01 bmbell

Can someone provide the explanation that clarifies how this is not confusing? I would like to know how to explain it to the consumers to my APIs.

RolandoAvila avatar Feb 07 '19 21:02 RolandoAvila

@RolandoAvila, @webron summed it up above pretty well IMO.

shockey avatar Feb 07 '19 22:02 shockey

Thanks @shockey , yes I read his comment, but I guess it's not convincing me that is an intuitive solution. If it was, I shouldn't have to continuously explain this to those new to swagger's documentation (which I love working with btw). I even surveyed some random non-technical people and sure enough all had the same confused conclusion I had.

RolandoAvila avatar Feb 08 '19 14:02 RolandoAvila

Very poor design. The API is locked until I authenticate then it is unlocked to me. I suggest you guys change this. It's beyond wondering how the thinking behind the current logic came about? Makes zero sense the way you have it now.

MikeGriffinReborn avatar Oct 24 '19 20:10 MikeGriffinReborn

This is actually a defect to be fixed since it is showing wrong information about API method on Swagger UI . It should be unlocked image for API method attributed [AllowAnonymous].

anasoftinc avatar Apr 12 '20 17:04 anasoftinc

Agreed, the logic on this is completely backwards. I'd suggest listening to user feedback on this one as it's clearly a common complaint.

jrnxf avatar Jul 06 '20 21:07 jrnxf

I'll add my voice to this as well. The current implementation is confusing. The open padlock to me symbolises that the end point is unlocked and can be used.The black padlock means that the end point is currently locked and can not be used without authorisation  

FirstChoice-Robert avatar Aug 07 '20 10:08 FirstChoice-Robert

I was about to raise this issue but found this existing ticket.

It may be splitting hair, but I very much agree that the notion of a closed=locked padlock to the common user is that something is secured against unauthorised access and will require a key to gain access, whereas an open=unlocked padlock would normally be interpreted as something that may otherwise be locked, but has already been unlocked.

As such, I too suggest an overhaul of the icons used to represent the different security requirements and Swagger UI session states:

Operations could have:

  • No padlock icon when no security has been defined
  • An open padlock when authentication is optional or after credentials matching any of the defined security schemes have been provided
  • A closed padlock when security is mandatory and no suitable credentials have been given

The Authorize button should use a closed/locked padlock before and an open/unlocked padlock after the user has provided credentials, so essentially the exact reverse of the current behaviour. Alternatively, a crossed-out or grey key icon could be displayed, which turns black once the user has logged in.

andi-livn avatar Aug 11 '20 04:08 andi-livn

This is nutty. If you picked a random sampling of 100 Swagger consumers and showed them the current behavior, I bet 95 of them would say that it's backwards. Would a PR be accepted to change this UI?

ericsampson avatar Aug 19 '20 18:08 ericsampson

Yes me too.. This is very confusing :) I agree with @bmbell I think this should be reversed :D ...

I found the current implementation very confusing. Just take a real-life scenario - if something is locked (in this case, the black lock symbol), then it generally means it cannot be accessed without a key (e.g. an api key). The key "unlocks" the service, and grants access to it. Thus, the open lock symbol would be shown after credentials were entered.

MohammedAlasaad avatar Sep 23 '20 20:09 MohammedAlasaad

Another vote for reversing here - to me displaying a closed padlock implies access to the method is locked/prohibited, an open padlock implies that the method is secured but I have provided the correct "key", and no padlock implies free/public access.

JonPugsley avatar Oct 26 '20 14:10 JonPugsley

I'd add that in addition to this fix, swagger-ui should detect when the only entry in the security array is the empty object, and mark that method as anonymous (no padlock), as I've detailed in #1819. The reason for this is that with Swashbuckle / OpenAPI.NET, there is no way to actually override an operation's global security and remove it completely to create an empty array - the most that can be done is an array with one empty object in it. Apparently this is supported, and indicates an anonymous operation - NOT one with optional security.

jez9999 avatar Jan 14 '21 11:01 jez9999

An average user aka customer aka human looking at a locked and unlocked icon instinctively thinks that an unlocked icon requires no authentication and that a locked icon requires authentication to access that endpoint.

The current behavior is a good example of how developers sometimes live on the moon.

Samyssmile avatar Oct 27 '21 12:10 Samyssmile

i too found the lock and unlock icon confusing. im just new and still learning about dotnet and swagger.

what i did is that, i added some js to swap the icons 😂

app.UseSwaggerUI(
swagger => {
    swagger.InjectJavascript("/changelock.js");
}

and on that js file is this:

window.addEventListener("load", (event) => {
  setTimeout(() => {
    var lockedIcon = document.getElementById("locked");
    var unlockedIcon = document.getElementById("unlocked");
    lockedIcon.id = "unlocked";
    unlockedIcon.id = "locked";
  }, 100);
});

marcotitoL avatar Apr 14 '22 04:04 marcotitoL

i too found the lock and unlock icon confusing. im just new and still learning about dotnet and swagger.

what i did is that, i added some js to swap the icons 😂

app.UseSwaggerUI(
swagger => {
    swagger.InjectJavascript("/changelock.js");
}

and on that js file is this:

window.addEventListener("load", (event) => {
  setTimeout(() => {
    var lockedIcon = document.getElementById("locked");
    var unlockedIcon = document.getElementById("unlocked");
    lockedIcon.id = "unlocked";
    unlockedIcon.id = "locked";
  }, 100);
});

Don't attempt to make it work this way. It doesn't work and if it did it would be very prone to fail. The swagger scripts don't only rely on class names or Ids to assert the style/action as other metadata is needed. The current locked/unlocked icon logic is broken and needs a fix. You can't argue that a closed lock indicates a successful access over an open one.

angelorscoelho avatar May 04 '22 15:05 angelorscoelho

Don't attempt to make it work this way. It doesn't work and if it did it would be very prone to fail. The swagger scripts don't only rely on class names or Ids to assert the style/action as other metadata is needed. The current locked/unlocked icon logic is broken and needs a fix. You can't argue that a closed lock indicates a successful access over an open one.

i am in no way suggesting this as a fix :D i was just doing it for my sanity at that time. and yes it works, as the icons are just html tags, you can do anything with html elements. but again, this is not the correct way to fix it.

marcotitoL avatar May 05 '22 02:05 marcotitoL

another vote for changing the current design - I was totally confused

fannypwr avatar Oct 21 '22 21:10 fannypwr

I was confused me to, why don't give a choice on User ?

Ceri48 avatar Nov 30 '22 16:11 Ceri48

How is this still going on 5 years later. A lock icon to anyone in the world is a symbol of no access without key/secret. The default should be lock icon set on all endpoints with security attribute applied, unlock icon for those that are unsecured with security or has been authenticated with a key in the UI.

If this is such a stickler thing that you do not agree with, then make it an option at least for others to opt in to one way or the other.

johnwc avatar Feb 25 '23 06:02 johnwc

I made a small improvement meanwhile we are waiting to fix this issue. Did not test it much but works with the latest version at least.

const ui = SwaggerUIBundle({
      url: "/swagger/docs/v1/",
      dom_id: '#swagger-ui',
      onComplete: function () {
         console.log(JSON.stringify(versions));
          var lockedIcon = document.querySelector(".svg-assets defs symbol#locked");
          var unlockedIcon = document.querySelector(".svg-assets defs symbol#unlocked");
          lockedIcon.id = "unlocked";
          unlockedIcon.id = "locked";
      },
});

bmja62 avatar Mar 09 '23 10:03 bmja62