Skip to content

Commit

Permalink
feat: preserve_host feature for oauth2_introspect, better tracing, in…
Browse files Browse the repository at this point in the history
…trospection prefixes (#1131)

This patch additionally allows selecting between the two authenticators based on a prefix to the token.
  • Loading branch information
alnr committed Sep 8, 2023
1 parent 1329413 commit b5d4d88
Show file tree
Hide file tree
Showing 38 changed files with 266 additions and 172 deletions.
2 changes: 1 addition & 1 deletion .docker/Dockerfile-build
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Workaround for https://github.com/GoogleContainerTools/distroless/issues/1342
FROM golang:1.20-bullseye AS builder
FROM golang:1.21-bullseye AS builder

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

Expand Down
7 changes: 5 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
key: ${{ needs.sdk-generate.outputs.sdk-cache-key }}
- uses: actions/setup-go@v2
with:
go-version: "1.20"
go-version: "1.21"
- run: go list -json > go.list
- name: Run nancy
uses: sonatype-nexus-community/[email protected]
Expand Down Expand Up @@ -70,7 +70,7 @@ jobs:
- uses: ory/ci/checkout@master
- uses: actions/setup-go@v2
with:
go-version: "1.20"
go-version: "1.21"
- run: |
./test/${{ matrix.name }}/run.sh
Expand All @@ -84,6 +84,9 @@ jobs:
with:
token: ${{ secrets.ORY_BOT_PAT }}
output-dir: docs/oathkeeper/cli
- uses: actions/setup-go@v2
with:
go-version: "1.21"

changelog:
name: Generate changelog
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/format.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: "1.20"
go-version: "1.21"
- run: make format
- name: Indicate formatting issues
run: git diff HEAD --exit-code --color
2 changes: 1 addition & 1 deletion .github/workflows/licenses.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-go@v2
with:
go-version: "1.20"
go-version: "1.21"
- uses: actions/setup-node@v2
with:
node-version: "18"
Expand Down
6 changes: 6 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ install:
docker:
DOCKER_BUILDKIT=1 DOCKER_CONTENT_TRUST=1 docker build -t oryd/oathkeeper:${IMAGE_TAG} --progress=plain -f .docker/Dockerfile-build .

.PHONY: docker-k3d
docker-k3d:
CGO_ENABLED=0 GOOS=linux go build
DOCKER_BUILDKIT=1 DOCKER_CONTENT_TRUST=1 docker build -t k3d-localhost:5111/oryd/oathkeeper:dev --push -f .docker/Dockerfile-distroless-static .
rm oathkeeper

docs/cli: .bin/clidoc
clidoc .

Expand Down
2 changes: 2 additions & 0 deletions api/credential_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

package api

//lint:file-ignore U1000 Used to generate Swagger/OpenAPI definitions.

// swagger:model jsonWebKeySet
type swaggerJSONWebKeySet struct {
// The value of the "keys" parameter is an array of JWK values. By
Expand Down
2 changes: 2 additions & 0 deletions api/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

package api

//lint:file-ignore U1000 Used to generate Swagger/OpenAPI definitions.

// Alive returns an ok status if the instance is ready to handle HTTP requests.
//
// swagger:route GET /health/alive api isInstanceAlive
Expand Down
2 changes: 2 additions & 0 deletions api/rule_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

package api

//lint:file-ignore U1000 Used to generate Swagger/OpenAPI definitions.

import "github.com/ory/oathkeeper/rule"

// A rule
Expand Down
28 changes: 5 additions & 23 deletions cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,12 @@ import (

var apiPort, proxyPort int

func freePort() (int, int) {
var err error
r := make([]int, 2)

if r[0], err = freeport.GetFreePort(); err != nil {
panic(err.Error())
}

tries := 0
for {
r[1], err = freeport.GetFreePort()
if r[0] != r[1] {
break
}
tries++
if tries > 10 {
panic("Unable to find free port")
}
}
return r[0], r[1]
}

func init() {
apiPort, proxyPort = freePort()
p, err := freeport.GetFreePorts(2)
if err != nil {
panic(err)
}
apiPort, proxyPort = p[0], p[1]

os.Setenv("SERVE_API_PORT", fmt.Sprintf("%d", apiPort))
os.Setenv("SERVE_PROXY_PORT", fmt.Sprintf("%d", proxyPort))
Expand Down
51 changes: 36 additions & 15 deletions credentials/fetcher_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"sync"
"time"

"go.opentelemetry.io/otel/trace"

"github.com/ory/oathkeeper/internal/cloudstorage"

"github.com/pkg/errors"
Expand Down Expand Up @@ -49,6 +51,11 @@ type FetcherDefault struct {
mux *blob.URLMux
}

type dependencies interface {
Logger() *logrusx.Logger
Tracer() trace.Tracer
}

type FetcherOption func(f *FetcherDefault)

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

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

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

go s.resolveAll(done, errs, locations)
go s.resolveAll(ctx, done, errs, locations)

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

func (s *FetcherDefault) isKeyExpired(expiredKeyAcceptable bool, fetchedAt time.Time) bool {
return expiredKeyAcceptable == false &&
fetchedAt.Add(s.ttl).Before(time.Now().UTC())
return !expiredKeyAcceptable && time.Since(fetchedAt) > s.ttl
}

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

for _, l := range locations {
l := l
wg.Add(1)
go s.resolve(&wg, errs, l)
go s.resolve(ctx, &wg, errs, l)
}

wg.Wait()
close(done)
close(errs)
}

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

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

case "http", "https":
res, err := s.client.Get(location.String())
req, err := http.NewRequestWithContext(ctx, "GET", location.String(), nil)
if err != nil {
errs <- errors.WithStack(herodot.
ErrInternalServerError.
WithReasonf(
`Unable to fetch JSON Web Keys from location "%s" because "%s".`,
location.String(),
err,
),
)
return
}
res, err := s.client.Do(req)
if err != nil {
errs <- errors.WithStack(herodot.
ErrInternalServerError.
Expand Down
19 changes: 15 additions & 4 deletions credentials/fetcher_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/trace"

"github.com/ory/x/logrusx"

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

type reg struct{}

func (*reg) Logger() *logrusx.Logger {
return logrusx.New("", "", logrusx.ForceLevel(logrus.DebugLevel))
}

func (*reg) Tracer() trace.Tracer {
return trace.NewNoopTracerProvider().Tracer("")
}

func TestFetcherDefault(t *testing.T) {
t.Parallel()

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

l := logrusx.New("", "", logrusx.ForceLevel(logrus.DebugLevel))
w := herodot.NewJSONWriter(l)
s := NewFetcherDefault(l, maxWait, JWKsTTL)
s := NewFetcherDefault(&reg{}, maxWait, JWKsTTL)

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

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

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

jwkKey, err := s.ResolveKey(ctx, []url.URL{
*urlx.ParseOrPanic("azblob://path/prefix/jwks.json"),
Expand Down
5 changes: 2 additions & 3 deletions credentials/signer_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@ import (
"github.com/stretchr/testify/require"

"github.com/ory/oathkeeper/x"
"github.com/ory/x/logrusx"
)

type defaultSignerMockRegistry struct {
f Fetcher
}

func newDefaultSignerMockRegistry() *defaultSignerMockRegistry {
return &defaultSignerMockRegistry{f: NewFetcherDefault(logrusx.New("", ""), time.Millisecond*100, time.Millisecond*500)}
return &defaultSignerMockRegistry{f: NewFetcherDefault(&reg{}, time.Millisecond*100, time.Millisecond*500)}
}

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

fetcher := NewFetcherDefault(logrusx.New("", ""), time.Second, time.Second)
fetcher := NewFetcherDefault(&reg{}, time.Second, time.Second)

_, err = verify(t, token, fetcher, src)
require.NoError(t, err)
Expand Down
2 changes: 2 additions & 0 deletions doc_swagger.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

package main

//lint:file-ignore U1000 Used to generate Swagger/OpenAPI definitions.

// The standard error format
// swagger:model genericError
type genericError struct {
Expand Down
9 changes: 5 additions & 4 deletions driver/configuration/provider_koanf_public_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/rs/cors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/trace"

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

require.NoError(t, p.PipelineConfig("authenticators", "oauth2_introspection", nil, &res))
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)
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)
})

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

t.Run("authenticator=cookie_session", func(t *testing.T) {
a := authn.NewAuthenticatorCookieSession(p, nil)
a := authn.NewAuthenticatorCookieSession(p, trace.NewNoopTracerProvider())
assert.True(t, p.AuthenticatorIsEnabled(a.GetID()))
require.NoError(t, a.Validate(nil))

Expand Down Expand Up @@ -285,7 +286,7 @@ func TestKoanfProvider(t *testing.T) {
})

t.Run("authenticator=oauth2_introspection", func(t *testing.T) {
a := authn.NewAuthenticatorOAuth2Introspection(p, logger)
a := authn.NewAuthenticatorOAuth2Introspection(p, logger, trace.NewNoopTracerProvider())
assert.True(t, p.AuthenticatorIsEnabled(a.GetID()))
require.NoError(t, a.Validate(nil))

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

config, _, err := a.Config(json.RawMessage(fmt.Sprintf(`{
"pre_authorization": {
Expand Down
Loading

0 comments on commit b5d4d88

Please sign in to comment.