Skip to content

Commit

Permalink
test: remove t.Parallel() from tests that use the same cache and key
Browse files Browse the repository at this point in the history
  • Loading branch information
zepatrik authored and aeneasr committed Dec 13, 2022
1 parent 32aa172 commit 7017fdf
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 22 deletions.
2 changes: 1 addition & 1 deletion pipeline/mutate/mutator_header.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ func (a *MutatorHeader) WithCache(t *template.Template) {
a.t = t
}

func (a *MutatorHeader) Mutate(r *http.Request, session *authn.AuthenticationSession, config json.RawMessage, rl pipeline.Rule) error {
func (a *MutatorHeader) Mutate(_ *http.Request, session *authn.AuthenticationSession, config json.RawMessage, rl pipeline.Rule) error {
cfg, err := a.config(config)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion pipeline/mutate/mutator_id_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func (a *MutatorIDToken) Mutate(r *http.Request, session *authn.AuthenticationSe
}

templateClaims = b.Bytes()
if err := json.NewDecoder(&b).Decode(&claims); err != nil {
if err := json.Unmarshal(templateClaims, &claims); err != nil {
return errors.WithStack(err)
}
}
Expand Down
26 changes: 6 additions & 20 deletions pipeline/mutate/mutator_id_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,58 +238,48 @@ func TestMutatorIDToken(t *testing.T) {
}

session := &authn.AuthenticationSession{Subject: "foo", Extra: map[string]interface{}{"bar": "baz"}}
config := json.RawMessage([]byte(`{"ttl": "3s", "claims": "{\"foo\": \"{{ print .Extra.bar }}\", \"aud\": [\"foo\"]}", "jwks_url": "file://../../test/stub/jwks-ecdsa.json"}`))
config := json.RawMessage(`{"ttl": "100ms", "claims": "{\"foo\": \"{{ print .Extra.bar }}\", \"aud\": [\"foo\"]}", "jwks_url": "file://../../test/stub/jwks-ecdsa.json"}`)

t.Run("subcase=different tokens because expired", func(t *testing.T) {
t.Parallel()
config, _ := sjson.SetBytes(config, "ttl", "100ms")
config, _ := sjson.SetBytes(config, "ttl", "1ms")
prev := mutate(t, *session, config)
time.Sleep(time.Millisecond * 90)
time.Sleep(time.Millisecond)
assert.NotEqual(t, prev, mutate(t, *session, config))
})

t.Run("subcase=same tokens because expired is long enough", func(t *testing.T) {
t.Parallel()
prev := mutate(t, *session, config)
time.Sleep(time.Second) // give the cache buffers some time
time.Sleep(10 * time.Millisecond) // give the cache buffers some time
assert.Equal(t, prev, mutate(t, *session, config))
})

t.Run("subcase=different tokens because expired is long but was reached", func(t *testing.T) {
t.Parallel()
prev := mutate(t, *session, config)
time.Sleep(time.Second * 3) // give the cache buffers some time
time.Sleep(150 * time.Millisecond) // give the cache buffers some time
assert.NotEqual(t, prev, mutate(t, *session, config))
})

t.Run("subcase=different tokens because different subjects", func(t *testing.T) {
t.Parallel()
prev := mutate(t, *session, config)
time.Sleep(time.Second)
s := *session
s.Subject = "not-foo"
assert.NotEqual(t, prev, mutate(t, s, config))
})

t.Run("subcase=different tokens because session extra changed", func(t *testing.T) {
t.Parallel()
prev := mutate(t, *session, config)
time.Sleep(time.Second)
s := *session
s.Extra = map[string]interface{}{"bar": "not-baz"}
assert.NotEqual(t, prev, mutate(t, s, config))
})

t.Run("subcase=different tokens because claim options changed", func(t *testing.T) {
t.Parallel()
prev := mutate(t, *session, config)
time.Sleep(time.Second)
config := json.RawMessage([]byte(`{"ttl": "3s", "claims": "{\"foo\": \"{{ print .Extra.bar }}\", \"aud\": [\"not-foo\"]}", "jwks_url": "file://../../test/stub/jwks-ecdsa.json"}`))
config := json.RawMessage(`{"ttl": "3s", "claims": "{\"foo\": \"{{ print .Extra.bar }}\", \"aud\": [\"not-foo\"]}", "jwks_url": "file://../../test/stub/jwks-ecdsa.json"}`)
assert.NotEqual(t, prev, mutate(t, *session, config))
})

t.Run("subcase=same tokens because session extra changed but claims ignore the extra claims", func(t *testing.T) {
t.Parallel()
t.Skip("Skipped because cache hit rate is too low, see: https://github.com/ory/oathkeeper/issues/371")

prev := mutate(t, *session, config)
Expand All @@ -300,17 +290,13 @@ func TestMutatorIDToken(t *testing.T) {
})

t.Run("subcase=different tokens because issuer changed", func(t *testing.T) {
t.Parallel()
prev := mutate(t, *session, config)
time.Sleep(time.Second)
config, _ := sjson.SetBytes(config, "issuer_url", "/not-baz/not-bar")
assert.NotEqual(t, prev, mutate(t, *session, config))
})

t.Run("subcase=different tokens because JWKS source changed", func(t *testing.T) {
t.Parallel()
prev := mutate(t, *session, config)
time.Sleep(time.Second)
config, _ := sjson.SetBytes(config, "jwks_url", "file://../../test/stub/jwks-hs.json")
assert.NotEqual(t, prev, mutate(t, *session, config))
})
Expand Down

0 comments on commit 7017fdf

Please sign in to comment.