fix: bound login redirects and handle session limits

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 `/`.

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

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())
+				}
+			}
+		})
+	}
+}

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)