Adding basic auth check

[samber]

Example:

# Caddyfile

  prometheus {
    basicauth my_username1 my_password1
    basicauth my_username2 my_password2
  }

#
# valid
#
$ curl localhost:80/metrics -u my_username1:my_password1
# HELP go_threads Number of OS threads created.
# TYPE go_threads gauge
go_threads 9
...

#
# invalid
#
$ curl localhost:80/metrics -u foo:bar -i
HTTP/1.1 401 Unauthorized
Content-Type: text/plain; charset=utf-8
Www-Authenticate: Basic realm="Restricted area"
X-Content-Type-Options: nosniff
Date: Tue, 13 Mar 2018 17:15:08 GMT
Content-Length: 13

Unauthorized

[samber]

If this gets merged, what is the process for releasing master in the Caddy official build server ?

[miekg]

I need to do click some buttons; which is manual and I always forget. So your better off building Caddy your self.

But thanks for your PR! One thing I don't understand is that this doesn't add HTTPS so you can easily sniff the password as it is plain text; I don't see how this improves security?

[samber]

Fixed tests.

In the following example, you can use both HTTPS and basic auth:

:443 {

  tls self_signed

  prometheus {
    use_caddy_addr
    basicauth prometheus foobar
  }

  ...

}

On a server, I use Caddy as a gateway and need to scrape metrics from outside.

I feel safer to expose my /metrics path on internet with basic auth, than using a random path such as /a/b/c/d/metrics to protect myself against data leak. ;)

[miekg]

[ Quoting notifications@github.com in "Re: [miekg/caddy-prometheus] Adding..." ]

Fixed tests.

In the following example, you can use both HTTPS and basic auth:

:443 {

 tls self_signed

 prometheus {
   basicauth prometheus foobar
 }

 ...

}

On a server, I use Caddy as a gateway and need to scrape metrics from outside.

I feel safer to expose my /metrics path on internet with basic auth, than using a random path such as /a/b/c/d/metrics to protect myself against data leak. ;)

ah cool. This setup should be documented, or better yet, can we error if tls is not enabled?

[samber]

IMHO, an error looks pretty aggressive and may break existing config. What about a simple warning ?

Would you like to open a separated issue to discuss about that ?

I need to dive more deeply into Caddy codebase, but it would be awesome to reuse an existing caddy vhost, instead of creating a new endpoint with http.ListenAndServe() from this plugin: tls, ip filtering and other cool directives for free.

[miekg]

[ Quoting notifications@github.com in "Re: [miekg/caddy-prometheus] Adding..." ]

IMHO, an error looks pretty aggressive and may break existing config.

How so? This syntax is new, so no one can't be using it. I think allowing a config that does not add any security is strange and we should just block it by erroring (if possible; not sure if caddy allows you to see other plugins).

But yes, we could discuss this in another issue/PR; not that I do care about this and gladly make a breaking change to avoid people from shooting themselves in the foot.

I need to dive more deeply into Caddy codebase, but it would be awesome to reuse an existing caddy vhost, instead of creating a new endpoint with http.ListenAndServe() from this plugin: tls, ip filtering and other cool directives for free.

Oh yes, that would be even better

/Miek

-- Miek Gieben