Skip to content

Commit ca2724e

Browse files
authored
sync: add missing policy annotations (#1446)
The allow_public_unauthenticated_access and allow_any_authenticated_user annotations are not honored by the current unified API sync logic. These are part of the "Base" annotations rather than the "Policy" annotations, but they should still affect the derived policy. Make sure the conversion logic accounts for these annotations.
1 parent 0d5a91c commit ca2724e

2 files changed

Lines changed: 60 additions & 3 deletions

File tree

pomerium/ingress_to_policy.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ import (
1414
// keysToPolicy translates Ingress annotations to a Policy proto compatible
1515
// with the unified API.
1616
func keysToPolicy(kv *keys, name string) (*configpb.Policy, error) {
17+
// Some policy-related annotations are part of the "base" annotations while
18+
// most are part of the "policy" annotations. Reassemble the complete policy
19+
// by combining both of these.
20+
r := new(configpb.Route)
21+
if err := unmarshalAnnotations(r, kv.Base); err != nil {
22+
return nil, fmt.Errorf("couldn't unmarshal base annotations: %w", err)
23+
}
1724
p := new(configpb.Policy)
1825
if err := unmarshalPolicyAnnotations(p, kv.Policy); err != nil {
1926
return nil, fmt.Errorf("couldn't unmarshal policy annotations: %w", err)
@@ -22,9 +29,11 @@ func keysToPolicy(kv *keys, name string) (*configpb.Policy, error) {
2229
// Use the same conversion logic from Core to translate the legacy
2330
// allowlist fields.
2431
configPolicy := config.Policy{
25-
AllowedDomains: p.AllowedDomains,
26-
AllowedUsers: p.AllowedUsers,
27-
AllowedIDPClaims: identity.NewFlattenedClaimsFromPB(p.AllowedIdpClaims),
32+
AllowAnyAuthenticatedUser: r.AllowAnyAuthenticatedUser,
33+
AllowPublicUnauthenticatedAccess: r.AllowPublicUnauthenticatedAccess,
34+
AllowedDomains: p.AllowedDomains,
35+
AllowedUsers: p.AllowedUsers,
36+
AllowedIDPClaims: identity.NewFlattenedClaimsFromPB(p.AllowedIdpClaims),
2837
}
2938
// Include any user-defined PPL.
3039
if p.SourcePpl != nil {

pomerium/ingress_to_policy_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,54 @@ func TestKeysToPolicy(t *testing.T) {
7878
]`, *p.SourcePpl)
7979
}
8080

81+
func TestKeysToPolicy_AnyAuthenticatedUser(t *testing.T) {
82+
kv, err := removeKeyPrefix(map[string]string{
83+
"a/allow_any_authenticated_user": "true",
84+
}, "a")
85+
require.NoError(t, err)
86+
87+
p, err := keysToPolicy(kv, "POLICY-NAME")
88+
require.NoError(t, err)
89+
require.NotNil(t, p.Name)
90+
assert.Equal(t, "POLICY-NAME", *p.Name)
91+
require.NotNil(t, p.SourcePpl)
92+
assert.JSONEq(t, `[
93+
{
94+
"allow": {
95+
"or": [
96+
{
97+
"authenticated_user": true
98+
}
99+
]
100+
}
101+
}
102+
]`, *p.SourcePpl)
103+
}
104+
105+
func TestKeysToPolicy_Public(t *testing.T) {
106+
kv, err := removeKeyPrefix(map[string]string{
107+
"a/allow_public_unauthenticated_access": "true",
108+
}, "a")
109+
require.NoError(t, err)
110+
111+
p, err := keysToPolicy(kv, "POLICY-NAME")
112+
require.NoError(t, err)
113+
require.NotNil(t, p.Name)
114+
assert.Equal(t, "POLICY-NAME", *p.Name)
115+
require.NotNil(t, p.SourcePpl)
116+
assert.JSONEq(t, `[
117+
{
118+
"allow": {
119+
"or": [
120+
{
121+
"accept": true
122+
}
123+
]
124+
}
125+
}
126+
]`, *p.SourcePpl)
127+
}
128+
81129
func TestKeysToPolicy_Empty(t *testing.T) {
82130
// keysToPolicy should return nil when there are no policy-related annotations.
83131
kv, err := removeKeyPrefix(map[string]string{

0 commit comments

Comments
 (0)