Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query params in URL bypasses BLACKLIST setting #3

Open
li6in9muyou opened this issue Jun 3, 2023 · 0 comments
Open

Query params in URL bypasses BLACKLIST setting #3

li6in9muyou opened this issue Jun 3, 2023 · 0 comments

Comments

@li6in9muyou
Copy link

To reproduce

  1. set PROXY_BLACKLIST to /blacklist
  2. send a GET request to /blacklist without any authorization header
  3. now send another GET request with query params /blacklist?foo=bar without any authorization header

Expected behavior

Both request get 401.

Actual behavior

The second request does not get 401.

Possible cause

I believe the cause is in this function

func VerifyJwtMiddleware(next http.Handler) http.Handler {
var isWhitelistMatch = func(url string, whitelistedURL string) bool {
whitelistedURL = strings.TrimSpace(whitelistedURL)
if strings.HasSuffix(whitelistedURL, "/") {
whitelistedURL = whitelistedURL[:len(whitelistedURL)-1]
}
if whitelistedURL != "" && (url == whitelistedURL || strings.HasPrefix(url, whitelistedURL+"/")) {
return true
}
return false
}
var IsWhitelisted = func(r *http.Request) bool {
url := r.URL.RequestURI()
// Check for whitelisted public API paths
for _, whitelistedURL := range unauthorizedRoutes {
if isWhitelistMatch(url, whitelistedURL) {
return true
}
}
// All other public API paths require a valid auth token
if strings.HasPrefix(url, GetConfig().PublicAPIPath) {
return false
}
// Whitelist Mode: Check is URL is whitelisted, else assume auth token is required
if len(GetConfig().ProxyWhitelist) > 0 {
for _, whitelistedURL := range GetConfig().ProxyWhitelist {
if isWhitelistMatch(url, whitelistedURL) {
return true
}
}
return false
}
// Blacklist Mode: Check is URL is blacklisted, else assume auth token is NOT required
for _, blacklistedURL := range GetConfig().ProxyBlacklist {
if isWhitelistMatch(url, blacklistedURL) {
return false
}
}
return true
}
var HandleWhitelistReq = func(w http.ResponseWriter, r *http.Request) {
claims, authHeader, err := ExtractClaimsFromRequest(r)
if err != nil {
next.ServeHTTP(w, r)
return
}
ctx := context.WithValue(r.Context(), contextKeyUserID, claims.UserID)
ctx = context.WithValue(ctx, contextKeyAuthHeader, authHeader)
next.ServeHTTP(w, r.WithContext(ctx))
}
var HandleNonWhitelistReq = func(w http.ResponseWriter, r *http.Request) {
claims, authHeader, err := ExtractClaimsFromRequest(r)
if err != nil {
log.Println(err)
SendUnauthorized(w)
return
}
ctx := context.WithValue(r.Context(), contextKeyUserID, claims.UserID)
ctx = context.WithValue(ctx, contextKeyAuthHeader, authHeader)
next.ServeHTTP(w, r.WithContext(ctx))
}
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == "OPTIONS" {
HandleWhitelistReq(w, r)
} else if IsWhitelisted(r) {
HandleWhitelistReq(w, r)
} else {
HandleNonWhitelistReq(w, r)
}
})
}

li6in9muyou added a commit to li6in9muyou/fix-my-issues-jwt-auth-proxy that referenced this issue Jun 3, 2023
li6in9muyou added a commit to li6in9muyou/fix-my-issues-jwt-auth-proxy that referenced this issue Jun 7, 2023
li6in9muyou added a commit to li6in9muyou/fix-my-issues-jwt-auth-proxy that referenced this issue Jun 7, 2023
(cherry picked from commit 7de85e0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant