Skip to content

Commit 6e095fd

Browse files
steveiliop56steveiliop56
authored
refactor: rework the way trusted proxies ip work (#1007)
1 parent 2a0d7bd commit 6e095fd

7 files changed

Lines changed: 105 additions & 47 deletions

File tree

‎internal/bootstrap/router_bootstrap.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@ func (app *BootstrapApp) setupRouter() error {
3030
if err != nil {
3131
return fmt.Errorf("failed to set trusted proxies: %w", err)
3232
}
33+
34+
app.runtime.TrustedProxiesConfigured = true
35+
} else {
36+
err := engine.SetTrustedProxies(nil)
37+
38+
if err != nil {
39+
return fmt.Errorf("failed to set trusted proxies: %w", err)
40+
}
41+
42+
app.log.App.Warn().Msg("Trusted proxies are not configured, IP access controls will NOT work")
3343
}
3444

3545
middlewareProvideFor := []any{

‎internal/controller/proxy_controller.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,10 @@ func (controller *ProxyController) proxyHandler(c *gin.Context) {
111111
clientIP := c.ClientIP()
112112

113113
aclsCtx := &service.ACLContext{
114-
ACLs: acls,
115-
IP: net.ParseIP(clientIP),
116-
Path: proxyCtx.Path,
114+
ACLs: acls,
115+
IP: net.ParseIP(clientIP),
116+
Path: proxyCtx.Path,
117+
TrustedProxiesConfigured: controller.runtime.TrustedProxiesConfigured,
117118
}
118119

119120
if controller.policyEngine.Evaluate(service.RuleIPBypassed, aclsCtx) {

‎internal/model/runtime.go‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
package model
22

33
type RuntimeConfig struct {
4-
AppURL string
5-
UUID string
6-
CookieDomain string
7-
SessionCookieName string
8-
CSRFCookieName string
9-
RedirectCookieName string
10-
OAuthSessionCookieName string
11-
LocalUsers []LocalUser
12-
OAuthProviders map[string]OAuthServiceConfig
13-
OAuthWhitelist []string
14-
ConfiguredProviders []Provider
4+
AppURL string
5+
UUID string
6+
CookieDomain string
7+
SessionCookieName string
8+
CSRFCookieName string
9+
RedirectCookieName string
10+
OAuthSessionCookieName string
11+
LocalUsers []LocalUser
12+
OAuthProviders map[string]OAuthServiceConfig
13+
OAuthWhitelist []string
14+
ConfiguredProviders []Provider
15+
TrustedProxiesConfigured bool
1516
}
1617

1718
type Provider struct {

‎internal/service/access_controls_rules.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,10 @@ type IPAllowedRule struct {
215215
}
216216

217217
func (rule *IPAllowedRule) Evaluate(ctx *ACLContext) Effect {
218+
if !ctx.TrustedProxiesConfigured {
219+
return EffectAllow // We can't block the proxy
220+
}
221+
218222
// merge global and per-app block/allow lists
219223
blockedIps := append([]string{}, rule.Config.Auth.IP.Block...)
220224
allowedIPs := append([]string{}, rule.Config.Auth.IP.Allow...)
@@ -263,6 +267,10 @@ type IPBypassedRule struct {
263267
}
264268

265269
func (rule *IPBypassedRule) Evaluate(ctx *ACLContext) Effect {
270+
if !ctx.TrustedProxiesConfigured {
271+
return EffectDeny
272+
}
273+
266274
// merge global and per-app bypass lists
267275
bypassList := append([]string{}, rule.Config.Auth.IP.Bypass...)
268276
if ctx.ACLs != nil {

‎internal/service/access_controls_rules_test.go‎

Lines changed: 62 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -612,20 +612,30 @@ func TestIPAllowedRule(t *testing.T) {
612612
expected Effect
613613
}{
614614
{
615-
name: "allows when ACLs are nil and no global lists configured",
615+
name: "when trusted proxies are not configured, IP is allowed",
616616
ctx: &ACLContext{
617-
ACLs: nil,
617+
ACLs: &model.App{},
618618
IP: net.ParseIP("10.0.0.1"),
619619
},
620620
expected: EffectAllow,
621621
},
622+
{
623+
name: "allows when ACLs are nil and no global lists configured",
624+
ctx: &ACLContext{
625+
ACLs: nil,
626+
IP: net.ParseIP("10.0.0.1"),
627+
TrustedProxiesConfigured: true,
628+
},
629+
expected: EffectAllow,
630+
},
622631
{
623632
name: "denies when IP matches app block list",
624633
ctx: &ACLContext{
625634
ACLs: &model.App{
626635
IP: model.AppIP{Block: []string{"10.0.0.1"}},
627636
},
628-
IP: net.ParseIP("10.0.0.1"),
637+
IP: net.ParseIP("10.0.0.1"),
638+
TrustedProxiesConfigured: true,
629639
},
630640
expected: EffectDeny,
631641
},
@@ -637,8 +647,9 @@ func TestIPAllowedRule(t *testing.T) {
637647
},
638648
},
639649
ctx: &ACLContext{
640-
ACLs: &model.App{},
641-
IP: net.ParseIP("10.0.0.5"),
650+
ACLs: &model.App{},
651+
IP: net.ParseIP("10.0.0.5"),
652+
TrustedProxiesConfigured: true,
642653
},
643654
expected: EffectDeny,
644655
},
@@ -648,7 +659,8 @@ func TestIPAllowedRule(t *testing.T) {
648659
ACLs: &model.App{
649660
IP: model.AppIP{Allow: []string{"192.168.1.0/24"}},
650661
},
651-
IP: net.ParseIP("192.168.1.10"),
662+
IP: net.ParseIP("192.168.1.10"),
663+
TrustedProxiesConfigured: true,
652664
},
653665
expected: EffectAllow,
654666
},
@@ -660,8 +672,9 @@ func TestIPAllowedRule(t *testing.T) {
660672
},
661673
},
662674
ctx: &ACLContext{
663-
ACLs: &model.App{},
664-
IP: net.ParseIP("192.168.1.10"),
675+
ACLs: &model.App{},
676+
IP: net.ParseIP("192.168.1.10"),
677+
TrustedProxiesConfigured: true,
665678
},
666679
expected: EffectAllow,
667680
},
@@ -671,15 +684,17 @@ func TestIPAllowedRule(t *testing.T) {
671684
ACLs: &model.App{
672685
IP: model.AppIP{Allow: []string{"192.168.1.0/24"}},
673686
},
674-
IP: net.ParseIP("10.0.0.1"),
687+
IP: net.ParseIP("10.0.0.1"),
688+
TrustedProxiesConfigured: true,
675689
},
676690
expected: EffectDeny,
677691
},
678692
{
679693
name: "allows when no block or allow lists are configured",
680694
ctx: &ACLContext{
681-
ACLs: &model.App{},
682-
IP: net.ParseIP("10.0.0.1"),
695+
ACLs: &model.App{},
696+
IP: net.ParseIP("10.0.0.1"),
697+
TrustedProxiesConfigured: true,
683698
},
684699
expected: EffectAllow,
685700
},
@@ -692,7 +707,8 @@ func TestIPAllowedRule(t *testing.T) {
692707
Allow: []string{"10.0.0.1"},
693708
},
694709
},
695-
IP: net.ParseIP("10.0.0.1"),
710+
IP: net.ParseIP("10.0.0.1"),
711+
TrustedProxiesConfigured: true,
696712
},
697713
expected: EffectDeny,
698714
},
@@ -705,7 +721,8 @@ func TestIPAllowedRule(t *testing.T) {
705721
Allow: []string{"10.0.0.1"},
706722
},
707723
},
708-
IP: net.ParseIP("10.0.0.1"),
724+
IP: net.ParseIP("10.0.0.1"),
725+
TrustedProxiesConfigured: true,
709726
},
710727
expected: EffectAllow,
711728
},
@@ -735,30 +752,43 @@ func TestIPBypassedRule(t *testing.T) {
735752
ctx *ACLContext
736753
expected Effect
737754
}{
755+
{
756+
name: "when trusted proxies are not configured, IP is not bypassed",
757+
rule: defaultIPBR,
758+
ctx: &ACLContext{
759+
ACLs: &model.App{},
760+
IP: net.ParseIP("10.0.0.1"),
761+
TrustedProxiesConfigured: false,
762+
},
763+
expected: EffectDeny,
764+
},
738765
{
739766
name: "deny when ACLs are nil and no global bypass",
740767
rule: defaultIPBR,
741768
ctx: &ACLContext{
742-
ACLs: nil,
743-
IP: net.ParseIP("10.0.0.1"),
769+
ACLs: nil,
770+
IP: net.ParseIP("10.0.0.1"),
771+
TrustedProxiesConfigured: true,
744772
},
745773
expected: EffectDeny,
746774
},
747775
{
748776
name: "allows when ACLs are nil but IP matches global bypass",
749777
rule: globBypassIPBR,
750778
ctx: &ACLContext{
751-
ACLs: nil,
752-
IP: net.ParseIP("10.0.0.5"),
779+
ACLs: nil,
780+
IP: net.ParseIP("10.0.0.5"),
781+
TrustedProxiesConfigured: true,
753782
},
754783
expected: EffectAllow,
755784
},
756785
{
757786
name: "denies when ACLs are nil and IP does not match global bypass",
758787
rule: globBypassIPBR,
759788
ctx: &ACLContext{
760-
ACLs: nil,
761-
IP: net.ParseIP("192.168.1.1"),
789+
ACLs: nil,
790+
IP: net.ParseIP("192.168.1.1"),
791+
TrustedProxiesConfigured: true,
762792
},
763793
expected: EffectDeny,
764794
},
@@ -769,7 +799,8 @@ func TestIPBypassedRule(t *testing.T) {
769799
ACLs: &model.App{
770800
IP: model.AppIP{Bypass: []string{"10.0.0.0/24"}},
771801
},
772-
IP: net.ParseIP("10.0.0.5"),
802+
IP: net.ParseIP("10.0.0.5"),
803+
TrustedProxiesConfigured: true,
773804
},
774805
expected: EffectAllow,
775806
},
@@ -780,7 +811,8 @@ func TestIPBypassedRule(t *testing.T) {
780811
ACLs: &model.App{
781812
IP: model.AppIP{Bypass: []string{"172.16.0.0/24"}},
782813
},
783-
IP: net.ParseIP("10.0.0.5"),
814+
IP: net.ParseIP("10.0.0.5"),
815+
TrustedProxiesConfigured: true,
784816
},
785817
expected: EffectAllow,
786818
},
@@ -791,7 +823,8 @@ func TestIPBypassedRule(t *testing.T) {
791823
ACLs: &model.App{
792824
IP: model.AppIP{Bypass: []string{"10.0.0.0/24"}},
793825
},
794-
IP: net.ParseIP("10.0.0.5"),
826+
IP: net.ParseIP("10.0.0.5"),
827+
TrustedProxiesConfigured: true,
795828
},
796829
expected: EffectAllow,
797830
},
@@ -802,16 +835,18 @@ func TestIPBypassedRule(t *testing.T) {
802835
ACLs: &model.App{
803836
IP: model.AppIP{Bypass: []string{"10.0.0.0/24"}},
804837
},
805-
IP: net.ParseIP("192.168.1.1"),
838+
IP: net.ParseIP("192.168.1.1"),
839+
TrustedProxiesConfigured: true,
806840
},
807841
expected: EffectDeny,
808842
},
809843
{
810844
name: "denies when bypass list is empty",
811845
rule: defaultIPBR,
812846
ctx: &ACLContext{
813-
ACLs: &model.App{},
814-
IP: net.ParseIP("10.0.0.1"),
847+
ACLs: &model.App{},
848+
IP: net.ParseIP("10.0.0.1"),
849+
TrustedProxiesConfigured: true,
815850
},
816851
expected: EffectDeny,
817852
},
@@ -822,7 +857,8 @@ func TestIPBypassedRule(t *testing.T) {
822857
ACLs: &model.App{
823858
IP: model.AppIP{Bypass: []string{"not-an-ip", "10.0.0.1"}},
824859
},
825-
IP: net.ParseIP("10.0.0.1"),
860+
IP: net.ParseIP("10.0.0.1"),
861+
TrustedProxiesConfigured: true,
826862
},
827863
expected: EffectAllow,
828864
},

‎internal/service/policy_engine.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,11 @@ type Rule interface {
2929
}
3030

3131
type ACLContext struct {
32-
ACLs *model.App
33-
UserContext *model.UserContext
34-
IP net.IP
35-
Path string
32+
ACLs *model.App
33+
UserContext *model.UserContext
34+
IP net.IP
35+
Path string
36+
TrustedProxiesConfigured bool
3637
}
3738

3839
type PolicyEngine struct {

‎internal/test/test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,10 @@ func CreateTestConfigs(t *testing.T) (model.Config, model.RuntimeConfig) {
163163
},
164164
},
165165
},
166-
CookieDomain: "example.com",
167-
AppURL: "https://tinyauth.example.com",
168-
SessionCookieName: "tinyauth-session",
166+
CookieDomain: "example.com",
167+
AppURL: "https://tinyauth.example.com",
168+
SessionCookieName: "tinyauth-session",
169+
TrustedProxiesConfigured: true,
169170
}
170171

171172
return config, runtime

0 commit comments

Comments
 (0)