Skip to content

Commit

Permalink
use nil instead of empty array, rename from pCE to providerMap
Browse files Browse the repository at this point in the history
  • Loading branch information
@roncodingenthusiast
roncodingenthusiast committed Jul 14, 2023
1 parent 258645e commit 00d045c
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 120 deletions.
8 changes: 4 additions & 4 deletions agent/xds/jwt_authn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const (
//
// Eg. If you have three providers: okta, auth0 and fusionAuth and only okta is referenced in your intentions, then this
// will create a jwt-auth filter containing just okta in the list of providers.
func makeJWTAuthFilter(pCE map[string]*structs.JWTProviderConfigEntry, intentions structs.SimplifiedIntentions) (*envoy_http_v3.HttpFilter, error) {
func makeJWTAuthFilter(providerMap map[string]*structs.JWTProviderConfigEntry, intentions structs.SimplifiedIntentions) (*envoy_http_v3.HttpFilter, error) {
providers := map[string]*envoy_http_jwt_authn_v3.JwtProvider{}
var jwtRequirements []*envoy_http_jwt_authn_v3.JwtRequirement

Expand All @@ -41,7 +41,7 @@ func makeJWTAuthFilter(pCE map[string]*structs.JWTProviderConfigEntry, intention
continue
}

providerCE, ok := pCE[providerName]
providerCE, ok := providerMap[providerName]
if !ok {
return nil, fmt.Errorf("provider specified in intention does not exist. Provider name: %s", providerName)
}
Expand All @@ -56,8 +56,8 @@ func makeJWTAuthFilter(pCE map[string]*structs.JWTProviderConfigEntry, intention
}
}

if len(intentions) == 0 && len(providers) == 0 {
//do not add jwt_authn filter when intentions don't have JWT
if len(jwtRequirements) == 0 {
//do not add jwt_authn filter when intentions don't have JWTs
return nil, nil
}

Expand Down
4 changes: 4 additions & 0 deletions agent/xds/jwt_authn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ func TestMakeJWTAUTHFilters(t *testing.T) {
intentions structs.SimplifiedIntentions
provider map[string]*structs.JWTProviderConfigEntry
}{
"no-provider": {
intentions: simplified(makeTestIntention(t, ixnOpts{src: "web", action: structs.IntentionActionAllow})),
provider: nil,
},
"remote-provider": {
intentions: simplified(makeTestIntention(t, ixnOpts{src: "web", action: structs.IntentionActionAllow, jwt: oktaIntention})),
provider: remoteCE,
Expand Down
50 changes: 30 additions & 20 deletions agent/xds/rbac.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func makeRBACNetworkFilter(
localInfo rbacLocalInfo,
peerTrustBundles []*pbpeering.PeeringTrustBundle,
) (*envoy_listener_v3.Filter, error) {
rules, err := makeRBACRules(intentions, intentionDefaultAllow, localInfo, false, peerTrustBundles, map[string]*structs.JWTProviderConfigEntry{})
rules, err := makeRBACRules(intentions, intentionDefaultAllow, localInfo, false, peerTrustBundles, nil)
if err != nil {
return nil, err
}
Expand All @@ -45,9 +45,9 @@ func makeRBACHTTPFilter(
intentionDefaultAllow bool,
localInfo rbacLocalInfo,
peerTrustBundles []*pbpeering.PeeringTrustBundle,
pCE map[string]*structs.JWTProviderConfigEntry,
providerMap map[string]*structs.JWTProviderConfigEntry,
) (*envoy_http_v3.HttpFilter, error) {
rules, err := makeRBACRules(intentions, intentionDefaultAllow, localInfo, true, peerTrustBundles, pCE)
rules, err := makeRBACRules(intentions, intentionDefaultAllow, localInfo, true, peerTrustBundles, providerMap)
if err != nil {
return nil, err
}
Expand All @@ -63,7 +63,7 @@ func intentionListToIntermediateRBACForm(
localInfo rbacLocalInfo,
isHTTP bool,
trustBundlesByPeer map[string]*pbpeering.PeeringTrustBundle,
pCE map[string]*structs.JWTProviderConfigEntry,
providerMap map[string]*structs.JWTProviderConfigEntry,
) ([]*rbacIntention, error) {
sort.Sort(structs.IntentionPrecedenceSorter(intentions))

Expand All @@ -81,7 +81,7 @@ func intentionListToIntermediateRBACForm(
continue
}

rixn, err := intentionToIntermediateRBACForm(ixn, localInfo, isHTTP, trustBundle, pCE)
rixn, err := intentionToIntermediateRBACForm(ixn, localInfo, isHTTP, trustBundle, providerMap)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -249,7 +249,7 @@ func intentionToIntermediateRBACForm(
localInfo rbacLocalInfo,
isHTTP bool,
bundle *pbpeering.PeeringTrustBundle,
pCE map[string]*structs.JWTProviderConfigEntry,
providerMap map[string]*structs.JWTProviderConfigEntry,
) (*rbacIntention, error) {
rixn := &rbacIntention{
Source: rbacService{
Expand All @@ -269,7 +269,7 @@ func intentionToIntermediateRBACForm(
if isHTTP && ixn.JWT != nil {
var jwts []*JWTInfo
for _, prov := range ixn.JWT.Providers {
jwtProvider, ok := pCE[prov.Name]
jwtProvider, ok := providerMap[prov.Name]

if !ok {
return nil, fmt.Errorf("provider specified in intention does not exist. Provider name: %s", prov.Name)
Expand All @@ -294,7 +294,7 @@ func intentionToIntermediateRBACForm(
if perm.JWT != nil {
var jwts []*JWTInfo
for _, prov := range perm.JWT.Providers {
jwtProvider, ok := pCE[prov.Name]
jwtProvider, ok := providerMap[prov.Name]
if !ok {
return nil, fmt.Errorf("provider specified in intention does not exist. Provider name: %s", prov.Name)
}
Expand Down Expand Up @@ -601,7 +601,7 @@ func makeRBACRules(
localInfo rbacLocalInfo,
isHTTP bool,
peerTrustBundles []*pbpeering.PeeringTrustBundle,
pCE map[string]*structs.JWTProviderConfigEntry,
providerMap map[string]*structs.JWTProviderConfigEntry,
) (*envoy_rbac_v3.RBAC, error) {
// TODO(banks,rb): Implement revocation list checking?

Expand All @@ -622,7 +622,7 @@ func makeRBACRules(
}

// First build up just the basic principal matches.
rbacIxns, err := intentionListToIntermediateRBACForm(intentions, localInfo, isHTTP, trustBundlesByPeer, pCE)
rbacIxns, err := intentionListToIntermediateRBACForm(intentions, localInfo, isHTTP, trustBundlesByPeer, providerMap)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -963,22 +963,32 @@ func countWild(src rbacService) int {
}

func andPrincipals(ids []*envoy_rbac_v3.Principal) *envoy_rbac_v3.Principal {
return &envoy_rbac_v3.Principal{
Identifier: &envoy_rbac_v3.Principal_AndIds{
AndIds: &envoy_rbac_v3.Principal_Set{
Ids: ids,
switch len(ids) {
case 1:
return ids[0]
default:
return &envoy_rbac_v3.Principal{
Identifier: &envoy_rbac_v3.Principal_AndIds{
AndIds: &envoy_rbac_v3.Principal_Set{
Ids: ids,
},
},
},
}
}
}

func orPrincipals(ids []*envoy_rbac_v3.Principal) *envoy_rbac_v3.Principal {
return &envoy_rbac_v3.Principal{
Identifier: &envoy_rbac_v3.Principal_OrIds{
OrIds: &envoy_rbac_v3.Principal_Set{
Ids: ids,
switch len(ids) {
case 1:
return ids[0]
default:
return &envoy_rbac_v3.Principal{
Identifier: &envoy_rbac_v3.Principal_OrIds{
OrIds: &envoy_rbac_v3.Principal_Set{
Ids: ids,
},
},
},
}
}
}

Expand Down
1 change: 1 addition & 0 deletions agent/xds/testdata/jwt_authn/no-provider.golden
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
58 changes: 26 additions & 32 deletions agent/xds/testdata/rbac/top-level-jwt-no-permissions--httpfilter.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,40 +27,34 @@
}
},
{
"orIds": {
"andIds": {
"ids": [
{
"andIds": {
"ids": [
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "iss"}
],
"value": {
"stringMatch": {
"exact": "mytest.okta-issuer"
}
}
}
},
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "roles"}
],
"value": {
"stringMatch": {
"exact": "testing"
}
}
}
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "iss"}
],
"value": {
"stringMatch": {
"exact": "mytest.okta-issuer"
}
]
}
}
},
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "roles"}
],
"value": {
"stringMatch": {
"exact": "testing"
}
}
}
}
]
Expand Down
58 changes: 26 additions & 32 deletions agent/xds/testdata/rbac/top-level-jwt-with-multiple-permissions--httpfilter.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,40 +135,34 @@
}
},
{
"orIds": {
"andIds": {
"ids": [
{
"andIds": {
"ids": [
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "iss"}
],
"value": {
"stringMatch": {
"exact": "mytest.okta-issuer"
}
}
}
},
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "roles"}
],
"value": {
"stringMatch": {
"exact": "testing"
}
}
}
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "iss"}
],
"value": {
"stringMatch": {
"exact": "mytest.okta-issuer"
}
]
}
}
},
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "roles"}
],
"value": {
"stringMatch": {
"exact": "testing"
}
}
}
}
]
Expand Down
58 changes: 26 additions & 32 deletions agent/xds/testdata/rbac/top-level-jwt-with-one-permission--httpfilter.golden
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,40 +94,34 @@
}
},
{
"orIds": {
"andIds": {
"ids": [
{
"andIds": {
"ids": [
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "iss"}
],
"value": {
"stringMatch": {
"exact": "mytest.okta-issuer"
}
}
}
},
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "roles"}
],
"value": {
"stringMatch": {
"exact": "testing"
}
}
}
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "iss"}
],
"value": {
"stringMatch": {
"exact": "mytest.okta-issuer"
}
]
}
}
},
{
"metadata": {
"filter":"envoy.filters.http.jwt_authn",
"path": [
{"key": "jwt_payload_okta"},
{"key": "roles"}
],
"value": {
"stringMatch": {
"exact": "testing"
}
}
}
}
]
Expand Down

0 comments on commit 00d045c

Please sign in to comment.