Skip to content

Commit d0925bd

Browse files
authored
Merge pull request #442 from DEEIX-AI/fix
fix: Add logo URL validation and update dompurify version
2 parents ad55e3a + 37ed3e7 commit d0925bd

5 files changed

Lines changed: 69 additions & 15 deletions

File tree

backend/internal/application/auth/provider.go

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -598,13 +598,8 @@ func (s *Service) normalizeProviderInput(input UpsertIdentityProviderInput, curr
598598
return nil, ErrIdentityProviderSuperAdminDefaultRoleNotAllowed
599599
}
600600
logoURL := strings.TrimSpace(input.LogoURL)
601-
if logoURL != "" {
602-
parsedLogoURL, err := url.Parse(logoURL)
603-
isHTTPLogoURL := (parsedLogoURL.Scheme == "http" || parsedLogoURL.Scheme == "https") && parsedLogoURL.Host != ""
604-
isAbsolutePathLogoURL := strings.HasPrefix(logoURL, "/")
605-
if err != nil || (!isHTTPLogoURL && !isAbsolutePathLogoURL) {
606-
return nil, fmt.Errorf("logo url must be a valid http(s) or absolute path")
607-
}
601+
if logoURL != "" && !isValidProviderLogoURL(logoURL) {
602+
return nil, fmt.Errorf("logo url must be a valid http(s) or absolute path")
608603
}
609604
provider := &domainuser.IdentityProvider{
610605
Type: providerType,
@@ -660,6 +655,21 @@ func (s *Service) normalizeProviderInput(input UpsertIdentityProviderInput, curr
660655
return provider, nil
661656
}
662657

658+
func isValidProviderLogoURL(value string) bool {
659+
parsedLogoURL, err := url.Parse(value)
660+
if err != nil {
661+
return false
662+
}
663+
if (parsedLogoURL.Scheme == "http" || parsedLogoURL.Scheme == "https") && parsedLogoURL.Host != "" {
664+
return true
665+
}
666+
return parsedLogoURL.Scheme == "" &&
667+
parsedLogoURL.Host == "" &&
668+
strings.HasPrefix(value, "/") &&
669+
!strings.HasPrefix(value, "//") &&
670+
!strings.Contains(value, "\\")
671+
}
672+
663673
func toProviderViews(items []domainuser.IdentityProvider, includeSensitive bool) []IdentityProviderView {
664674
results := make([]IdentityProviderView, 0, len(items))
665675
for _, item := range items {

backend/internal/application/auth/provider_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,47 @@ func TestNormalizeProviderInputProtectsSuperAdminDefaultRole(t *testing.T) {
9090
}
9191
}
9292

93+
func TestNormalizeProviderInputValidatesLogoURL(t *testing.T) {
94+
service := NewService(config.Config{JWTSecret: "test-secret"}, &providerLoginRepo{}, nil)
95+
96+
cases := []struct {
97+
name string
98+
logoURL string
99+
wantErr bool
100+
}{
101+
{name: "https url", logoURL: "https://example.com/logo.svg"},
102+
{name: "http url", logoURL: "http://example.com/logo.svg"},
103+
{name: "absolute path", logoURL: "/identity-providers/acme.svg"},
104+
{name: "protocol relative url", logoURL: "//example.com/logo.svg", wantErr: true},
105+
{name: "data url", logoURL: "data:image/svg+xml,<svg/>", wantErr: true},
106+
{name: "javascript url", logoURL: "javascript:alert(1)", wantErr: true},
107+
{name: "relative path", logoURL: "identity-providers/acme.svg", wantErr: true},
108+
{name: "backslash path", logoURL: `/\example.svg`, wantErr: true},
109+
}
110+
111+
for _, tc := range cases {
112+
t.Run(tc.name, func(t *testing.T) {
113+
_, err := service.normalizeProviderInput(UpsertIdentityProviderInput{
114+
ActorRole: domainuser.RoleAdmin,
115+
Type: domainuser.IdentityProviderTypeOIDC,
116+
Name: "Acme SSO",
117+
LogoURL: tc.logoURL,
118+
ClientID: "client",
119+
ClientSecret: "secret",
120+
DiscoveryURL: "https://example.com/.well-known/openid-configuration",
121+
RegistrationEnabled: boolPtr(true),
122+
DefaultRole: domainuser.RoleUser,
123+
}, nil)
124+
if tc.wantErr && err == nil {
125+
t.Fatalf("expected logo URL %q to be rejected", tc.logoURL)
126+
}
127+
if !tc.wantErr && err != nil {
128+
t.Fatalf("expected logo URL %q to be accepted, got %v", tc.logoURL, err)
129+
}
130+
})
131+
}
132+
}
133+
93134
func TestResolveProviderUserAutoRegistrationAddsUsernameSuffixOnCollision(t *testing.T) {
94135
repo := &providerLoginRepo{duplicateUsernameAttempts: 1}
95136
service := NewService(config.Config{JWTSecret: "test-secret"}, repo, nil)

frontend/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"@babel/core": "7.29.7",
99
"brace-expansion@1": "1.1.15",
1010
"brace-expansion@5": "5.0.6",
11-
"dompurify": "3.4.10",
11+
"dompurify": "3.4.11",
1212
"flatted": "3.4.2",
1313
"js-yaml": "4.2.0",
1414
"mermaid": "11.15.0",

frontend/pnpm-lock.yaml

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

frontend/shared/components/identity-provider-icon.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ function resolveCustomIdentityProviderLogoURL(logoURL: string | undefined, slug:
1717
if (/^https?:\/\//i.test(trimmed)) {
1818
return `/api/v1/auth/providers/${encodeURIComponent(slug)}/logo`;
1919
}
20-
return trimmed;
20+
if (trimmed.startsWith("/") && !trimmed.startsWith("//") && !trimmed.includes("\\")) {
21+
return trimmed;
22+
}
23+
return "";
2124
}
2225

2326
export function IdentityProviderIcon({

0 commit comments

Comments
 (0)