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

feat: preserve_host feature for oauth2_introspect #1131

Merged
merged 14 commits into from
Sep 8, 2023
Merged
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/nancy-github-action@v1.0.2
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
53 changes: 36 additions & 17 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,12 @@ 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:
alnr marked this conversation as resolved.
Show resolved Hide resolved
// We're done!
return nil
Expand Down Expand Up @@ -183,25 +191,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 +218,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 +262,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
7 changes: 4 additions & 3 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 @@ -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
Loading