diff --git a/README.md b/README.md index 5e8b8e3..e39e5de 100644 --- a/README.md +++ b/README.md @@ -12,3 +12,8 @@ OIDC-verified authentication for [JaWS](https://github.com/linkdata/jaws) sessio - Adds PKCE (S256) and OIDC nonce verification to the authorization-code flow. - Automatically refreshes the `id_token` in the background before it expires. - Supports admin-only handlers: `Wrap`/`Handler` for any authenticated user, `WrapAdmin`/`HandlerAdmin` gated by `SetAdmins`. + +Protected routes create JaWS sessions for unauthenticated visitors. Set +`Jaws.MaxSessions` and `Jaws.MaxSessionsPerIP` to bound session counts. When a +session cannot be created, login returns 503 with `Retry-After: 60`. +Post-login return targets longer than 8192 bytes fall back to `/`. diff --git a/oauth2.go b/oauth2.go index ee6b35d..9205652 100644 --- a/oauth2.go +++ b/oauth2.go @@ -27,6 +27,9 @@ const oauth2PKCEVerifierKey = "oauth2pkceverifier" const oauth2NonceKey = "oauth2nonce" const oauth2IDTokenExpiryKey = "oauth2idtokenexpiry" // #nosec G101 +// maxRedirectTargetLen bounds return targets retained in pre-auth sessions. +const maxRedirectTargetLen = 8192 + func normalizeHost(hostport string) (normalized string) { normalized = strings.TrimSpace(hostport) if normalized != "" { @@ -56,15 +59,8 @@ func sanitizeRedirectTarget(requestHost, location string) (sanitized string) { } } sanitized = strings.TrimSpace(sanitized) - if sanitized != "" { - if !strings.HasPrefix(sanitized, "/") { - sanitized = "/" + sanitized - } - for strings.HasPrefix(sanitized, "//") { - sanitized = "/" + strings.TrimLeft(sanitized, "/") - } - } - if sanitized == "" { + sanitized = "/" + strings.TrimLeft(sanitized, "/\\") + if len(sanitized) > maxRedirectTargetLen { sanitized = "/" } return @@ -86,6 +82,7 @@ func (srv *Server) begin(hr *http.Request) (oauth2cfg *oauth2.Config, location s // // For GET requests it generates and stores the state, nonce and PKCE verifier in the // session, then responds with a 302 redirect to the provider's authorization URL. +// If no session can be created, it returns 503 with Retry-After instead. // Non-GET requests receive 405. func (srv *Server) HandleLogin(hw http.ResponseWriter, hr *http.Request) { statusCode := http.StatusMethodNotAllowed @@ -96,19 +93,22 @@ func (srv *Server) HandleLogin(hw http.ResponseWriter, hr *http.Request) { if sess == nil { sess = srv.Jaws.NewSession(hw, hr) } - if sess != nil { - authOptions := append([]oauth2.AuthCodeOption{}, srv.Options...) - state := randomHexString() - sess.Set(oauth2StateKey, state) - nonce := randomHexString() - sess.Set(oauth2NonceKey, nonce) - authOptions = append(authOptions, oidc.Nonce(nonce)) - verifier := oauth2.GenerateVerifier() - sess.Set(oauth2PKCEVerifierKey, verifier) - authOptions = append(authOptions, oauth2.S256ChallengeOption(verifier)) - sess.Set(oauth2ReferrerKey, location) - location = oauth2cfg.AuthCodeURL(state, authOptions...) + if sess == nil { + hw.Header().Set("Retry-After", "60") + srv.writeResult(hw, http.StatusServiceUnavailable, ErrOAuth2MissingSession, nil) + return } + authOptions := append([]oauth2.AuthCodeOption{}, srv.Options...) + state := randomHexString() + sess.Set(oauth2StateKey, state) + nonce := randomHexString() + sess.Set(oauth2NonceKey, nonce) + authOptions = append(authOptions, oidc.Nonce(nonce)) + verifier := oauth2.GenerateVerifier() + sess.Set(oauth2PKCEVerifierKey, verifier) + authOptions = append(authOptions, oauth2.S256ChallengeOption(verifier)) + sess.Set(oauth2ReferrerKey, location) + location = oauth2cfg.AuthCodeURL(state, authOptions...) } hw.Header().Set("Location", location) statusCode = http.StatusFound diff --git a/oauth2_test.go b/oauth2_test.go index ef354f3..1327ec6 100644 --- a/oauth2_test.go +++ b/oauth2_test.go @@ -1468,3 +1468,77 @@ func Test_handlersRejectNonGet(t *testing.T) { t.Fatal(string(body)) } } + +func TestRedirectTargetBounds(t *testing.T) { + for _, target := range []string{`/\docs`, `\docs`, `/\/docs`} { + if got := sanitizeRedirectTarget("example.com", target); got != "/docs" { + t.Errorf("target %q: got %q", target, got) + } + } + for _, referer := range []bool{false, true} { + for _, tc := range []struct { + length int + accepted bool + }{ + // RFC 9110 section 4.1 recommends support for 8000-octet URIs. + {8000, true}, + {maxRedirectTargetLen, true}, + {maxRedirectTargetLen + 1, false}, + } { + jw, err := jaws.New() + if err != nil { + t.Fatal(err) + } + srv := newWrapperTestServer(jw, "https://issuer.example") + target := "/" + strings.Repeat("a", tc.length-1) + req := httptest.NewRequest(http.MethodGet, target, nil) + if referer { + req = httptest.NewRequest(http.MethodGet, "/oauth2/login", nil) + req.Header.Set("Referer", target) + } + srv.HandleLogin(httptest.NewRecorder(), req) + want := target + if !tc.accepted { + want = "/" + } + if got := jw.GetSession(req).Get(oauth2ReferrerKey); got != want { + t.Errorf("referer=%v length=%d: unexpected stored target", referer, tc.length) + } + jw.Close() + } + } +} + +func TestLoginSessionUnavailable(t *testing.T) { + for _, limit := range []string{"global", "perIP", "closed"} { + t.Run(limit, func(t *testing.T) { + jw, err := jaws.New() + if err != nil { + t.Fatal(err) + } + defer jw.Close() + switch limit { + case "global": + jw.MaxSessions = 1 + case "perIP": + jw.MaxSessionsPerIP = 1 + } + jw.NewSession(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/", nil)) + if limit == "closed" { + jw.Close() + } + srv := newWrapperTestServer(jw, "https://issuer.example") + for _, handler := range []http.Handler{ + http.HandlerFunc(srv.HandleLogin), + srv.Wrap(testStatusHandler{http.StatusOK}), + srv.WrapAdmin(testStatusHandler{http.StatusOK}), + } { + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/protected", nil)) + if rec.Code != http.StatusServiceUnavailable || rec.Header().Get("Location") != "" || rec.Header().Get("Retry-After") != "60" { + t.Fatalf("status=%d headers=%v", rec.Code, rec.Header()) + } + } + }) + } +} diff --git a/wrapper.go b/wrapper.go index a7e05f9..289850f 100644 --- a/wrapper.go +++ b/wrapper.go @@ -14,9 +14,6 @@ type wrapper struct { func (w wrapper) ServeHTTP(hw http.ResponseWriter, hr *http.Request) { h := w.handler sess := w.server.Jaws.GetSession(hr) - if sess == nil { - sess = w.server.Jaws.NewSession(hw, hr) - } if current, present := w.server.sessionAuthStatus(sess, time.Now); !current { if present { w.server.clearSessionAuth(sess, hr, true, false, nil)