Remove Link headers for management interface

[felixfontein]

Fixes #253.

I'm not sure that this is the best implementation. Another way would be to add a boolean argument (I don't think this is good, though), or to have a more generic function which is used by HandleFunc and HandleManagementFunc.

Any opinions on this?

[jsha]

I think you could probably remove HandleManagementFunc and just call m.Handle directly:

Suggested change

	wfe.HandleManagementFunc(m, RootCertPath, wfe.handleCert(wfe.ca.GetRootCert, RootCertPath), "GET")
	m.Handle(RootCertPath, wfe.handleCert(wfe.ca.GetRootCert, RootCertPath))

Most of the stuff that's in HandleFunc (and your copy of it in HandleManagementFunc) is specific to quirks of ACME, and we don't really need it in Pebble. For that matter, we probably don't even need most of it for ACME - for instance, in RFC8555 everything is a POST except for new-nonce and directory; we could clean up the code that deals with tying things to specific HTTP methods.

[felixfontein]

It would have to be

Suggested change

	wfe.HandleManagementFunc(m, RootCertPath, wfe.handleCert(wfe.ca.GetRootCert, RootCertPath), "GET")
	wfe.HandleManagementFunc(m, RootCertPath, wfe.handleCert(wfe.ca.GetRootCert, RootCertPath), "GET")
	m.Handle(RootCertPath, http.StripPrefix(RootCertPath, wfeHandlerFunc(wfe.handleCert(wfe.ca.GetRootCert, RootCertPath))))

StripPrefix is needed since otherwise RootCertPath needs to cut off the path prefix itself, and the wfeHandlerFunc() cast is needed since otherwise there's a compile error. While this works, it is pretty explicit (and that has to be repeated for every management endpoint). Having a small wfe.HandleManagementFunc like

func (wfe *WebFrontEndImpl) HandleManagementFunc(
	mux *http.ServeMux,
	pattern string,
	handler wfeHandlerFunc) {
	mux.Handle(pattern, http.StripPrefix(pattern, handler))
}

makes this already much nicer.

One disadvantage this approach has is that the method checking is gone, i.e. the handlers reply to all HTTP methods independently of HTTP semantics. Also, there's no longer a no-cache header automatically added.

[jsha]

Your simplified wrapper function looks nicer. I'm not too worried about losing the method checking and no-cache header.

[felixfontein]

I've updated the code with the simplified wrapper.

[golangcibot]

handler can be net/http.Handler (from interfacer)

[felixfontein]

When I do that, I get

wfe/wfe.go:359:58: cannot use wfe.handleCert(wfe.ca.GetRootCert, RootCertPath) (type func(context.Context, http.ResponseWriter, *http.Request)) as type http.Handler in argument to wfe.HandleManagementFunc:
	func(context.Context, http.ResponseWriter, *http.Request) does not implement http.Handler (missing ServeHTTP method)
wfe/wfe.go:360:56: cannot use wfe.handleKey(wfe.ca.GetRootKey, rootKeyPath) (type func(context.Context, http.ResponseWriter, *http.Request)) as type http.Handler in argument to wfe.HandleManagementFunc:
	func(context.Context, http.ResponseWriter, *http.Request) does not implement http.Handler (missing ServeHTTP method)
wfe/wfe.go:361:66: cannot use wfe.handleCert(wfe.ca.GetIntermediateCert, intermediateCertPath) (type func(context.Context, http.ResponseWriter, *http.Request)) as type http.Handler in argument to wfe.HandleManagementFunc:
	func(context.Context, http.ResponseWriter, *http.Request) does not implement http.Handler (missing ServeHTTP method)
wfe/wfe.go:362:64: cannot use wfe.handleKey(wfe.ca.GetIntermediateKey, intermediateKeyPath) (type func(context.Context, http.ResponseWriter, *http.Request)) as type http.Handler in argument to wfe.HandleManagementFunc:
	func(context.Context, http.ResponseWriter, *http.Request) does not implement http.Handler (missing ServeHTTP method)

[jsha]

Yeah I think golangcibot may be confused here and can probably be ignored.

That said, Travis jobs are failing, so let's see whether golangcibot still reports this once those tests are fixed.

[felixfontein]

Both Travis and AppVeyor fail because of

$ golangci-lint run
wfe/wfe.go:227:2: `handler` can be `net/http.Handler` (interfacer)
	handler wfeHandlerFunc) {
	^

So exactly the same "problem". Every other test passed.

[jsha]

Can you add interfacer to the disable list in .golangci.yml?

[felixfontein]

That sounds too general for me. I've first tried to disable this particular test for this line (according to https://github.com/golangci/golangci-lint#nolint). I'm not sure whether it's the correct line, but since GolangCI seems already to be happy, I guess it was the correct one :)

[felixfontein]

@jsha thanks for handling this!