Skip to content

Commit 558bcb1

Browse files
committed
fix: add missing username for private scraper on installer form
- Added a new credential field for private username in the ScraperFormModel. - Introduced comprehensive tests for the browser tool, covering URL resolution and availability checks, ensuring robust handling of private and public URLs.
1 parent 4b90669 commit 558bcb1

File tree

3 files changed

+213
-19
lines changed

3 files changed

+213
-19
lines changed

backend/cmd/installer/wizard/models/scraper_form.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ func (m *ScraperFormModel) BuildForm() tea.Cmd {
9595
locale.ToolsScraperPrivateURLDesc,
9696
"https://scraper-internal.example.com",
9797
))
98+
fields = append(fields, m.createCredentialField(config, "private_username",
99+
locale.ToolsScraperPrivateUsername,
100+
locale.ToolsScraperPrivateUsernameDesc,
101+
))
98102
fields = append(fields, m.createCredentialField(config, "private_password",
99103
locale.ToolsScraperPrivatePassword,
100104
locale.ToolsScraperPrivatePasswordDesc,

backend/pkg/tools/browser.go

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ var localZones = []string{
2929
".localdomain",
3030
".local",
3131
".lan",
32+
".htb",
3233
".dev",
3334
".test",
3435
".corp",
@@ -215,36 +216,50 @@ func (b *browser) resolveUrl(targetURL string) (*url.URL, error) {
215216
host = u.Host
216217
}
217218

219+
// determine if target is private or public
220+
isPrivate := false
221+
218222
hostIP := net.ParseIP(host)
219223
if hostIP != nil {
220-
if hostIP.IsPrivate() {
221-
return url.Parse(b.scPrvURL)
224+
isPrivate = hostIP.IsPrivate() || hostIP.IsLoopback()
225+
} else {
226+
ip, err := net.ResolveIPAddr("ip", host)
227+
if err == nil {
228+
isPrivate = ip.IP.IsPrivate() || ip.IP.IsLoopback()
222229
} else {
223-
return url.Parse(b.scPubURL)
230+
lowerHost := strings.ToLower(host)
231+
if strings.Contains(lowerHost, "localhost") || !strings.Contains(lowerHost, ".") {
232+
isPrivate = true
233+
} else {
234+
for _, zone := range localZones {
235+
if strings.HasSuffix(lowerHost, zone) {
236+
isPrivate = true
237+
break
238+
}
239+
}
240+
}
224241
}
225242
}
226243

227-
ip, err := net.ResolveIPAddr("ip", host)
228-
if err == nil {
229-
if ip.IP.IsPrivate() {
230-
return url.Parse(b.scPrvURL)
231-
} else {
232-
return url.Parse(b.scPubURL)
244+
// select appropriate scraper URL with fallback
245+
var scraperURL string
246+
if isPrivate {
247+
scraperURL = b.scPrvURL
248+
if scraperURL == "" {
249+
scraperURL = b.scPubURL
250+
}
251+
} else {
252+
scraperURL = b.scPubURL
253+
if scraperURL == "" {
254+
scraperURL = b.scPrvURL
233255
}
234256
}
235257

236-
lowerHost := strings.ToLower(host)
237-
if strings.Contains(lowerHost, "localhost") || !strings.Contains(lowerHost, ".") {
238-
return url.Parse(b.scPrvURL)
239-
}
240-
241-
for _, zone := range localZones {
242-
if strings.HasSuffix(lowerHost, zone) {
243-
return url.Parse(b.scPrvURL)
244-
}
258+
if scraperURL == "" {
259+
return nil, fmt.Errorf("no scraper URL configured")
245260
}
246261

247-
return url.Parse(b.scPubURL)
262+
return url.Parse(scraperURL)
248263
}
249264

250265
func (b *browser) writeScreenshotToFile(screenshot []byte) (string, error) {

backend/pkg/tools/browser_test.go

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
package tools
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestBrowserResolveUrl(t *testing.T) {
8+
tests := []struct {
9+
name string
10+
scPrvURL string
11+
scPubURL string
12+
targetURL string
13+
wantURL string
14+
wantErr bool
15+
}{
16+
{
17+
name: "both URLs set, private target uses private",
18+
scPrvURL: "http://scraper-prv:8080",
19+
scPubURL: "http://scraper-pub:8080",
20+
targetURL: "http://192.168.1.1/test",
21+
wantURL: "http://scraper-prv:8080",
22+
wantErr: false,
23+
},
24+
{
25+
name: "both URLs set, public target uses public",
26+
scPrvURL: "http://scraper-prv:8080",
27+
scPubURL: "http://scraper-pub:8080",
28+
targetURL: "https://google.com",
29+
wantURL: "http://scraper-pub:8080",
30+
wantErr: false,
31+
},
32+
{
33+
name: "only private URL set, private target uses private",
34+
scPrvURL: "http://scraper-prv:8080",
35+
scPubURL: "",
36+
targetURL: "http://localhost:3000",
37+
wantURL: "http://scraper-prv:8080",
38+
wantErr: false,
39+
},
40+
{
41+
name: "only private URL set, public target falls back to private",
42+
scPrvURL: "http://scraper-prv:8080",
43+
scPubURL: "",
44+
targetURL: "https://example.com",
45+
wantURL: "http://scraper-prv:8080",
46+
wantErr: false,
47+
},
48+
{
49+
name: "only public URL set, public target uses public",
50+
scPrvURL: "",
51+
scPubURL: "http://scraper-pub:8080",
52+
targetURL: "https://google.com",
53+
wantURL: "http://scraper-pub:8080",
54+
wantErr: false,
55+
},
56+
{
57+
name: "only public URL set, private target falls back to public",
58+
scPrvURL: "",
59+
scPubURL: "http://scraper-pub:8080",
60+
targetURL: "http://10.0.0.1",
61+
wantURL: "http://scraper-pub:8080",
62+
wantErr: false,
63+
},
64+
{
65+
name: "no URLs set, returns error",
66+
scPrvURL: "",
67+
scPubURL: "",
68+
targetURL: "https://example.com",
69+
wantURL: "",
70+
wantErr: true,
71+
},
72+
{
73+
name: "localhost target uses private",
74+
scPrvURL: "http://scraper-prv:8080",
75+
scPubURL: "http://scraper-pub:8080",
76+
targetURL: "http://localhost:8080",
77+
wantURL: "http://scraper-prv:8080",
78+
wantErr: false,
79+
},
80+
{
81+
name: "local zone target uses private",
82+
scPrvURL: "http://scraper-prv:8080",
83+
scPubURL: "http://scraper-pub:8080",
84+
targetURL: "http://myapp.local",
85+
wantURL: "http://scraper-prv:8080",
86+
wantErr: false,
87+
},
88+
{
89+
name: "10.x.x.x private IP uses private",
90+
scPrvURL: "http://scraper-prv:8080",
91+
scPubURL: "http://scraper-pub:8080",
92+
targetURL: "http://10.1.2.3:8000",
93+
wantURL: "http://scraper-prv:8080",
94+
wantErr: false,
95+
},
96+
{
97+
name: "172.16.x.x private IP uses private",
98+
scPrvURL: "http://scraper-prv:8080",
99+
scPubURL: "http://scraper-pub:8080",
100+
targetURL: "http://172.16.0.1",
101+
wantURL: "http://scraper-prv:8080",
102+
wantErr: false,
103+
},
104+
}
105+
106+
for _, tt := range tests {
107+
t.Run(tt.name, func(t *testing.T) {
108+
b := &browser{
109+
scPrvURL: tt.scPrvURL,
110+
scPubURL: tt.scPubURL,
111+
}
112+
113+
gotURL, err := b.resolveUrl(tt.targetURL)
114+
115+
if (err != nil) != tt.wantErr {
116+
t.Errorf("resolveUrl() error = %v, wantErr %v", err, tt.wantErr)
117+
return
118+
}
119+
120+
if err == nil {
121+
got := gotURL.Scheme + "://" + gotURL.Host
122+
if got != tt.wantURL {
123+
t.Errorf("resolveUrl() = %v, want %v", got, tt.wantURL)
124+
}
125+
}
126+
})
127+
}
128+
}
129+
130+
func TestBrowserIsAvailable(t *testing.T) {
131+
tests := []struct {
132+
name string
133+
scPrvURL string
134+
scPubURL string
135+
want bool
136+
}{
137+
{
138+
name: "both URLs set",
139+
scPrvURL: "http://scraper-prv:8080",
140+
scPubURL: "http://scraper-pub:8080",
141+
want: true,
142+
},
143+
{
144+
name: "only private URL set",
145+
scPrvURL: "http://scraper-prv:8080",
146+
scPubURL: "",
147+
want: true,
148+
},
149+
{
150+
name: "only public URL set",
151+
scPrvURL: "",
152+
scPubURL: "http://scraper-pub:8080",
153+
want: true,
154+
},
155+
{
156+
name: "no URLs set",
157+
scPrvURL: "",
158+
scPubURL: "",
159+
want: false,
160+
},
161+
}
162+
163+
for _, tt := range tests {
164+
t.Run(tt.name, func(t *testing.T) {
165+
b := &browser{
166+
scPrvURL: tt.scPrvURL,
167+
scPubURL: tt.scPubURL,
168+
}
169+
170+
if got := b.IsAvailable(); got != tt.want {
171+
t.Errorf("IsAvailable() = %v, want %v", got, tt.want)
172+
}
173+
})
174+
}
175+
}

0 commit comments

Comments
 (0)