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.

Sign up for GitHub

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

fix: OAuth2 iss claim verification in JWT/OIDC authenticators when used with metadata_endpoint #1660

Merged
merged 1 commit into from
Jul 25, 2024
Merged

fix: OAuth2 iss claim verification in JWT/OIDC authenticators when used with metadata_endpoint #1660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions internal/rules/mechanisms/authenticators/jwt_authenticator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,9 @@ func (a *jwtAuthenticator) WithConfig(config map[string]any) (Authenticator, err
}

type Config struct {
Assertions *oauth2.Expectation `mapstructure:"assertions" validate:"-"`
CacheTTL *time.Duration `mapstructure:"cache_ttl"`
AllowFallbackOnError *bool `mapstructure:"allow_fallback_on_error"`
Assertions oauth2.Expectation `mapstructure:"assertions" validate:"-"`
CacheTTL *time.Duration `mapstructure:"cache_ttl"`
AllowFallbackOnError *bool `mapstructure:"allow_fallback_on_error"`
}

var conf Config
Expand All @@ -221,7 +221,7 @@ func (a *jwtAuthenticator) WithConfig(config map[string]any) (Authenticator, err
return &jwtAuthenticator{
id: a.id,
r: a.r,
a: conf.Assertions.Merge(&a.a),
a: conf.Assertions.Merge(a.a),
ttl: x.IfThenElse(conf.CacheTTL != nil, conf.CacheTTL, a.ttl),
sf: a.sf,
ads: a.ads,
Expand Down Expand Up @@ -314,7 +314,7 @@ func (a *jwtAuthenticator) verifyToken(ctx heimdall.Context, token *jwt.JSONWebT
}

// configured assertions take precedence over those available in the metadata
assertions := a.a.Merge(&oauth2.Expectation{
assertions := a.a.Merge(oauth2.Expectation{
TrustedIssuers: []string{metadata.Issuer},
})

Expand Down
10 changes: 5 additions & 5 deletions internal/rules/mechanisms/authenticators/oauth2_introspection_authenticator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,9 @@ func (a *oauth2IntrospectionAuthenticator) WithConfig(rawConfig map[string]any)
}

type Config struct {
Assertions *oauth2.Expectation `mapstructure:"assertions"`
CacheTTL *time.Duration `mapstructure:"cache_ttl"`
AllowFallbackOnError *bool `mapstructure:"allow_fallback_on_error"`
Assertions oauth2.Expectation `mapstructure:"assertions"`
CacheTTL *time.Duration `mapstructure:"cache_ttl"`
AllowFallbackOnError *bool `mapstructure:"allow_fallback_on_error"`
}

var conf Config
Expand All @@ -205,7 +205,7 @@ func (a *oauth2IntrospectionAuthenticator) WithConfig(rawConfig map[string]any)
return &oauth2IntrospectionAuthenticator{
id: a.id,
r: a.r,
a: conf.Assertions.Merge(&a.a),
a: conf.Assertions.Merge(a.a),
sf: a.sf,
ads: a.ads,
ttl: x.IfThenElse(conf.CacheTTL != nil, conf.CacheTTL, a.ttl),
Expand Down Expand Up @@ -299,7 +299,7 @@ func (a *oauth2IntrospectionAuthenticator) getSubjectInformation(ctx heimdall.Co
}

// configured assertions take precedence over those available in the metadata
assertions := a.a.Merge(&oauth2.Expectation{
assertions := a.a.Merge(oauth2.Expectation{
TrustedIssuers: []string{metadata.Issuer},
})

Expand Down
20 changes: 8 additions & 12 deletions ...l/rules/mechanisms/oauth2/expectations.go → ...al/rules/mechanisms/oauth2/expectation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,37 +38,33 @@ type Expectation struct {
ValidityLeeway time.Duration `mapstructure:"validity_leeway"`
}

func (e *Expectation) Merge(other *Expectation) Expectation {
if e == nil {
return *other
}

func (e Expectation) Merge(other Expectation) Expectation {
e.TrustedIssuers = x.IfThenElse(len(e.TrustedIssuers) != 0, e.TrustedIssuers, other.TrustedIssuers)
e.ScopesMatcher = x.IfThenElse(e.ScopesMatcher != nil, e.ScopesMatcher, other.ScopesMatcher)
e.Audiences = x.IfThenElse(len(e.Audiences) != 0, e.Audiences, other.Audiences)
e.AllowedAlgorithms = x.IfThenElse(len(e.AllowedAlgorithms) != 0, e.AllowedAlgorithms, other.AllowedAlgorithms)
e.ValidityLeeway = x.IfThenElse(e.ValidityLeeway != 0, e.ValidityLeeway, other.ValidityLeeway)

return *e
return e
}

func (e *Expectation) AssertAlgorithm(alg string) error {
func (e Expectation) AssertAlgorithm(alg string) error {
if !slices.Contains(e.AllowedAlgorithms, alg) {
return errorchain.NewWithMessagef(ErrAssertion, "algorithm %s is not allowed", alg)
}

return nil
}

func (e *Expectation) AssertIssuer(issuer string) error {
func (e Expectation) AssertIssuer(issuer string) error {
if !slices.Contains(e.TrustedIssuers, issuer) {
return errorchain.NewWithMessagef(ErrAssertion, "issuer %s is not trusted", issuer)
}

return nil
}

func (e *Expectation) AssertAudience(audience []string) error {
func (e Expectation) AssertAudience(audience []string) error {
if len(e.Audiences) == 0 {
return nil
}
Expand All @@ -80,7 +76,7 @@ func (e *Expectation) AssertAudience(audience []string) error {
return nil
}

func (e *Expectation) AssertValidity(notBefore, notAfter time.Time) error {
func (e Expectation) AssertValidity(notBefore, notAfter time.Time) error {
leeway := int64(x.IfThenElse(e.ValidityLeeway != 0, e.ValidityLeeway, defaultLeeway).Seconds())
now := time.Now().Unix()
nbf := notBefore.Unix()
Expand All @@ -97,7 +93,7 @@ func (e *Expectation) AssertValidity(notBefore, notAfter time.Time) error {
return nil
}

func (e *Expectation) AssertIssuanceTime(issuedAt time.Time) error {
func (e Expectation) AssertIssuanceTime(issuedAt time.Time) error {
leeway := x.IfThenElse(e.ValidityLeeway != 0, e.ValidityLeeway, defaultLeeway)

// IssuedAt is optional but cannot be in the future. This is not required by the RFC, but
Expand All @@ -109,4 +105,4 @@ func (e *Expectation) AssertIssuanceTime(issuedAt time.Time) error {
return nil
}

func (e *Expectation) AssertScopes(scopes []string) error { return e.ScopesMatcher.Match(scopes) }
func (e Expectation) AssertScopes(scopes []string) error { return e.ScopesMatcher.Match(scopes) }
75 changes: 43 additions & 32 deletions ...es/mechanisms/oauth2/expectations_test.go → ...les/mechanisms/oauth2/expectation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,51 +524,42 @@ func TestExpectationAssertScopes(t *testing.T) {
}
}

func TestMergeExpectations(t *testing.T) {
func TestExpectationMerge(t *testing.T) {
t.Parallel()

for _, tc := range []struct {
uc string
target *Expectation
source *Expectation
assert func(t *testing.T, merged *Expectation, source *Expectation, target *Expectation)
target Expectation
source Expectation
assert func(t *testing.T, merged Expectation, source Expectation, target Expectation)
}{
{
uc: "with nil target",
source: &Expectation{},
assert: func(t *testing.T, merged *Expectation, source *Expectation, _ *Expectation) {
t.Helper()

require.Equal(t, source, merged)
},
},
{
uc: "with empty target",
source: &Expectation{
source: Expectation{
ScopesMatcher: ExactScopeStrategyMatcher{},
Audiences: []string{"foo"},
TrustedIssuers: []string{"bar"},
AllowedAlgorithms: []string{"RS512"},
ValidityLeeway: 10 * time.Second,
},
target: &Expectation{},
assert: func(t *testing.T, merged *Expectation, source *Expectation, _ *Expectation) {
target: Expectation{},
assert: func(t *testing.T, merged Expectation, source Expectation, _ Expectation) {
t.Helper()

require.Equal(t, source, merged)
},
},
{
uc: "with target having only scopes configured",
source: &Expectation{
source: Expectation{
ScopesMatcher: ExactScopeStrategyMatcher{},
Audiences: []string{"foo"},
TrustedIssuers: []string{"bar"},
AllowedAlgorithms: []string{"RS512"},
ValidityLeeway: 10 * time.Second,
},
target: &Expectation{ScopesMatcher: HierarchicScopeStrategyMatcher{}},
assert: func(t *testing.T, merged *Expectation, source *Expectation, target *Expectation) {
target: Expectation{ScopesMatcher: HierarchicScopeStrategyMatcher{}},
assert: func(t *testing.T, merged Expectation, source Expectation, target Expectation) {
t.Helper()

assert.NotEqual(t, source, merged)
Expand All @@ -582,18 +573,18 @@ func TestMergeExpectations(t *testing.T) {
},
{
uc: "with target having scopes and audience configured",
source: &Expectation{
source: Expectation{
ScopesMatcher: ExactScopeStrategyMatcher{},
Audiences: []string{"foo"},
TrustedIssuers: []string{"bar"},
AllowedAlgorithms: []string{"RS512"},
ValidityLeeway: 10 * time.Second,
},
target: &Expectation{
target: Expectation{
ScopesMatcher: HierarchicScopeStrategyMatcher{},
Audiences: []string{"baz"},
},
assert: func(t *testing.T, merged *Expectation, source *Expectation, target *Expectation) {
assert: func(t *testing.T, merged Expectation, source Expectation, target Expectation) {
t.Helper()

assert.NotEqual(t, source, merged)
Expand All @@ -608,19 +599,19 @@ func TestMergeExpectations(t *testing.T) {
},
{
uc: "with target having scopes, audience and trusted issuers configured",
source: &Expectation{
source: Expectation{
ScopesMatcher: ExactScopeStrategyMatcher{},
Audiences: []string{"foo"},
TrustedIssuers: []string{"bar"},
AllowedAlgorithms: []string{"RS512"},
ValidityLeeway: 10 * time.Second,
},
target: &Expectation{
target: Expectation{
ScopesMatcher: HierarchicScopeStrategyMatcher{},
Audiences: []string{"baz"},
TrustedIssuers: []string{"zab"},
},
assert: func(t *testing.T, merged *Expectation, source *Expectation, target *Expectation) {
assert: func(t *testing.T, merged Expectation, source Expectation, target Expectation) {
t.Helper()

assert.NotEqual(t, source, merged)
Expand All @@ -636,20 +627,20 @@ func TestMergeExpectations(t *testing.T) {
},
{
uc: "with target having scopes, audience, trusted issuers and allowed algorithms configured",
source: &Expectation{
source: Expectation{
ScopesMatcher: ExactScopeStrategyMatcher{},
Audiences: []string{"foo"},
TrustedIssuers: []string{"bar"},
AllowedAlgorithms: []string{"RS512"},
ValidityLeeway: 10 * time.Second,
},
target: &Expectation{
target: Expectation{
ScopesMatcher: HierarchicScopeStrategyMatcher{},
Audiences: []string{"baz"},
TrustedIssuers: []string{"zab"},
AllowedAlgorithms: []string{"BAR128"},
},
assert: func(t *testing.T, merged *Expectation, source *Expectation, target *Expectation) {
assert: func(t *testing.T, merged Expectation, source Expectation, target Expectation) {
t.Helper()

assert.NotEqual(t, source, merged)
Expand All @@ -666,21 +657,21 @@ func TestMergeExpectations(t *testing.T) {
},
{
uc: "with target having everything reconfigured",
source: &Expectation{
source: Expectation{
ScopesMatcher: ExactScopeStrategyMatcher{},
Audiences: []string{"foo"},
TrustedIssuers: []string{"bar"},
AllowedAlgorithms: []string{"RS512"},
ValidityLeeway: 10 * time.Second,
},
target: &Expectation{
target: Expectation{
ScopesMatcher: HierarchicScopeStrategyMatcher{},
Audiences: []string{"baz"},
TrustedIssuers: []string{"zab"},
AllowedAlgorithms: []string{"BAR128"},
ValidityLeeway: 20 * time.Minute,
},
assert: func(t *testing.T, merged *Expectation, source *Expectation, target *Expectation) {
assert: func(t *testing.T, merged Expectation, source Expectation, target Expectation) {
t.Helper()

assert.NotEqual(t, source, merged)
Expand All @@ -702,7 +693,27 @@ func TestMergeExpectations(t *testing.T) {
exp := tc.target.Merge(tc.source)

// THEN
tc.assert(t, &exp, tc.source, tc.target)
tc.assert(t, exp, tc.source, tc.target)
})
}
}

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

exp := Expectation{}

exp.Merge(Expectation{
ScopesMatcher: ExactScopeStrategyMatcher{},
Audiences: []string{"foo"},
TrustedIssuers: []string{"bar"},
AllowedAlgorithms: []string{"RS512"},
ValidityLeeway: 10 * time.Second,
})

assert.Nil(t, exp.ScopesMatcher)
assert.Empty(t, exp.Audiences)
assert.Empty(t, exp.TrustedIssuers)
assert.Empty(t, exp.AllowedAlgorithms)
assert.Equal(t, 0*time.Second, exp.ValidityLeeway)
}
Loading