AbsaOSS / enceladus

Dynamic Conformance Engine
Apache License 2.0
29 stars 14 forks source link

Feature/2160 openapi3 #2178

Closed dk1844 closed 1 year ago

dk1844 commented 1 year ago

Swagger-reated

Versions update notes

~Patch version update for <spring.version>: 2.0.0.RELEASE -> 2.0.9.RELEASE~ Reverted - just doing this breaks correct 401 reponses on JWT verification failing and yields 404 instead.

I have considered updating to

<spring.ldap.version>5.5.8</spring.ldap.version>
<spring.version>2.5.14</spring.version>

which seems to work ok (except for the 401->404 issue mentioned above), but the integration tests with embedded mongo do not work with this out of the box (or with light touchups at least - and there is no newer version of embedded mongo in the same major-version ballpark). However, a possible future consideration for a version update. Note, for such a case, following config may be neede:

# https://stackoverflow.com/a/74430247
management.metrics.mongo.command.enabled=false
management.metrics.mongo.connectionpool.enabled=false

Closes #2160

Zejnilovic commented 1 year ago

Have you thought about adding auth to it? Link to docs - https://swagger.io/docs/specification/authentication/bearer-authentication/

EDIT

Just tested. Now API returns 404 instead of 401 if I have a missing JWT header.

dk1844 commented 1 year ago

EDIT

Just tested. Now API returns 404 instead of 401 if I have a missing JWT header.

This part seems fixed now (I have used the original spring boot version from develop 🙄).

Now, the API should behave correctly and it's time to change auth that Swagger/openApi documents (I'm on it). Thanks big time for both of these realizations, @Zejnilovic!

// edit: custom JWT auth option is now available in Swagger, too. Hint: try in Incognito, otherwise your browser might slip in SPNEGO token.

swagger-custom-jwt

Zejnilovic commented 1 year ago

All looks good now but I don't see a login documentation

dk1844 commented 1 year ago

All looks good now but I don't see a login documentation

@Zejnilovic, a correct observation! The endpoint is a special one introduced via the authorization config, so the usual annotation on the controller cannot be used. I added a manual ApiOperation description, heavily inspired by https://stackoverflow.com/a/74574834/1773349. So fixed in my 👀 .

api-login-swagger

miroslavpojer commented 1 year ago

When using swagger switch from v2+3(dev) to v3, there are missing APIs for:

@benedeki what is your opinion? //edit: @dk1844 agreed as keep as-is (do not include these in v3 API)

miroslavpojer commented 1 year ago

Missing documented return status 403 for all endpoint which require admin rights. See endpoints with defined: @PreAuthorize("@authConstants.hasAdminRole(authentication)")

miroslavpojer commented 1 year ago

Is possible to hide the password here:

image

miroslavpojer commented 1 year ago

Missing method names (next to end point paths).

image

See "/api/login Login" as expected example.

dk1844 commented 1 year ago

Missing method names (next to end point paths).

image

See "/api/login Login" as expected example.

Hey, the closest thing I could find (without resorting to manual annotations for every method) is: springdoc.swagger-ui.displayOperationId=true

Then, it looks like this: swagger-op-id

(basically just methodName if globally unique, otherwise methodName_#)

Better than nothing or just too ugly?

dk1844 commented 1 year ago

Is possible to hide the password here:

image

The easiest option here is to define the password param's schema as new Schema().type("string").format("password") The result (masked entry field, but visible parameter): passwordVisibleInParamSwagger

Using x-www-form-urlencoded (www form sending data in POST payload)

        .requestBody(new RequestBody()
          .required(true)
          .content(new Content()
            .addMediaType(
              "application/x-www-form-urlencoded",
              new MediaType().schema(new Schema()
                  .addProperty("username", new Schema().`type`("string"))
                .addRequiredItem("username")
                .addProperty("password", new Schema().`type`("string").format("password"))
                .addRequiredItem("password")
              )
            )
          )
        )

does not behave any better from the password-visibility-in-swagger-perspective: www-form-swagger-passwordVisibleInSwagger

For simplicity, choosing the former, then.

sonarcloud[bot] commented 1 year ago

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
11.3% 11.3% Duplication

miroslavpojer commented 1 year ago

Missing method names (next to end point paths). image See "/api/login Login" as expected example.

Hey, the closest thing I could find (without resorting to manual annotations for every method) is: springdoc.swagger-ui.displayOperationId=true

Then, it looks like this: swagger-op-id

(basically just methodName if globally unique, otherwise methodName_#)

Better than nothing or just too ugly?

This solution is great.

miroslavpojer commented 1 year ago

Is possible to hide the password here: image

The easiest option here is to define the password param's schema as new Schema().type("string").format("password") The result (masked entry field, but visible parameter): passwordVisibleInParamSwagger

Using x-www-form-urlencoded (www form sending data in POST payload)

        .requestBody(new RequestBody()
          .required(true)
          .content(new Content()
            .addMediaType(
              "application/x-www-form-urlencoded",
              new MediaType().schema(new Schema()
                  .addProperty("username", new Schema().`type`("string"))
                .addRequiredItem("username")
                .addProperty("password", new Schema().`type`("string").format("password"))
                .addRequiredItem("password")
              )
            )
          )
        )

does not behave any better from the password-visibility-in-swagger-perspective: www-form-swagger-passwordVisibleInSwagger

For simplicity, choosing the former, then.

Solution improved situation. Well done.

miroslavpojer commented 1 year ago

Missing documented return status 403 for all endpoint which require admin rights. See endpoints with defined: @PreAuthorize("@authConstants.hasAdminRole(authentication)")

Solved.