Implement core/site/data/connection REST route

[felixarntz]

Feature Description

There should be a REST route exposing basic connection information and related data that is site-specific (explicitly not user-specific), inspired by what is currently handled in Authentication::inline_js_setup_data().

This is PHP work, but still a requirement for following JS refactoring.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• There should be a core/site/data/connection REST route that is accessible via GET.
• The route should expose an object with the following properties:
  • connected: boolean whether there are site credentials
  • resettable: boolean whether there is data to reset

Implementation Brief

• Register a new endpoint in REST_Routes
  • uri: core/site/data/connection
  • methods: GET
  • 'permission_callback' => $can_setup,
• Return
  • connected: Credentials->has()
  • resettable: true if credentials option is set

Settings

• Register Credentials in Authentication::register()
• Update Data_Encryption::get() to remove early return if ! $raw_value
  • Only call decrypt if $raw_value is a string (if not, return $raw_value as it is not encrypted)
• Add public has( $option ): bool to Options_Interface
• Update Options and Encrypted_Options to implement the new method
  • For Options::has(), call get() and then check if the option name exists as a key on wp_cache_get( 'notoptions', 'options' ) to prevent unnecessary DB queries
  • For Encrypted_Options::has() proxy the call to $this->options->has( $option )
• Update Setting to include a has method which calls $this->options->has( static::OPTION )

Changelog entry

• Introduce core/site/data/connection REST API route for retrieving site connection info.

[aaemnnosttv]

@felixarntz is this a public endpoint? If not, what permissions does it require?

[felixarntz]

@aaemnnosttv It should be using Permissions::SETUP as requirement.

[aaemnnosttv]

@felixarntz I'm noticing while looking into this that Credentials is now an instance of Setting but isn't registered. After registering, it produces errors because Data_Encryption is trying to decrypt the default value (array) which should be an encrypted string instead. We can either keep it unregistered, or register it (maybe in Authentication?) and remove its get_defaults method so that the setting doesn't return the wrong type if it isn't set. What do you think?

[felixarntz]

@aaemnnosttv Hmm that's tricky. I feel like we should try to find a solution that is internal to Encrypted_Options / Encrypted_User_Options:

• In the respective get( $option ) method, maybe use something to check whether an option value exists in the DB? Alternatively, we could as a minimum not attempt to decrypt if ! is_string( $raw_value ), but that would be unreliable for default values that are a string.
• Note also that the current if ( ! $raw_value ) { return false; } should no longer be there as is, because if there is a default registered that is e.g. 0, it should still be returned as such.

Not sure on the solution, but let's try to find something that generally works for the encryption "adapters" internally.

[aaemnnosttv]

@felixarntz One solution here is to change Encrypted_*_Options::get() to only call decrypt internally if the returned option value is a string (i.e. encrypted). That would solve this problem for serialized encrypted values I think. However, if the setting was a string itself (like access tokens) then we could attempt decryption and then return the $raw_value if decryption failed.

Adding an exists() method to the Options_Interface would be good. We could add a method by the same name to Setting as well which would be all we really need in this specific situation. Encrypted Options could also return early if the option didn't exist: if ( ! exists( option ) ) return $options->get() before calling decrypt (assume the default value is never encrypted).

...

After a bit more thinking, I'm thinking maybe we should re-implement the encryption to happen using WordPress sanitization callbacks and option_ filters. That's the only way I think we can get a consistent behavior. Basically, encrypt when sanitizing for the DB, and then decrypt when the saved value is filtered before returning. The default would be returned as-is as option_ filters don't apply to default values. This is a bit more problematic for user meta I think as WP doesn't have filter-parity for options and meta but this sounds like it may be the better approach at the moment (at least for options). Registered defaults for meta don't function the same with get_*_meta as get_option does either.

[felixarntz]

@aaemnnosttv

Adding an exists() method to the Options_Interface would be good.

That sounds good to me. Let's name it has( $option ) though, this way it has naming parity with the existing Setting::has(). We can then use that as you're suggesting. The User_Options_Interface (see #1029) should then receive this method too.

After a bit more thinking, I'm thinking maybe we should re-implement the encryption to happen using WordPress sanitization callbacks and option_ filters.

I thought about this yesterday as well, and it seems clean to me, but I'm a bit wary of unexpected side effects here because of how obscure core behaves sometimes - particularly because we would be changing the data type of the value by encrypting/decrypting. It's something to keep in mind for potential refactoring and testing in the future, but for now, let's keep the encryption out of tying in too deeply with core option functions - especially given that meta wouldn't support it anyway.

[aaemnnosttv]

@felixarntz Setting doesn't implement Options_Interface interface. Would you like to add the same method to Setting as well or treat Credentials as a special case?

It's something to keep in mind for potential refactoring and testing in the future, but for now, let's keep the encryption out of tying in too deeply with core option functions - especially given that meta wouldn't support it anyway.

Yeah, the lack of support for the same pattern with meta is a deal-breaker for sure.

[felixarntz]

@aaemnnosttv

Setting doesn't implement Options_Interface interface. Would you like to add the same method to Setting as well or treat Credentials as a special case?

What do you mean? What I wanted to say is just to introduce Options_Interface::has( $option ) so that it has a similar name like the existing Setting::has(), regardless of the two being not technically tied to each other. The Setting::has() implementation should then be changed to call $this->options->has( static::OPTION ) - Credentials should remain as is because it's indeed special (if only for BC).

[aaemnnosttv]

Ok that makes sense. For Credentials specifically, we would need an additional method then to call parent::has() to check for existence, correct?

[felixarntz]

@aaemnnosttv

Ok that makes sense. For Credentials specifically, we would need an additional method then to call parent::has() to check for existence, correct?

I don't think so, it's just overloaded. We could actually add a parent::has() call to Credentials::has() before checking for the array keys.

[aaemnnosttv]

I don't think so, it's just overloaded. We could actually add a parent::has() call to Credentials::has() before checking for the array keys.

In the case of isResettable though, we want to know specifically if the option is actually set in the DB. Even if we were to add that parent call, Credentials::has() also returns true for credentials provided via the googlesitekit_oauth_secret filter so we can't use the same method for both connected and resettable logic.

[felixarntz]

In the case of isResettable though, we want to know specifically if the option is actually set in the DB.

We can just maintain how it is currently - we would call $this->credentials->has() for connected and $this->options->has( Credentials::OPTION ) for resettable.

[aaemnnosttv]

Ok, that works. I was thinking that we would add a method to Credentials to do the same thing but I suppose there isn't really a need for that yet. I'll update the IB shortly.

[felixarntz]

IB ✅

[aaemnnosttv]

CR ✅ Ready for merge 👍

[tofumatt]

QA ✅ The REST API endpoint exists and works as expected here.