elazarl / goproxy

An HTTP proxy library for Go
BSD 3-Clause "New" or "Revised" License
6.06k stars 1.1k forks source link

Basic auth fixed #428

Open clburlison opened 3 years ago

clburlison commented 3 years ago

Issue: When using AlwaysMitm ext/auth/basic.go would attempt to authenticate requests twice resulting in the 407 unauthorized message.

This fixes two outstanding issues with the basic auth behavior of this library.

  1. BasicConnect handler was preventing other handlers from running (AlwaysMitm)
  2. Basic was trying to authenticate connection that was already authenticated by BasicConnect

Resolves: https://github.com/elazarl/goproxy/issues/309, https://github.com/elazarl/goproxy/issues/416

Below is a simple test case to validate the behavior before/after the patch:

package main

import (
    "flag"
    "log"
    "net"
    "net/http"
    "regexp"

    "github.com/elazarl/goproxy"
    "github.com/elazarl/goproxy/ext/auth"
)

func handleRequest(req *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) {
    ip, _, err := net.SplitHostPort(req.RemoteAddr)
    if err != nil {
        log.Print(err)
    }

    log.Printf("[%d] %s --> %s %s", ctx.Session, ip, req.Method, req.URL)

    return req, nil
}

func main() {
    verbose := flag.Bool("v", false, "should every proxy request be logged to stdout")
    addr := flag.String("addr", ":8080", "proxy listen address")
    flag.Parse()
    username, password := "foo", "bar"
    hostmatch := "google.com|neverssl.com|apache.org"
    proxy := goproxy.NewProxyHttpServer()
    auth.ProxyBasic(proxy, "Auth", func(user, passwd string) bool {
        return user == username && password == passwd
    })
    proxy.OnRequest(goproxy.ReqHostMatches(regexp.MustCompile(hostmatch))).
        HandleConnect(goproxy.AlwaysMitm)
    proxy.OnRequest().DoFunc(handleRequest)
    proxy.Verbose = *verbose
    log.Fatal(http.ListenAndServe(*addr, proxy))
}
batuzyn commented 3 years ago

@elazarl Could you merge this PR?

elazarl commented 3 years ago

I'm very careful about adding yet another field to the context. Can we do it in some other way?

batuzyn commented 3 years ago

Actually I did't think another way :) But this PR working correctly for me.

batuzyn commented 3 years ago

Sorry, I had a problem after i implemented it. 'ctx.Authenticated' not returns 'true' for http connect methods.

I am getting 407 (proxy auth required) for this code. Is there something I did wrong?

`addr := flag.String("addr", ":8080", "proxy listen address") hostmatch := flag.String("hostmatch", "^.*$", "hosts to trace (regexp pattern)") verbose := flag.Bool("v", true, "verbose output") flag.Parse()

log.SetFlags(log.Lmicroseconds)

proxy := goproxy.NewProxyHttpServer()

auth.ProxyBasic(proxy, "Auth", func(user, passwd string) bool {
    return true
})

proxy.OnRequest(goproxy.ReqHostMatches(regexp.MustCompile(*hostmatch))).
    HandleConnect(goproxy.AlwaysMitm)

proxy.OnRequest().DoFunc(handleRequest)

proxy.Verbose = *verbose
log.Fatal(http.ListenAndServe(*addr, proxy))`
thalesfsp commented 3 years ago

Any update on this?

elazarl commented 3 years ago

@thalesfsp as I said, I'm reluctant to add another field to the Ctx without a really good reason.

I reviewed the code and asked isn't using the User pointer possible, but didn't receive response yet.

thalesfsp commented 3 years ago

@elazarl Given the importance of the MR, could you do the change you are suggesting? Best regards!

faf-xff commented 2 years ago

这个啥时候能修复呢

thalesfsp commented 1 year ago

@guanyufen123 just fork and merge the change. This is what happens when the community needs and demands things but authors are reluctant to listen.

elazarl commented 1 year ago

@thalesfsp can you please explain me again, why using the User pointer is not a good solution in this case?

I'm very open to listen, but was not able to spurr discussion.