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

Scopes enum is required even for Bearer authentication #65

Open
sigmanil opened this issue Jul 20, 2020 · 3 comments
Open

Scopes enum is required even for Bearer authentication #65

sigmanil opened this issue Jul 20, 2020 · 3 comments

Comments

@sigmanil
Copy link

As far as I understand the OpenAPI documentation, Scopes should be an empty list when using Bearer authentication. See: https://swagger.io/docs/specification/authentication/bearer-authentication/

In Ktor-OpenAPI-Generator, due to the way com.papsign.ktor.openapigen.modules.providers.AuthProvider.Security is defined, it seems that we must still have an Enum class with scopes even when only supporting Bearer authentication: requierments must be a List, "where T: Enum" and T is a Described.

Intuitively, I think a bearer scheme should be possible to define either as follows, skipping the requirements parameter:

class JwtProvider : AuthProvider<UserPrincipal> {
    override val security: Iterable<Iterable<Security<*>>> =
        listOf(listOf(Security(SecuritySchemeModel(SecuritySchemeType.openIdConnect, scheme = HttpSecurityScheme.bearer, bearerFormat = "JWT", name = "JWT"))))

Or as follows, providing an empty list:

class JwtProvider : AuthProvider<UserPrincipal> {
    override val security: Iterable<Iterable<Security<*>>> =
        listOf(listOf(Security(SecuritySchemeModel(SecuritySchemeType.openIdConnect, scheme = HttpSecurityScheme.bearer, bearerFormat = "JWT", name = "JWT"), emptyList()))

But both of these result in compilation errors.

The solution for now has been to have "dummy enum" that's a subtype of Described, as follows:

enum class Scopes(override val description: String) : Described {
    DummyScope("Some scope")
}

...and then using that to define an empty list that can be sent to the Security class:

    override val security: Iterable<Iterable<Security<*>>> =
        listOf(listOf(Security(SecuritySchemeModel(SecuritySchemeType.openIdConnect, scheme = HttpSecurityScheme.bearer, bearerFormat = "JWT", name = "JWT"), emptyList<Scopes>())))

But the "dummy enum" feels out of place, and is never used anywhere apart from placating the type system. Unless I'm missing something?

@Wicpar
Copy link
Collaborator

Wicpar commented Jul 22, 2020

Yes, that restriction is quite bothersome and should be relaxed. I unfortunately haven't gotten around to it.
The security was designed with OAuth2 in mind, that is why typed enforcement of flows seemed a good idea.

@sigmanil
Copy link
Author

sigmanil commented Jul 23, 2020

I haven't really thought this through very thoroughly, but would an easy improvement be to change the type parameter in AuthProvider.Security, SecurityShemeModel to just require a Described, and not an Enum? For example:

Security<TScope : Described>(val scheme: SecuritySchemeModel<TScope>, val requirements: List<TScope>)

Or are you using the fact that it's an Enum for something somewhere? If not, that seems something that could be left to the discretion of whoever is using the library. In my head it wouldn't be a breaking change for any current users (though again, I might easily be missing something). And in my case it would allow me to say emptyList<Described>(), and forego the dummy enum.

@Wicpar
Copy link
Collaborator

Wicpar commented Jul 23, 2020

Enum is used for the ".name" and serialization, but besides that it was only meant as type enforcement. I think the issue with removing the Enum would be that the security model would be improperly generated. Replacing it with a String is possible, but generally using language types directly is unadvisable. Maybe a wrapper that handles the serialization properly ?

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