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

Inconsistency between [AllowAny] and [] with permission_classes in schema generation #1382

Open
ldeluigi opened this issue Feb 17, 2025 · 6 comments

Comments

@ldeluigi
Copy link

Describe the bug
When using AllowAny or IsAuthenticatedOrReadOnly, opening the openapi doc using swagger looks like this:

Image

To Reproduce
Create an APIView or an @api_view that overrides the permission_classes with [AllowAny]

Expected behavior
Instead, the swagger page should look like this for those endpoints, because they don't require any authentication:

Image

Workaround
In Django, omitting AllowAny when it's the only permission_class doesn't change any behavior, but in drf-spectacular fixes this issue and properly displays the open lock icon in swagger.

Insight
I think the issue comes from these lines:

if permissions.AllowAny in perms:
auths.append({})
elif permissions.IsAuthenticatedOrReadOnly in perms and self.method in permissions.SAFE_METHODS:
auths.append({})

where adding {} as schemes has the effect to require auth in swagger

@tfranzel
Copy link
Owner

likely duplicate of #328

tldr, this is a design choice of SwaggerUI and is counter-intuitive. It is correct as is.

  • locked means you can use the endpoint.
  • open lock means either you do not have permission or a token is not set.
  • removing the whole security section will remove the lock completely
  • {} means allow any and as such closes the lock in the UI

check this with: https://editor-next.swagger.io/

@ldeluigi
Copy link
Author

There is the same result in the editor from swagger.io:
Image

I understand that the issue comes from a weird Swagger UI choice, I've read that {} means allowing no security in the openapi spec, but I'd like to stress that:

  • [AllowAny] and [] are equivalent in Django but not with drf-spectacular
  • drf-spectacular could ignore AllowAny without violating the OpenApi spec when it's the only element of the list, circumventing the problem

@ldeluigi
Copy link
Author

Found the root cause: swagger-api/swagger-ui#4402

@tfranzel
Copy link
Owner

tfranzel commented Feb 17, 2025

I've read that {} means allowing no security in the openapi spec

maybe you mistyped but {} means "allow any", not "allowing no security"

you need to have the {} in there in case you also use Oauth2PasswordBearer. It signals that you accept a bearer, but no auth is also fine.

If you have [] in Django DRF, it means the list is empty and then the security section is completely removed from the schema.

However, only having [AllowAny] in Django DRF will result in security: {}, which would be identical to omiting the security section. Its debatable whether we should remove it. In theory both are identical then.

@ldeluigi
Copy link
Author

ldeluigi commented Feb 17, 2025

However, only having [AllowAny] in Django DRF will result in security: {}, which would be identical to omiting the security section. Its debatable whether we should remove it. In theory both are identical then.

Yes, correct, only exception would be this icon issue with Swagger UI. But in general, there's an inconsistency between [AllowAny] and [] in the schema generation, and in my opinion omitting the security section should be prefered where possible because it results in a cleaner generated yaml.

@ldeluigi ldeluigi changed the title When using AllowAny the swagger doc suggests a mandatory authentication Inconsistency between [AllowAny] and [] with permission_classes in schema generation Feb 17, 2025
@ldeluigi
Copy link
Author

I've renamed the issue after your helpful insight

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

No branches or pull requests

2 participants