Switch to forked httprouter and enable UseRawPath option

[Tener]

The httprouter package mishandles the URL-encoded / present in request paths. Despite requests and available PRs, this hasn't been fixed:

• RawPathRouting julienschmidt/httprouter#55
• allow router to use request.URL.RawPath julienschmidt/httprouter#209

We are affected by this issue in many ways, e.g.:

• Allow MFA devices with / in names to be deleted from UI #11011
• Creating the user with / in name is possible and breaks things #10576

This PR fixes the problem by switching to our fork of httprouter: https://github.com/gravitational/httprouter, which has one of the proposed patches applied. The fork's main branch is teleport, which is based on tag 1.3.0v we were using.

The fork adds UseRawPath option to the router, which is false by default to maintain backward compatibility.

Here we enable that option for all our usages of the router. To verify the fix to our routing, a single test is added (taken from #11011).

Fixes TEL-Q122-7.

[zmb3]

I see that this will indeed fix the problem, but I can't help but ask if we should really be using httprouter at all? It's clearly no longer maintained, and each new fork we pull in creates additional burden on us.

I'm fine with merging this, but I wonder if we should also evaluate using a different router that is actively maintained.

[Tener]

I see that this will indeed fix the problem, but I can't help but ask if we should really be using httprouter at all? It's clearly no longer maintained, and each new fork we pull in creates additional burden on us.

I'm fine with merging this, but I wonder if we should also evaluate using a different router that is actively maintained.

Indeed the httprouter doesn't look active right now: after 1.3.0 there was ~1 year of commits, but now it is 2 years of silence. Strangely, the PRs I linked were from 2015 and 2017 when the library was in active development. Ignoring these PRs is an argument in favor of moving away from httprouter.

Having said that, I'm worried about the potential costs of the switch. There may be hard-to-find compatibility issues lurking, while the benefits are seemingly few or none?

[codingllama]

Looks reasonable to me, although I didn't go all the way digging on httprouter or UseRawPath.

[codingllama]

Having said that, I'm worried about the potential costs of the switch. There may be hard-to-find compatibility issues lurking, while the benefits are seemingly few or none?

That's true, it's a scary change to make. I think the lesson is to be very careful about the dependencies we pull in, specially far reaching ones like httprouter.

My 2c is that best way to "fix" this is to kill all REST APIs in favor of gRPC. One can dream. :)

@zmb3

[Tener]

My 2c is that best way to "fix" this is to kill all REST APIs in favor of gRPC. One can dream. :)

There are at least two of us dreaming the same dream 😄

[zmb3]

There are at least two three of us dreaming the same dream