From 170489745b11317fa8e0323cfb70bc200c895b13 Mon Sep 17 00:00:00 2001 From: mattthew Date: Fri, 2 Oct 2026 13:43:05 -0400 Subject: [PATCH 1/6] feat(security): add a --security "anonymous=..." posture, defaulting to full Three controls gate Alpha's privileged operations -- the --security whitelist, the --security token, and ACL -- and each passes when its own feature is unconfigured. The protection an operator gets is whatever they configured rather than the union of the three. The gap that leaves is specific: the whitelist answers where a request came from and carries no credential, so widening it is by itself enough to make administrative operations reachable anonymously from anywhere inside the range. "whitelist=0.0.0.0/0" is a common setting and nothing is left to check. This adds a key that names the missing decision: --security "anonymous=full|data| none", where an anonymous caller is one whose request produced no x.Principal. full today's behavior, and the default. Nothing about an unconfigured cluster changes. data queries, mutations, commits, login, and health stay open; every Capability check denies regardless of the whitelist. none additionally denies queries, mutations, and commits. Implemented as an AccessController that wraps whichever policy is installed rather than replacing it. "Has this caller been identified at all" is prior to and independent of "what may this identity do", so answering it inside the capability rules would mean every policy reimplementing the same check. It installs after ConfigureIdentity so a deployment policy is wrapped, not discarded, and under anonymous=full it installs nothing at all. The --security token now mints a Principal (x.MethodPreshared) instead of answering a yes/no, which is what makes a closed posture usable without standing up full ACL. ACL is tried first so a request carrying both resolves to the user and keeps userDataFromPrincipal's fast path. Principal.Groups is left empty on purpose: x.IsSuperAdmin reads it by name, so a "guardians" entry would turn a shared secret into cluster authority without passing authorizeClusterAdmin. Subject matches audit.PoorManAuth so audit output for token-bearing callers is unchanged, pinned by a test. Zero honors the same key. Its admin HTTP handler stops letting a whitelisted source IP stand in for the token once the posture is closed, and enforces the non-strict routes (/state, /assign) that are otherwise open until something is configured. Two startup warnings, both pure and table-tested: a widened whitelist with no credential, and a closed posture with no credential (where every capability check denies, including the operator's own, so the cluster cannot be administered). Also attaches full request identity on /query, /mutate, /commit, /state and /health?all, which previously took only the access JWT. A live run caught this: with anonymous=data and a token configured, /state refused a caller presenting that token, because the header never became auth-token metadata. TestHTTPEdgeResolvesIdentityThroughOneHelper pins it -- handlers go through x.AttachRequestIdentity or appear in an exception list with a reason, and a stale exception fails too. Note for review: --security is shared with Zero and z.SuperFlag log.Fatals on an unknown key, so a config that sets anonymous= will not start on a pre-v25 binary. Nothing in-tree sets it, and no docker-compose or dgraphtest default was changed. Verified against a live local cluster across all three values: the stock default is byte-for-byte unchanged and silent; anonymous=data with whitelist=0.0.0.0/0 turns /state and listBackups from answered into Unauthenticated while queries keep working; the token re-opens them; anonymous=none closes the data plane while /health stays open for readiness. Co-Authored-By: Claude Opus 5 --- dgraph/cmd/alpha/http.go | 7 +- dgraph/cmd/alpha/run.go | 41 +++- dgraph/cmd/alpha/security_posture.go | 63 ++++++ dgraph/cmd/alpha/security_posture_test.go | 199 +++++++++++++++++ dgraph/cmd/zero/admin.go | 18 +- dgraph/cmd/zero/admin_test.go | 62 ++++++ dgraph/cmd/zero/run.go | 17 +- edgraph/anonymous.go | 99 +++++++++ edgraph/anonymous_test.go | 256 ++++++++++++++++++++++ edgraph/authn.go | 87 ++++++++ edgraph/authn_identity_test.go | 157 +++++++++++++ edgraph/server.go | 18 ++ worker/server_state.go | 2 +- x/anonymous.go | 94 ++++++++ x/anonymous_test.go | 91 ++++++++ x/config.go | 4 + 16 files changed, 1201 insertions(+), 14 deletions(-) create mode 100644 dgraph/cmd/alpha/security_posture.go create mode 100644 dgraph/cmd/alpha/security_posture_test.go create mode 100644 edgraph/anonymous.go create mode 100644 edgraph/anonymous_test.go create mode 100644 edgraph/authn.go create mode 100644 edgraph/authn_identity_test.go create mode 100644 x/anonymous.go create mode 100644 x/anonymous_test.go diff --git a/dgraph/cmd/alpha/http.go b/dgraph/cmd/alpha/http.go index 4f0f2ff48ef..9cdaad63f0d 100644 --- a/dgraph/cmd/alpha/http.go +++ b/dgraph/cmd/alpha/http.go @@ -240,8 +240,7 @@ func queryHandler(w http.ResponseWriter, r *http.Request) { } ctx := context.WithValue(r.Context(), query.DebugKey, isDebugMode) - ctx = x.AttachAccessJwt(ctx, r) - ctx = x.AttachRemoteIP(ctx, r) + ctx = x.AttachRequestIdentity(ctx, r) if queryTimeout != 0 { var cancel context.CancelFunc @@ -467,7 +466,7 @@ func mutationHandler(w http.ResponseWriter, r *http.Request) { req.Hash = hash req.CommitNow = commitNow - ctx := x.AttachAccessJwt(context.Background(), r) + ctx := x.AttachRequestIdentity(context.Background(), r) resp, err := (&edgraph.Server{}).QueryNoGrpc(ctx, req) if err != nil { x.SetStatusWithData(w, x.ErrorInvalidRequest, err.Error()) @@ -530,7 +529,7 @@ func commitHandler(w http.ResponseWriter, r *http.Request) { return } - ctx := x.AttachAccessJwt(context.Background(), r) + ctx := x.AttachRequestIdentity(context.Background(), r) var response map[string]interface{} if abort { response, err = handleAbort(ctx, startTs, hash) diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go index 14da3a3e257..b662d627cc9 100644 --- a/dgraph/cmd/alpha/run.go +++ b/dgraph/cmd/alpha/run.go @@ -173,7 +173,18 @@ they form a Raft group and provide synchronous replication. "A comma separated list of IP addresses, IP ranges, CIDR blocks, or hostnames you wish "+ "to whitelist for performing admin actions (i.e., --security "+ `"whitelist=144.142.126.254,127.0.0.1:127.0.0.3,192.168.0.0/16,host.docker.`+ - `internal").`). + `internal"). Empty by default, which admits loopback only. This is a network `+ + "location check, NOT authentication: every address in the range can run "+ + "privileged operations without a credential unless you also set token= or enable "+ + "ACL."). + Flag("anonymous", + "[full, data, none] What a caller that presents no verified credential may do. "+ + "full (default) leaves authorization to whatever the whitelist, token, and ACL "+ + "settings decide, which is the behavior of every earlier release. data allows "+ + "queries, mutations, commits, and login while denying every administrative "+ + "operation regardless of the whitelist. none additionally denies queries, "+ + "mutations, and commits, leaving only login and the health endpoints. data and "+ + "none require token= or ACL, otherwise no caller can ever be identified."). String()) flag.String("limit", worker.LimitDefaults, z.NewSuperFlagHelp(worker.LimitDefaults). @@ -310,7 +321,10 @@ func healthCheck(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - ctx := x.AttachAccessJwt(context.Background(), r) + // Full identity, not just the access JWT: Health(all) is a capability check, + // and under a closed --security "anonymous=..." posture a caller presenting + // only the --security token would otherwise arrive unidentified. + ctx := x.AttachRequestIdentity(context.Background(), r) var resp *api.Response if resp, err = (&edgraph.Server{}).Health(ctx, true); err != nil { x.SetStatus(w, x.Error, err.Error()) @@ -355,8 +369,9 @@ func stateHandler(w http.ResponseWriter, r *http.Request) { x.AddCorsHeaders(w) w.Header().Set("Content-Type", "application/json") - ctx := context.Background() - ctx = x.AttachAccessJwt(ctx, r) + // Full identity, not just the access JWT: State is a capability check. See the + // note on the /health?all branch above. + ctx := x.AttachRequestIdentity(context.Background(), r) var aResp *api.Response if aResp, err = (&edgraph.Server{}).State(ctx); err != nil { @@ -652,6 +667,8 @@ func run() { FromSuperFlag(Alpha.Conf.GetString("badger")) security := z.NewSuperFlag(Alpha.Conf.GetString("security")).MergeAndCheckDefault( worker.SecurityDefaults) + anonymous, err := x.ParseAnonymousPosture(security.GetString("anonymous")) + x.Check(err) conf := audit.GetAuditConf(Alpha.Conf.GetString("audit")) x.Config.Limit = z.NewSuperFlag(Alpha.Conf.GetString("limit")).MergeAndCheckDefault( @@ -703,6 +720,11 @@ func run() { ips, err := getIPsFromString(security.GetString("whitelist")) x.Check(err) + for _, msg := range securityWarnings(anonymous, security.GetString("whitelist"), ips, + opts.AuthToken, keys.AclSecretKey != nil, httpPort()) { + glog.Warning(msg) + } + tlsClientConf, err := x.LoadClientTLSConfigForInternalPort(Alpha.Conf) x.Check(err) tlsServerConf, err := x.LoadServerTLSConfigForInternalPort(Alpha.Conf) @@ -720,6 +742,7 @@ func run() { AbortOlderThan: abortDur, StartTime: startTime, Security: security, + Anonymous: anonymous, TLSClientConfig: tlsClientConf, TLSServerConfig: tlsServerConf, AclJwtAlg: keys.AclJwtAlg, @@ -729,11 +752,21 @@ func run() { } x.WorkerConfig.Parse(Alpha.Conf) + // The built-in authenticator: ACL's access JWT, plus the --security auth token as + // an identity rather than a boolean check. Installed before ConfigureIdentity so a + // deployment can compose with or replace it; see edgraph.PresharedAuthenticator. + x.SetAuthenticator(edgraph.PresharedAuthenticator()) + // Install deployment-specific authentication and authorization now: the config // is parsed, and nothing is serving yet. A misconfiguration here is fatal, which // is why it runs before any listener rather than lazily on the first request. ConfigureIdentity() + // The --security "anonymous=..." floor goes on last, so that it wraps whatever + // policy ConfigureIdentity installed rather than being replaced by it. Under the + // shipped anonymous=full this installs nothing. + edgraph.EnforceAnonymousPosture(anonymous) + // Set the directory for temporary buffers. z.SetTmpDir(x.WorkerConfig.TmpDir) diff --git a/dgraph/cmd/alpha/security_posture.go b/dgraph/cmd/alpha/security_posture.go new file mode 100644 index 00000000000..532a2571a0b --- /dev/null +++ b/dgraph/cmd/alpha/security_posture.go @@ -0,0 +1,63 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package alpha + +import ( + "fmt" + + "github.com/dgraph-io/dgraph/v25/x" +) + +// securityWarnings returns the startup warnings for a --security configuration +// whose parts do not add up, or nil when they do. +// +// Three controls gate Alpha's privileged operations -- the whitelist, the auth +// token, and ACL -- and each one passes when its own feature is unconfigured. The +// protection an operator gets is whatever they configured rather than the union of +// the three, and the two combinations below are the ones where that produces an +// outcome they almost certainly did not intend. +// +// It is pure and returns strings rather than logging, so the combinations can be +// pinned by a table test. +func securityWarnings(posture x.AnonymousPosture, whitelist string, ips []x.IPRange, + authToken string, aclEnabled bool, httpPort int) []string { + + hasCredential := aclEnabled || authToken != "" + var out []string + + // The shipped default is an empty whitelist, which admits loopback only, so the + // admin plane does not leave the host without an operator widening it. This + // warns at exactly that point: whitelisting answers where a request came from + // and has no credential in it, so a widened range with nothing else configured + // means every address inside it can run privileged operations anonymously. + if posture == x.AnonymousFull && len(ips) > 0 && !hasCredential { + out = append(out, fmt.Sprintf( + `SECURITY: --security "whitelist=%s" admits non-loopback callers, but neither ACL nor `+ + `an admin token is configured. Privileged operations (backup, restore, export, `+ + `shutdown, removeNode, moveTablet, assign, draining, config, namespace create and `+ + `drop) are reachable from that range WITHOUT ANY CREDENTIAL: anyone who can reach `+ + `port %d can read the whole database via backup or export, restore over it, or shut `+ + `the cluster down. Set --security "token=..." or enable ACL, and narrow the `+ + `whitelist to the addresses that actually administer this cluster. `+ + `--security "anonymous=data" additionally denies every administrative operation to `+ + `a caller that presents no credential.`, whitelist, httpPort)) + } + + // A closed posture with nothing that can produce an identity. Every capability + // check will deny, including the operator's own, so the cluster cannot be + // administered at all. This is a misconfiguration rather than a hardening, and + // it is worth saying so loudly at boot instead of letting it surface as a + // permission error during an incident. + if posture.RequiresIdentityForCapability() && !hasCredential { + out = append(out, fmt.Sprintf( + `SECURITY: --security "anonymous=%s" requires an identified caller, but neither ACL nor `+ + `an admin token is configured, so no request can ever be identified. Every `+ + `administrative operation will be denied, including from loopback, and this cluster `+ + `cannot be administered. Set --security "token=..." or enable ACL.`, posture)) + } + + return out +} diff --git a/dgraph/cmd/alpha/security_posture_test.go b/dgraph/cmd/alpha/security_posture_test.go new file mode 100644 index 00000000000..4f649acac5f --- /dev/null +++ b/dgraph/cmd/alpha/security_posture_test.go @@ -0,0 +1,199 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package alpha + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/dgraph-io/dgraph/v25/x" +) + +func TestSecurityWarnings(t *testing.T) { + tests := []struct { + name string + posture x.AnonymousPosture + whitelist string + authToken string + acl bool + want []string // substrings that must appear, one per expected warning + }{{ + // The shipped default. An empty whitelist admits loopback only, so the admin + // plane has not left the host and there is nothing to say. + name: "stock alpha is quiet", + posture: x.AnonymousFull, + }, { + name: "widened whitelist with no credential", + posture: x.AnonymousFull, + whitelist: "0.0.0.0/0", + want: []string{"WITHOUT ANY CREDENTIAL"}, + }, { + // A plausible-looking CIDR is still every pod in the cluster. + name: "private CIDR with no credential", + posture: x.AnonymousFull, + whitelist: "10.0.0.0/8", + want: []string{"WITHOUT ANY CREDENTIAL"}, + }, { + name: "widened whitelist with a token", + posture: x.AnonymousFull, + whitelist: "0.0.0.0/0", + authToken: "s3cr3t", + }, { + name: "widened whitelist with ACL", + posture: x.AnonymousFull, + whitelist: "0.0.0.0/0", + acl: true, + }, { + // The closed posture answers the exposure, so the first warning must stand + // down rather than tell an operator to fix something they already fixed. + name: "widened whitelist with a closed posture", + posture: x.AnonymousData, + whitelist: "0.0.0.0/0", + authToken: "s3cr3t", + }, { + // A closed posture with nothing that can produce an identity. Every + // capability check denies, including the operator's own. + name: "closed posture with no credential at all", + posture: x.AnonymousData, + want: []string{"cannot be administered"}, + }, { + name: "anonymous=none with no credential at all", + posture: x.AnonymousNone, + whitelist: "0.0.0.0/0", + want: []string{"cannot be administered"}, + }, { + name: "closed posture with ACL is fine", + posture: x.AnonymousNone, + whitelist: "0.0.0.0/0", + acl: true, + }} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ips, err := getIPsFromString(tt.whitelist) + require.NoError(t, err) + + got := securityWarnings(tt.posture, tt.whitelist, ips, tt.authToken, tt.acl, 8080) + require.Len(t, got, len(tt.want)) + for i, want := range tt.want { + require.Contains(t, got[i], "SECURITY:") + require.Contains(t, got[i], want) + } + }) + } +} + +// TestDefaultWhitelistAdmitsLoopbackOnly pins the model the v25 default rests on: +// a stock `dgraph alpha`, with no --security flag, serves admin operations to this +// host and nothing else. That is what makes anonymous=full a defensible default, +// and what makes widening the whitelist the moment the admin plane leaves the +// host. +func TestDefaultWhitelistAdmitsLoopbackOnly(t *testing.T) { + prev := x.WorkerConfig.WhiteListedIPRanges + t.Cleanup(func() { x.WorkerConfig.WhiteListedIPRanges = prev }) + + ips, err := getIPsFromString("") + require.NoError(t, err) + x.WorkerConfig.WhiteListedIPRanges = ips + + for _, ip := range []string{"127.0.0.1", "::1"} { + require.Truef(t, x.IsIpWhitelisted(ip), "loopback %s must reach admin operations", ip) + } + for _, ip := range []string{ + "203.0.113.7", // arbitrary remote host + "172.17.0.1", // Docker bridge, i.e. the host reaching a published port + "10.0.5.7", // private network peer + "192.168.1.20", // LAN peer + } { + require.Falsef(t, x.IsIpWhitelisted(ip), + "non-loopback %s must not reach admin operations by default", ip) + } +} + +// identityExceptions names the handlers that deliberately assemble the request +// context by hand, and why. Everything else must go through +// x.AttachRequestIdentity. +var identityExceptions = map[string]string{ + // Login is how an access JWT is obtained, so it must not depend on one being + // present, and it needs no Principal of its own: edgraph.Login authorizes on + // hasAdminAuth, which reads the peer and the auth token straight out of the + // metadata this prelude attaches. + "loginHandler": "login must not require a credential it is the means of issuing", +} + +// TestHTTPEdgeResolvesIdentityThroughOneHelper pins an invariant that a live test +// caught the hard way. +// +// x.AttachRequestIdentity is the whole HTTP-edge prelude: access JWT, remote IP, +// auth token, then identity resolution. A handler that reaches for the individual +// pieces instead gets some of them, and the one it is most likely to omit is the +// auth token -- which is invisible until a closed --security "anonymous=..." +// posture is configured, at which point a caller presenting the token arrives +// unidentified and is refused. /state and /health?all both had exactly that shape. +// +// So: no handler in this package resolves identity by hand unless it is listed +// above with a reason. +func TestHTTPEdgeResolvesIdentityThroughOneHelper(t *testing.T) { + banned := map[string]string{ + "AttachAccessJwt": "attaches the access JWT but not the --security auth token", + "AttachAuthToken": "attaches the auth token but resolves no Principal", + "AttachRemoteIP": "attaches the peer but resolves no Principal", + } + + fset := token.NewFileSet() + pkgs, err := parser.ParseDir(fset, ".", func(fi os.FileInfo) bool { + return !strings.HasSuffix(fi.Name(), "_test.go") + }, 0) + require.NoError(t, err) + + seenExceptions := map[string]bool{} + for _, pkg := range pkgs { + for _, file := range pkg.Files { + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok { + continue + } + _, exempt := identityExceptions[fn.Name.Name] + ast.Inspect(fn, func(n ast.Node) bool { + sel, ok := n.(*ast.SelectorExpr) + if !ok { + return true + } + pkgIdent, ok := sel.X.(*ast.Ident) + if !ok || pkgIdent.Name != "x" { + return true + } + why, bad := banned[sel.Sel.Name] + if !bad { + return true + } + if exempt { + seenExceptions[fn.Name.Name] = true + return true + } + t.Errorf("%s: %s calls x.%s, which %s. Use x.AttachRequestIdentity, or add "+ + "it to identityExceptions with a reason.", + fset.Position(sel.Pos()), fn.Name.Name, sel.Sel.Name, why) + return true + }) + } + } + } + + // A stale exception is its own problem: it reads as a documented carve-out for + // something that no longer exists. + for name := range identityExceptions { + require.Truef(t, seenExceptions[name], + "identityExceptions lists %q, but it no longer assembles the context by hand", name) + } +} diff --git a/dgraph/cmd/zero/admin.go b/dgraph/cmd/zero/admin.go index d2accb0b3b4..1f3b617a59b 100644 --- a/dgraph/cmd/zero/admin.go +++ b/dgraph/cmd/zero/admin.go @@ -34,11 +34,18 @@ func adminSecurityConfigured() bool { // A request is authorized when it carries the configured poor man's auth token // (--security "token=...") in the X-Dgraph-AuthToken header, or its source IP is loopback or // within a configured whitelist range (--security "whitelist=..."). +// +// Under a closed --security "anonymous=..." posture the whitelist stops standing in for the +// token. The two answer different questions -- where a request came from, and who sent it -- +// and the whole point of the posture is that only the second one counts. func isAdminRequestAuthorized(r *http.Request) bool { if token := worker.Config.AuthToken; token != "" && subtle.ConstantTimeCompare([]byte(token), []byte(r.Header.Get("X-Dgraph-AuthToken"))) == 1 { return true } + if x.WorkerConfig.Anonymous.RequiresIdentityForCapability() { + return false + } return x.IsIpWhitelisted(remoteHost(r)) } @@ -54,16 +61,19 @@ func isAdminRequestAuthorized(r *http.Request) bool { // // - strict=false is used for the informational and allocation endpoints (/state, /assign), // which existing tooling (dashboards, monitoring, loaders) reads over HTTP. Enforcement is -// opt-in: auth is applied only once the operator sets a token or whitelist via --security. -// Until then the request is allowed, preserving prior behavior. Network isolation of the -// port remains the primary control for these, as it already is for Zero's gRPC surface. +// opt-in: auth is applied only once the operator sets a token or whitelist via --security, +// or selects a closed --security "anonymous=..." posture. Until then the request is allowed, +// preserving prior behavior. Network isolation of the port remains the primary control for +// these, as it already is for Zero's gRPC surface. func adminAuthHandler(strict bool, next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { x.AddCorsHeaders(w) if r.Method == http.MethodOptions { return } - if (strict || adminSecurityConfigured()) && !isAdminRequestAuthorized(r) { + enforce := strict || adminSecurityConfigured() || + x.WorkerConfig.Anonymous.RequiresIdentityForCapability() + if enforce && !isAdminRequestAuthorized(r) { w.WriteHeader(http.StatusUnauthorized) x.SetStatus(w, x.ErrorUnauthorized, "Request is not from a whitelisted IP and does not carry a valid X-Dgraph-AuthToken. "+ diff --git a/dgraph/cmd/zero/admin_test.go b/dgraph/cmd/zero/admin_test.go index fee6f48db13..e81ee8a1c73 100644 --- a/dgraph/cmd/zero/admin_test.go +++ b/dgraph/cmd/zero/admin_test.go @@ -31,11 +31,14 @@ func resetSecurityConfig(t *testing.T) { t.Helper() prevToken := worker.Config.AuthToken prevRanges := x.WorkerConfig.WhiteListedIPRanges + prevAnon := x.WorkerConfig.Anonymous worker.Config.AuthToken = "" x.WorkerConfig.WhiteListedIPRanges = nil + x.WorkerConfig.Anonymous = x.AnonymousFull t.Cleanup(func() { worker.Config.AuthToken = prevToken x.WorkerConfig.WhiteListedIPRanges = prevRanges + x.WorkerConfig.Anonymous = prevAnon }) } @@ -139,3 +142,62 @@ func TestZeroAdminAuth_WhitelistAllowsRemote(t *testing.T) { require.False(t, reached, "non-whitelisted remote request was admitted") require.Equal(t, http.StatusUnauthorized, rr.Code) } + +// TestZeroAdminAuth_ClosedPostureRequiresToken pins what --security +// "anonymous=..." means on Zero. The whitelist and the token answer different +// questions -- where a request came from, and who sent it -- and a closed posture +// says only the second one counts. Without that, an operator who set +// anonymous=data on Alpha would still have a Zero whose control plane admits +// anyone inside the range. +func TestZeroAdminAuth_ClosedPostureRequiresToken(t *testing.T) { + for _, posture := range []x.AnonymousPosture{x.AnonymousData, x.AnonymousNone} { + t.Run(posture.String(), func(t *testing.T) { + resetSecurityConfig(t) + x.WorkerConfig.Anonymous = posture + worker.Config.AuthToken = "s3cr3t" + ips, err := x.GetIPsFromString("0.0.0.0/0") + require.NoError(t, err) + x.WorkerConfig.WhiteListedIPRanges = ips + + // Whitelisted and loopback callers are both refused without the token. + for _, addr := range []string{"203.0.113.7:12345", "127.0.0.1:12345"} { + rr, reached := doAdminRequest(t, false, addr, "") + require.False(t, reached, "%s must not reach the handler without the token", addr) + require.Equal(t, http.StatusUnauthorized, rr.Code) + } + + // The token is sufficient, from anywhere. + rr, reached := doAdminRequest(t, false, "203.0.113.7:12345", "s3cr3t") + require.True(t, reached) + require.Equal(t, http.StatusOK, rr.Code) + }) + } +} + +// TestZeroAdminAuth_ClosedPostureEnforcesNonStrictRoutes: /state and /assign are +// registered non-strict, so they are unguarded until an operator configures +// something. A closed posture is that configuration. +func TestZeroAdminAuth_ClosedPostureEnforcesNonStrictRoutes(t *testing.T) { + resetSecurityConfig(t) + + // Control: with nothing configured, a non-strict route is open. + _, reached := doAdminRequest(t, false, "203.0.113.7:12345", "") + require.True(t, reached, "precondition: non-strict routes are open by default") + + x.WorkerConfig.Anonymous = x.AnonymousData + rr, reached := doAdminRequest(t, false, "203.0.113.7:12345", "") + require.False(t, reached) + require.Equal(t, http.StatusUnauthorized, rr.Code) +} + +// TestZeroAdminAuth_FullPostureIsUnchanged is the v25 default: every existing +// behavior of adminAuthHandler survives. +func TestZeroAdminAuth_FullPostureIsUnchanged(t *testing.T) { + resetSecurityConfig(t) + ips, err := x.GetIPsFromString("203.0.113.0/24") + require.NoError(t, err) + x.WorkerConfig.WhiteListedIPRanges = ips + + _, reached := doAdminRequest(t, true, "203.0.113.7:12345", "") + require.True(t, reached, "a whitelisted IP still stands in for a credential under full") +} diff --git a/dgraph/cmd/zero/run.go b/dgraph/cmd/zero/run.go index ce4ee233c5e..c693bde6a76 100644 --- a/dgraph/cmd/zero/run.go +++ b/dgraph/cmd/zero/run.go @@ -115,7 +115,13 @@ instances to achieve high-availability. Flag("whitelist", "A comma separated list of IP addresses, IP ranges, CIDR blocks, or hostnames that are "+ "allowed to reach Zero's administrative HTTP endpoints (loopback is always allowed). "+ - `e.g. --security "whitelist=127.0.0.1,192.168.0.0/16,host.docker.internal".`). + `e.g. --security "whitelist=127.0.0.1,192.168.0.0/16,host.docker.internal". This is `+ + "a network location check, NOT authentication."). + Flag("anonymous", + "[full, data, none] What a caller that presents no verified credential may do. full "+ + "(default) is the behavior of every earlier release. Any other value requires the "+ + "token above on every administrative HTTP endpoint, including /state and /assign, "+ + "and stops a whitelisted source IP from standing in for a credential."). String()) flag.String("audit", worker.AuditDefaults, z.NewSuperFlagHelp(worker.AuditDefaults). @@ -252,6 +258,15 @@ func run() { ips, err := x.GetIPsFromString(security.GetString("whitelist")) x.Check(err) x.WorkerConfig.WhiteListedIPRanges = ips + anonymous, err := x.ParseAnonymousPosture(security.GetString("anonymous")) + x.Check(err) + x.WorkerConfig.Anonymous = anonymous + if anonymous.RequiresIdentityForCapability() && worker.Config.AuthToken == "" { + glog.Warningf(`SECURITY: --security "anonymous=%s" requires an identified caller, but no `+ + `token is configured, so no request can ever be identified. Every administrative HTTP `+ + `endpoint will be denied, including from loopback. Set --security "token=...".`, + anonymous) + } if opts.numReplicas < 0 || opts.numReplicas%2 == 0 { log.Fatalf("ERROR: Number of replicas must be odd for consensus. Found: %d", diff --git a/edgraph/anonymous.go b/edgraph/anonymous.go new file mode 100644 index 00000000000..1b31b514ee8 --- /dev/null +++ b/edgraph/anonymous.go @@ -0,0 +1,99 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package edgraph + +import ( + "context" + + "github.com/golang/glog" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/dgraph-io/dgraph/v25/acl" + "github.com/dgraph-io/dgraph/v25/x" +) + +// anonymousPosture wraps an AccessController so that a caller with no verified +// identity is denied every capability, whatever the wrapped policy would have +// said. +// +// It wraps rather than replaces because the posture is a floor, not a policy. The +// question "has this caller been identified at all" is prior to and independent of +// "what may this identity do", and answering the first one in the capability rules +// themselves would mean every policy -- the built-in one, and any a deployment +// installs -- reimplementing the same check and getting it consistent. +// +// Predicate authorization is passed straight through. A query naming predicates +// the caller may not read is answered with the rest dropped rather than refused +// (see AccessController.AuthorizePredicates), so there is no "deny" to express +// here. Whether an anonymous caller may run the query at all is decided earlier, +// by RequireIdentifiedCaller. +type anonymousPosture struct { + inner AccessController + posture x.AnonymousPosture +} + +func (a anonymousPosture) Name() string { + return a.inner.Name() + " (anonymous=" + a.posture.String() + ")" +} + +func (a anonymousPosture) AuthorizeCapability(ctx context.Context, c Capability) error { + if a.posture.RequiresIdentityForCapability() && x.PrincipalFrom(ctx) == nil { + return status.Errorf(codes.Unauthenticated, + `the %s capability requires an identified caller, and this request presented no `+ + `credential that verified. This cluster runs with --security "anonymous=%s". `+ + `Present the --security auth token, or log in with ACL.`, + c, a.posture) + } + return a.inner.AuthorizeCapability(ctx, c) +} + +func (a anonymousPosture) AuthorizePredicates(ctx context.Context, preds []string, + op *acl.Operation) (*PredResult, error) { + return a.inner.AuthorizePredicates(ctx, preds, op) +} + +// EnforceAnonymousPosture installs the --security "anonymous=..." floor over +// whatever policy is currently installed. AnonymousFull installs nothing, so a +// cluster running the shipped default carries no extra indirection at all. +// +// Call it during command setup, after any deployment-specific SetAccessController +// and before any listener starts serving: it captures the policy installed at the +// moment it runs, so installing one afterwards would silently discard the floor. +func EnforceAnonymousPosture(posture x.AnonymousPosture) { + if !posture.RequiresIdentityForCapability() { + return + } + inner := currentAccessController() + SetAccessController(anonymousPosture{inner: inner, posture: posture}) + glog.Infof(`--security "anonymous=%s": capabilities now require an identified caller `+ + `(policy: %s)`, posture, inner.Name()) +} + +// RequireIdentifiedCaller rejects an anonymous caller's ordinary data access under +// --security "anonymous=none". +// +// Separate from AuthorizeCapability because queries, mutations, and commits are +// not capabilities and should not become them: a capability is cluster or tenant +// authority, deliberately coarse and deliberately few, while data access is +// decided per predicate. This is the one question the posture asks that predicate +// authorization cannot, because an anonymous caller on an ACL-off cluster has +// blanket predicate access by construction. +// +// It is a no-op under anonymous=full and anonymous=data, which is why the call +// sites can be unconditional. +func RequireIdentifiedCaller(ctx context.Context, op string) error { + if !x.WorkerConfig.Anonymous.RequiresIdentityForData() { + return nil + } + if x.PrincipalFrom(ctx) != nil { + return nil + } + return status.Errorf(codes.Unauthenticated, + `%s requires an identified caller, and this request presented no credential that `+ + `verified. This cluster runs with --security "anonymous=none". Present the `+ + `--security auth token, or log in with ACL.`, op) +} diff --git a/edgraph/anonymous_test.go b/edgraph/anonymous_test.go new file mode 100644 index 00000000000..eb973f9233e --- /dev/null +++ b/edgraph/anonymous_test.go @@ -0,0 +1,256 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package edgraph + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/dgraph-io/dgraph/v25/acl" + "github.com/dgraph-io/dgraph/v25/worker" + "github.com/dgraph-io/dgraph/v25/x" +) + +// allCapabilities is every capability the posture must cover. A new constant with +// no entry here fails TestAnonymousPostureCoversEveryCapability. +var allCapabilities = []Capability{CapClusterAdmin, CapTenantAdmin, CapAssumeTenant, CapLeaseUIDs} + +func restoreSecurityConfig(t *testing.T) { + t.Helper() + prevAcl, prevSecret := x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey + prevToken, prevIPs := worker.Config.AuthToken, x.WorkerConfig.WhiteListedIPRanges + prevAnon := x.WorkerConfig.Anonymous + t.Cleanup(func() { + x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey = prevAcl, prevSecret + worker.Config.AuthToken, x.WorkerConfig.WhiteListedIPRanges = prevToken, prevIPs + x.WorkerConfig.Anonymous = prevAnon + SetAccessController(nil) + x.SetAuthenticator(nil) + }) +} + +// withBuiltinAuthenticator installs the composition dgraph/cmd/alpha/run.go +// installs, so that x.WithResolvedIdentity resolves a --security token the way it +// does on a live Alpha. Without it the package default is ACL only, and the token +// half of these tests would pass vacuously. +func withBuiltinAuthenticator(t *testing.T) { + t.Helper() + x.SetAuthenticator(PresharedAuthenticator()) +} + +// TestAnonymousPostureCoversEveryCapability pins that the floor is capability +// independent. A posture that denied only the capabilities someone remembered to +// list would reintroduce the omission class it exists to close. +func TestAnonymousPostureCoversEveryCapability(t *testing.T) { + restoreSecurityConfig(t) + + for _, posture := range []x.AnonymousPosture{x.AnonymousData, x.AnonymousNone} { + t.Run(posture.String(), func(t *testing.T) { + inner := &fakeController{} + ac := anonymousPosture{inner: inner, posture: posture} + + for _, c := range allCapabilities { + err := ac.AuthorizeCapability(context.Background(), c) + require.Errorf(t, err, "%s must be denied to a caller with no Principal", c) + require.Equal(t, codes.Unauthenticated, status.Code(err)) + require.Contains(t, status.Convert(err).Message(), c.String(), + "the denial must name the capability so an operator knows what to grant") + } + require.Empty(t, inner.asked, + "the wrapped policy must not be consulted once the floor denies") + }) + } +} + +// TestAnonymousPostureAdmitsIdentifiedCallers pins the other half: the floor asks +// only whether the caller was identified, and hands every other question to the +// wrapped policy. +func TestAnonymousPostureAdmitsIdentifiedCallers(t *testing.T) { + restoreSecurityConfig(t) + + for _, posture := range []x.AnonymousPosture{x.AnonymousFull, x.AnonymousData, x.AnonymousNone} { + t.Run(posture.String(), func(t *testing.T) { + inner := &fakeController{} + ac := anonymousPosture{inner: inner, posture: posture} + ctx := x.WithPrincipal(context.Background(), + &x.Principal{Subject: "someone", Method: x.MethodPreshared}) + + for _, c := range allCapabilities { + require.NoError(t, ac.AuthorizeCapability(ctx, c)) + } + require.Equal(t, allCapabilities, inner.asked) + }) + } +} + +// TestAnonymousPostureUnderFullIsTransparent is the v25 default. Nothing about an +// unconfigured cluster's authorization may change. +func TestAnonymousPostureUnderFullIsTransparent(t *testing.T) { + restoreSecurityConfig(t) + + inner := &fakeController{} + ac := anonymousPosture{inner: inner, posture: x.AnonymousFull} + for _, c := range allCapabilities { + require.NoError(t, ac.AuthorizeCapability(context.Background(), c), + "anonymous=full must defer entirely to the wrapped policy") + } + require.Equal(t, allCapabilities, inner.asked) +} + +func TestAnonymousPosturePassesPredicatesThrough(t *testing.T) { + restoreSecurityConfig(t) + + inner := &fakeController{} + ac := anonymousPosture{inner: inner, posture: x.AnonymousNone} + + // No Principal, the strictest posture, and the predicate half still delegates: + // a query that names predicates the caller may not read is answered with those + // dropped, so there is no denial for the floor to express here. + res, err := ac.AuthorizePredicates(context.Background(), []string{"name"}, acl.Read) + require.NoError(t, err) + require.NotNil(t, res) + require.Equal(t, [][]string{{"name"}}, inner.askedPreds) +} + +func TestEnforceAnonymousPosture(t *testing.T) { + t.Run("full installs nothing", func(t *testing.T) { + restoreSecurityConfig(t) + EnforceAnonymousPosture(x.AnonymousFull) + require.Equal(t, "predicate-acl", currentAccessController().Name(), + "the shipped default must not add indirection") + }) + + t.Run("wraps the built-in policy", func(t *testing.T) { + restoreSecurityConfig(t) + EnforceAnonymousPosture(x.AnonymousData) + require.Equal(t, "predicate-acl (anonymous=data)", currentAccessController().Name()) + }) + + // The floor goes on after ConfigureIdentity precisely so a deployment policy is + // wrapped rather than replaced. If the order were reversed the floor would be + // silently discarded, which is the kind of failure that shows up as an + // advisory rather than a test failure. + t.Run("wraps an installed policy rather than replacing it", func(t *testing.T) { + restoreSecurityConfig(t) + fake := &fakeController{} + SetAccessController(fake) + EnforceAnonymousPosture(x.AnonymousNone) + + require.Equal(t, "fake (anonymous=none)", currentAccessController().Name()) + + ctx := x.WithPrincipal(context.Background(), &x.Principal{Subject: "s"}) + require.NoError(t, AuthorizeCapability(ctx, CapTenantAdmin)) + require.Equal(t, []Capability{CapTenantAdmin}, fake.asked, + "the wrapped policy must still decide for an identified caller") + }) +} + +// TestBreakGlassIsNotAnIdentity is the behavioral heart of the flag. +// +// With ACL off and no token configured, break-glass grants cluster admin to any +// whitelisted source IP -- and the standalone image ships whitelist=0.0.0.0/0, +// which makes that every caller. A whitelist answers where a request came from and +// carries no credential, so under a closed posture it must stop being sufficient. +func TestBreakGlassIsNotAnIdentity(t *testing.T) { + restoreSecurityConfig(t) + x.WorkerConfig.AclEnabled = false + worker.Config.AuthToken = "" + + // A loopback caller: break-glass is fully satisfied for this context. + ctx := fromIP(t, "127.0.0.1") + require.True(t, breakGlassSource{}.Grants(ctx, nil, CapClusterAdmin), + "precondition: break-glass grants on its own") + + require.NoError(t, predicateACL{}.AuthorizeCapability(ctx, CapClusterAdmin), + "anonymous=full: unchanged, the whitelist is enough") + + EnforceAnonymousPosture(x.AnonymousData) + err := AuthorizeCapability(ctx, CapClusterAdmin) + require.Error(t, err, "anonymous=data: a location check is not a credential") + require.Equal(t, codes.Unauthenticated, status.Code(err)) +} + +// TestSecurityTokenIsAnIdentityUnderClosedPosture is the end-to-end claim the flag +// makes: a closed posture is usable without standing up full ACL, because the +// --security token now produces a Principal instead of answering a yes/no. +func TestSecurityTokenIsAnIdentityUnderClosedPosture(t *testing.T) { + restoreSecurityConfig(t) + withBuiltinAuthenticator(t) + x.WorkerConfig.AclEnabled = false + worker.Config.AuthToken = "s3cr3t-admin-token" + EnforceAnonymousPosture(x.AnonymousData) + + t.Run("without the token", func(t *testing.T) { + ctx := x.WithResolvedIdentity(fromIP(t, "127.0.0.1")) + err := AuthorizeCapability(ctx, CapClusterAdmin) + require.Error(t, err) + require.Equal(t, codes.Unauthenticated, status.Code(err)) + }) + + t.Run("with the wrong token", func(t *testing.T) { + ctx := x.WithResolvedIdentity(withAuthToken(fromIP(t, "127.0.0.1"), "wrong")) + err := AuthorizeCapability(ctx, CapClusterAdmin) + require.Error(t, err) + require.Equal(t, codes.Unauthenticated, status.Code(err)) + }) + + t.Run("with the token", func(t *testing.T) { + ctx := x.WithResolvedIdentity(withAuthToken(fromIP(t, "127.0.0.1"), "s3cr3t-admin-token")) + require.NotNil(t, x.PrincipalFrom(ctx), "the token must resolve to a Principal") + require.NoError(t, AuthorizeCapability(ctx, CapClusterAdmin), + "an identified caller then falls through to break-glass as before") + }) + + // The token is an identity, not a bypass: a non-whitelisted caller presenting it + // is identified, so the floor admits them, and break-glass then refuses on the IP. + t.Run("the token does not bypass the whitelist", func(t *testing.T) { + ctx := x.WithResolvedIdentity(withAuthToken(fromIP(t, "203.0.113.7"), "s3cr3t-admin-token")) + require.NotNil(t, x.PrincipalFrom(ctx)) + err := AuthorizeCapability(ctx, CapClusterAdmin) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, status.Code(err), + "denied by break-glass on the source IP, not by the posture floor") + }) +} + +func TestRequireIdentifiedCaller(t *testing.T) { + restoreSecurityConfig(t) + identified := x.WithPrincipal(context.Background(), &x.Principal{Subject: "s"}) + + tests := []struct { + posture x.AnonymousPosture + wantAnonErr bool + wantIdentErr bool + }{ + {posture: x.AnonymousFull, wantAnonErr: false}, + // data deliberately leaves the data plane alone: an unauthenticated query is + // the posture's whole point of difference from none. + {posture: x.AnonymousData, wantAnonErr: false}, + {posture: x.AnonymousNone, wantAnonErr: true}, + } + + for _, tt := range tests { + t.Run(tt.posture.String(), func(t *testing.T) { + x.WorkerConfig.Anonymous = tt.posture + + err := RequireIdentifiedCaller(context.Background(), "query") + if tt.wantAnonErr { + require.Error(t, err) + require.Equal(t, codes.Unauthenticated, status.Code(err)) + require.Contains(t, status.Convert(err).Message(), "query") + } else { + require.NoError(t, err) + } + + require.NoError(t, RequireIdentifiedCaller(identified, "query"), + "an identified caller is never refused by the data-plane floor") + }) + } +} diff --git a/edgraph/authn.go b/edgraph/authn.go new file mode 100644 index 00000000000..4d15e30d41a --- /dev/null +++ b/edgraph/authn.go @@ -0,0 +1,87 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package edgraph + +import ( + "context" + + "github.com/dgraph-io/dgraph/v25/worker" + "github.com/dgraph-io/dgraph/v25/x" +) + +// PresharedSubject is the Principal.Subject a caller authenticated by the +// --security auth token carries. +// +// The value matches audit.PoorManAuth deliberately. Audit prefers the resolved +// Principal over re-parsing the credential, so minting a Principal for a +// token-bearing caller would otherwise change what every such request logs. Pinned +// by TestPresharedSubjectMatchesAudit. +const PresharedSubject = "PoorManAuth" + +// PresharedIssuer identifies the --security auth token as the thing that vouched +// for a Principal. +const PresharedIssuer = "dgraph-security-token" + +// PresharedAuthenticator returns the built-in authenticator: Dgraph's own ACL +// access JWT first, then the --security auth token. +// +// The token half is what makes a closed --security "anonymous=..." posture usable +// without standing up full ACL. Before this, the token was a boolean check -- +// hasPoormansAuth answered "may this request proceed" and produced no identity -- +// so a token-bearing caller was indistinguishable from an anonymous one, and a +// posture keyed on "did this caller present a credential" would have denied it. +// +// Ordering is ACL first because ACL identifies a user while the token identifies a +// client service, and the more specific identity should win when a request carries +// both. It also costs nothing when ACL is off: aclAuthenticator returns +// (nil, nil) immediately in that configuration. +func PresharedAuthenticator() x.Authenticator { return presharedAuthenticator{} } + +type presharedAuthenticator struct{} + +func (presharedAuthenticator) Name() string { return "acl+preshared" } + +func (presharedAuthenticator) Authenticate(ctx context.Context) (*x.Principal, error) { + p, aclErr := x.ACLAuthenticator().Authenticate(ctx) + if p != nil { + return p, nil + } + // No ACL identity, either because ACL is off, the request carried no access + // JWT, or the one it carried did not verify. The token is the remaining route. + // On a verification failure we still try it, so a client service with a valid + // token and a stale JWT is identified as the service rather than as nobody. + if tp := presharedPrincipal(ctx); tp != nil { + return tp, nil + } + return nil, aclErr +} + +// presharedPrincipal returns a Principal for a caller presenting the configured +// --security auth token, or nil. +// +// It deliberately does not reuse hasPoormansAuth's result alone. That function +// returns nil when no token is configured, which is the right answer for "may this +// request proceed" and the wrong one here: an unconfigured token means there is no +// credential to verify and therefore no identity to assert. Minting a Principal in +// that case would make every anonymous caller look authenticated, which is exactly +// the fail-open this work exists to close. +// +// Groups is deliberately empty. x.IsSuperAdmin consults Principal.Groups by name, +// so putting "guardians" here would turn a shared secret into cluster authority +// without going through authorizeClusterAdmin and its ACL-on exclusion. +func presharedPrincipal(ctx context.Context) *x.Principal { + if worker.Config.AuthToken == "" { + return nil + } + if err := hasPoormansAuth(ctx); err != nil { + return nil + } + return &x.Principal{ + Issuer: PresharedIssuer, + Subject: PresharedSubject, + Method: x.MethodPreshared, + } +} diff --git a/edgraph/authn_identity_test.go b/edgraph/authn_identity_test.go new file mode 100644 index 00000000000..7a32dbcfb6f --- /dev/null +++ b/edgraph/authn_identity_test.go @@ -0,0 +1,157 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package edgraph + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc/metadata" + + "github.com/dgraph-io/dgraph/v25/audit" + "github.com/dgraph-io/dgraph/v25/worker" + "github.com/dgraph-io/dgraph/v25/x" +) + +// withAccessJwt puts a raw access token on the context the way a gRPC client does. +func withAccessJwt(ctx context.Context, token string) context.Context { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + md = metadata.New(nil) + } else { + md = md.Copy() + } + md.Set("accessJwt", token) + return metadata.NewIncomingContext(ctx, md) +} + +// aclCtxNoResolve is aclCtx without the identity resolution, so a test can drive +// an Authenticator directly rather than observe the result of the package default. +func aclCtxNoResolve(t *testing.T, namespace uint64, userID string, groups []string) context.Context { + t.Helper() + token := generateJWT(namespace, userID, groups, time.Now().Add(30*time.Minute).Unix()) + return withAccessJwt(context.Background(), token) +} + +// TestPresharedSubjectMatchesAudit pins the one cross-package coupling in this +// change. Audit prefers the resolved Principal over re-parsing the credential, so +// minting one for a token-bearing caller changes what every such request logs +// unless the Subject is the label audit already used. +func TestPresharedSubjectMatchesAudit(t *testing.T) { + require.Equal(t, audit.PoorManAuth, PresharedSubject, + "audit output for token-authenticated callers must not change") +} + +func TestPresharedPrincipal(t *testing.T) { + restoreSecurityConfig(t) + x.WorkerConfig.AclEnabled = false + + t.Run("no token configured yields no identity", func(t *testing.T) { + worker.Config.AuthToken = "" + // Deliberately presenting a token the server is not configured with. An + // unconfigured token means there is nothing to verify, so there is no + // identity to assert -- reusing hasPoormansAuth's nil here would make every + // anonymous caller look authenticated. + require.Nil(t, presharedPrincipal(withAuthToken(context.Background(), "anything"))) + require.Nil(t, presharedPrincipal(context.Background())) + }) + + t.Run("token configured but not presented", func(t *testing.T) { + worker.Config.AuthToken = "s3cr3t" + require.Nil(t, presharedPrincipal(context.Background())) + }) + + t.Run("token configured and wrong", func(t *testing.T) { + worker.Config.AuthToken = "s3cr3t" + require.Nil(t, presharedPrincipal(withAuthToken(context.Background(), "nope"))) + }) + + t.Run("token configured and correct", func(t *testing.T) { + worker.Config.AuthToken = "s3cr3t" + p := presharedPrincipal(withAuthToken(context.Background(), "s3cr3t")) + require.NotNil(t, p) + require.Equal(t, x.MethodPreshared, p.Method) + require.Equal(t, PresharedIssuer, p.Issuer) + require.Equal(t, PresharedSubject, p.Subject) + }) +} + +// TestPresharedPrincipalConfersNoGroups guards the escalation the Principal type +// warns about: x.IsSuperAdmin reads Principal.Groups by name, so a "guardians" +// entry here would turn a shared secret into cluster authority without passing +// through authorizeClusterAdmin and its ACL-on exclusion. +func TestPresharedPrincipalConfersNoGroups(t *testing.T) { + restoreSecurityConfig(t) + x.WorkerConfig.AclEnabled = false + worker.Config.AuthToken = "s3cr3t" + + p := presharedPrincipal(withAuthToken(context.Background(), "s3cr3t")) + require.NotNil(t, p) + require.Empty(t, p.Groups) + require.False(t, x.IsSuperAdmin(p.Groups)) +} + +// TestPresharedAuthenticatorPrefersACL pins the ordering. ACL identifies a user +// and the token identifies a client service, so a request carrying both must +// resolve to the user -- which also keeps userDataFromPrincipal's fast path, since +// it requires Method == MethodACL. +func TestPresharedAuthenticatorPrefersACL(t *testing.T) { + restoreSecurityConfig(t) + x.WorkerConfig.AclEnabled = true + worker.Config.AclSecretKey = x.Sensitive("6ABBAA2014CFF00289D20D20DA296F67") + worker.Config.AuthToken = "s3cr3t" + + ctx := withAuthToken(aclCtxNoResolve(t, 0, "groot", []string{"guardians"}), "s3cr3t") + p, err := presharedAuthenticator{}.Authenticate(ctx) + require.NoError(t, err) + require.NotNil(t, p) + require.Equal(t, x.MethodACL, p.Method) + require.Equal(t, "groot", p.Subject) +} + +// TestPresharedAuthenticatorFallsBackOnAnUnusableJWT: a client service with a +// valid token and a stale access JWT is the service, not nobody. +func TestPresharedAuthenticatorFallsBackOnAnUnusableJWT(t *testing.T) { + restoreSecurityConfig(t) + x.WorkerConfig.AclEnabled = true + worker.Config.AclSecretKey = x.Sensitive("6ABBAA2014CFF00289D20D20DA296F67") + worker.Config.AuthToken = "s3cr3t" + + ctx := withAuthToken(withAccessJwt(context.Background(), "not-a-jwt"), "s3cr3t") + p, err := presharedAuthenticator{}.Authenticate(ctx) + require.NoError(t, err) + require.NotNil(t, p) + require.Equal(t, x.MethodPreshared, p.Method) +} + +// TestPresharedAuthenticatorPropagatesACLErrors: with no token to fall back on, a +// credential that did not verify is still reported, so WithResolvedIdentity can +// log it. +func TestPresharedAuthenticatorPropagatesACLErrors(t *testing.T) { + restoreSecurityConfig(t) + x.WorkerConfig.AclEnabled = true + worker.Config.AclSecretKey = x.Sensitive("6ABBAA2014CFF00289D20D20DA296F67") + worker.Config.AuthToken = "" + + p, err := presharedAuthenticator{}.Authenticate(withAccessJwt(context.Background(), "not-a-jwt")) + require.Error(t, err) + require.Nil(t, p) +} + +// TestPresharedAuthenticatorIsSilentWithNothingConfigured pins the shipped +// default: no ACL, no token, no identity, and no error either -- "no credential +// presented" is the normal state for the endpoints that cannot present one. +func TestPresharedAuthenticatorIsSilentWithNothingConfigured(t *testing.T) { + restoreSecurityConfig(t) + x.WorkerConfig.AclEnabled = false + worker.Config.AuthToken = "" + + p, err := presharedAuthenticator{}.Authenticate(context.Background()) + require.NoError(t, err) + require.Nil(t, p) +} diff --git a/edgraph/server.go b/edgraph/server.go index 684a18e5073..6050206c14e 100644 --- a/edgraph/server.go +++ b/edgraph/server.go @@ -1514,6 +1514,20 @@ func (s *Server) doQuery(ctx context.Context, req *Request) (resp *api.Response, ostats.Record(ctx, x.NumMutations.M(1)) } + if req.doAuth == NeedAuthorize { + // Gated on NeedAuthorize, which is what distinguishes a network-originated + // request from an in-process one: QueryNoAuth and the internal DQL callers + // run with NoAuthorize and a context.Background() that could never carry a + // Principal. + op := "query" + if isMutation { + op = "mutation" + } + if err := RequireIdentifiedCaller(ctx, op); err != nil { + return nil, err + } + } + if req.doAuth == NeedAuthorize && x.IsRootNsOperation(ctx) { // Only the guardian of the galaxy can do a galaxy wide query/mutation. This operation is // needed by live loader. @@ -2157,6 +2171,10 @@ func (s *Server) CommitOrAbort(ctx context.Context, tc *api.TxnContext) (*api.Tx return &api.TxnContext{}, err } + if err := RequireIdentifiedCaller(ctx, "commit"); err != nil { + return &api.TxnContext{}, err + } + tctx := &api.TxnContext{} if tc.StartTs == 0 { return &api.TxnContext{}, errors.Errorf( diff --git a/worker/server_state.go b/worker/server_state.go index 4c2058165a1..d9905132442 100644 --- a/worker/server_state.go +++ b/worker/server_state.go @@ -32,7 +32,7 @@ const ( BadgerDefaults = `compression=snappy; numgoroutines=8;` RaftDefaults = `learner=false; snapshot-after-entries=10000; ` + `snapshot-after-duration=30m; pending-proposals=256; idx=; group=;` - SecurityDefaults = `token=; whitelist=;` + SecurityDefaults = `token=; whitelist=; anonymous=full;` CDCDefaults = `file=; kafka=; sasl_user=; sasl_password=; ca_cert=; client_cert=; ` + `client_key=; sasl-mechanism=PLAIN; tls=false;` LimitDefaults = `mutations=allow; query-edge=1000000; normalize-node=10000; ` + diff --git a/x/anonymous.go b/x/anonymous.go new file mode 100644 index 00000000000..f0f9aae6085 --- /dev/null +++ b/x/anonymous.go @@ -0,0 +1,94 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package x + +import ( + "strings" + + "github.com/pkg/errors" +) + +// AnonymousPosture says what a caller with no verified identity may do. +// +// It exists because the three controls that gate Alpha's privileged operations -- +// the --security whitelist, the --security token, and ACL -- each pass when their +// own feature is unconfigured. The protection an operator gets is whatever they +// configured rather than the union of the three, and the gap that leaves is +// specific: the whitelist answers where a request came from and carries no +// credential in it, so widening the range is by itself enough to make +// administrative operations reachable anonymously from anywhere inside it. +// "whitelist=0.0.0.0/0" is a common setting, and in that posture nothing is left +// to check. +// +// This type names the missing decision directly. An anonymous caller is one whose +// request produced no Principal: either it presented no credential, or the one it +// presented did not verify. See PrincipalFrom. +type AnonymousPosture int + +const ( + // AnonymousFull leaves authorization exactly as the configured controls decide + // it. An anonymous caller reaching an operation whose gate is unconfigured is + // allowed through, which is the behavior every release before this one had. + // + // It is the zero value, so a WorkerOptions that was never populated behaves as + // it did before this flag existed. + AnonymousFull AnonymousPosture = iota + + // AnonymousData lets an anonymous caller read and write data -- queries, + // mutations, commits, Login, and the health endpoints -- while every + // administrative capability is denied regardless of the whitelist. + // + // "Administrative" means a Capability check: namespace create/drop/list, UID + // leasing, drop-all, arming an external-snapshot import, reading cluster state, + // and the privileged GraphQL admin surface (backup, restore, export, shutdown, + // removeNode, moveTablet, assign, draining, config). + AnonymousData + + // AnonymousNone additionally denies ordinary data access, leaving an anonymous + // caller only the operations that cannot present a credential in the first + // place: Login, CheckVersion, and the health and readiness endpoints. + AnonymousNone +) + +func (p AnonymousPosture) String() string { + switch p { + case AnonymousFull: + return "full" + case AnonymousData: + return "data" + case AnonymousNone: + return "none" + } + return "unknown" +} + +// ParseAnonymousPosture reads the --security "anonymous=..." value. An empty +// string is AnonymousFull, so an operator who never set the key gets the +// pre-existing behavior. +func ParseAnonymousPosture(s string) (AnonymousPosture, error) { + switch strings.ToLower(strings.TrimSpace(s)) { + case "", "full": + return AnonymousFull, nil + case "data": + return AnonymousData, nil + case "none": + return AnonymousNone, nil + } + return AnonymousFull, errors.Errorf( + `--security "anonymous=%s" is not a valid value; it must be one of full, data, or none`, s) +} + +// RequiresIdentityForCapability reports whether this posture denies an anonymous +// caller every administrative capability. +func (p AnonymousPosture) RequiresIdentityForCapability() bool { + return p != AnonymousFull +} + +// RequiresIdentityForData reports whether this posture denies an anonymous caller +// ordinary queries, mutations, and commits. +func (p AnonymousPosture) RequiresIdentityForData() bool { + return p == AnonymousNone +} diff --git a/x/anonymous_test.go b/x/anonymous_test.go new file mode 100644 index 00000000000..ccd88ccde95 --- /dev/null +++ b/x/anonymous_test.go @@ -0,0 +1,91 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package x + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestAnonymousFullIsTheZeroValue pins the property the v25 rollout rests on: a +// WorkerOptions that was never populated, and a --security string that never +// mentions the key, both behave exactly as they did before the flag existed. +func TestAnonymousFullIsTheZeroValue(t *testing.T) { + var p AnonymousPosture + require.Equal(t, AnonymousFull, p) + require.False(t, p.RequiresIdentityForCapability()) + require.False(t, p.RequiresIdentityForData()) + + parsed, err := ParseAnonymousPosture("") + require.NoError(t, err) + require.Equal(t, AnonymousFull, parsed) +} + +func TestParseAnonymousPosture(t *testing.T) { + tests := []struct { + in string + want AnonymousPosture + wantErr bool + }{ + {in: "", want: AnonymousFull}, + {in: "full", want: AnonymousFull}, + {in: "FULL", want: AnonymousFull}, + {in: " full ", want: AnonymousFull}, + {in: "data", want: AnonymousData}, + {in: "Data", want: AnonymousData}, + {in: "none", want: AnonymousNone}, + {in: "NONE", want: AnonymousNone}, + // A typo must fail at startup rather than silently selecting a posture the + // operator did not ask for. "off" and "all" are the plausible ones. + {in: "off", wantErr: true}, + {in: "all", wantErr: true}, + {in: "true", wantErr: true}, + {in: "admin", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + got, err := ParseAnonymousPosture(tt.in) + if tt.wantErr { + require.Error(t, err) + // A rejected value still reports the safe posture, so a caller that + // ignores the error does not accidentally get a closed cluster. + require.Equal(t, AnonymousFull, got) + return + } + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +} + +func TestAnonymousPostureEnforcement(t *testing.T) { + tests := []struct { + posture AnonymousPosture + str string + capability bool + data bool + }{ + {posture: AnonymousFull, str: "full", capability: false, data: false}, + {posture: AnonymousData, str: "data", capability: true, data: false}, + {posture: AnonymousNone, str: "none", capability: true, data: true}, + } + + for _, tt := range tests { + t.Run(tt.str, func(t *testing.T) { + require.Equal(t, tt.str, tt.posture.String()) + require.Equal(t, tt.capability, tt.posture.RequiresIdentityForCapability()) + require.Equal(t, tt.data, tt.posture.RequiresIdentityForData()) + + // Every value this type can hold must survive a round trip through the + // superflag, since that is the only way one is ever constructed. + parsed, err := ParseAnonymousPosture(tt.posture.String()) + require.NoError(t, err) + require.Equal(t, tt.posture, parsed) + }) + } +} diff --git a/x/config.go b/x/config.go index dc5371f8225..15f4123c17e 100644 --- a/x/config.go +++ b/x/config.go @@ -123,7 +123,11 @@ type WorkerOptions struct { // // whitelist string - comma separated IP addresses // token string - if set, all Admin requests to Dgraph will have this token. + // anonymous string - what a caller with no verified identity may do. Security *z.SuperFlag + // Anonymous is the parsed --security "anonymous=..." posture. Its zero value is + // AnonymousFull, which is the behavior every release before the flag existed. + Anonymous AnonymousPosture // EncryptionKey is the key used for encryption at rest, backups, exports. EncryptionKey Sensitive // LogDQLRequest indicates whether alpha should log all query/mutation requests coming to it. From 5b591e3c296296453261cdc923f5eac37ef972af Mon Sep 17 00:00:00 2001 From: mattthew Date: Fri, 2 Oct 2026 14:06:51 -0400 Subject: [PATCH 2/6] fix(test): alias the go/token import to avoid a collision under the integration tag run_test.go is //go:build integration and declares a package-level `token *Token`, so an unaliased `go/token` import compiled fine untagged and broke the integration build. Caught by CI, not locally, for exactly that reason. Co-Authored-By: Claude Opus 5 --- dgraph/cmd/alpha/security_posture_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dgraph/cmd/alpha/security_posture_test.go b/dgraph/cmd/alpha/security_posture_test.go index 4f649acac5f..7e596f9e3e5 100644 --- a/dgraph/cmd/alpha/security_posture_test.go +++ b/dgraph/cmd/alpha/security_posture_test.go @@ -8,7 +8,7 @@ package alpha import ( "go/ast" "go/parser" - "go/token" + gotoken "go/token" "os" "strings" "testing" @@ -149,7 +149,8 @@ func TestHTTPEdgeResolvesIdentityThroughOneHelper(t *testing.T) { "AttachRemoteIP": "attaches the peer but resolves no Principal", } - fset := token.NewFileSet() + // Aliased: the integration-tagged run_test.go declares a package-level `token`. + fset := gotoken.NewFileSet() pkgs, err := parser.ParseDir(fset, ".", func(fi os.FileInfo) bool { return !strings.HasSuffix(fi.Name(), "_test.go") }, 0) From e74554c6a23e4bbfa74b3b00460ae4e4a7aa01bd Mon Sep 17 00:00:00 2001 From: mattthew Date: Fri, 2 Oct 2026 14:24:03 -0400 Subject: [PATCH 3/6] fix(test): replace deprecated parser.ParseDir in the HTTP identity test staticcheck SA1019: go/parser.ParseDir is deprecated as of Go 1.25. Parse each non-test file with parser.ParseFile over a glob instead. That also scans files regardless of build tags, which is what this test wants: a handler compiled on one platform only is still a handler. Co-Authored-By: Claude Opus 5.5 --- dgraph/cmd/alpha/security_posture_test.go | 73 ++++++++++++----------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/dgraph/cmd/alpha/security_posture_test.go b/dgraph/cmd/alpha/security_posture_test.go index 7e596f9e3e5..f6c1dab7e65 100644 --- a/dgraph/cmd/alpha/security_posture_test.go +++ b/dgraph/cmd/alpha/security_posture_test.go @@ -9,7 +9,7 @@ import ( "go/ast" "go/parser" gotoken "go/token" - "os" + "path/filepath" "strings" "testing" @@ -149,45 +149,50 @@ func TestHTTPEdgeResolvesIdentityThroughOneHelper(t *testing.T) { "AttachRemoteIP": "attaches the peer but resolves no Principal", } - // Aliased: the integration-tagged run_test.go declares a package-level `token`. - fset := gotoken.NewFileSet() - pkgs, err := parser.ParseDir(fset, ".", func(fi os.FileInfo) bool { - return !strings.HasSuffix(fi.Name(), "_test.go") - }, 0) + // Every non-test file in the package, regardless of build tags. parser.ParseDir is + // deprecated, and ignoring tags is what this test wants anyway: a handler compiled + // only on one platform is still a handler. + paths, err := filepath.Glob("*.go") require.NoError(t, err) + // Aliased: the integration-tagged run_test.go declares a package-level `token`. + fset := gotoken.NewFileSet() seenExceptions := map[string]bool{} - for _, pkg := range pkgs { - for _, file := range pkg.Files { - for _, decl := range file.Decls { - fn, ok := decl.(*ast.FuncDecl) + for _, path := range paths { + if strings.HasSuffix(path, "_test.go") { + continue + } + file, err := parser.ParseFile(fset, path, nil, 0) + require.NoError(t, err) + + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok { + continue + } + _, exempt := identityExceptions[fn.Name.Name] + ast.Inspect(fn, func(n ast.Node) bool { + sel, ok := n.(*ast.SelectorExpr) if !ok { - continue + return true } - _, exempt := identityExceptions[fn.Name.Name] - ast.Inspect(fn, func(n ast.Node) bool { - sel, ok := n.(*ast.SelectorExpr) - if !ok { - return true - } - pkgIdent, ok := sel.X.(*ast.Ident) - if !ok || pkgIdent.Name != "x" { - return true - } - why, bad := banned[sel.Sel.Name] - if !bad { - return true - } - if exempt { - seenExceptions[fn.Name.Name] = true - return true - } - t.Errorf("%s: %s calls x.%s, which %s. Use x.AttachRequestIdentity, or add "+ - "it to identityExceptions with a reason.", - fset.Position(sel.Pos()), fn.Name.Name, sel.Sel.Name, why) + pkgIdent, ok := sel.X.(*ast.Ident) + if !ok || pkgIdent.Name != "x" { return true - }) - } + } + why, bad := banned[sel.Sel.Name] + if !bad { + return true + } + if exempt { + seenExceptions[fn.Name.Name] = true + return true + } + t.Errorf("%s: %s calls x.%s, which %s. Use x.AttachRequestIdentity, or add "+ + "it to identityExceptions with a reason.", + fset.Position(sel.Pos()), fn.Name.Name, sel.Sel.Name, why) + return true + }) } } From 9fbb6a66e6473ea6b0f494fc60ce3788de656d2f Mon Sep 17 00:00:00 2001 From: mattthew Date: Fri, 2 Oct 2026 14:32:44 -0400 Subject: [PATCH 4/6] fix(zero): correct the --security anonymous help text Superflag help lists keys alphabetically, so "anonymous" prints before "token" and "the token above" pointed at nothing. Name the option instead. Co-Authored-By: Claude Opus 5.5 --- dgraph/cmd/zero/run.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dgraph/cmd/zero/run.go b/dgraph/cmd/zero/run.go index c693bde6a76..f870120b94c 100644 --- a/dgraph/cmd/zero/run.go +++ b/dgraph/cmd/zero/run.go @@ -120,7 +120,7 @@ instances to achieve high-availability. Flag("anonymous", "[full, data, none] What a caller that presents no verified credential may do. full "+ "(default) is the behavior of every earlier release. Any other value requires the "+ - "token above on every administrative HTTP endpoint, including /state and /assign, "+ + "token= option on every administrative HTTP endpoint, including /state and /assign, "+ "and stops a whitelisted source IP from standing in for a credential."). String()) From 2d779c2b40cd2952c3c7d9418b976b6db9196098 Mon Sep 17 00:00:00 2001 From: mattthew Date: Fri, 2 Oct 2026 15:45:43 -0400 Subject: [PATCH 5/6] fix(security): address review on the anonymous posture warnings and identity test Three review findings, each verified before changing anything. The widened-whitelist warning fired on whitelist=127.0.0.1, because it treated any non-empty whitelist as exposure. Loopback is admitted regardless, and the public docs give exactly that value as the "allow localhost only" example, so anyone following them got a false SECURITY warning. It now warns only when some range admits a non-loopback address. A range is loopback-only when both of its ends are loopback, which is exact: IPv4 loopback is contiguous and IPv6 loopback is a single address. The lockout warning ("this cluster cannot be administered") ran before ConfigureIdentity, so it could not see a deployment authenticator. An external JWT issuer installed there identifies callers with neither ACL nor a token, and the warning was false for it. The check now runs after ConfigureIdentity and stands down when a non-default authenticator is installed, via a new x.AuthenticatorName(). Moving it also fixed a second bug: it ran before the port offset was set, so under --port_offset it named the wrong HTTP port. x.ACLAuthenticator's doc told deployments to compose with it, which now drops the --security token as an identity. It points at edgraph.PresharedAuthenticator instead. TestHTTPEdgeResolvesIdentityThroughOneHelper only caught a partial prelude. A handler that dropped the prelude entirely and started from context.Background() called none of the banned helpers and passed. It now also requires x.AttachRequestIdentity in every handler that reaches an authorization decision, including resolveWithAdminServer, which carries the HTTP admin routes, and fails on a stale entry. Verified by removing the prelude from stateHandler: the old test passed, and the new one fails and names the handler. Co-Authored-By: Claude Opus 5.5 --- dgraph/cmd/alpha/run.go | 14 ++-- dgraph/cmd/alpha/security_posture.go | 30 +++++++- dgraph/cmd/alpha/security_posture_test.go | 84 ++++++++++++++++++++++- x/anonymous_test.go | 22 ++++++ x/authn.go | 26 +++++-- 5 files changed, 159 insertions(+), 17 deletions(-) diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go index b662d627cc9..3bbc7a70122 100644 --- a/dgraph/cmd/alpha/run.go +++ b/dgraph/cmd/alpha/run.go @@ -720,11 +720,6 @@ func run() { ips, err := getIPsFromString(security.GetString("whitelist")) x.Check(err) - for _, msg := range securityWarnings(anonymous, security.GetString("whitelist"), ips, - opts.AuthToken, keys.AclSecretKey != nil, httpPort()) { - glog.Warning(msg) - } - tlsClientConf, err := x.LoadClientTLSConfigForInternalPort(Alpha.Conf) x.Check(err) tlsServerConf, err := x.LoadServerTLSConfigForInternalPort(Alpha.Conf) @@ -774,6 +769,15 @@ func run() { setupCustomTokenizers() x.Config.PortOffset = Alpha.Conf.GetInt("port_offset") + + // Deliberately this late. It has to follow ConfigureIdentity, because a + // deployment authenticator changes who can be identified, and it has to follow + // the port offset, because the warning names the HTTP port. + builtinIdentity := x.AuthenticatorName() == edgraph.PresharedAuthenticator().Name() + for _, msg := range securityWarnings(anonymous, security.GetString("whitelist"), ips, + opts.AuthToken, keys.AclSecretKey != nil, builtinIdentity, httpPort()) { + glog.Warning(msg) + } x.Config.LimitMutationsNquad = int(x.Config.Limit.GetInt64("mutations-nquad")) x.Config.LimitQueryEdge = x.Config.Limit.GetUint64("query-edge") x.Config.BlockClusterWideDrop = x.Config.Limit.GetBool("disallow-drop") diff --git a/dgraph/cmd/alpha/security_posture.go b/dgraph/cmd/alpha/security_posture.go index 532a2571a0b..a70422199e4 100644 --- a/dgraph/cmd/alpha/security_posture.go +++ b/dgraph/cmd/alpha/security_posture.go @@ -22,8 +22,13 @@ import ( // // It is pure and returns strings rather than logging, so the combinations can be // pinned by a table test. +// +// builtinIdentity is false when a deployment installed its own authenticator via +// ConfigureIdentity. Such an authenticator can produce identities from credentials +// this function knows nothing about -- an external JWT, say -- so whether a closed +// posture can be administered is no longer something it can judge. func securityWarnings(posture x.AnonymousPosture, whitelist string, ips []x.IPRange, - authToken string, aclEnabled bool, httpPort int) []string { + authToken string, aclEnabled, builtinIdentity bool, httpPort int) []string { hasCredential := aclEnabled || authToken != "" var out []string @@ -33,7 +38,7 @@ func securityWarnings(posture x.AnonymousPosture, whitelist string, ips []x.IPRa // warns at exactly that point: whitelisting answers where a request came from // and has no credential in it, so a widened range with nothing else configured // means every address inside it can run privileged operations anonymously. - if posture == x.AnonymousFull && len(ips) > 0 && !hasCredential { + if posture == x.AnonymousFull && admitsNonLoopback(ips) && !hasCredential { out = append(out, fmt.Sprintf( `SECURITY: --security "whitelist=%s" admits non-loopback callers, but neither ACL nor `+ `an admin token is configured. Privileged operations (backup, restore, export, `+ @@ -51,7 +56,7 @@ func securityWarnings(posture x.AnonymousPosture, whitelist string, ips []x.IPRa // administered at all. This is a misconfiguration rather than a hardening, and // it is worth saying so loudly at boot instead of letting it surface as a // permission error during an incident. - if posture.RequiresIdentityForCapability() && !hasCredential { + if posture.RequiresIdentityForCapability() && !hasCredential && builtinIdentity { out = append(out, fmt.Sprintf( `SECURITY: --security "anonymous=%s" requires an identified caller, but neither ACL nor `+ `an admin token is configured, so no request can ever be identified. Every `+ @@ -61,3 +66,22 @@ func securityWarnings(posture x.AnonymousPosture, whitelist string, ips []x.IPRa return out } + +// admitsNonLoopback reports whether any whitelist range admits an address outside +// loopback. Loopback is always admitted regardless of the whitelist, so a range +// that covers only loopback -- whitelist=127.0.0.1, or the documented "localhost +// only" example -- does not take the admin plane off the host and is not worth a +// warning. +// +// A range is loopback-only when both of its ends are loopback. That is exact +// rather than approximate: IPv4 loopback is the contiguous block 127.0.0.0/8, and +// IPv6 loopback is the single address ::1, so a range whose ends both fall inside +// one of those cannot contain anything else. +func admitsNonLoopback(ips []x.IPRange) bool { + for _, r := range ips { + if !r.Lower.IsLoopback() || !r.Upper.IsLoopback() { + return true + } + } + return false +} diff --git a/dgraph/cmd/alpha/security_posture_test.go b/dgraph/cmd/alpha/security_posture_test.go index f6c1dab7e65..d28b0f7b2b5 100644 --- a/dgraph/cmd/alpha/security_posture_test.go +++ b/dgraph/cmd/alpha/security_posture_test.go @@ -25,7 +25,9 @@ func TestSecurityWarnings(t *testing.T) { whitelist string authToken string acl bool - want []string // substrings that must appear, one per expected warning + // customIdentity means ConfigureIdentity installed its own authenticator. + customIdentity bool + want []string // substrings that must appear, one per expected warning }{{ // The shipped default. An empty whitelist admits loopback only, so the admin // plane has not left the host and there is nothing to say. @@ -42,6 +44,33 @@ func TestSecurityWarnings(t *testing.T) { posture: x.AnonymousFull, whitelist: "10.0.0.0/8", want: []string{"WITHOUT ANY CREDENTIAL"}, + }, { + // Loopback is admitted regardless of the whitelist, so naming it explicitly + // does not take the admin plane off the host. The public docs give exactly + // this as the "allow localhost only" example. + name: "explicit loopback whitelist is quiet", + posture: x.AnonymousFull, + whitelist: "127.0.0.1", + }, { + name: "loopback range is quiet", + posture: x.AnonymousFull, + whitelist: "127.0.0.1:127.0.0.3", + }, { + name: "IPv6 loopback is quiet", + posture: x.AnonymousFull, + whitelist: "::1", + }, { + name: "loopback alongside a real address still warns", + posture: x.AnonymousFull, + whitelist: "127.0.0.1,10.0.0.1", + want: []string{"WITHOUT ANY CREDENTIAL"}, + }, { + // A range starting at loopback but running past it admits non-loopback + // addresses, so a check on Lower alone would be wrong. + name: "range leaving loopback still warns", + posture: x.AnonymousFull, + whitelist: "127.0.0.1:128.0.0.1", + want: []string{"WITHOUT ANY CREDENTIAL"}, }, { name: "widened whitelist with a token", posture: x.AnonymousFull, @@ -75,6 +104,21 @@ func TestSecurityWarnings(t *testing.T) { posture: x.AnonymousNone, whitelist: "0.0.0.0/0", acl: true, + }, { + // A deployment authenticator (an external JWT issuer, say) can identify + // callers with neither ACL nor a token, so claiming the cluster cannot be + // administered would be false. + name: "closed posture with a deployment authenticator is quiet", + posture: x.AnonymousData, + customIdentity: true, + }, { + // The other warning does not depend on identity at all: under full, a + // widened whitelist with no credential is exposed whoever installed what. + name: "deployment authenticator does not silence the exposure warning", + posture: x.AnonymousFull, + whitelist: "0.0.0.0/0", + customIdentity: true, + want: []string{"WITHOUT ANY CREDENTIAL"}, }} for _, tt := range tests { @@ -82,7 +126,8 @@ func TestSecurityWarnings(t *testing.T) { ips, err := getIPsFromString(tt.whitelist) require.NoError(t, err) - got := securityWarnings(tt.posture, tt.whitelist, ips, tt.authToken, tt.acl, 8080) + got := securityWarnings(tt.posture, tt.whitelist, ips, tt.authToken, tt.acl, + !tt.customIdentity, 8080) require.Len(t, got, len(tt.want)) for i, want := range tt.want { require.Contains(t, got[i], "SECURITY:") @@ -130,6 +175,20 @@ var identityExceptions = map[string]string{ "loginHandler": "login must not require a credential it is the means of issuing", } +// identityRequired names the handlers that reach an authorization decision and so +// must resolve the caller's identity. Checking for the banned helpers alone only +// catches a partial prelude. A handler that drops the prelude entirely and starts +// from a bare context.Background() calls none of them, and would pass. +var identityRequired = map[string]string{ + "queryHandler": "/query, refused to anonymous callers under anonymous=none", + "mutationHandler": "/mutate, refused to anonymous callers under anonymous=none", + "commitHandler": "/commit, refused to anonymous callers under anonymous=none", + "alterHandler": "/alter, which can drop all data", + "healthCheck": "/health?all, a tenant-admin capability check", + "stateHandler": "/state, a tenant-admin capability check", + "resolveWithAdminServer": "the HTTP admin routes, which re-enter the /admin GraphQL server", +} + // TestHTTPEdgeResolvesIdentityThroughOneHelper pins an invariant that a live test // caught the hard way. // @@ -158,6 +217,7 @@ func TestHTTPEdgeResolvesIdentityThroughOneHelper(t *testing.T) { // Aliased: the integration-tagged run_test.go declares a package-level `token`. fset := gotoken.NewFileSet() seenExceptions := map[string]bool{} + seenRequired := map[string]bool{} for _, path := range paths { if strings.HasSuffix(path, "_test.go") { continue @@ -171,6 +231,7 @@ func TestHTTPEdgeResolvesIdentityThroughOneHelper(t *testing.T) { continue } _, exempt := identityExceptions[fn.Name.Name] + resolves := false ast.Inspect(fn, func(n ast.Node) bool { sel, ok := n.(*ast.SelectorExpr) if !ok { @@ -180,6 +241,10 @@ func TestHTTPEdgeResolvesIdentityThroughOneHelper(t *testing.T) { if !ok || pkgIdent.Name != "x" { return true } + if sel.Sel.Name == "AttachRequestIdentity" { + resolves = true + return true + } why, bad := banned[sel.Sel.Name] if !bad { return true @@ -193,9 +258,24 @@ func TestHTTPEdgeResolvesIdentityThroughOneHelper(t *testing.T) { fset.Position(sel.Pos()), fn.Name.Name, sel.Sel.Name, why) return true }) + + if reason, required := identityRequired[fn.Name.Name]; required { + seenRequired[fn.Name.Name] = true + if !resolves { + t.Errorf("%s: %s serves %s but never calls x.AttachRequestIdentity, so a "+ + "caller presenting a credential arrives unidentified.", + fset.Position(fn.Pos()), fn.Name.Name, reason) + } + } } } + // A renamed or deleted handler would otherwise drop out of the check silently. + for name := range identityRequired { + require.Truef(t, seenRequired[name], + "identityRequired lists %q, but no function by that name exists in this package", name) + } + // A stale exception is its own problem: it reads as a documented carve-out for // something that no longer exists. for name := range identityExceptions { diff --git a/x/anonymous_test.go b/x/anonymous_test.go index ccd88ccde95..42e2e64b13d 100644 --- a/x/anonymous_test.go +++ b/x/anonymous_test.go @@ -6,6 +6,7 @@ package x import ( + "context" "testing" "github.com/stretchr/testify/require" @@ -89,3 +90,24 @@ func TestAnonymousPostureEnforcement(t *testing.T) { }) } } + +type namedAuthenticator struct{ name string } + +func (n namedAuthenticator) Name() string { return n.name } +func (namedAuthenticator) Authenticate(context.Context) (*Principal, error) { + return nil, nil +} + +// TestAuthenticatorName pins the accessor dgraph alpha uses to tell whether a +// deployment replaced the default authenticator during ConfigureIdentity. +func TestAuthenticatorName(t *testing.T) { + t.Cleanup(func() { SetAuthenticator(nil) }) + + require.Equal(t, "acl", AuthenticatorName(), "the package default is ACL only") + + SetAuthenticator(namedAuthenticator{name: "deployment"}) + require.Equal(t, "deployment", AuthenticatorName()) + + SetAuthenticator(nil) + require.Equal(t, "acl", AuthenticatorName(), "nil restores the default") +} diff --git a/x/authn.go b/x/authn.go index 9400f81714d..f57f5713a24 100644 --- a/x/authn.go +++ b/x/authn.go @@ -46,16 +46,28 @@ func SetAuthenticator(a Authenticator) { authenticator.Store(&a) } -// ACLAuthenticator returns the built-in authenticator, which verifies Dgraph's own -// ACL access token. +// ACLAuthenticator returns the authenticator that verifies Dgraph's own ACL access +// token, and nothing else. // -// Exported so a deployment installing its own Authenticator can compose with this -// one rather than displace it. That matters more than it looks: Login is how an -// ACL token is obtained, so an installed authenticator that cannot also verify an -// ACL token takes away the cluster's ability to log in — and the failure surfaces -// as an authorization error somewhere unrelated. +// A deployment installing its own Authenticator should usually compose with +// edgraph.PresharedAuthenticator instead, which is what dgraph alpha installs by +// default: it verifies the ACL token first and then the --security auth token. +// Composing with this one alone silently drops the auth token as an identity, so +// under a closed --security "anonymous=..." posture a caller presenting the token +// is turned away as anonymous. (edgraph cannot be named from here, so this is the +// only place to say it.) +// +// Either way, compose rather than displace. Login is how an ACL token is obtained, +// so an installed authenticator that cannot also verify one takes away the +// cluster's ability to log in, and the failure surfaces as an authorization error +// somewhere unrelated. func ACLAuthenticator() Authenticator { return aclAuthenticator{} } +// AuthenticatorName reports the Name of the authenticator currently in effect, +// whether installed or built in. It lets startup checks tell whether a deployment +// replaced the default, without exposing the authenticator itself. +func AuthenticatorName() string { return currentAuthenticator().Name() } + // currentAuthenticator returns the installed authenticator, or the built-in ACL // one when none is installed. Mirrors how ResolveTenant falls back to // aclTenantResolver, so "installed" and "default" behave the same way in both From 79de59da612d655386c153831550d54791497770 Mon Sep 17 00:00:00 2001 From: mattthew Date: Tue, 6 Oct 2026 15:08:53 -0400 Subject: [PATCH 6/6] fix(security): gate every Alter on the posture, and resolve identity for subscriptions Two gaps from review (thanks @shiva-istari), both confirmed against a live cluster before changing anything. Alter was not gated by the posture at all. Only drop_all reaches a Capability, and the remaining gates on Alter -- hasAdminAuth (whitelist, plus the token if one is configured) and authorizeAlter (ACL) -- never consult the Principal. So with an open whitelist and nothing else configured, an anonymous caller under anonymous=data or anonymous=none could still change the schema, drop a predicate, or drop_op DATA, which empties the namespace. That is one-request data destruction in exactly the posture meant to close it. Every network Alter now requires an identified caller under data and none, via a new RequireIdentifiedAdmin called in validateAlterOperation after the in-process short-circuit, so AlterNoAuth is unaffected. Schema changes count, not only drops: Dgraph already classifies /alter as an admin operation, and a posture that admitted schema changes but refused drops is a line operators would have to learn. Pinned by driving the real validateAlterOperation across five Alter shapes and all three postures; removing the call site fails exactly the ten closed-posture cases. Subscriptions refused every subscriber under anonymous=none, authenticated or not. The poller rebuilt each poll's context from the stored headers with x.AttachAccessJwt alone, so the --security auth token never reached the context and no Principal was ever resolved. Both sites now go through one subscriberContext, which uses x.AttachRequestIdentity. A unit test pins that a stored token or ACL JWT resolves to a Principal, and fails on the old JWT-only prelude. The AST invariant could not have caught the second gap, because it only scanned dgraph/cmd/alpha. It now also scans graphql/subscription and requires subscriberContext to resolve identity. Reintroducing the original poller bug fails it with both checks naming the line. Also names schema changes and drops explicitly in the anonymous help text and in the AnonymousData doc comment. Co-Authored-By: Claude Opus 5.5 --- dgraph/cmd/alpha/run.go | 3 +- dgraph/cmd/alpha/security_posture_test.go | 22 +++++- edgraph/anonymous.go | 30 ++++++++ edgraph/anonymous_test.go | 86 +++++++++++++++++++++++ edgraph/server.go | 8 +++ graphql/subscription/poller.go | 24 +++++-- graphql/subscription/poller_test.go | 83 ++++++++++++++++++++++ x/anonymous.go | 5 +- 8 files changed, 252 insertions(+), 9 deletions(-) create mode 100644 graphql/subscription/poller_test.go diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go index 3bbc7a70122..d89a8c0cc0a 100644 --- a/dgraph/cmd/alpha/run.go +++ b/dgraph/cmd/alpha/run.go @@ -182,7 +182,8 @@ they form a Raft group and provide synchronous replication. "full (default) leaves authorization to whatever the whitelist, token, and ACL "+ "settings decide, which is the behavior of every earlier release. data allows "+ "queries, mutations, commits, and login while denying every administrative "+ - "operation regardless of the whitelist. none additionally denies queries, "+ + "operation, including schema changes and drops, regardless of the whitelist. "+ + "none additionally denies queries, "+ "mutations, and commits, leaving only login and the health endpoints. data and "+ "none require token= or ACL, otherwise no caller can ever be identified."). String()) diff --git a/dgraph/cmd/alpha/security_posture_test.go b/dgraph/cmd/alpha/security_posture_test.go index d28b0f7b2b5..8512d0018b7 100644 --- a/dgraph/cmd/alpha/security_posture_test.go +++ b/dgraph/cmd/alpha/security_posture_test.go @@ -187,6 +187,19 @@ var identityRequired = map[string]string{ "healthCheck": "/health?all, a tenant-admin capability check", "stateHandler": "/state, a tenant-admin capability check", "resolveWithAdminServer": "the HTTP admin routes, which re-enter the /admin GraphQL server", + // graphql/subscription. Every subscription poll is a fresh query with no request + // behind it, so this is where its identity comes from. It sat outside this test's + // reach until a reviewer found it attaching only the access JWT. + "subscriberContext": "every GraphQL subscription poll", +} + +// identityScanDirs are the packages whose code builds a request context from +// client-supplied headers. The invariant is about that edge, not about a package, +// which is why this test reaches outside its own directory: the subscription +// poller is the same edge, and the same bug, in a different package. +var identityScanDirs = []string{ + ".", + "../../../graphql/subscription", } // TestHTTPEdgeResolvesIdentityThroughOneHelper pins an invariant that a live test @@ -211,8 +224,13 @@ func TestHTTPEdgeResolvesIdentityThroughOneHelper(t *testing.T) { // Every non-test file in the package, regardless of build tags. parser.ParseDir is // deprecated, and ignoring tags is what this test wants anyway: a handler compiled // only on one platform is still a handler. - paths, err := filepath.Glob("*.go") - require.NoError(t, err) + var paths []string + for _, dir := range identityScanDirs { + matches, err := filepath.Glob(filepath.Join(dir, "*.go")) + require.NoError(t, err) + require.NotEmptyf(t, matches, "no Go files in %s; has the package moved?", dir) + paths = append(paths, matches...) + } // Aliased: the integration-tagged run_test.go declares a package-level `token`. fset := gotoken.NewFileSet() diff --git a/edgraph/anonymous.go b/edgraph/anonymous.go index 1b31b514ee8..8fc5db0a103 100644 --- a/edgraph/anonymous.go +++ b/edgraph/anonymous.go @@ -73,6 +73,36 @@ func EnforceAnonymousPosture(posture x.AnonymousPosture) { `(policy: %s)`, posture, inner.Name()) } +// RequireIdentifiedAdmin rejects an anonymous caller of an administrative +// operation that is not a Capability check, under --security "anonymous=data" and +// "anonymous=none". +// +// Alter is the case that needs it. Only drop_all reaches a Capability, so before +// this an anonymous caller under a closed posture could still change the schema, +// drop a predicate, or drop_op DATA -- which empties the namespace -- because the +// remaining gates are the whitelist and token check (hasAdminAuth) and ACL +// (authorizeAlter), and neither consults the Principal. With an open whitelist and +// nothing else configured, that left one-request data destruction reachable under +// exactly the posture meant to close it. +// +// It treats every Alter as administrative, schema changes included. Dgraph already +// does: /alter is gated by hasAdminAuth, whose error reads "Token needed for Admin +// operations". A posture that admitted schema changes but refused drops would be a +// line operators had to learn, and drop_op DATA is what getting it wrong costs. +func RequireIdentifiedAdmin(ctx context.Context, op string) error { + if !x.WorkerConfig.Anonymous.RequiresIdentityForCapability() { + return nil + } + if x.PrincipalFrom(ctx) != nil { + return nil + } + return status.Errorf(codes.Unauthenticated, + `%s is an administrative operation and requires an identified caller, and this `+ + `request presented no credential that verified. This cluster runs with `+ + `--security "anonymous=%s". Present the --security auth token, or log in with ACL.`, + op, x.WorkerConfig.Anonymous) +} + // RequireIdentifiedCaller rejects an anonymous caller's ordinary data access under // --security "anonymous=none". // diff --git a/edgraph/anonymous_test.go b/edgraph/anonymous_test.go index eb973f9233e..51376447b42 100644 --- a/edgraph/anonymous_test.go +++ b/edgraph/anonymous_test.go @@ -9,6 +9,7 @@ import ( "context" "testing" + "github.com/dgraph-io/dgo/v250/protos/api" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -254,3 +255,88 @@ func TestRequireIdentifiedCaller(t *testing.T) { }) } } + +// TestAlterRequiresIdentityUnderClosedPosture pins the fix for a gap a reviewer +// found and a live run confirmed: under a closed posture, only drop_all reached a +// Capability check, so an anonymous caller with an open whitelist could still +// change the schema, drop a predicate, or drop_op DATA -- which empties the +// namespace. It drives the real validateAlterOperation rather than the helper, so +// the call site is pinned as well as the rule. +func TestAlterRequiresIdentityUnderClosedPosture(t *testing.T) { + restoreSecurityConfig(t) + x.UpdateHealthStatus(true) + t.Cleanup(func() { x.UpdateHealthStatus(false) }) + + // The standalone posture: every address whitelisted, no token, no ACL. Under + // anonymous=full this admits everyone, which is the exposure being closed. + x.WorkerConfig.AclEnabled = false + worker.Config.AclSecretKey = nil + worker.Config.AuthToken = "" + ips, err := x.GetIPsFromString("0.0.0.0/0") + require.NoError(t, err) + x.WorkerConfig.WhiteListedIPRanges = ips + + ops := map[string]*api.Operation{ + "schema change": {Schema: "name: string @index(exact) ."}, + "drop_attr": {DropAttr: "name"}, + "drop_op DATA": {DropOp: api.Operation_DATA}, + "drop_op TYPE": {DropOp: api.Operation_TYPE, DropValue: "Person"}, + "drop_op ATTR": {DropOp: api.Operation_ATTR, DropValue: "name"}, + } + anonymous := fromIP(t, "203.0.113.7") + identified := x.WithPrincipal(fromIP(t, "203.0.113.7"), + &x.Principal{Subject: PresharedSubject, Method: x.MethodPreshared}) + + for _, posture := range []x.AnonymousPosture{x.AnonymousFull, x.AnonymousData, x.AnonymousNone} { + for name, op := range ops { + t.Run(posture.String()+"/"+name, func(t *testing.T) { + x.WorkerConfig.Anonymous = posture + + err := validateAlterOperation(anonymous, op, NeedAuthorize) + if posture == x.AnonymousFull { + require.NoError(t, err, "anonymous=full must keep the pre-existing behavior") + } else { + require.Error(t, err, "an anonymous Alter must be refused under a closed posture") + require.Equal(t, codes.Unauthenticated, status.Code(err)) + require.Contains(t, status.Convert(err).Message(), "administrative operation") + } + + require.NoError(t, validateAlterOperation(identified, op, NeedAuthorize), + "an identified caller is never refused by the floor") + + // In-process callers (AlterNoAuth) run with NoAuthorize and a context + // that could never carry a Principal. They must be unaffected. + require.NoError(t, validateAlterOperation(context.Background(), op, NoAuthorize), + "trusted in-process Alter must not be gated by the posture") + }) + } + } +} + +func TestRequireIdentifiedAdmin(t *testing.T) { + restoreSecurityConfig(t) + identified := x.WithPrincipal(context.Background(), &x.Principal{Subject: "s"}) + + for _, tt := range []struct { + posture x.AnonymousPosture + wantErr bool + }{ + {posture: x.AnonymousFull, wantErr: false}, + // Unlike RequireIdentifiedCaller, this applies under data too: data closes the + // control plane, and Alter is control plane. + {posture: x.AnonymousData, wantErr: true}, + {posture: x.AnonymousNone, wantErr: true}, + } { + t.Run(tt.posture.String(), func(t *testing.T) { + x.WorkerConfig.Anonymous = tt.posture + err := RequireIdentifiedAdmin(context.Background(), "alter") + if tt.wantErr { + require.Error(t, err) + require.Equal(t, codes.Unauthenticated, status.Code(err)) + } else { + require.NoError(t, err) + } + require.NoError(t, RequireIdentifiedAdmin(identified, "alter")) + }) + } +} diff --git a/edgraph/server.go b/edgraph/server.go index 6050206c14e..3a03282fcd4 100644 --- a/edgraph/server.go +++ b/edgraph/server.go @@ -222,6 +222,14 @@ func validateAlterOperation(ctx context.Context, op *api.Operation, doAuth AuthM return nil } + // Before hasAdminAuth, so an anonymous caller under a closed posture is told the + // actual reason -- no identity -- rather than whichever whitelist or token check + // happens to fail first. See RequireIdentifiedAdmin for why every Alter counts. + if err := RequireIdentifiedAdmin(ctx, "alter"); err != nil { + glog.Warningf("Alter denied with error: %v\n", err) + return err + } + if _, err := hasAdminAuth(ctx, "Alter"); err != nil { glog.Warningf("Alter denied with error: %v\n", err) return err diff --git a/graphql/subscription/poller.go b/graphql/subscription/poller.go index 64ae410bbca..01778b1893d 100644 --- a/graphql/subscription/poller.go +++ b/graphql/subscription/poller.go @@ -55,6 +55,24 @@ type subscriber struct { // AddSubscriber tries to add subscription into the existing polling goroutine if it exists. // If it doesn't exist, then it creates a new polling goroutine for the given request. +// subscriberContext builds the context each subscription query runs under, from +// the headers the subscriber presented when it connected. +// +// It resolves identity the same way the HTTP edge does, through +// x.AttachRequestIdentity, rather than attaching only the access JWT. A poll is a +// fresh query on every tick with no request of its own behind it, so these stored +// headers are the only place a credential can come from. With the JWT alone, the +// --security auth token never reached the context and no Principal was ever +// resolved, so under --security "anonymous=none" every poll was refused -- even for +// a subscriber that had presented a valid token or ACL JWT. +// +// The synthetic request has no RemoteAddr, so no peer is attached. Nothing on this +// path reads one: a subscription runs ordinary GraphQL queries, which are +// authorized on identity and predicates, never on the caller's address. +func subscriberContext(h http.Header) context.Context { + return x.AttachRequestIdentity(context.Background(), &http.Request{Header: h}) +} + func (p *Poller) AddSubscriber(req *schema.Request) (*SubscriberResponse, error) { p.RLock() resolver := p.resolver @@ -99,8 +117,7 @@ func (p *Poller) AddSubscriber(req *schema.Request) (*SubscriberResponse, error) p.Lock() defer p.Unlock() - res := resolver.Resolve(x.AttachAccessJwt(context.Background(), - &http.Request{Header: req.Header}), req) + res := resolver.Resolve(subscriberContext(req.Header), req) if len(res.Errors) != 0 { return nil, res.Errors } @@ -179,8 +196,7 @@ func (p *Poller) poll(req *pollRequest) { p.terminateSubscriptions(req.bucketID) } - ctx := x.AttachAccessJwt(context.Background(), &http.Request{Header: req.graphqlReq.Header}) - res := resolver.Resolve(ctx, req.graphqlReq) + res := resolver.Resolve(subscriberContext(req.graphqlReq.Header), req.graphqlReq) currentHash := farm.Fingerprint64(res.Data.Bytes()) diff --git a/graphql/subscription/poller_test.go b/graphql/subscription/poller_test.go new file mode 100644 index 00000000000..78d78412d55 --- /dev/null +++ b/graphql/subscription/poller_test.go @@ -0,0 +1,83 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package subscription + +import ( + "context" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc/metadata" + + "github.com/dgraph-io/dgraph/v25/x" +) + +// recordingAuthenticator reports what credential metadata reached it, so the test +// observes the context subscriberContext built without depending on how any real +// authenticator verifies a credential. +type recordingAuthenticator struct{} + +func (recordingAuthenticator) Name() string { return "recording" } + +func (recordingAuthenticator) Authenticate(ctx context.Context) (*x.Principal, error) { + md, _ := metadata.FromIncomingContext(ctx) + if t := md.Get("auth-token"); len(t) > 0 { + return &x.Principal{Subject: "token:" + t[0], Method: x.MethodPreshared}, nil + } + if t := md.Get("accessJwt"); len(t) > 0 { + return &x.Principal{Subject: "jwt:" + t[0], Method: x.MethodACL}, nil + } + return nil, nil +} + +// headerWith builds a stored header the way the subscription handler does, with +// Set, so the key goes through the same canonicalization a real request's would. +func headerWith(key, value string) http.Header { + h := http.Header{} + h.Set(key, value) + return h +} + +// TestSubscriberContextResolvesIdentity pins the fix for a gap a reviewer found: +// the poller attached only the access JWT, so the --security auth token never +// reached the context and no Principal was resolved for any subscriber. Under +// --security "anonymous=none" that refused every poll, authenticated or not. +func TestSubscriberContextResolvesIdentity(t *testing.T) { + x.SetAuthenticator(recordingAuthenticator{}) + t.Cleanup(func() { x.SetAuthenticator(nil) }) + + tests := []struct { + name string + header http.Header + wantSubject string + }{{ + name: "the --security auth token reaches the authenticator", + header: headerWith("X-Dgraph-AuthToken", "s3cr3t"), + wantSubject: "token:s3cr3t", + }, { + name: "the ACL access JWT still reaches it", + header: headerWith("X-Dgraph-AccessToken", "a.b.c"), + wantSubject: "jwt:a.b.c", + }, { + // A subscriber that presented nothing is anonymous, and must stay so: this + // path must not invent an identity. + name: "no credential, no Principal", + header: http.Header{}, + }} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := x.PrincipalFrom(subscriberContext(tt.header)) + if tt.wantSubject == "" { + require.Nil(t, p) + return + } + require.NotNil(t, p, "the stored header must resolve to a Principal") + require.Equal(t, tt.wantSubject, p.Subject) + }) + } +} diff --git a/x/anonymous.go b/x/anonymous.go index f0f9aae6085..8e095cbb9b5 100644 --- a/x/anonymous.go +++ b/x/anonymous.go @@ -41,10 +41,11 @@ const ( // mutations, commits, Login, and the health endpoints -- while every // administrative capability is denied regardless of the whitelist. // - // "Administrative" means a Capability check: namespace create/drop/list, UID + // "Administrative" means a Capability check -- namespace create/drop/list, UID // leasing, drop-all, arming an external-snapshot import, reading cluster state, // and the privileged GraphQL admin surface (backup, restore, export, shutdown, - // removeNode, moveTablet, assign, draining, config). + // removeNode, moveTablet, assign, draining, config) -- and every Alter, schema + // changes and drops alike. AnonymousData // AnonymousNone additionally denies ordinary data access, leaving an anonymous