Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Project Guidance

## Commands

| Command | Purpose |
|---------|---------|
| `go test -p 1 ./...` | Run the root `github.com/go-pkgz/auth` module tests. |
| `cd v2 && go test -p 1 ./...` | Run the `github.com/go-pkgz/auth/v2` module tests. |
| `go test -timeout=60s -v -race -p 1 -covermode=atomic -coverprofile=$GITHUB_WORKSPACE/profile.cov ./...` | Root-module CI test command from `.github/workflows/ci.yml`. |
| `cd v2 && go test -timeout=60s -v -race -p 1 -covermode=atomic -coverprofile=$GITHUB_WORKSPACE/profile.cov ./...` | v2-module CI test command from `.github/workflows/ci-v2.yml`. |
| `go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.6.2 run --max-issues-per-linter=0 --max-same-issues=0` | Reproduce root-module CI lint locally. |
| `cd v2 && go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.6.2 run --config ../.golangci.yml --max-issues-per-linter=0 --max-same-issues=0` | Reproduce v2-module CI lint locally. |
| `~/.claude/format.sh` | Format Go code when available; it runs gofmt/goimports. |

## Architecture

- root module `github.com/go-pkgz/auth` is the v1 API; `v2/` is a separate Go module `github.com/go-pkgz/auth/v2` with mirrored packages.
- `_example/` is a separate example module that replaces `github.com/go-pkgz/auth/v2 => ../v2`; v2 CI builds it before testing `v2/`.
- `auth.go` wires the public `Service`, providers, middleware, token service, avatar proxy, and security headers.
- `provider/` contains OAuth1/OAuth2, Apple, direct, verify, Telegram, dev, and custom-provider flows; most provider changes need the same change under `v2/provider/`.
- `token/` wraps JWT/cookie/XSRF behavior; v1 uses `github.com/golang-jwt/jwt` v3 claims, v2 uses `github.com/golang-jwt/jwt/v5` registered claims.
- `avatar/` owns avatar proxying/storage and has filesystem, bbolt, GridFS, and no-op stores.
- `middleware/` owns auth, trace, admin-only, RBAC, basic auth, validator, token refresh, and request user context.

## CI and Lint Notes

- CI pins golangci-lint through GitHub Actions (`golangci/golangci-lint-action@v7` with `version: v2.6.2` in `.github/workflows/ci*.yml`). Newer local `golangci-lint` versions can report extra `gosec` findings that CI does not enforce.
- Root CI (`.github/workflows/ci.yml`) ignores `v2/**`, `_example/**`, and `ci-v2.yml`; v2 CI (`.github/workflows/ci-v2.yml`) runs only for `v2/**`, `_example/**`, and `ci-v2.yml` changes.
- Mongo-backed tests run in CI with `ENABLE_MONGO_TESTS=true` after `wbari/start-mongoDB@v0.2` starts MongoDB 6.0.
- The workflow coverage step currently installs `github.com/mattn/goveralls@latest` with `COVERALLS_TOKEN=${{ secrets.GITHUB_TOKEN }}`; security reviews should keep checking this supply-chain path.

## Project Rules

- For behavior shared by v1 and v2, update root and `v2/` implementations and tests together unless the difference is explicitly version-specific.
- Preserve backward compatibility for public auth flows and documented query parameters. For example, direct login still supports `passwd` in the URL, so sensitive-query fixes must redact rather than remove that input path.
- Avoid logging raw user profiles, mapped `token.User` values, JWTs, OAuth tokens, confirmation tokens, or credential-bearing URLs. Use redacted request copies when passing URLs with secrets to `rest.SendErrorJSON`.
- `AllowedRedirectHosts` hardening is opt-in. Nil keeps legacy permissive `from` redirects; a non-nil getter enables host validation.
- Verify-provider confirmation tokens are one-shot only when `VerifConfirmationStore` is configured; nil installs an in-memory store via `AddVerifProvider`, suitable only for single-instance deployments.
- Avatar handling is security-sensitive: keep content-type validation, image dimension/size caps, and bot-token URL redaction intact in both root and v2.

## Testing Gotchas

- Run root and v2 tests separately; `go test ./...` from the root does not test the separate `v2` module.
- When reproducing CI lint, use the pinned `v2.6.2` command above, not the globally installed `golangci-lint` binary unless its version matches CI.
- Many tests bind fixed localhost ports in the 898x/899x range; use `-p 1` for package test runs to reduce port-collision risk.
- `go test ./...` in the root includes Mongo tests only when `ENABLE_MONGO_TESTS=true`; without it, CI-only Mongo coverage may not run locally.
4 changes: 1 addition & 3 deletions provider/apple.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,6 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) {
return
}

ah.Logf("[DEBUG] user info %+v", u)

// redirect to back url if presented in login query params
if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" {
if !isAllowedRedirect(oauthClaims.Handshake.From, ah.URL, ah.AllowedRedirectHosts) {
Expand Down Expand Up @@ -516,7 +514,7 @@ func (ah *AppleHandler) parseUserData(user *token.User, jUser string) {

// catch error for log only. No need break flow if user name doesn't exist
if err := json.Unmarshal([]byte(jUser), &userData); err != nil {
ah.Logf("[DEBUG] failed to parse user data %s: %v", user, err)
ah.Logf("[DEBUG] failed to parse apple user data: %v", err)
user.Name = "noname_" + user.ID[6:12] // paste noname if user name failed to parse
return
}
Expand Down
29 changes: 22 additions & 7 deletions provider/direct.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,24 +76,25 @@ func (p DirectHandler) Name() string { return p.ProviderName }
// "aud": "bar",
// }
func (p DirectHandler) LoginHandler(w http.ResponseWriter, r *http.Request) {
logReq := p.scrubPasswordFromRequest(r)
creds, err := p.getCredentials(w, r)
if err != nil {
rest.SendErrorJSON(w, r, p.L, http.StatusBadRequest, err, "failed to parse credentials")
rest.SendErrorJSON(w, logReq, p.L, http.StatusBadRequest, err, "failed to parse credentials")
return
}
sessOnly := r.URL.Query().Get("sess") == "1"
if p.CredChecker == nil {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError,
rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError,
fmt.Errorf("no credential checker"), "no credential checker")
return
}
ok, err := p.CredChecker.Check(creds.User, creds.Password)
if err != nil {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to check user credentials")
rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "failed to check user credentials")
return
}
if !ok {
rest.SendErrorJSON(w, r, p.L, http.StatusForbidden, nil, "incorrect user or password")
rest.SendErrorJSON(w, logReq, p.L, http.StatusForbidden, nil, "incorrect user or password")
return
}

Expand All @@ -108,13 +109,13 @@ func (p DirectHandler) LoginHandler(w http.ResponseWriter, r *http.Request) {
}
u, err = setAvatar(p.AvatarSaver, u, &http.Client{Timeout: 5 * time.Second})
if err != nil {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to save avatar to proxy")
rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "failed to save avatar to proxy")
return
}

cid, err := randToken()
if err != nil {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "can't make token id")
rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "can't make token id")
return
}

Expand All @@ -132,12 +133,26 @@ func (p DirectHandler) LoginHandler(w http.ResponseWriter, r *http.Request) {
}

if _, err = p.TokenService.Set(w, claims); err != nil {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to set token")
rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "failed to set token")
return
}
rest.RenderJSON(w, claims.User)
}

func (p DirectHandler) scrubPasswordFromRequest(r *http.Request) *http.Request {
if r == nil || r.URL == nil {
return r
}
if _, ok := r.URL.Query()["passwd"]; !ok {
return r
}
rc := r.Clone(r.Context())
q := rc.URL.Query()
q.Set("passwd", "<redacted>")
rc.URL.RawQuery = q.Encode()
return rc
}

// getCredentials extracts user and password from request
func (p DirectHandler) getCredentials(w http.ResponseWriter, r *http.Request) (credentials, error) {

Expand Down
28 changes: 28 additions & 0 deletions provider/direct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,34 @@ func TestDirect_LoginHandlerFailed(t *testing.T) {
}
}

func TestDirect_LoginHandlerRedactsQueryPasswordInLogs(t *testing.T) {
logBuf := strings.Builder{}
d := DirectHandler{
ProviderName: "test",
CredChecker: &mockCredsChecker{ok: false},
TokenService: token.NewService(token.Opts{
SecretReader: token.SecretFunc(func(string) (string, error) { return "secret", nil }),
TokenDuration: time.Hour,
CookieDuration: time.Hour * 24 * 31,
}),
Issuer: "iss-test",
L: logger.Func(func(format string, args ...any) {
fmt.Fprintf(&logBuf, format, args...)
}),
}

rr := httptest.NewRecorder()
req, err := http.NewRequest("GET", "/login?user=myuser&passwd=secret-password&aud=xyz123", http.NoBody)
require.NoError(t, err)

d.LoginHandler(rr, req)

assert.Equal(t, http.StatusForbidden, rr.Code)
assert.Equal(t, "secret-password", req.URL.Query().Get("passwd"), "original request must not be mutated")
assert.NotContains(t, logBuf.String(), "secret-password")
assert.Contains(t, logBuf.String(), "passwd=<redacted>")
}

func TestDirect_Logout(t *testing.T) {
d := DirectHandler{
ProviderName: "test",
Expand Down
4 changes: 0 additions & 4 deletions provider/oauth1.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,6 @@ func (h Oauth1Handler) AuthHandler(w http.ResponseWriter, r *http.Request) {
rest.SendErrorJSON(w, r, h.L, http.StatusInternalServerError, err, "failed to unmarshal user info")
return
}
h.Logf("[DEBUG] got raw user info %+v", jData)

u := h.mapUser(jData, data)
u, err = setAvatar(h.AvatarSaver, u, &http.Client{Timeout: 5 * time.Second})
if err != nil {
Expand Down Expand Up @@ -159,8 +157,6 @@ func (h Oauth1Handler) AuthHandler(w http.ResponseWriter, r *http.Request) {
return
}

h.Logf("[DEBUG] user info %+v", u)

// redirect to back url if presented in login query params
if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" {
if !isAllowedRedirect(oauthClaims.Handshake.From, h.URL, h.AllowedRedirectHosts) {
Expand Down
25 changes: 25 additions & 0 deletions provider/oauth1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,30 @@ func TestOauth1LoginFromRejectsExternalHost(t *testing.T) {
assert.NotContains(t, string(body), "evil.example.com")
}

func TestOauth1LoginDoesNotLogUserProfile(t *testing.T) {
logBuf := strings.Builder{}
captureLog := func(p *Params) {
p.L = logger.Func(func(format string, args ...any) {
fmt.Fprintf(&logBuf, format, args...)
})
}
teardown := prepOauth1Test(t, 8993, 8994, captureLog)
defer teardown()

jar, err := cookiejar.New(nil)
require.NoError(t, err)
client := &http.Client{Jar: jar, Timeout: timeout * time.Second}

resp, err := client.Get("http://localhost:8993/login?site=remark")
require.NoError(t, err)
defer resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode)

logged := logBuf.String()
assert.NotContains(t, logged, "oauth-sensitive@example.com")
assert.NotContains(t, logged, "mock_myuser1")
}

func prepOauth1Test(t *testing.T, loginPort, authPort int, paramOpts ...func(*Params)) func() { //nolint

provider := Oauth1Handler{
Expand Down Expand Up @@ -305,6 +329,7 @@ func prepOauth1Test(t *testing.T, loginPort, authPort int, paramOpts ...func(*Pa
res := fmt.Sprintf(`{
"id": "%s",
"name":"blah",
"email":"oauth-sensitive@example.com",
"picture":"http://exmple.com/pic1.png"
}`, useIDs[count])
count++
Expand Down
4 changes: 0 additions & 4 deletions provider/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,6 @@ func (p Oauth2Handler) AuthHandler(w http.ResponseWriter, r *http.Request) {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to unmarshal user info")
return
}
p.Logf("[DEBUG] got raw user info %+v", jData)

u := p.mapUser(jData, data)
if oauthClaims.NoAva {
u.Picture = "" // reset picture on no avatar request
Expand Down Expand Up @@ -243,8 +241,6 @@ func (p Oauth2Handler) AuthHandler(w http.ResponseWriter, r *http.Request) {
p.bearerTokenHook(p.Name(), u, *tok)
}

p.Logf("[DEBUG] user info %+v", u)

// redirect to back url if presented in login query params
if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" {
if !isAllowedRedirect(oauthClaims.Handshake.From, p.URL, p.AllowedRedirectHosts) {
Expand Down
25 changes: 25 additions & 0 deletions provider/oauth2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,30 @@ func TestOauth2LoginFromAllowsAllowlistedHost(t *testing.T) {
assert.Contains(t, lastRedirect, "trusted.example.com")
}

func TestOauth2LoginDoesNotLogUserProfile(t *testing.T) {
logBuf := strings.Builder{}
captureLog := func(p *Params) {
p.L = logger.Func(func(format string, args ...any) {
fmt.Fprintf(&logBuf, format, args...)
})
}
teardown := prepOauth2Test(t, 8991, 8992, nil, captureLog)
defer teardown()

jar, err := cookiejar.New(nil)
require.NoError(t, err)
client := &http.Client{Jar: jar, Timeout: 5 * time.Second}

resp, err := client.Get("http://localhost:8991/login?site=remark")
require.NoError(t, err)
defer resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode)

logged := logBuf.String()
assert.NotContains(t, logged, "oauth-sensitive@example.com")
assert.NotContains(t, logged, "mock_myuser1")
}

func TestMakeRedirURL(t *testing.T) {
cases := []struct{ rootURL, route, out string }{
{"localhost:8080/", "/my/auth/path/google", "localhost:8080/my/auth/path/callback"},
Expand Down Expand Up @@ -415,6 +439,7 @@ func prepOauth2Test(t *testing.T, loginPort, authPort int, btHook BearerTokenHoo
res := fmt.Sprintf(`{
"id": "%s",
"name":"blah",
"email":"oauth-sensitive@example.com",
"picture":"http://exmple.com/pic1.png"
}`, useIDs[count])
count++
Expand Down
4 changes: 1 addition & 3 deletions v2/provider/apple.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,6 @@ func (ah AppleHandler) AuthHandler(w http.ResponseWriter, r *http.Request) {
return
}

ah.Logf("[DEBUG] user info %+v", u)

// redirect to back url if presented in login query params
if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" {
if !isAllowedRedirect(oauthClaims.Handshake.From, ah.URL, ah.AllowedRedirectHosts) {
Expand Down Expand Up @@ -522,7 +520,7 @@ func (ah *AppleHandler) parseUserData(user *token.User, jUser string) {

// catch error for log only. No need break flow if user name doesn't exist
if err := json.Unmarshal([]byte(jUser), &userData); err != nil {
ah.Logf("[DEBUG] failed to parse user data %s: %v", user, err)
ah.Logf("[DEBUG] failed to parse apple user data: %v", err)
user.Name = "noname_" + user.ID[6:12] // paste noname if user name failed to parse
return
}
Expand Down
29 changes: 22 additions & 7 deletions v2/provider/direct.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,24 +76,25 @@ func (p DirectHandler) Name() string { return p.ProviderName }
// "aud": "bar",
// }
func (p DirectHandler) LoginHandler(w http.ResponseWriter, r *http.Request) {
logReq := p.scrubPasswordFromRequest(r)
creds, err := p.getCredentials(w, r)
if err != nil {
rest.SendErrorJSON(w, r, p.L, http.StatusBadRequest, err, "failed to parse credentials")
rest.SendErrorJSON(w, logReq, p.L, http.StatusBadRequest, err, "failed to parse credentials")
return
}
sessOnly := r.URL.Query().Get("sess") == "1"
if p.CredChecker == nil {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError,
rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError,
fmt.Errorf("no credential checker"), "no credential checker")
return
}
ok, err := p.CredChecker.Check(creds.User, creds.Password)
if err != nil {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to check user credentials")
rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "failed to check user credentials")
return
}
if !ok {
rest.SendErrorJSON(w, r, p.L, http.StatusForbidden, nil, "incorrect user or password")
rest.SendErrorJSON(w, logReq, p.L, http.StatusForbidden, nil, "incorrect user or password")
return
}

Expand All @@ -108,13 +109,13 @@ func (p DirectHandler) LoginHandler(w http.ResponseWriter, r *http.Request) {
}
u, err = setAvatar(p.AvatarSaver, u, &http.Client{Timeout: 5 * time.Second})
if err != nil {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to save avatar to proxy")
rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "failed to save avatar to proxy")
return
}

cid, err := randToken()
if err != nil {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "can't make token id")
rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "can't make token id")
return
}

Expand All @@ -132,12 +133,26 @@ func (p DirectHandler) LoginHandler(w http.ResponseWriter, r *http.Request) {
}

if _, err = p.TokenService.Set(w, claims); err != nil {
rest.SendErrorJSON(w, r, p.L, http.StatusInternalServerError, err, "failed to set token")
rest.SendErrorJSON(w, logReq, p.L, http.StatusInternalServerError, err, "failed to set token")
return
}
rest.RenderJSON(w, claims.User)
}

func (p DirectHandler) scrubPasswordFromRequest(r *http.Request) *http.Request {
if r == nil || r.URL == nil {
return r
}
if _, ok := r.URL.Query()["passwd"]; !ok {
return r
}
rc := r.Clone(r.Context())
q := rc.URL.Query()
q.Set("passwd", "<redacted>")
rc.URL.RawQuery = q.Encode()
return rc
}

// getCredentials extracts user and password from request
func (p DirectHandler) getCredentials(w http.ResponseWriter, r *http.Request) (credentials, error) {

Expand Down
Loading
Loading