Allow per-app allow/deny.

2026-08-06 13:53:24 -04:00
parent c15a1cc14c
commit 8ce2919627
19 changed files with 1151 additions and 187 deletions
@@ -0,0 +1,261 @@
+package authproxy
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+// writeAppLists lays out one app's config the way the dokku plugin does.
+func writeAppLists(t *testing.T, dir, app string, files map[string]string) {
+	t.Helper()
+	appDir := filepath.Join(dir, app)
+	if err := os.MkdirAll(appDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	for name, content := range files {
+		if err := os.WriteFile(filepath.Join(appDir, name), []byte(content), 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+}
+
+func TestAppStoreReadsLists(t *testing.T) {
+	dir := t.TempDir()
+	writeAppLists(t, dir, "my-app", map[string]string{
+		"allowed-domains": "Signal.org\n@example.com\n",
+		"allowed-emails":  "Guest@Partner.com\n",
+		"denied-emails":   "former@signal.org\n",
+	})
+	store := newAppStore(dir)
+
+	got := store.lists("my-app")
+	if len(got.AllowedDomains) != 2 || got.AllowedDomains[0] != "signal.org" || got.AllowedDomains[1] != "example.com" {
+		t.Errorf("AllowedDomains = %v", got.AllowedDomains)
+	}
+	if len(got.AllowedEmails) != 1 || got.AllowedEmails[0] != "guest@partner.com" {
+		t.Errorf("AllowedEmails = %v", got.AllowedEmails)
+	}
+	if len(got.DeniedEmails) != 1 || got.DeniedEmails[0] != "former@signal.org" {
+		t.Errorf("DeniedEmails = %v", got.DeniedEmails)
+	}
+
+	// An app with no directory, and a disabled store, both mean "inherit".
+	if store.lists("other-app").hasAllowRules() {
+		t.Error("an app with no config should have no allow rules")
+	}
+	if newAppStore("").lists("my-app").hasAllowRules() {
+		t.Error("an empty config dir should disable per-app lists")
+	}
+}
+
+// The app name reaches the service in a header, so it must never be able to
+// walk out of the config directory.
+func TestAppStoreRejectsUnusableNames(t *testing.T) {
+	dir := t.TempDir()
+	writeAppLists(t, dir, "escaped", map[string]string{"allowed-emails": "attacker@evil.com\n"})
+	store := newAppStore(dir)
+
+	for _, name := range []string{
+		"", ".", "..", "../escaped", "..%2fescaped", "/escaped", "sub/escaped",
+		".hidden", "app name", "app;rm", "app\x00", "app\n",
+	} {
+		if got := store.lists(name); got.hasAllowRules() || len(got.DeniedEmails) > 0 {
+			t.Errorf("lists(%q) returned rules; want empty", name)
+		}
+		if validAppName(name) {
+			t.Errorf("validAppName(%q) = true, want false", name)
+		}
+	}
+	for _, name := range []string{"my-app", "my_app", "app.example", "App2"} {
+		if !validAppName(name) {
+			t.Errorf("validAppName(%q) = false, want true", name)
+		}
+	}
+}
+
+func TestAppStoreCachesUntilTTL(t *testing.T) {
+	dir := t.TempDir()
+	writeAppLists(t, dir, "my-app", map[string]string{"allowed-emails": "first@signal.org\n"})
+
+	now := time.Unix(1700000000, 0)
+	store := newAppStore(dir)
+	store.now = func() time.Time { return now }
+
+	if got := store.lists("my-app").AllowedEmails[0]; got != "first@signal.org" {
+		t.Fatalf("first read = %q", got)
+	}
+	writeAppLists(t, dir, "my-app", map[string]string{"allowed-emails": "second@signal.org\n"})
+	if got := store.lists("my-app").AllowedEmails[0]; got != "first@signal.org" {
+		t.Errorf("within the TTL the cached value should stand, got %q", got)
+	}
+	now = now.Add(store.ttl + time.Second)
+	if got := store.lists("my-app").AllowedEmails[0]; got != "second@signal.org" {
+		t.Errorf("after the TTL the change should be picked up, got %q", got)
+	}
+}
+
+// newPerAppServer is a server whose global list allows all of signal.org, plus
+// per-app overrides on disk.
+func newPerAppServer(t *testing.T, apps map[string]map[string]string) *Server {
+	t.Helper()
+	dir := t.TempDir()
+	for app, files := range apps {
+		writeAppLists(t, dir, app, files)
+	}
+	s := newTestServer(t, "http://unused.invalid")
+	s.cfg.AppConfigDir = dir
+	s.apps = newAppStore(dir)
+	return s
+}
+
+func TestPerAppAllowReplacesGlobal(t *testing.T) {
+	s := newPerAppServer(t, map[string]map[string]string{
+		"narrowed": {"allowed-emails": "ceo@signal.org\n"},
+		"widened":  {"allowed-emails": "guest@partner.com\n"},
+	})
+
+	cases := []struct {
+		app, email string
+		want       bool
+	}{
+		// No per-app config: the global list applies.
+		{"inherits", "anyone@signal.org", true},
+		{"inherits", "guest@partner.com", false},
+		{"", "anyone@signal.org", true},
+		// Own allow list: it replaces the global one entirely.
+		{"narrowed", "ceo@signal.org", true},
+		{"narrowed", "anyone@signal.org", false},
+		// An app may admit someone the global list does not cover.
+		{"widened", "guest@partner.com", true},
+		{"widened", "anyone@signal.org", false},
+		// An unknown app name falls back to global rather than failing open.
+		{"no-such-app", "anyone@signal.org", true},
+		{"../narrowed", "anyone@signal.org", true},
+	}
+	for _, c := range cases {
+		if got := s.emailAllowedFor(c.app, c.email); got != c.want {
+			t.Errorf("emailAllowedFor(%q, %q) = %v, want %v", c.app, c.email, got, c.want)
+		}
+	}
+}
+
+func TestPerAppDenyCombinesWithGlobal(t *testing.T) {
+	s := newPerAppServer(t, map[string]map[string]string{
+		"app-a": {"denied-emails": "bob@signal.org\n"},
+		// An app's own deny list must not resurrect a globally denied account.
+		"app-b": {"denied-emails": "someone@signal.org\n"},
+		// Nor may an app's allow list override a global denial.
+		"app-c": {"allowed-emails": "gone@signal.org\n"},
+	})
+	s.cfg.DeniedEmails = []string{"gone@signal.org"}
+
+	cases := []struct {
+		app, email string
+		want       bool
+	}{
+		{"app-a", "bob@signal.org", false},  // denied for this app
+		{"app-b", "bob@signal.org", true},   // ...but not for another
+		{"app-a", "gone@signal.org", false}, // global denial still applies
+		{"app-b", "gone@signal.org", false}, // even with its own deny list
+		{"app-c", "gone@signal.org", false}, // even when the app allows them
+		{"inherits", "gone@signal.org", false},
+	}
+	for _, c := range cases {
+		if got := s.emailAllowedFor(c.app, c.email); got != c.want {
+			t.Errorf("emailAllowedFor(%q, %q) = %v, want %v", c.app, c.email, got, c.want)
+		}
+	}
+}
+
+// The app name is read from a header, so verify must use the value nginx set
+// and a session must not be portable to an app with stricter rules.
+func TestVerifyUsesAppHeader(t *testing.T) {
+	s := newPerAppServer(t, map[string]map[string]string{
+		"strict": {"allowed-emails": "ceo@signal.org\n"},
+	})
+	sess := sessionClaims{Email: "dev@signal.org", User: "1", Host: appHost,
+		Exp: time.Now().Add(time.Hour).Unix()}
+	val, err := s.box.seal("session", sess)
+	if err != nil {
+		t.Fatal(err)
+	}
+	verify := func(app string) int {
+		r := httptest.NewRequest("GET", "http://"+appHost+RoutePrefix+"/verify", nil)
+		r.AddCookie(&http.Cookie{Name: "_google_auth", Value: val})
+		if app != "" {
+			r.Header.Set(AppHeader, app)
+		}
+		return do(s.Routes(), r).Code
+	}
+
+	if got := verify("lenient"); got != http.StatusOK {
+		t.Errorf("app inheriting the global list: got %d, want 200", got)
+	}
+	if got := verify("strict"); got != http.StatusUnauthorized {
+		t.Errorf("app whose list excludes the user: got %d, want 401", got)
+	}
+	if got := verify(""); got != http.StatusOK {
+		t.Errorf("no app header: got %d, want 200 (global rules)", got)
+	}
+}
+
+// The callback runs on the auth host, which may serve a different app than the
+// one being signed in to. Authorization must follow the destination app.
+func TestCallbackAuthorizesAgainstDestinationApp(t *testing.T) {
+	google := fakeGoogle(t, goodClaims()) // signs in greyson@signal.org
+	defer google.Close()
+	s := newPerAppServer(t, map[string]map[string]string{
+		"strict": {"allowed-emails": "ceo@signal.org\n"},
+	})
+	s.cfg.TokenURL = google.URL
+
+	callback := func(app string) *httptest.ResponseRecorder {
+		tok, err := s.box.seal("state", stateClaims{
+			Host: appHost, App: app, RD: "/", Proto: "https", Nonce: randToken(),
+			Exp: time.Now().Add(10 * time.Minute).Unix(),
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		r := httptest.NewRequest("GET",
+			"http://"+authHost+RoutePrefix+"/callback?code=good-code&state="+url.QueryEscape(tok), nil)
+		// The auth host's own app name, which must not be what decides.
+		r.Header.Set(AppHeader, "auth-host-app")
+		return do(s.Routes(), r)
+	}
+
+	if w := callback("lenient"); w.Code != http.StatusFound {
+		t.Errorf("destination inheriting the global list: got %d, want 302; body: %s", w.Code, w.Body.String())
+	}
+	if w := callback("strict"); w.Code != http.StatusForbidden {
+		t.Errorf("destination whose list excludes the user: got %d, want 403", w.Code)
+	}
+}
+
+// The hd hint should follow the app's effective domain list, not the global one.
+func TestStartHintsAppDomain(t *testing.T) {
+	s := newPerAppServer(t, map[string]map[string]string{
+		"partner-app": {"allowed-domains": "partner.com\n"},
+	})
+	start := func(app string) string {
+		r := httptest.NewRequest("GET", "http://"+appHost+RoutePrefix+"/start", nil)
+		r.Header.Set("Accept", "text/html")
+		r.Header.Set(AppHeader, app)
+		loc, err := url.Parse(do(s.Routes(), r).Header().Get("Location"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		return loc.Query().Get("hd")
+	}
+	if got := start("inherits"); got != "signal.org" {
+		t.Errorf("hd for an inheriting app = %q, want signal.org", got)
+	}
+	if got := start("partner-app"); got != "partner.com" {
+		t.Errorf("hd for an app with its own domain = %q, want partner.com", got)
+	}
+}