V8-archive: Prevent URL unsafe characters in primary keys

[EPMatt]

Bug Report

When selecting an element whose primary key contains / characters Directus App displays the Sign In page and the error Unauthorized Request.

Steps to Reproduce

1. Create a Collection test with just the primary key as a field, named ID by default-
2. Update the ID field: select String as Field Type,VARCHAR as MySQL Datatype and switch off the Hidden on Detail option.
3. Add an item to the test collection with the value of ID field set to a/b.
4. Save the newly created item.
5. In the test collection page click on the newly created item.

Expected Behavior

The item with ID set to a/b is shown.

Actual Behavior

The Sign in page is displayed and the error Unauthorized request appears, below Something is wrong with this project (see screenshot).

Other Context & Screenshots

Screenshot:

I think the issue is related to how links for items are generated. In many sections of the Directus App source I see code similar to this:

 `/${this.currentProjectKey}/collections/${this.collection}/${pk}`

A / character in a primary key is directly injected in the URL, thus tricking the router which will consider the generated URL as a bad route.
I tried to replicate the issue in 190927A: when clicking on the a/b item I received the error Item not found and on the top of the screen the breadcrumb was Collections > Test > A > B (confirming that the URL was not interpreted correctly).

Technical Details

• Device: Desktop
• OS: Windows 10
• Web Server: Apache 2.4.38
• PHP Version: 7.3.3
• Database: MySQL 5.7.17
• Directus Release Version: v8.0.0-rc.2
• Install Method: cloned master branch

[rijkvanzanten]

Could you try what happens if you open the item in the API directly URL escaping the primary key?

[rijkvanzanten]

I'm also curious to the opposite side of this. Lets say we support escaped characters in primary keys. That means that a/b would be a%2Fb. What happens if you have both a/b and a%2Fb as primary keys (they're technically different in the DB). Or what happens if you have a primary key that has %2F in it (generated string maybe), but now you're not able to access it through the API because it'll search for / in the DB column?

[EPMatt]

Could you try what happens if you open the item in the API directly URL escaping the primary key?

Accessing the URL http://[path_to_directus]/public/[project_name]/items/test/a%2Fb returns Object not found (404).

If the AllowEncodedSlashes Option is set to On in Apache configuration, then accessing the API with the same URL returns: {"data":{"id":"a\/b"}}

I'm also curious to the opposite side of this. Lets say we support escaped characters in primary keys. That means that a/b would be a%2Fb. What happens if you have both a/b and a%2Fb as primary keys (they're technically different in the DB). Or what happens if you have a primary key that has %2F in it (generated string maybe), but now you're not able to access it through the API because it'll search for / in the DB column?

Yep, that creates lot of confusion...just tried to add an element with a%2Fb as a primary key and accessing it (with AllowEncodedSlashes Option set to Off): the UI displays "Server Trouble". In the requests it looks like it's trying to access the item a/b as it probably encodes the primary key.

More experiments I did:

• removed a/b item via SQL query;
• created a%2Fb via UI;
• accessed http://[path_to_directus]/public/[project_name]/items/test/a%2Fb:
  
  
  
  • with AllowEncodedSlashes Option set to Off: 404
  
  
  • with AllowEncodedSlashes Option set to On: {"error":{"code":203,"message":"Item with id \"a\/b\" not found"}}
  
  

[rijkvanzanten]

I'm wondering if all the added server complexity and confusion is worth the ability to have a / in the primary key name. Seeing that we always retrieve the item through its primary key in the URL, I think we should state that we don't allow (#, ?, &, /) in the primary key..

@benhaynes thoughts?

[benhaynes]

Agree, and I think that definitely fits with our 80/20 rule. Most string PKs I've seen are alphanumeric hashes, UUIDs, or something similar. It sucks for those using those characters... but it really does make things _much_ harder to build/maintain.

[EPMatt]

@rijkvanzanten @benhaynes what about not only stating in docs that those characters are not allowed in primary keys, but also implementing a check on string primary keys, such that it's impossible for the user to insert those characters by mistake?

[rijkvanzanten]

Yeah I agree. @bjgajjar Is that something we can do based on the primary-key directus datatype?

[benhaynes]

Yes, and Docs have fallen behind (we were busy with Client work,etc for the last 10 months). So that will be getting some much needed attention over the coming weeks.