diff --git a/CHANGELOG.md b/CHANGELOG.md index 622e22f..f37a102 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # CHANGELOG +## 0.12.4 + +- Key changes: + - From now on, `Authorization` header will be considered only if it's of `Bearer` type: + - In practical terms, it means that requests in a misconfigured setup will fail earlier than before; + - There's no change for properly configured setups. +- Runtime and dependencies: + - Go: `1.18.2` -> `1.19.5`; + - Alpine: `3.15.4` -> `3.17.1`; + - `VictoriaMetrics/metrics`: `v1.18.1` -> `v1.23.0`; + - `VictoriaMetrics/metricsql` `v0.43.0` -> `v0.51.1`; + - `coreos/go-oidc/v3`: `v3.2.0` -> `v3.5.0`; + - `rs/zerolog v1.26.1`: -> `v1.28.0`; + - `stretchr/testify`: `v1.7.1` -> `v1.8.1`; + - `urfave/cli/v2`: `v2.6.0` -> `v2.23.7`. + ## 0.12.3 - Key changes: diff --git a/Dockerfile b/Dockerfile index 4ea9344..a84271a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.19.3-alpine3.15 as builder +FROM golang:1.19.5-alpine3.17 as builder ARG COMMIT ARG VERSION diff --git a/README.md b/README.md index 8591598..d5fffc5 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,8 @@ Example of `keycloak + grafana + lfgw` setup is described [here](docs/oidc.md). ### Environment variables +#### Basic settings + | Variable | Default Value | Description | | --------------------------- | ------------- | ------------------------------------------------------------ | | `UPSTREAM_URL` | | Prometheus URL, e.g. `http://prometheus.localhost`. | @@ -43,6 +45,13 @@ Example of `keycloak + grafana + lfgw` setup is described [here](docs/oidc.md). | `OIDC_CLIENT_ID` | | OIDC Client ID (1*) | | `ACL_PATH` | `./acl.yaml` | Path to a file with ACL definitions (OIDC role to namespace bindings). Skipped if `ACL_PATH` is empty (might be useful when autoconfiguration is enabled through `ASSUMED_ROLES=true`). | | `ASSUMED_ROLES` | `false` | In environments, where OIDC-role names match names of namespaces, ACLs can be constructed on the fly (e.g. `["role1", "role2"]` will give access to metrics from namespaces `role1` and `role2`). The roles specified in `acl.yaml` are still considered and get merged with assumed roles. Role names may contain regular expressions, including the admin definition `.*`. | + +(1*): since it's grafana who obtains jwt-tokens in the first place, the specified client id must also be present in the forwarded token (the `aud` claim). + +#### Additional settings + +| Variable | Default Value | Description | +| --------------------------- | ------------- | ------------------------------------------------------------ | | `ENABLE_DEDUPLICATION` | `true` | Whether to enable deduplication, which leaves some of the requests unmodified if they match the target policy. Examples can be found in the "acl.yaml syntax" section. | | `OPTIMIZE_EXPRESSIONS` | `true` | Whether to automatically optimize expressions for non-full access requests. [More details](https://pkg.go.dev/github.com/VictoriaMetrics/metricsql#Optimize) | | `SAFE_MODE` | `true` | Whether to block requests to sensitive endpoints like `/api/v1/admin/tsdb`, `/api/v1/insert`. | @@ -57,9 +66,7 @@ Example of `keycloak + grafana + lfgw` setup is described [here](docs/oidc.md). | `WRITE_TIMEOUT` | `10s` | `WriteTimeout` normally covers the time from the end of the request header read to the end of the response write (a.k.a. the lifetime of the ServeHTTP). [More details](https://blog.cloudflare.com/the-complete-guide-to-golang-net-http-timeouts/) | | `GRACEFUL_SHUTDOWN_TIMEOUT` | `20s` | Maximum amount of time to wait for all connections to be closed. [More details](https://pkg.go.dev/net/http#Server.Shutdown) | -(1*): since it's grafana who obtains jwt-tokens in the first place, the specified client id must also be present in the forwarded token (the `aud` claim). - -### ACL +### ACL syntax The file with ACL definitions (`./acl.yaml` by default) has a simple structure: @@ -106,11 +113,3 @@ Note: a user is free to have multiple roles matching the contents of `acl.yaml`. ## Licensing lfgw code is licensed under MIT, though its dependencies might have other licenses. Please, inspect the modules listed in [go.mod](go.mod) if needed. - -## TODO - -* improve naming; -* log slow requests; -* metrics; -* OIDC callback to support for proxying Prometheus web-interface itself; -* add a helm chart. diff --git a/internal/lfgw/helpers.go b/internal/lfgw/helpers.go index 247ac2f..4f631a5 100644 --- a/internal/lfgw/helpers.go +++ b/internal/lfgw/helpers.go @@ -35,6 +35,11 @@ func (app *application) getRawAccessToken(r *http.Request) (string, error) { t := r.Header.Get(h) if h == "Authorization" { + // Consider only Bearer tokens + if !strings.HasPrefix(t, "Bearer ") { + continue + } + t = strings.TrimPrefix(t, "Bearer ") } diff --git a/internal/lfgw/helpers_test.go b/internal/lfgw/helpers_test.go index b1bc26c..a3f49f8 100644 --- a/internal/lfgw/helpers_test.go +++ b/internal/lfgw/helpers_test.go @@ -1,7 +1,6 @@ package lfgw import ( - "fmt" "net/http" "testing" @@ -15,76 +14,83 @@ func TestGetRawAccessToken(t *testing.T) { logger: &logger, } - testsWithToken := []struct { - name string - header string - want string + userAgentGrafana := "Grafana/8.5.0" + userAgentCurl := "curl/7.81.0" + + tests := []struct { + name string + userAgent string + header string + headerValue string + want string + wantErr error }{ { - name: "X-Forwarded-Access-Token", - header: "X-Forwarded-Access-Token", - want: "FAKE_TOKEN", + name: "X-Forwarded-Access-Token", + userAgent: userAgentGrafana, + header: "X-Forwarded-Access-Token", + headerValue: "FAKE_TOKEN", + want: "FAKE_TOKEN", + wantErr: nil, }, { - name: "X-Auth-Request-Access-Token", - header: "X-Auth-Request-Access-Token", - want: "FAKE_TOKEN", + name: "X-Auth-Request-Access-Token", + userAgent: userAgentGrafana, + header: "X-Auth-Request-Access-Token", + headerValue: "FAKE_TOKEN", + want: "FAKE_TOKEN", + wantErr: nil, }, { - name: "Authorization", - header: "Authorization", - want: "FAKE_TOKEN", + name: "Authorization Bearer", + userAgent: userAgentGrafana, + header: "Authorization", + headerValue: "Bearer FAKE_TOKEN", + want: "FAKE_TOKEN", + wantErr: nil, + }, + { + name: "No token: Authorization Basic", + userAgent: userAgentGrafana, + header: "Authorization", + headerValue: "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==", + want: "", + wantErr: errNoTokenGrafana, }, - } - - for _, tt := range testsWithToken { - t.Run(tt.name, func(t *testing.T) { - r, err := http.NewRequest(http.MethodGet, "/", nil) - if err != nil { - t.Fatal(err) - } - - switch tt.header { - case "Authorization": - r.Header.Set(tt.header, fmt.Sprintf("Bearer %s", tt.want)) - default: - r.Header.Set(tt.header, tt.want) - } - - got, err := app.getRawAccessToken(r) - assert.Nil(t, err) - assert.Equal(t, tt.want, got) - }) - } - - testsNoToken := []struct { - name string - userAgent string - want error - }{ { - name: "Request from Grafana", - userAgent: "Grafana/8.5.0", - want: errNoTokenGrafana, + name: "No token: request from grafana", + userAgent: userAgentGrafana, + header: "", + headerValue: "", + want: "", + wantErr: errNoTokenGrafana, }, { - name: "Request from another client", - userAgent: "curl/7.81.0", - want: errNoToken, + name: "No token: request from curl", + userAgent: userAgentCurl, + header: "", + headerValue: "", + want: "", + wantErr: errNoToken, }, } - for _, tt := range testsNoToken { + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r, err := http.NewRequest(http.MethodGet, "/", nil) if err != nil { t.Fatal(err) } + if tt.header != "" && tt.headerValue != "" { + r.Header.Set(tt.header, tt.headerValue) + } + r.Header.Set("User-Agent", tt.userAgent) + got, err := app.getRawAccessToken(r) - assert.Empty(t, got) - assert.ErrorIs(t, err, tt.want) + assert.ErrorIs(t, err, tt.wantErr) + assert.Equal(t, tt.want, got) }) } }