Add support for HTTP 'Accept' header to '/webapi/auth/export' endpoint

[fheinecke]

This adds support for the HTTP 'Accept' header to the /webapi/auth/export and /webapi/sites/:site/auth/export endpoints. Clients can specify this header to receive either the current text/plain response body, or a JSON-encoded (quoted) response body.

My use case for this is pulling the Teleport Database Access client CA public cert into a k8s cluster via Kyverno. Kyverno only supports HTTP endpoints that return a properly-formatted JSON response.

This endpoint was originally added to make Teleport setup easier. The PR is intended to extend this.

changelog: Added JSON response support to the /webapi/auth/export public certificate API endpoint.

[kopiczko]

LGTM but the Marshal error should be handled properly as @marcoandredinis noted

[zmb3]

You should probably be using mime.ParseMediaType here.

While using the Accept header is fine, we would be complying with only a small portion of spec as there is a lot of other scope:

• The accept header can specify multiple content types, which is a case you're not handling.
• The accept header can also specify a quality factor, which is another case we're not handling.

We already use the type query parameter to alter the format of the returned data. For example, ?type=windows gets you a binary DER-encoded cert, not a plain text response.

If you combine this new Accept header with type=windows you get some odd results. Not necessarily wrong, but a bit awkward:

 ❯ curl -H 'Accept: application/json' https://proxy.127.0.0.1.nip.io:3080/webapi/auth/export\?type\=windows
"0\ufffd\u0003\ufffd0\ufffd\u0002h\ufffd\u0003\u0002\u0001\u0002\u0002\u0010b\ufffd\ufffdU7\ufffd\u003c=5\ufffd9SU_=!0\r\u0006\t*\ufffdH\ufffd\ufffd\r\u0001\0006\u0003U\u0004\n\u0013\tzac-local1\u00120\u0010\u0006\u0003U\u0004\u0003\u0013\tzac-local100.\u0006\u0003U\u0004\u0005\u0013'1311395565953840698110626561911831421770\u001e\u0017\r220810172452Z\u0017\r320807172452Z0Z1\u00120\u0010\u0006\u0003U\u0004\n\u0013\tzac-local1\u00120\u0010\u0006\u0003U\u0004\u0003\u0013\tzac-local100.\u0006\u0003U\u0004\u0005\u0013'1311395565953840698110626561911831421770\ufffd\u0001\"0\r\u0006\t*\ufffdH\ufffd\ufffd\r\u0001\u0001\u0001\u0005\u0000\u0003\ufffd\u0001\u000f\u00000\ufffd\u0001\n\u0002\ufffd\u0001\u0001\u0000\ufffd_k\tpA\ufffd7\ufffd6|\ufffd³\ufffdz,\ufffd\ufffdm\ufffd޸t\ufffd\ufffd\ufffdq\ufffd\ufffd$\ufffd\u0004\u0019\ufffd\ufffd\ufffd�=.\ufffd\ufffdC\u0007\f\u0019IW\u0019\ufffd\ufffdr\ufffd\ufffdRm7\u000fdl#\ufffd8c\ufffd\f\u001fd\ufffd\ufffd\ufffdq\t\ufffd\"\ufffd\ufffd\ufffdtt-\fy\ufffd[1\ufffd\\$\u0000\ufffd\ufffd]\u0006u\ufffd[3\\vVw3+\ufffdYֵ{\ufffd\ufffd7d\ufffd\ufffd\ufffdiM\u001f\u0017ə\ufffd\ufffdj\ufffd\ufffdaA먷x\ufffdl\ufffd\ufffdg\u001e8B\u003e\u0003LC\r�\ufffd\ufffdM4Ì=\ufffdn\ufffd\ufffd\ufffd\u0015\ufffd\ufffdC\u0002b;\ufffd\ufffd\ufffd\ufffd\ufffdp\ufffd\ufffd\ufffdno\ufffdw\ufffd\u0005\ufffdW6\ufffdTX(\ufffd䴷l\u001bm\ufffdME*XvGa\ufffdo\ufffd\u0010\ufffd\u000eD|\ufffdܑG\ufffd\ufffdŞ_\ufffdp\ufffd\ufffd'\ufffd\ufffd\ufffd\ufffd)1\ufffd\ufffd\ufffde;�n\ufffd\ufffd\ufffd\ufffd/{\ufffd0ޣ\ufffd͛c\ufffd`\ufffd\ufffd\u0002\u0003\u0001\u0000\u0001\ufffdB0@0\u000e\u0006\u0003U\u001d\u000f\u0001\u0001\ufffd\u0004\u0004\u0003\u0002\u0001\ufffd0\u000f\u0006\u0003U\u001d\u0013\u0001\u0001\ufffd\u0004\u00050\u0003\u0001\u0001\ufffd0\u001d\u0006\u0003U\u001d\u000e\u0004\u0016\u0004\u0014\ufffd-\u000e\ufffd\ufffdA\ufffdC\u00076@(@Gd\u0007\ufffd\ufffd\u0005\ufffd0\r\u0006\t*\ufffdH\ufffd\ufffd\r\u0001\u0001\u000b\u0005\u0000\u0003\ufffd\u0001\u0001\u0000F\ufffd\ufffd\ufffd˧\ufffd\ufffd\ufffd`J\ufffd\ufffd\ufffdש(UB\ufffd\ufffd\ufffd'\ufffd\u0026\ufffd~\ufffdǽw\u0012\ufffd\ufffd\ufffd6*\ufffd*\ufffd\u003e\ufffd\u0006\ufffd\ufffd\u0014\ufffd\ufffdq=\u0012\ufffd2\ufffd\ufffd\ufffd\ufffd:e\ufffd\ufffd\ufffd\ufffdDV\ufffda\ufffd:\ufffdC(]\ufffd\ufffd\u0013\ufffdu,\ufffd\ufffd2fT\ufffd\ufffd\ufffd\u0005\ufffd\ufffdN\ufffd\ufffd\u0010\ufffdU\ufffd\ufffd\ufffd\ufffd\ufffd\ufffdMl\ufffd=\ufffd\u001fg0\u0012\ufffd\ufffd\ufffdm\ufffd\ufffd\ufffd\ufffde܉\ufffdm\u0016\ufffd\ufffdOID\ufffd\ufffdk\u001f\ufffd\ufffdnW\ufffd�\ufffd\ufffd\ufffd\ufffd\ufffd}\ufffd\uff\ufffd\ufffd!ҙ\u0000\u0001,R\ufffd\ufffd\u000b\u001a\ufffdͮA\ufffd٦\ufffd\ufffd\\iZ\ufffd}X\ufffd\ufffd!w\ufffdE\ufffd\ufffd䕟\ufffd\ufffd\ufffd\ufffdԆIΨ\ufffd\ufffdK:޷\ufffd\ufffdn\ufffdF\ufffd6\ufffd\ufffd\u0011K\ufffd\ufffdx\"P\ufffd\u0003\"b\ufffd{I\ufffd\ufffd\u0012{R`\u0015\u0018\ufffd\u00146'\ufffd\ufffd\ufffd\ufffdـ\ufffd"%

Would it be simpler to just add a new value for the type param?

$ curl example.com/webapi/auth/export?type=dbjson
{
  ...
}

[fheinecke]

You should probably be using mime.ParseMediaType here.

Thank you! I was looking for a function to handle this but couldn't find one.

we would be complying with only a small portion of spec

I considered this while writing this logic, but I think it's fine for spec compliance and at the very least shouldn't break anything. According to the spec (here), the header relays preference rather than a requirement. The spec also says that the quality factor "should" be respected, so it's not a hard requirement.

Would it be simpler to just add a new value for the type param?

I can certainly do that, and will defer to whichever solution you prefer. I the current approach will solve this issue for every cert type, but could possibly break existing clients if they're sending an application/json Accept header but still expecting a text/plain response.

Let me know which route you'd prefer.

[zmb3]

Thank you! I was looking for a function to handle this but couldn't find one.

It was good timing, I just (re)discovered this function earlier this week looking at some CSRF mitigations.

Let me know which route you'd prefer.

I don't have a strong preference either way, as I suspect this will be rarely used.

Technically with your current implementation a client could say they accept both XML and JSON and we'd send them plain text since JSON wasn't the first value. Not optimal, but not something I'm particularly concerned about either.

[fheinecke]

I personally would see value in exporting some of the other types as valid JSON, but I could also add *-json types for them as well.

as I suspect this will be rarely used

100% agree. It's great specifically for when you're:

• Using an in-cluster database, and
• You already have a policy/scripting tool like Kyverno deployed, and
• Want to 100% end to end "gitops" database deployments

Not a lot of use outside of this IMO.

Technically with your current implementation a client could say they accept both XML and JSON and we'd send them plain text since JSON wasn't the first value

I see your point. Maybe I should look over all provided values until a supported MIME type is found? And if none is found, then send the default of text/plain. How does this sound?

[zmb3]

That works for me!

[zmb3]

Might be overkill, but http.Error is going to send a plain text response. Should we also send errors in JSON if the client requested JSON?

[zmb3]

This will work for:

Accept: content-type1
Accept: content-type2

But will it work for:

Accept: content-type1, content-type2

??

Might need to use something like req.Header().Values("Accept")

[fheinecke]

I checked the source for MIMEHeader.Values() (which is what implements req.Header().Values("Accept") and it basically just does req.Header()["Accept"], with no additional splitting.

That said, you're right about splitting the values. I misread the docs and thought this was handled automatically.

[zmb3]

Can you use httptest.NewRecorder as the responsewriter instead of making your own type?

[fheinecke]

Oh that's great. I was looking for a built-in implementation but couldn't find one.

[marcoandredinis]

💅 I think we can swap these two assignments.
Otherwise, we can end up with two text/plain (which is not a big deal, anyway).

[marcoandredinis]

Suggested change

	// Default to text/plain
	case "text/plain":
	return []byte(responseBody), "text/plain; charset=utf-8", trackedErr
	}
	// Default to text/plain
	case default:
	return []byte(responseBody), "text/plain; charset=utf-8", trackedErr
	}

Or just remove it from the switch case and always return it as text/plain

[marcoandredinis]

The error message is not very helpful for the end user (which would be fine), but, as I said above, you can move the text/plain branch and use it as the default condition.