OpenAPI is returning invalid security definitions

🙏

I originally posted the below in regard to removing the security definitions in #2982691: A few items not conforming to spec. I don't know if just stripping out the bad definitions is the best approach here.

While I would like the swagger doc to conform to complete spec the cookie and csrf_token can be used by decoupled projects to authenticate to Drupal. As a result the should be included in the api. I don't have a recommended solution here. I would rather not just take out the definitions, which is why they were added using the version 3 spec.

I think that the best option, would be to allow them to be in the spec, but find a why to put them behind a toggle. Thus allowing the user to allow these auth options, if they need them for their project and accepting the fact that their site would no longer conform to the 2.0 spec.

At this point, this module doesn't have any other settings or configuration, so I am hesitant to implement it just for this functionality.

I am open to suggestions. Any thoughts?

Now that #2948570: Refactor doc module implementations has been released, I am going over issues and figuring out a path forward. I arrived at this issue via #2886726: Allow authentication information to be added to ReDoc OpenAPI docs. Ultimately i think that its a vote in favor of providing correct security definitions, so I think the patch here has the correct intent going forward, once we look into #2988277: [Meta] Implement OpenAPI 3.0 spec, we can revisit the extended auth options.

I took another look over this patch. Ultimately, it looks like its mostly there, but made a few changes to ensure that the expected data is returned.

We effectively drop cookie and oauth, and don't return the unknown definition.

+    // Strip any empty arrays which aren't required.
+    $required = ['swagger',  'info', 'paths'];
+    foreach ($spec as $key => $item) {
+      if (!in_array($key, $required) && is_array($item) && !count($item)) {
+        unset($spec[$key]);
+      }
+    }

@justafish +1 for this. I like the idea. I opened #2988867: Don't return empty values for non required keys. to make this change since its not directly related to the rest of this patch.

Sometimes I suck at this coding thing.... just a spelling mistake.

Committed and pushed. @justafish, Thanks for doing the initial work on this!

Correct. I choose to leave them in as the apiKey type is within spec. And thought there were cases where this could be leveraged for authentication for core rest. I don't use core rest in any projects, so i might be entirely wrong. If there is no reason why this might be helpful to a user, we can remove it.

Unless of course it breaks spec in some other way I am not aware of?

I think, this still does not work as expected. The oauth2 authentication flow is hardcoded to "password". We are using the "Authorization Code Flow" in the simple_oauth_extras module.

See https://oauth2.thephpleague.com/authorization-server/auth-code-grant/

This is not reflected in the OpenAPI Doc because the Password Grant is hardcoded.

@ayalon,

I am not familiar with the "Authorization Code Flow", but if drupal uses it, i would support it here. The challenge is that simple_oauth, does use the password flow, so I can't just swap it. I haven't used the simple_oauth_extras module, but I would assume that you can still use the password flow even with it enabled? As a result, I would need to publish both correct?

Due to the design of the 2.0 spec, which is what the OpenAPI module currently publishes, only one OAuth flow is supported. In the 3.0 spec, which I hope to implement, does support multiple OAuth flows. This might be argument enough for moving to the 3.0 version of the spec. However, I don't know if I can facilitate it at the moment.

That leaves a few options, let me know what you think makes sense.

1. either go with a single flow ("password" is more useful since it will be supported on all sites)
2. Use "password" by default, and switch to "Authorization Code Flow" if simple_oauth_extras is installed
3. implement 2 instances of OAuth in the results. One for "password" and one for "Authorization Code Flow"

What are your thoughts?

I think number 3. is correct. As far as I can say (and also tested) if the simple_oauth_extras module is enabled, both flows work. The flow is identified by the grant_type "password" or "authentication_code".

It's hard for me to believe, that only one grant type is allowed by the specs. It simply does not make sense to me.

Just for you an example how the "Authorization Code Flow" works:
Think of a native iOS app. Tha app should have access to the API of the Drupal Backend. The app opens a browser, opens an url. The website displays a message: "Do you want to grant the native app access?". If you click yes, the browser redirects back to a local url of the app giving you an authorization_code. With this code you then can make a request to get the access token and a refresh token.

I'm pretty sure that this flow can exists in coexistance with the password flow. The advantage is, that the native app does not have to have any hardcoded passwords.

Regarding spec, as any standard, the spec had some poor decisions early on, such as limiting one flow per authentication method. This has been fixed in the latest (3.0.0 and 3.0.1) versions of the spec, but at the moment this module only complies with the 2.0 version, which allows each oAuth security definition to supply a single "flow" property. In 3.x variants, this is replaced with a "flows" property, which contains an array of object which represent the different flows possible through OAuth. Ultimately, I do want to transition to the 3.x spec, but want to clean up some more project architecture before doing so. Once things are a little more organized, the move will be easier.

In the mean time, I will work on getting the other OAuth flow into the current doc. I have opened #2995867: Add support for simple_oauth_extras to track the changes in order to support this. If you have time, feel free on taking a shot at a patch. If you put one together I will be happy to review it.