Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jwks max stale #502

Merged
merged 9 commits into from
May 12, 2022
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ Unreleased changes are available as `avenga/couper:edge` container.
* `backends.<name>.health` variable to access the current health-check state _(subject to change)_
* Log malformed duration settings ([#487](https://github.com/avenga/couper/pull/487))
* `url` attribute could make use of our wildcard pattern `/**` and relative urls in combination with a backend reference ([#480](https://github.com/avenga/couper/pull/480))
* `jwks_max_stale` in [`jwt` block](./docs/REFERENCE.md#jwt-block) ([#502](https://github.com/avenga/couper/pull/502))
* `jwks_ttl`, `jwks_max_stale` and `configuration_max_stale` in [`oidc` block](./docs/REFERENCE.md#oidc-block) ([#502](https://github.com/avenga/couper/pull/502))
* Error handling for `backend`, `backend_openapi_validation` and `backend_timeout` [error types](./docs/ERRORS.md) ([#490](https://github.com/avenga/couper/pull/490))
* `response.bytes` log-field to backend logs if read from body, fallback is the `Content-Length` header ([#494](https://github.com/avenga/couper/pull/494))
* [Error types](./docs/ERRORS.md) `endpoint` and `access_control` ([#500](https://github.com/avenga/couper/pull/500))
Expand Down
20 changes: 9 additions & 11 deletions accesscontrol/jwk/jwks.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/dgrijalva/jwt-go/v4"

"github.com/avenga/couper/config"
jsn "github.com/avenga/couper/json"
)

Expand All @@ -29,12 +30,12 @@ type JWKS struct {
syncedJSON *jsn.SyncedJSON
}

func NewJWKS(uri string, ttl string, transport http.RoundTripper) (*JWKS, error) {
if ttl == "" {
ttl = "1h"
func NewJWKS(uri string, ttl string, maxStale string, transport http.RoundTripper) (*JWKS, error) {
timetolive, err := config.ParseDuration("jwks_ttl", ttl, time.Hour)
if err != nil {
return nil, err
}

timetolive, err := time.ParseDuration(ttl)
maxStaleTime, err := config.ParseDuration("jwks_max_stale", maxStale, time.Hour)
if err != nil {
return nil, err
}
Expand All @@ -46,7 +47,7 @@ func NewJWKS(uri string, ttl string, transport http.RoundTripper) (*JWKS, error)
}

jwks := &JWKS{}
jwks.syncedJSON, err = jsn.NewSyncedJSON(file, "jwks_url", uri, transport, "jwks", timetolive, jwks)
jwks.syncedJSON, err = jsn.NewSyncedJSON(file, "jwks_url", uri, transport, "jwks", timetolive, maxStaleTime, jwks)
return jwks, err
}

Expand Down Expand Up @@ -114,13 +115,10 @@ func (j *JWKS) getKeys(kid string) ([]*JWK, error) {

func (j *JWKS) Data() (*JWKSData, error) {
data, err := j.syncedJSON.Data()
if err != nil {
return nil, err
}

// Ignore backend errors as long as we still get cached (stale) data.
jwksData, ok := data.(*JWKSData)
if !ok {
return nil, fmt.Errorf("data not JWKS data: %#v", data)
return nil, fmt.Errorf("received no valid JWKs data: %#v, %w", data, err)
}

return jwksData, nil
Expand Down
10 changes: 5 additions & 5 deletions accesscontrol/jwk/jwks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func Test_JWKS(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(subT *testing.T) {
_, err := jwk.NewJWKS(tt.url, "", backend)
_, err := jwk.NewJWKS(tt.url, "", "", backend)
if err == nil && tt.error != "" {
subT.Errorf("Missing error:\n\tWant: %v\n\tGot: %v", tt.error, nil)
}
Expand All @@ -64,7 +64,7 @@ func Test_JWKS_Load(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(subT *testing.T) {
jwks, err := jwk.NewJWKS("file:"+tt.file, "", nil)
jwks, err := jwk.NewJWKS("file:"+tt.file, "", "", nil)
helper.Must(err)
_, err = jwks.Data()
if err != nil && tt.expParsed {
Expand Down Expand Up @@ -93,7 +93,7 @@ func Test_JWKS_GetSigKeyForToken(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(subT *testing.T) {
helper := test.New(subT)
jwks, err := jwk.NewJWKS("file:"+tt.file, "", nil)
jwks, err := jwk.NewJWKS("file:"+tt.file, "", "", nil)
helper.Must(err)
_, err = jwks.Data()
helper.Must(err)
Expand Down Expand Up @@ -145,7 +145,7 @@ func Test_JWKS_GetKey(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(subT *testing.T) {
helper := test.New(subT)
jwks, err := jwk.NewJWKS("file:"+tt.file, "", nil)
jwks, err := jwk.NewJWKS("file:"+tt.file, "", "", nil)
helper.Must(err)
_, err = jwks.Data()
helper.Must(err)
Expand Down Expand Up @@ -175,7 +175,7 @@ func Test_JWKS_LoadSynced(t *testing.T) {
io.Copy(writer, bytes.NewReader(f))
}))

jwks, err := jwk.NewJWKS(jwksOrigin.URL, "10s", http.DefaultTransport)
jwks, err := jwk.NewJWKS(jwksOrigin.URL, "10s", "", http.DefaultTransport)
helper.Must(err)

wg := sync.WaitGroup{}
Expand Down
4 changes: 2 additions & 2 deletions accesscontrol/jwt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -811,12 +811,12 @@ func TestJwtConfig(t *testing.T) {
server "test" {}
definitions {
jwt "myac" {
jwks_url = "http://no-back.end"
jwks_url = "file://...",
header = "..."
}
}
`,
`backend error: anonymous_5_8_jwks_url: connecting to anonymous_5_8_jwks_url "no-back.end:80" failed: dial tcp: lookup no-back.end`,
"",
},
{
"signature_algorithm + jwks_url",
Expand Down
1 change: 1 addition & 0 deletions config/ac_jwt.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ type JWT struct {
Header string `hcl:"header,optional"`
JWKsURL string `hcl:"jwks_url,optional"`
JWKsTTL string `hcl:"jwks_ttl,optional"`
JWKsMaxStale string `hcl:"jwks_max_stale,optional"`
Key string `hcl:"key,optional"`
KeyFile string `hcl:"key_file,optional"`
Name string `hcl:"name,label"`
Expand Down
3 changes: 3 additions & 0 deletions config/ac_oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ type OIDC struct {
ClientID string `hcl:"client_id"`
ClientSecret string `hcl:"client_secret"`
ConfigurationURL string `hcl:"configuration_url"`
JWKsTTL string `hcl:"jwks_ttl,optional"`
JWKsMaxStale string `hcl:"jwks_max_stale,optional"`
Name string `hcl:"name,label"`
Remain hcl.Body `hcl:",remain"`
Scope *string `hcl:"scope,optional"`
TokenEndpointAuthMethod *string `hcl:"token_endpoint_auth_method,optional"`
ConfigurationTTL string `hcl:"configuration_ttl,optional"`
ConfigurationMaxStale string `hcl:"configuration_max_stale,optional"`
VerifierMethod string `hcl:"verifier_method,optional"`

// configuration related backends
Expand Down
22 changes: 22 additions & 0 deletions config/duration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package config

import (
"fmt"
"time"
)

func ParseDuration(attribute string, value string, _default time.Duration) (time.Duration, error) {
if value == "" {
return _default, nil
}

duration, err := time.ParseDuration(value)
if err != nil {
return 0, fmt.Errorf("%s: %s", attribute, err)
}
if duration < 0 {
return 0, fmt.Errorf("%s: cannot be negative: %q", attribute, value)
}

return duration, nil
}
39 changes: 39 additions & 0 deletions config/duration_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package config_test

import (
"testing"
"time"

"github.com/avenga/couper/config"
)

func TestParseDuration(t *testing.T) {
tests := []struct {
duration string
_default time.Duration
want time.Duration
err string
}{
{"1s", time.Hour, time.Second, ""},
{"0m", time.Hour, 0, ""},
{"1h1s1m", time.Hour, 3661 * time.Second, ""},
{"", time.Hour, time.Hour, ""},
{"invalid", time.Hour, 0, `my-duration: time: invalid duration "invalid"`},
{"1sec", time.Hour, 0, `my-duration: time: unknown unit "sec" in duration "1sec"`},
{"-3s", time.Hour, 0, `my-duration: cannot be negative: "-3s"`},
}
for _, tt := range tests {
t.Run("", func(subT *testing.T) {
duration, err := config.ParseDuration("my-duration", tt.duration, tt._default)
if duration != tt.want {
subT.Errorf("unexpected duration, want: %q, got: %q", tt.want, duration)
}
if err != nil && err.Error() != tt.err {
subT.Errorf("unexpected error,\n\twant: %q\n\tgot: %q", tt.err, err.Error())
}
if err == nil && tt.err != "" {
subT.Errorf("expected error %q, got: %v", tt.err, nil)
}
})
}
}
2 changes: 1 addition & 1 deletion config/runtime/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ func configureJWKS(jwtConf *config.JWT, confContext *hcl.EvalContext, log *logru
}
}

return jwk.NewJWKS(jwtConf.JWKsURL, jwtConf.JWKsTTL, backend)
return jwk.NewJWKS(jwtConf.JWKsURL, jwtConf.JWKsTTL, jwtConf.JWKsMaxStale, backend)
}

type protectedOptions struct {
Expand Down
9 changes: 7 additions & 2 deletions docs/REFERENCE.md
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ Since responses from endpoints protected by JWT access controls are not publicly
| `beta_roles_map` |object (string)|-| mapping of roles to granted permissions | Non-mapped roles can be assigned with `*` to specific permissions. |`beta_roles_map = { role1 = ["p1", "p2"], role2 = ["p3"], "*" = ["public"] }`|
| `jwks_url` | string | - | URI pointing to a set of [JSON Web Keys (RFC 7517)](https://datatracker.ietf.org/doc/html/rfc7517) | - | `jwks_url = "http://identityprovider:8080/jwks.json"` |
| `jwks_ttl` | [duration](#duration) | `"1h"` | Time period the JWK set stays valid and may be cached. | - | `jwks_ttl = "1800s"` |
| `jwks_max_stale` | [duration](#duration) | `"1h"` | Time period the cached JWK set stays valid after its TTL has passed. | - | `jwks_max_stale = "45m"` |
| `backend` | string| - | [backend reference](#backend-block) for enhancing JWKS requests| - | `backend = "jwks_backend"` |
| `disable_private_caching` | bool | `false` | If set to `true`, Couper does not add the `private` directive to the `Cache-Control` HTTP header field value. | - | - |

Expand Down Expand Up @@ -487,20 +488,24 @@ Like all [Access Control](#access-control) types, the `oidc` block is defined in
|:-----------------------------|:----------------------|:----------------------|:-------------------------------------------------------------------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------------------------------------------|
| `backend` | string | - | [Backend Block Reference](#backend-block) | &#9888; Do not disable the peer certificate validation with `disable_certificate_validation = true`! | - |
| `configuration_url` | string | - | The OpenID configuration URL. | &#9888; required | - |
| `configuration_ttl` | [duration](#duration) | `1h` | The duration to cache the OpenID configuration located at `configuration_url`. | - | `configuration_ttl = "1d"` |
| `token_endpoint_auth_method` | string | `client_secret_basic` | Defines the method to authenticate the client at the token endpoint. | If set to `client_secret_post`, the client credentials are transported in the request body. If set to `client_secret_basic`, the client credentials are transported via Basic Authentication. | - |
| `configuration_ttl` | [duration](#duration) | `"1h"` | The duration to cache the OpenID configuration located at `configuration_url`. | - | `configuration_ttl = "1d"` |
| `configuration_max_stale` | [duration](#duration) | `"1h"` | Duration a cached OpenID configuration stays valid after its TTL has passed. | - | `configuration_max_stale = "2h"` |
| `token_endpoint_auth_method` | string | `"client_secret_basic"` | Defines the method to authenticate the client at the token endpoint. | If set to `client_secret_post`, the client credentials are transported in the request body. If set to `client_secret_basic`, the client credentials are transported via Basic Authentication. | - |
| `redirect_uri` | string | - | The Couper endpoint for receiving the authorization code. | &#9888; required. Relative URL references are resolved against the origin of the current request URL. The origin can be changed with the [`accept_forwarded_url`](#settings-block) attribute if Couper is running behind a proxy. | - |
| `client_id` | string | - | The client identifier. | &#9888; required | - |
| `client_secret` | string | - | The client password. | &#9888; required. | - |
| `scope` | string | - | A space separated list of requested scope values for the access token. | `openid` is automatically added. | `scope = "profile read"` |
| `verifier_method` | string | - | The method to verify the integrity of the authorization code flow | available values: `ccm_s256` (`code_challenge` parameter with `code_challenge_method` `S256`), `nonce` (`nonce` parameter) | `verifier_method = "nonce"` |
| `verifier_value` | string or expression | - | The value of the (unhashed) verifier. | &#9888; required; e.g. using cookie value created with [`oauth2_verifier()` function](#functions) | `verifier_value = request.cookies.verifier` |
| `custom_log_fields` | object | - | Defines log fields for [Custom Logging](LOGS.md#custom-logging). | &#9888; Inherited by nested blocks. | - |
| `jwks_ttl` | [duration](#duration) | `"1h"` | Time period the JWK set stays valid and may be cached. | - | `jwks_ttl = "3h"` |
| `jwks_max_stale` | [duration](#duration) | `"1h"` | Time period the cached JWK set stays valid after its TTL has passed. | - | `jwks_max_stale = "1h30m"` |
| `configuration_backend` | string | - | [Backend Block Reference](#backend-block) | &#9888; Do not disable the peer certificate validation with `disable_certificate_validation = true`! | - |
| `jwks_uri_backend` | string | - | [Backend Block Reference](#backend-block) | &#9888; Do not disable the peer certificate validation with `disable_certificate_validation = true`! | - |
| `token_backend` | string | - | [Backend Block Reference](#backend-block) | &#9888; Do not disable the peer certificate validation with `disable_certificate_validation = true`! | - |
| `userinfo_backend` | string | - | [Backend Block Reference](#backend-block) | &#9888; Do not disable the peer certificate validation with `disable_certificate_validation = true`! | - |


In most cases, referencing one `backend` (backend attribute) for all the backend requests sent by the OIDC client is enough.
You should only use `configuration_backend`, `jwks_uri_backend`, `token_backend` or `userinfo_backend` if you need to configure a specific behaviour for the respective request (e.g. timeouts).

Expand Down
11 changes: 6 additions & 5 deletions handler/transport/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"

"github.com/avenga/couper/config"
"github.com/avenga/couper/config/request"
"github.com/avenga/couper/telemetry"
"golang.org/x/net/http/httpproxy"
Expand Down Expand Up @@ -154,15 +155,15 @@ func (c *Config) WithTarget(scheme, origin, hostname, proxyURL string) *Config {

func (c *Config) WithTimings(connect, ttfb, timeout string, logger *logrus.Entry) *Config {
conf := *c
parseDuration(connect, &conf.ConnectTimeout, logger)
parseDuration(ttfb, &conf.TTFBTimeout, logger)
parseDuration(timeout, &conf.Timeout, logger)
parseDuration(connect, &conf.ConnectTimeout, "connect_timeout", logger)
parseDuration(ttfb, &conf.TTFBTimeout, "ttfb_timeout", logger)
parseDuration(timeout, &conf.Timeout, "timeout", logger)
return &conf
}

// parseDuration sets the target value if the given duration string is not empty.
func parseDuration(src string, target *time.Duration, logger *logrus.Entry) {
d, err := time.ParseDuration(src)
func parseDuration(src string, target *time.Duration, attributeName string, logger *logrus.Entry) {
d, err := config.ParseDuration(attributeName, src, *target)
if err != nil {
logger.WithError(err).Warning("using default timing of ", target, " because an error occured")
}
Expand Down
Loading