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

Authorization (lock symbol) is rendered incorrectly #4402

Open
m-mohr opened this issue Apr 3, 2018 · 26 comments
Open

Authorization (lock symbol) is rendered incorrectly #4402

m-mohr opened this issue Apr 3, 2018 · 26 comments

Comments

@m-mohr
Copy link

m-mohr commented Apr 3, 2018

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

@webron
Copy link
Contributor

webron commented Apr 3, 2018

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.

@IceflowRE
Copy link

IceflowRE commented Sep 25, 2018

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.

@bmbell
Copy link

bmbell commented Jan 16, 2019

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.

@RolandoAvila
Copy link

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.

@shockey
Copy link
Contributor

shockey commented Feb 7, 2019

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

@RolandoAvila
Copy link

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.

@MikeGriffinReborn
Copy link

MikeGriffinReborn commented Oct 24, 2019

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.

@anasoftinc
Copy link

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

@jrnxf
Copy link

jrnxf commented Jul 6, 2020

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.

@FirstChoice-Robert
Copy link

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  

@andi-livn
Copy link

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.

@ericsampson
Copy link

ericsampson commented Aug 19, 2020

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?

@MohammedAlasaad
Copy link

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.

@JonPugsley
Copy link

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.

@jez9999
Copy link

jez9999 commented Jan 14, 2021

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.

@Samyssmile
Copy link

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.

@marcotitoL
Copy link

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);
});

@angelorscoelho
Copy link

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.

@marcotitoL
Copy link

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.

@angrocode
Copy link

angrocode commented Jun 9, 2022

@fannypwr
Copy link

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

@Ceri48
Copy link

Ceri48 commented Nov 30, 2022

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

@johnwc
Copy link

johnwc commented Feb 25, 2023

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.

@bmja62
Copy link

bmja62 commented Mar 9, 2023

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";
      },
});

@maxenceTroislouche
Copy link

I think the current implementation very confusing and should actually be the opposite :/

@andrea14polanco-sa
Copy link

This is still very confusing. I was about to raise a bug but then found this.

nfacha pushed a commit to wireless-broadband-alliance/wba-openroaming-portal that referenced this issue Nov 12, 2024
swagger-api/swagger-ui#4402

this link expains how it should work
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests