Skip to content

Commit b5d4d88

Browse files
authored
feat: preserve_host feature for oauth2_introspect, better tracing, introspection prefixes (#1131)
This patch additionally allows selecting between the two authenticators based on a prefix to the token.
1 parent 1329413 commit b5d4d88

38 files changed

+266
-172
lines changed

.docker/Dockerfile-build

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Workaround for https://github.com/GoogleContainerTools/distroless/issues/1342
2-
FROM golang:1.20-bullseye AS builder
2+
FROM golang:1.21-bullseye AS builder
33

44
WORKDIR /go/src/github.com/ory/oathkeeper
55

.github/workflows/ci.yml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ jobs:
3737
key: ${{ needs.sdk-generate.outputs.sdk-cache-key }}
3838
- uses: actions/setup-go@v2
3939
with:
40-
go-version: "1.20"
40+
go-version: "1.21"
4141
- run: go list -json > go.list
4242
- name: Run nancy
4343
uses: sonatype-nexus-community/[email protected]
@@ -70,7 +70,7 @@ jobs:
7070
- uses: ory/ci/checkout@master
7171
- uses: actions/setup-go@v2
7272
with:
73-
go-version: "1.20"
73+
go-version: "1.21"
7474
- run: |
7575
./test/${{ matrix.name }}/run.sh
7676
@@ -84,6 +84,9 @@ jobs:
8484
with:
8585
token: ${{ secrets.ORY_BOT_PAT }}
8686
output-dir: docs/oathkeeper/cli
87+
- uses: actions/setup-go@v2
88+
with:
89+
go-version: "1.21"
8790

8891
changelog:
8992
name: Generate changelog

.github/workflows/format.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ jobs:
1111
- uses: actions/checkout@v3
1212
- uses: actions/setup-go@v3
1313
with:
14-
go-version: "1.20"
14+
go-version: "1.21"
1515
- run: make format
1616
- name: Indicate formatting issues
1717
run: git diff HEAD --exit-code --color

.github/workflows/licenses.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jobs:
1414
- uses: actions/checkout@v2
1515
- uses: actions/setup-go@v2
1616
with:
17-
go-version: "1.20"
17+
go-version: "1.21"
1818
- uses: actions/setup-node@v2
1919
with:
2020
node-version: "18"

Makefile

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,12 @@ install:
8484
docker:
8585
DOCKER_BUILDKIT=1 DOCKER_CONTENT_TRUST=1 docker build -t oryd/oathkeeper:${IMAGE_TAG} --progress=plain -f .docker/Dockerfile-build .
8686

87+
.PHONY: docker-k3d
88+
docker-k3d:
89+
CGO_ENABLED=0 GOOS=linux go build
90+
DOCKER_BUILDKIT=1 DOCKER_CONTENT_TRUST=1 docker build -t k3d-localhost:5111/oryd/oathkeeper:dev --push -f .docker/Dockerfile-distroless-static .
91+
rm oathkeeper
92+
8793
docs/cli: .bin/clidoc
8894
clidoc .
8995

api/credential_doc.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
package api
55

6+
//lint:file-ignore U1000 Used to generate Swagger/OpenAPI definitions.
7+
68
// swagger:model jsonWebKeySet
79
type swaggerJSONWebKeySet struct {
810
// The value of the "keys" parameter is an array of JWK values. By

api/health.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
package api
55

6+
//lint:file-ignore U1000 Used to generate Swagger/OpenAPI definitions.
7+
68
// Alive returns an ok status if the instance is ready to handle HTTP requests.
79
//
810
// swagger:route GET /health/alive api isInstanceAlive

api/rule_doc.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
package api
55

6+
//lint:file-ignore U1000 Used to generate Swagger/OpenAPI definitions.
7+
68
import "github.com/ory/oathkeeper/rule"
79

810
// A rule

cmd/root_test.go

Lines changed: 5 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,30 +20,12 @@ import (
2020

2121
var apiPort, proxyPort int
2222

23-
func freePort() (int, int) {
24-
var err error
25-
r := make([]int, 2)
26-
27-
if r[0], err = freeport.GetFreePort(); err != nil {
28-
panic(err.Error())
29-
}
30-
31-
tries := 0
32-
for {
33-
r[1], err = freeport.GetFreePort()
34-
if r[0] != r[1] {
35-
break
36-
}
37-
tries++
38-
if tries > 10 {
39-
panic("Unable to find free port")
40-
}
41-
}
42-
return r[0], r[1]
43-
}
44-
4523
func init() {
46-
apiPort, proxyPort = freePort()
24+
p, err := freeport.GetFreePorts(2)
25+
if err != nil {
26+
panic(err)
27+
}
28+
apiPort, proxyPort = p[0], p[1]
4729

4830
os.Setenv("SERVE_API_PORT", fmt.Sprintf("%d", apiPort))
4931
os.Setenv("SERVE_PROXY_PORT", fmt.Sprintf("%d", proxyPort))

credentials/fetcher_default.go

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import (
1414
"sync"
1515
"time"
1616

17+
"go.opentelemetry.io/otel/trace"
18+
1719
"github.com/ory/oathkeeper/internal/cloudstorage"
1820

1921
"github.com/pkg/errors"
@@ -49,6 +51,11 @@ type FetcherDefault struct {
4951
mux *blob.URLMux
5052
}
5153

54+
type dependencies interface {
55+
Logger() *logrusx.Logger
56+
Tracer() trace.Tracer
57+
}
58+
5259
type FetcherOption func(f *FetcherDefault)
5360

5461
func WithURLMux(mux *blob.URLMux) FetcherOption {
@@ -60,15 +67,18 @@ func WithURLMux(mux *blob.URLMux) FetcherOption {
6067
// - cancelAfter: If reached, the fetcher will stop waiting for responses and return an error.
6168
// - waitForResponse: While the fetcher might stop waiting for responses, we will give the server more time to respond
6269
// and add the keys to the registry unless waitForResponse is reached in which case we'll terminate the request.
63-
func NewFetcherDefault(l *logrusx.Logger, cancelAfter time.Duration, ttl time.Duration, opts ...FetcherOption) *FetcherDefault {
70+
func NewFetcherDefault(d dependencies, cancelAfter time.Duration, ttl time.Duration, opts ...FetcherOption) *FetcherDefault {
6471
f := &FetcherDefault{
6572
cancelAfter: cancelAfter,
66-
l: l,
73+
l: d.Logger(),
6774
ttl: ttl,
6875
keys: make(map[string]jose.JSONWebKeySet),
6976
fetchedAt: make(map[string]time.Time),
70-
client: httpx.NewResilientClient(httpx.ResilientClientWithConnectionTimeout(15 * time.Second)).StandardClient(),
71-
mux: cloudstorage.NewURLMux(),
77+
client: httpx.NewResilientClient(
78+
httpx.ResilientClientWithConnectionTimeout(15*time.Second),
79+
httpx.ResilientClientWithTracer(d.Tracer()),
80+
).StandardClient(),
81+
mux: cloudstorage.NewURLMux(),
7282
}
7383
for _, o := range opts {
7484
o(f)
@@ -94,8 +104,6 @@ func (s *FetcherDefault) ResolveSets(ctx context.Context, locations []url.URL) (
94104
}
95105

96106
func (s *FetcherDefault) fetchParallel(ctx context.Context, locations []url.URL) error {
97-
ctx, cancel := context.WithTimeout(ctx, s.cancelAfter)
98-
defer cancel()
99107
errs := make(chan error)
100108
done := make(chan struct{})
101109

@@ -112,12 +120,14 @@ func (s *FetcherDefault) fetchParallel(ctx context.Context, locations []url.URL)
112120
}
113121
}()
114122

115-
go s.resolveAll(done, errs, locations)
123+
go s.resolveAll(ctx, done, errs, locations)
116124

117125
select {
118126
case <-ctx.Done():
119-
s.l.WithError(ctx.Err()).Errorf("Ignoring JSON Web Keys from at least one URI because the request timed out waiting for a response.")
120127
return ctx.Err()
128+
case <-time.After(s.cancelAfter):
129+
s.l.Errorf("Ignoring JSON Web Keys from at least one URI because the request timed out waiting for a response.")
130+
return context.DeadlineExceeded
121131
case <-done:
122132
// We're done!
123133
return nil
@@ -183,25 +193,25 @@ func (s *FetcherDefault) set(locations []url.URL, staleKeyAcceptable bool) []jos
183193
}
184194

185195
func (s *FetcherDefault) isKeyExpired(expiredKeyAcceptable bool, fetchedAt time.Time) bool {
186-
return expiredKeyAcceptable == false &&
187-
fetchedAt.Add(s.ttl).Before(time.Now().UTC())
196+
return !expiredKeyAcceptable && time.Since(fetchedAt) > s.ttl
188197
}
189198

190-
func (s *FetcherDefault) resolveAll(done chan struct{}, errs chan error, locations []url.URL) {
199+
func (s *FetcherDefault) resolveAll(ctx context.Context, done chan struct{}, errs chan error, locations []url.URL) {
200+
ctx = context.WithoutCancel(ctx)
191201
var wg sync.WaitGroup
192202

193203
for _, l := range locations {
194204
l := l
195205
wg.Add(1)
196-
go s.resolve(&wg, errs, l)
206+
go s.resolve(ctx, &wg, errs, l)
197207
}
198208

199209
wg.Wait()
200210
close(done)
201211
close(errs)
202212
}
203213

204-
func (s *FetcherDefault) resolve(wg *sync.WaitGroup, errs chan error, location url.URL) {
214+
func (s *FetcherDefault) resolve(ctx context.Context, wg *sync.WaitGroup, errs chan error, location url.URL) {
205215
defer wg.Done()
206216
var (
207217
reader io.ReadCloser
@@ -210,7 +220,6 @@ func (s *FetcherDefault) resolve(wg *sync.WaitGroup, errs chan error, location u
210220

211221
switch location.Scheme {
212222
case "azblob", "gs", "s3":
213-
ctx := context.Background()
214223
bucket, err := s.mux.OpenBucket(ctx, location.Scheme+"://"+location.Host)
215224
if err != nil {
216225
errs <- errors.WithStack(herodot.
@@ -255,7 +264,19 @@ func (s *FetcherDefault) resolve(wg *sync.WaitGroup, errs chan error, location u
255264
defer reader.Close()
256265

257266
case "http", "https":
258-
res, err := s.client.Get(location.String())
267+
req, err := http.NewRequestWithContext(ctx, "GET", location.String(), nil)
268+
if err != nil {
269+
errs <- errors.WithStack(herodot.
270+
ErrInternalServerError.
271+
WithReasonf(
272+
`Unable to fetch JSON Web Keys from location "%s" because "%s".`,
273+
location.String(),
274+
err,
275+
),
276+
)
277+
return
278+
}
279+
res, err := s.client.Do(req)
259280
if err != nil {
260281
errs <- errors.WithStack(herodot.
261282
ErrInternalServerError.

credentials/fetcher_default_test.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/sirupsen/logrus"
1616
"github.com/stretchr/testify/assert"
1717
"github.com/stretchr/testify/require"
18+
"go.opentelemetry.io/otel/trace"
1819

1920
"github.com/ory/x/logrusx"
2021

@@ -33,6 +34,16 @@ var sets = [...]json.RawMessage{
3334
json.RawMessage(`invalid json ¯\_(ツ)_/¯`),
3435
}
3536

37+
type reg struct{}
38+
39+
func (*reg) Logger() *logrusx.Logger {
40+
return logrusx.New("", "", logrusx.ForceLevel(logrus.DebugLevel))
41+
}
42+
43+
func (*reg) Tracer() trace.Tracer {
44+
return trace.NewNoopTracerProvider().Tracer("")
45+
}
46+
3647
func TestFetcherDefault(t *testing.T) {
3748
t.Parallel()
3849

@@ -42,7 +53,7 @@ func TestFetcherDefault(t *testing.T) {
4253

4354
l := logrusx.New("", "", logrusx.ForceLevel(logrus.DebugLevel))
4455
w := herodot.NewJSONWriter(l)
45-
s := NewFetcherDefault(l, maxWait, JWKsTTL)
56+
s := NewFetcherDefault(&reg{}, maxWait, JWKsTTL)
4657

4758
timeOutServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
4859
time.Sleep(timeoutServerDelay)
@@ -177,7 +188,7 @@ func TestFetcherDefault(t *testing.T) {
177188
t.Run("name=should fetch from s3 object storage", func(t *testing.T) {
178189
t.Parallel()
179190
ctx := context.Background()
180-
s := NewFetcherDefault(l, maxWait, JWKsTTL, WithURLMux(cloudstorage.NewTestURLMux(t)))
191+
s := NewFetcherDefault(&reg{}, maxWait, JWKsTTL, WithURLMux(cloudstorage.NewTestURLMux(t)))
181192

182193
key, err := s.ResolveKey(ctx, []url.URL{
183194
*urlx.ParseOrPanic("s3://oathkeeper-test-bucket/path/prefix/jwks.json"),
@@ -189,7 +200,7 @@ func TestFetcherDefault(t *testing.T) {
189200
t.Run("name=should fetch from gs object storage", func(t *testing.T) {
190201
t.Parallel()
191202
ctx := context.Background()
192-
s := NewFetcherDefault(l, maxWait, JWKsTTL, WithURLMux(cloudstorage.NewTestURLMux(t)))
203+
s := NewFetcherDefault(&reg{}, maxWait, JWKsTTL, WithURLMux(cloudstorage.NewTestURLMux(t)))
193204

194205
key, err := s.ResolveKey(ctx, []url.URL{
195206
*urlx.ParseOrPanic("gs://oathkeeper-test-bucket/path/prefix/jwks.json"),
@@ -201,7 +212,7 @@ func TestFetcherDefault(t *testing.T) {
201212
t.Run("name=should fetch from azure object storage", func(t *testing.T) {
202213
t.Parallel()
203214
ctx := context.Background()
204-
s := NewFetcherDefault(l, maxWait, JWKsTTL, WithURLMux(cloudstorage.NewTestURLMux(t)))
215+
s := NewFetcherDefault(&reg{}, maxWait, JWKsTTL, WithURLMux(cloudstorage.NewTestURLMux(t)))
205216

206217
jwkKey, err := s.ResolveKey(ctx, []url.URL{
207218
*urlx.ParseOrPanic("azblob://path/prefix/jwks.json"),

credentials/signer_default_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,14 @@ import (
1515
"github.com/stretchr/testify/require"
1616

1717
"github.com/ory/oathkeeper/x"
18-
"github.com/ory/x/logrusx"
1918
)
2019

2120
type defaultSignerMockRegistry struct {
2221
f Fetcher
2322
}
2423

2524
func newDefaultSignerMockRegistry() *defaultSignerMockRegistry {
26-
return &defaultSignerMockRegistry{f: NewFetcherDefault(logrusx.New("", ""), time.Millisecond*100, time.Millisecond*500)}
25+
return &defaultSignerMockRegistry{f: NewFetcherDefault(&reg{}, time.Millisecond*100, time.Millisecond*500)}
2726
}
2827

2928
func (m *defaultSignerMockRegistry) CredentialsFetcher() Fetcher {
@@ -42,7 +41,7 @@ func TestSignerDefault(t *testing.T) {
4241
token, err := signer.Sign(context.Background(), x.ParseURLOrPanic(src), jwt.MapClaims{"sub": "foo"})
4342
require.NoError(t, err)
4443

45-
fetcher := NewFetcherDefault(logrusx.New("", ""), time.Second, time.Second)
44+
fetcher := NewFetcherDefault(&reg{}, time.Second, time.Second)
4645

4746
_, err = verify(t, token, fetcher, src)
4847
require.NoError(t, err)

doc_swagger.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
package main
55

6+
//lint:file-ignore U1000 Used to generate Swagger/OpenAPI definitions.
7+
68
// The standard error format
79
// swagger:model genericError
810
type genericError struct {

driver/configuration/provider_koanf_public_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/rs/cors"
1414
"github.com/stretchr/testify/assert"
1515
"github.com/stretchr/testify/require"
16+
"go.opentelemetry.io/otel/trace"
1617

1718
"github.com/ory/x/configx"
1819
"github.com/ory/x/logrusx"
@@ -47,7 +48,7 @@ func TestPipelineConfig(t *testing.T) {
4748
p := setup(t)
4849

4950
require.NoError(t, p.PipelineConfig("authenticators", "oauth2_introspection", nil, &res))
50-
assert.JSONEq(t, `{"cache":{"enabled":false, "max_cost":1000},"introspection_url":"https://override/path","pre_authorization":{"client_id":"some_id","client_secret":"some_secret","enabled":true,"audience":"some_audience","scope":["foo","bar"],"token_url":"https://my-website.com/oauth2/token"},"retry":{"max_delay":"100ms", "give_up_after":"1s"},"scope_strategy":"exact"}`, string(res), "%s", res)
51+
assert.JSONEq(t, `{"cache":{"enabled":false, "max_cost":1000},"introspection_url":"https://override/path","preserve_host":false,"pre_authorization":{"client_id":"some_id","client_secret":"some_secret","enabled":true,"audience":"some_audience","scope":["foo","bar"],"token_url":"https://my-website.com/oauth2/token"},"retry":{"max_delay":"100ms", "give_up_after":"1s"},"scope_strategy":"exact"}`, string(res), "%s", res)
5152
})
5253

5354
t.Run("case=should setup", func(t *testing.T) {
@@ -247,7 +248,7 @@ func TestKoanfProvider(t *testing.T) {
247248
})
248249

249250
t.Run("authenticator=cookie_session", func(t *testing.T) {
250-
a := authn.NewAuthenticatorCookieSession(p, nil)
251+
a := authn.NewAuthenticatorCookieSession(p, trace.NewNoopTracerProvider())
251252
assert.True(t, p.AuthenticatorIsEnabled(a.GetID()))
252253
require.NoError(t, a.Validate(nil))
253254

@@ -285,7 +286,7 @@ func TestKoanfProvider(t *testing.T) {
285286
})
286287

287288
t.Run("authenticator=oauth2_introspection", func(t *testing.T) {
288-
a := authn.NewAuthenticatorOAuth2Introspection(p, logger)
289+
a := authn.NewAuthenticatorOAuth2Introspection(p, logger, trace.NewNoopTracerProvider())
289290
assert.True(t, p.AuthenticatorIsEnabled(a.GetID()))
290291
require.NoError(t, a.Validate(nil))
291292

@@ -433,7 +434,7 @@ func TestAuthenticatorOAuth2TokenIntrospectionPreAuthorization(t *testing.T) {
433434
{enabled: true, id: "a", secret: "b", turl: "https://some-url", err: false},
434435
} {
435436
t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
436-
a := authn.NewAuthenticatorOAuth2Introspection(p, logrusx.New("", ""))
437+
a := authn.NewAuthenticatorOAuth2Introspection(p, logrusx.New("", ""), trace.NewNoopTracerProvider())
437438

438439
config, _, err := a.Config(json.RawMessage(fmt.Sprintf(`{
439440
"pre_authorization": {

0 commit comments

Comments
 (0)