jsonapi is great. jsonapi_extras is even cooler!

I think they would be even better if all resources were disabled by default. This way, you don't have to spend time disabling each one by one. You can spend time configuring what you want to expose.

A compromise would be to at least be able to disable all resources with the push of a button, and then you can continue configuring. I will be glad to work on this issue.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sebi created an issue. See original summary.

mkolar’s picture

same here :) it took a while to disable everything :) button would be great

sebi’s picture

Yeah, the other issue is if you add something new, after you've done all that clicking, you have to remember to disable the new thing in case you don't want that exposed either.

e0ipso’s picture

I do not think that disabling resources by default will be useful for everyone. However I do think that having "Enable All" and "Disable All" buttons can be interesting.

I wonder if that could be a views bulk operation (just like the /admin/content page). If views is enabled then the bulk operations are made available, if views is not enabled then things stay as is.

sebi’s picture

Thanks for your reply! I think the enable/disable all is useful as a compromise for sure. I'm sure there is some historical reason for enabling by default. I'm just concerned that someone who isn't aware of jsonapi creates a content type and now you can technically get to all the fields if it hasn't been "locked down". One less thing to worry about. I like to err on providing information as you need it than giving too much if not necessary.

e0ipso’s picture

Issue tags: -Security Issue

I see where you are coming from. However I think that full linkability is more important in this case. Moreover, this policy goes in the same line that the JSON API uses, which is to expose all that the user can read & write based on the entity permissions.

Remember, you don't expose everything. You only expose the information that the user already has access on the website via HTML. If they can access it via HTML it's not a big deal that they can access it via JSON.

e0ipso’s picture

Title: Disable resources by default, configure what you WANT to expose » Allow bulk operations to enable/disable resources
Priority: Major » Normal
e0ipso’s picture

Title: Allow bulk operations to enable/disable resources » Add "Enable/Disable All" buttons in the configuration form
minnur’s picture

Here is a custom module for disable/enable all functionality. https://github.com/minnur/jsonapi_extras_bulk it might still have some bugs haven't fully tests, but might be something that we could put into JSON:API Extras module.

efpapado’s picture

I added minnur's code from his jsonapi_extras_bulk module to the jsonapi_extras module, and wrote some tests.
Attaching a patch.

jonnyeom’s picture

Status: Needs review » Reviewed & tested by the community

Tested against 3.13 and the latest 3.14 as well.
Works great! Thanks!

gitesh.koli’s picture

patch #10 works ! Tested this against 3.14

hoporr’s picture

Same here: path #10 works; tested against 3.14.
Very useful feature.

bbrala’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/src/Form/JsonapiExtrasBulkOpsForm.php
@@ -0,0 +1,249 @@
+  /**
+   * Gets all field names for a given entity type and bundle.
+   * (copied and modified from Drupal\jsonapi_extras\Form\JsonapiResourceConfigForm)
+   *
+   * @param \Drupal\Core\Entity\EntityTypeInterface $entity_type
+   *   The entity type for which to get all field names.
+   * @param string $bundle
+   *   The bundle for which to get all field names.
+   *
+   * @todo This is a copy of ResourceTypeRepository::getAllFieldNames. We can't
+   * reuse that code because it's protected.
+   *
+   * @return string[]
+   *   All field names.
+   */
+  protected function getAllFieldNames(EntityTypeInterface $entity_type, $bundle) {
+    if (is_a($entity_type->getClass(), FieldableEntityInterface::class, TRUE)) {
+      $field_definitions = $this->fieldManager->getFieldDefinitions(
+        $entity_type->id(),
+        $bundle
+      );
+      return array_keys($field_definitions);
+    }
+    elseif (is_a($entity_type->getClass(), ConfigEntityInterface::class, TRUE)) {
+      // @todo Uncomment the first line, remove everything else once https://www.drupal.org/project/drupal/issues/2483407 lands.
+      // return array_keys($entity_type->getPropertiesToExport());
+      $export_properties = $entity_type->getPropertiesToExport();
+      if ($export_properties !== NULL) {
+        return array_keys($export_properties);
+      }
+      else {
+        return ['id', 'type', 'uuid', '_core'];
+      }
+    }
+
+    return [];
+  }
+

Since this is duplicate code, we should find a way to extract this code to a service, parent or trait or something.

Also i'm wondering there should be confirmation before disabling all resources. It's relatively easy to misclick and break your configuration this way. What do you think?

brianperry’s picture

Applied patch in #10 and it worked well. Also agree that a confirmation step would be worthwhile.

mxr576’s picture

Moreover, this policy goes in the same line that the JSON API uses, which is to expose all that the user can read & write based on the entity permissions.

JSON API does a little bit more than that with its default behavior. When a module is enabled the /jsonapi exposes information about ALL entity types and bundles that the current site has, no matter if a user has access to any entity of entity type X or not. However, it is highly unlikely that a name of an entity or a bundle would lead to direct information disclosure, the response still provides a quite clear indirect idea about the enabled modules on a site, which can be leverage when there is a security issue with module X.
(Even though changing the default /jsonapi path is recommended, I do not think it mitigates this.)

bbrala’s picture

This is true, but by default i'd rather not change this. Although if i look at our default workflow, i'd also rather have new resources disabled by default then enable the ones I need. We could add an option to jaonapi_extra's for this behaviour, but that should be a separate issue.

mxr576’s picture

The problem with #10 is that it only provides a UI solution for bulk disabling/enabling API endpoints. It does not solve the problem that whenever a custom/contrib module introduces a new entity/bundle you have to keep in mind that you may want to disable the related JSONAPI endpoint _manually_ because it gets exposed by default.

bbrala’s picture

Yes i understand that, and as i said, i'm open to having an option to disable by default.

mxr576’s picture

Yes i understand that, and as i said, i'm open to having an option to disable by default.

Sorry, I did not see your previous comment when I submitted my new one after #17 :)

I am open a new issue later and reference this one, although maybe it should be rather a Drupal core issue for JSON API to have a switch for the default behavior.

e0ipso’s picture

It does not solve the problem that whenever a custom/contrib module introduces a new entity/bundle you have to keep in mind that you may want to disable the related JSONAPI endpoint _manually_ because it gets exposed by default

Remember that you are talking about information that you are declaring to be accessible, so it is exposed. There are several points where you might avoid that info being consumable:

  1. You can declare the entity type internal.
  2. You add access rules to your sensitive content.
  3. You go and disable it using JSON:API.

I think these are ample options to disable the entities to be exposed.

----

That being said, I have a client project that applies the "off by default" behavior. And believe me that there is more nuance than I anticipated in that scenario. My custom module allows config like this:

resource_types:
  whitelist:
    - file--file
    - menu_link_content--menu_link_content
    - node--case_study
    - node--post
    - taxonomy_term--*
    - taxonomy_vocabulary--taxonomy_vocabulary
  blacklist:
    - taxonomy_term--industry

Having implemented that, I would say this is out of scope of this issue.

e0ipso’s picture

I think the scope of this issue is solving the problem of: Oh my! Now I have to click all these pages manually...

mxr576’s picture

@e0ipso, thanks for your insides :) Yes, I am aware that both the current solution and the other one have their PROS and CONS. There is no perfect default configuration for everyone (for every project).

The background story where my frustration is coming from that enabling JSON API on an existing project causing a bunch of unexpected extra work for developers. (If they even know what "side-effects" this module comes.)
That gets even a bigger nightmare when an upstream adds JSON API as a dependency (f.e.: because it introduces one decoupled component) and just by updating the latest version on a downstream project, a bunch of API endpoints and information gets exposed from a site and downstream devs without preliminary knowledge may not even recognize this. So the unexpected extra work in this scenario is both on the upstream side and the downstream side. The upstream has to provide detailed documentation and manual steps for downstream devs about how they can achieve "an expected state" on site after updating to a news release.

I agree that this out of the scope of this issue.

My custom module allows config like this

Is there a chance to have that published on Drupal.org? :)

e0ipso’s picture

The background story where my frustration is coming from that enabling JSON API on an existing project causing a bunch of unexpected extra work for developers. (If they even know what "side-effects" this module comes.)

Can you clarify these two points? I am not sure I understand if the needs behind these are generic or a particular use case:

  1. What was the "unexpected extra work"?
  2. What are the "side-effects this module comes"?

----

Is there a chance to have that published on Drupal.org? :)

Sadly, I already maintain enough modules so it may be a while until I find the ability to do so.

mxr576’s picture

What was the "unexpected extra work"?
What are the "side-effects this module comes"?

What I have just described earlier, but in short: my expectation is that the JSON API module should just provide a programming API that simplifies the creation of JSON API compatible endpoints, instead of exposing a bunch of API endpoint on a site when it gets installed.

As you can see, just the name of the exposed endpoint can leak information from a site, even if they respect the entity/field access under the hood. Also, if you are enabling the JSON API module on upstream (f.e.: in a distribution), because you want to expose 1 entity API endpoint, you suddenly expose a bunch of other API endpoints on every dowstream site that uses your distro and downstream developers who are not familiar with this module, they may not even know about it.

Anyway, maybe my expectations are incorrect and this issue is definitely not the right one to discuss with a broader audience. Sorry for the noise :)

efpapado’s picture

What I have just described earlier, but in short: my expectation is that the JSON API module should just provide a programming API that simplifies the creation of JSON API compatible endpoints, instead of exposing a bunch of API endpoint on a site when it gets installed.

As you can see, just the name of the exposed endpoint can leak information from a site, even if they respect the entity/field access under the hood. Also, if you are enabling the JSON API module on upstream (f.e.: in a distribution), because you want to expose 1 entity API endpoint, you suddenly expose a bunch of other API endpoints on every dowstream site that uses your distro and downstream developers who are not familiar with this module, they may not even know about it.

I agree.

At least, there should be by default an easy differentiation between content and config entities.

uniquename’s picture

Just thinking: if the two buttons from the patch would also save a setting, if resources should be enabled or disabled by default, it might be possible to hook into the creation of an entity type, when a new module is installed, with hook_modules_installed and set the resource status accordingly.

bbrala’s picture

As #3169413: Add a configuration option to default resources without configuration to disabled was merged. Wouldn't that kinda cover this use case? Now you can default to disable unconfigured resources.

mxr576’s picture

Status: Needs work » Closed (duplicate)

@bbrala yes, it seems that is something we can use to cover this use case, although that commit is not available in a stable release yet. So I hope there will be a new release soon.

bbrala’s picture

I released 3.20 which should contain that feature? Have you tried that release?

mxr576’s picture

FileSize
479.85 KB

Hm, I used simplytest.me to quickly try this out but that versions was not and still not visible there. Which is possibly a bug in their end, also my bad. Sorry