diff --git a/audit/interceptor.go b/audit/interceptor.go index a602fe53e28..5f30cd020e9 100644 --- a/audit/interceptor.go +++ b/audit/interceptor.go @@ -175,6 +175,18 @@ func auditGrpc(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo) var namespace uint64 var err error extractUser := func(md metadata.MD) { + // Prefer the identity the interceptor already verified. Without this, + // auditing a request re-parses and re-verifies its access token, a second + // full JWT verification per RPC purely to recover a username the identity + // interceptor has already extracted. + // + // The fallback below is unchanged and still covers every case where no + // Principal exists: poor-man's auth, a token that failed to verify + // (UnknownUser), and no credential at all (UnauthorisedUser under ACL). + if p := x.PrincipalFrom(ctx); p != nil { + user = p.Subject + return + } if t := md.Get("accessJwt"); len(t) > 0 { user = getUser(t[0], false) } else if t := md.Get("auth-token"); len(t) > 0 { @@ -230,9 +242,24 @@ func auditHttp(w *ResponseWriter, r *http.Request) { user = getUser("", false) } + // Audit must never reject a request, so a resolver error becomes the explicit + // unknown sentinel rather than a failure. + // + // Worth knowing what this does and does not achieve today: the built-in resolver + // tolerates a token it cannot parse and reports no error, so a malformed + // credential is still recorded against the root namespace and this branch is + // reached only by an installed resolver that fails closed. Attributing a + // malformed token accurately needs the resolver to distinguish "no credential" + // from "unusable credential", which it cannot yet do: /admin and the login + // mutation legitimately carry no credential at all. + namespace, err := x.ResolveTenantHTTP(r) + if err != nil { + namespace = UnknownNamespace + } + auditor.Audit(&AuditEvent{ User: user, - Namespace: x.ExtractNamespaceHTTP(r), + Namespace: namespace, ServerHost: x.WorkerConfig.MyAddr, ClientHost: r.RemoteAddr, Endpoint: r.URL.Path, diff --git a/dgraph/cmd/alpha/hooks.go b/dgraph/cmd/alpha/hooks.go index d1ca3c65f66..8f9bf9b8ac0 100644 --- a/dgraph/cmd/alpha/hooks.go +++ b/dgraph/cmd/alpha/hooks.go @@ -21,6 +21,20 @@ var RegisterFlags = defaultRegisterFlags func defaultRegisterFlags(f *pflag.FlagSet) {} +// ConfigureIdentity installs deployment-specific authentication and authorization +// — an x.Authenticator, and any additional capability sources — from parsed +// configuration. Called from run() after x.WorkerConfig.Parse and before any +// listener starts serving, which is the only window where the config exists and +// nothing is being served yet. Default: no-op (OSS builds). +// +// Separate from the service-registration hooks below on purpose. What it installs +// is process-wide: it resolves the caller's identity for every API the Alpha +// serves, so tying it to whether one optional service happens to be enabled would +// misdescribe its reach. +var ConfigureIdentity = defaultConfigureIdentity + +func defaultConfigureIdentity() {} + // RegisterZanzibar wires the Zanzibar gRPC service onto s and bootstraps the // fixed predicate schema. Default: no-op (OSS builds). var RegisterZanzibar = defaultRegisterZanzibar diff --git a/dgraph/cmd/alpha/http.go b/dgraph/cmd/alpha/http.go index b4ba1baf1bc..4f0f2ff48ef 100644 --- a/dgraph/cmd/alpha/http.go +++ b/dgraph/cmd/alpha/http.go @@ -652,10 +652,9 @@ func alterHandler(w http.ResponseWriter, r *http.Request) { glog.Infof("The alter request is forwarded by %s\n", fwd) } - // Pass in PoorMan's auth, ACL and IP information if present. - ctx := x.AttachAuthToken(context.Background(), r) - ctx = x.AttachAccessJwt(ctx, r) - ctx = x.AttachRemoteIP(ctx, r) + // Pass in PoorMan's auth, ACL and IP information if present, and resolve the + // caller's identity from it. + ctx := x.AttachRequestIdentity(context.Background(), r) if _, err := (&edgraph.Server{}).Alter(ctx, op); err != nil { x.SetStatus(w, x.Error, err.Error()) return @@ -705,7 +704,12 @@ func graphqlProbeHandler(gqlHealthStore *admin.GraphQLHealthStore, globalEpoch m w.Header().Set("Content-Type", "application/json") // lazy load the schema so that just by making a probe request, // one can boot up GraphQL for their namespace - namespace := x.ExtractNamespaceHTTP(r) + namespace, err := x.ResolveTenantHTTP(r) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + x.Check2(fmt.Fprintf(w, `{"error":"%s"}`, err)) + return + } if err := admin.LazyLoadSchema(namespace); err != nil { w.WriteHeader(http.StatusInternalServerError) x.Check2(w.Write([]byte(fmt.Sprintf(`{"error":"%s"}`, err)))) @@ -732,10 +736,11 @@ func resolveWithAdminServer(gqlReq *schema.Request, r *http.Request, adminServer admin.IServeGraphQL) *schema.Response { md := metadata.New(nil) ctx := metadata.NewIncomingContext(context.Background(), md) - ctx = x.AttachAccessJwt(ctx, r) - ctx = x.AttachRemoteIP(ctx, r) - ctx = x.AttachAuthToken(ctx, r) - ctx = x.AttachJWTNamespace(ctx) + ctx = x.AttachRequestIdentity(ctx, r) + ctx, err := x.ResolveTenant(ctx) + if err != nil { + return schema.ErrorResponse(err) + } return adminServer.ResolveWithNs(ctx, x.RootNamespace, gqlReq) } diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go index c5118bbfab4..bd37960fac8 100644 --- a/dgraph/cmd/alpha/run.go +++ b/dgraph/cmd/alpha/run.go @@ -393,11 +393,15 @@ func serveGRPC(l net.Listener, tlsCfg *tls.Config, closer *z.Closer) { x.RegisterExporters(Alpha.Conf, "dgraph.alpha") - unary := []grpc.UnaryServerInterceptor{audit.AuditRequestGRPC} + // Identity resolution runs first so every later interceptor and handler sees + // the same verified Principal instead of re-parsing the credential. It never + // rejects — see x.WithResolvedIdentity — so ordering it ahead of audit cannot + // suppress an audit record. + unary := []grpc.UnaryServerInterceptor{x.IdentityUnaryInterceptor(), audit.AuditRequestGRPC} if zi := ZanzibarUnaryInterceptor(); zi != nil { unary = append(unary, zi) } - stream := []grpc.StreamServerInterceptor{audit.AuditStreamGRPC} + stream := []grpc.StreamServerInterceptor{x.IdentityStreamInterceptor(), audit.AuditStreamGRPC} if zs := ZanzibarStreamInterceptor(); zs != nil { stream = append(stream, zs) } @@ -521,7 +525,22 @@ func setupServer(closer *z.Closer, enableMcp bool) { mainServer, adminServer, gqlHealthStore = admin.NewServers(introspection, globalEpoch, closer) baseMux.HandleFunc("/graphql", func(w http.ResponseWriter, r *http.Request) { - namespace := x.ExtractNamespaceHTTP(r) + // Strict here because this handler routes by the resolved namespace: it sets + // the resolver header that selects which namespace's GraphQL schema serves + // the request, so a token it cannot resolve must be refused rather than + // quietly served the root namespace's. A request with no token still + // resolves to the root namespace — see x.ResolveTenantHTTPStrict. + // + // Two limits, neither of which this closes. The strict variant only rejects a + // token that was *presented* and could not be resolved, and its "was one + // presented" probe reads the ACL accessJwt channel, so an installed + // non-ACL resolver is not covered. And the graphql-ws subscription handler + // routes by the same header while still using the lenient resolver. + namespace, err := x.ResolveTenantHTTPStrict(r) + if err != nil { + admin.WriteErrorResponse(w, r, err) + return + } r.Header.Set("resolver", strconv.FormatUint(namespace, 10)) if err := admin.LazyLoadSchema(namespace); err != nil { admin.WriteErrorResponse(w, r, err) @@ -536,7 +555,12 @@ func setupServer(closer *z.Closer, enableMcp bool) { r.Header.Set("resolver", "0") // We don't need to load the schema for all the admin operations. // Only a few like getUser, queryGroup require this. So, this can be optimized. - if err := admin.LazyLoadSchema(x.ExtractNamespaceHTTP(r)); err != nil { + namespace, err := x.ResolveTenantHTTP(r) + if err != nil { + admin.WriteErrorResponse(w, r, err) + return + } + if err := admin.LazyLoadSchema(namespace); err != nil { admin.WriteErrorResponse(w, r, err) return } @@ -706,6 +730,11 @@ func run() { } x.WorkerConfig.Parse(Alpha.Conf) + // 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() + // Set the directory for temporary buffers. z.SetTmpDir(x.WorkerConfig.TmpDir) diff --git a/dgraphtest/config.go b/dgraphtest/config.go index b68be111619..610f328a902 100644 --- a/dgraphtest/config.go +++ b/dgraphtest/config.go @@ -14,6 +14,13 @@ import ( "github.com/golang-jwt/jwt/v5" ) +// defaultWhitelist is the --security whitelist every dgraphtest cluster gets +// unless a test overrides it with WithWhitelist. It has to stay wide open: +// LocalCluster.Start() probes GraphQL with an admin mutation, and every admin +// GraphQL operation carries IpWhitelistingMW, so a narrower default would break +// every existing test rather than only the ones doing admin work. +const defaultWhitelist = "0.0.0.0/0" + // UpgradeCombo represents a version combination before and // after the upgrade, and the strategy for upgrading type UpgradeCombo struct { @@ -112,6 +119,7 @@ type ClusterConfig struct { repoDir string mcp bool securityToken string + whitelist string } // NewClusterConfig generates a default ClusterConfig @@ -132,6 +140,7 @@ func NewClusterConfig() ClusterConfig { portOffset: -1, customPlugins: false, mcp: false, + whitelist: defaultWhitelist, } } @@ -174,6 +183,30 @@ func (cc ClusterConfig) WithSecurityToken(token string) ClusterConfig { return cc } +// WithWhitelist sets the Alpha's --security whitelist for admin operations, +// REPLACING the wide-open default rather than adding to it. Zero is unaffected: it +// reads its own whitelist from DGRAPH_ZERO_SECURITY, because older zero binaries used +// in upgrade tests would fail to start on an unrecognized flag. +// +// The value is the flag's own syntax: a comma-separated list of IP addresses, +// a.b.c.d:w.x.y.z ranges, CIDR blocks, or hostnames, e.g. +// "192.168.0.0/16,host.docker.internal". +// +// Replacing is the point. The default is 0.0.0.0/0, so an additive option could never +// express a cluster that denies anyone, which is the only configuration worth a test. +// Note that loopback is admitted unconditionally by x.isIpWhitelisted regardless of +// this setting, so a test that wants a denial has to reach the alpha over a +// non-loopback source — which a published Docker port gives it on Linux, though not +// necessarily on Docker Desktop. +// +// Setting a whitelist other than the default makes Start() wait on /probe/graphql +// instead of the admin GraphQL mutation it normally probes with; see +// LocalCluster.waitUntilGraphqlHealthCheck for why. +func (cc ClusterConfig) WithWhitelist(spec string) ClusterConfig { + cc.whitelist = spec + return cc +} + // WithAcl enables ACL feature for Dgraph cluster func (cc ClusterConfig) WithACL(aclTTL time.Duration) ClusterConfig { cc.acl = true diff --git a/dgraphtest/dgraph.go b/dgraphtest/dgraph.go index e49cf4582c3..09b251d7e21 100644 --- a/dgraphtest/dgraph.go +++ b/dgraphtest/dgraph.go @@ -256,9 +256,9 @@ func (a *alpha) cmd(c *LocalCluster) []string { "--bindall", "--logtostderr", fmt.Sprintf("-v=%d", c.conf.verbosity)} if c.lowerThanV21 { - acmd = append(acmd, `--whitelist=0.0.0.0/0`, "--telemetry=false") + acmd = append(acmd, fmt.Sprintf(`--whitelist=%s`, c.conf.whitelist), "--telemetry=false") } else { - security := `--security=whitelist=0.0.0.0/0` + security := fmt.Sprintf(`--security=whitelist=%s`, c.conf.whitelist) if c.conf.securityToken != "" { security += fmt.Sprintf(`;token=%s`, c.conf.securityToken) } diff --git a/dgraphtest/local_cluster.go b/dgraphtest/local_cluster.go index 99188303d5a..8594d6db475 100644 --- a/dgraphtest/local_cluster.go +++ b/dgraphtest/local_cluster.go @@ -864,6 +864,15 @@ func (c *LocalCluster) waitUntilLogin() error { } func (c *LocalCluster) waitUntilGraphqlHealthCheck() error { + // The probes below are admin GraphQL operations, and every admin GraphQL + // operation carries IpWhitelistingMW — login included. On a cluster whose + // whitelist does not admit this process, neither can ever succeed, so wait on + // /probe/graphql instead: no whitelist middleware, no auth, and its whole + // purpose is answering "is GraphQL serving yet". + if c.conf.whitelist != defaultWhitelist { + return c.waitUntilGraphqlProbe() + } + hc, err := c.HTTPClient() if err != nil { return errors.Wrap(err, "error creating http client while graphql health check") @@ -901,6 +910,33 @@ func (c *LocalCluster) waitUntilGraphqlHealthCheck() error { return errors.Wrap(err, "error during graphql health check") } +// waitUntilGraphqlProbe waits for GraphQL to serve using /probe/graphql, which +// carries no IP-whitelist and no auth middleware. It is the probe to use when the +// cluster's whitelist may not admit the test process. +func (c *LocalCluster) waitUntilGraphqlProbe() error { + url, err := c.serverURL("alpha", "/probe/graphql") + if err != nil { + return errors.Wrap(err, "error getting graphql probe URL") + } + + var lastErr error + for attempt := range 10 { + time.Sleep(waitDurBeforeRetry) + req, err := http.NewRequest(http.MethodGet, "http://"+url, nil) + if err != nil { + return errors.Wrap(err, "error building graphql probe request") + } + if _, lastErr = dgraphapi.DoReq(req); lastErr == nil { + log.Printf("[INFO] graphql probe succeeded for %v", c.conf.prefix) + return nil + } + if attempt > 5 { + log.Printf("[WARNING] problem during graphql probe: %v", lastErr) + } + } + return errors.Wrap(lastErr, "error during graphql probe") +} + // Upgrades the cluster to the provided dgraph version func (c *LocalCluster) Upgrade(version string, strategy UpgradeStrategy) error { if version == c.conf.version { diff --git a/edgraph/access.go b/edgraph/access.go index 15c0fb458ce..f56cc45b9fb 100644 --- a/edgraph/access.go +++ b/edgraph/access.go @@ -604,7 +604,40 @@ func upsertGroot(ctx context.Context, passwd string) error { } // extract the userId, groupIds from the accessJwt in the context +// extractUserAndGroups reports the ACL identity and tenant claim of the calling +// request. +// +// It prefers the Principal the identity interceptor already resolved, and parses +// the token itself only when there is none. Before that interceptor existed there +// was one JWT verification per request; adding it made two under ACL, and for the +// RS/PS/ES algorithms that is asymmetric crypto on every RPC. +// +// Two conditions on the fast path, both load-bearing: +// +// The Method must be MethodACL. Principal.Groups is whatever the issuer asserted, +// and x.IsSuperAdmin consults it by name, so accepting an external issuer's +// membership here would let anything that can mint a `guardians` group become +// superadmin. Only Dgraph's own token speaks for Dgraph's own groups. +// +// The namespace claim must be present. aclAuthenticator deliberately ignores it — +// tenancy is the resolver's concern — so the Principal alone cannot say whether +// the token carried one. validateToken requires it, and that requirement is doing +// real work: aclTenantResolver tolerates a token it cannot extract a namespace +// from and leaves whatever namespace the context already carries, which on the +// server side is client-controlled. It gets away with that precisely because this +// function rejects the same token a moment later. Reading the namespace from the +// resolved tenancy instead would remove the check the resolver depends on, and +// hand the caller their own namespace. +// +// So the value comes from the claim, exactly as validateToken produces it. Keying +// authorization on the resolved namespace is the right end state and it is a +// behavior change, so it belongs with the others rather than inside a change whose +// whole claim is that nothing moved. authorizePreds already keys on the resolved +// namespace; the remaining reader is shouldAllowAcls. func extractUserAndGroups(ctx context.Context) (*userData, error) { + if ud, ok := userDataFromPrincipal(ctx); ok { + return ud, nil + } accessJwt, err := x.ExtractJwt(ctx) if err != nil { return nil, err @@ -612,49 +645,21 @@ func extractUserAndGroups(ctx context.Context) (*userData, error) { return validateToken(accessJwt) } -type authPredResult struct { - allowed []string - blocked map[string]struct{} -} - -func authorizePreds(ctx context.Context, userData *userData, preds []string, - aclOp *acl.Operation) *authPredResult { - - if !worker.AclCachePtr.Loaded() { - RefreshACLs(ctx) - } - - userId := userData.userId - groupIds := userData.groupIds - ns := userData.namespace - blockedPreds := make(map[string]struct{}) - for _, pred := range preds { - nsPred := x.NamespaceAttr(ns, pred) - if err := worker.AclCachePtr.AuthorizePredicate(groupIds, nsPred, aclOp); err != nil { - logAccess(&accessEntry{ - userId: userId, - groups: groupIds, - preds: preds, - operation: aclOp, - allowed: false, - }) - blockedPreds[pred] = struct{}{} - } - } - if worker.HasAccessToAllPreds(ns, groupIds, aclOp) { - // Setting allowed to nil allows access to all predicates. Note that the access to ACL - // predicates will still be blocked. - return &authPredResult{allowed: nil, blocked: blockedPreds} - } - // User can have multiple permission for same predicate, add predicate - allowedPreds := make([]string, 0, len(worker.AclCachePtr.GetUserPredPerms(userId))) - // only if the acl.Op is covered in the set of permissions for the user - for predicate, perm := range worker.AclCachePtr.GetUserPredPerms(userId) { - if (perm & aclOp.Code) > 0 { - allowedPreds = append(allowedPreds, predicate) - } +// userDataFromPrincipal rebuilds userData from an already-verified Principal, +// reporting false when the fast path does not apply so the caller falls back to +// parsing — and to the error message parsing would have produced. +func userDataFromPrincipal(ctx context.Context) (*userData, bool) { + p := x.PrincipalFrom(ctx) + if p == nil || p.Method != x.MethodACL { + return nil, false + } + // float64 because JSON numbers are, which is also why validateToken caps the + // usable namespace at 1<<52; asserting the same type keeps that identical. + ns, ok := p.Claims["namespace"].(float64) + if !ok { + return nil, false } - return &authPredResult{allowed: allowedPreds, blocked: blockedPreds} + return &userData{namespace: uint64(ns), userId: p.Subject, groupIds: p.Groups}, true } // authorizeAlter parses the Schema in the operation and authorizes the operation @@ -709,10 +714,13 @@ func authorizeAlter(ctx context.Context, op *api.Operation) error { "only guardians are allowed to drop all data, but the current user is %s", userId) } - result := authorizePreds(ctx, userData, preds, acl.Modify) - if len(result.blocked) > 0 { + result, err := AuthorizePredicates(ctx, preds, acl.Modify) + if err != nil { + return err + } + if len(result.Blocked) > 0 { var msg strings.Builder - for key := range result.blocked { + for key := range result.Blocked { x.Check2(msg.WriteString(key)) x.Check2(msg.WriteString(" ")) } @@ -828,17 +836,20 @@ func authorizeMutation(ctx context.Context, gmu *dql.Mutation) error { } return nil } - result := authorizePreds(ctx, userData, preds, acl.Write) - if len(result.blocked) > 0 { + result, err := AuthorizePredicates(ctx, preds, acl.Write) + if err != nil { + return err + } + if len(result.Blocked) > 0 { var msg strings.Builder - for key := range result.blocked { + for key := range result.Blocked { x.Check2(msg.WriteString(key)) x.Check2(msg.WriteString(" ")) } return status.Errorf(codes.PermissionDenied, "unauthorized to mutate following predicates: %s\n", msg.String()) } - gmu.AllowedPreds = result.allowed + gmu.AllowedPreds = result.Allowed return nil } @@ -992,8 +1003,11 @@ func authorizeQuery(ctx context.Context, parsedReq *dql.Result, graphql bool) er return blockedPreds(preds), nil, nil } - result := authorizePreds(ctx, userData, preds, acl.Read) - return result.blocked, result.allowed, nil + result, err := AuthorizePredicates(ctx, preds, acl.Read) + if err != nil { + return nil, nil, err + } + return result.Blocked, result.Allowed, nil } blockedPreds, allowedPreds, err := doAuthorizeQuery() @@ -1014,7 +1028,7 @@ func authorizeQuery(ctx context.Context, parsedReq *dql.Result, graphql bool) er if len(blockedPreds) != 0 { // For GraphQL requests, we allow filtered access to the ACL predicates. // Filter for user_id and group_id is applied for the currently logged in user. - if graphql && shouldAllowAcls(namespace) { + if graphql && shouldAllowAcls(namespace) { // namespace is the token claim, as above for _, gq := range parsedReq.Query { addUserFilterToQuery(gq, userId, groupIds) } @@ -1081,8 +1095,11 @@ func authorizeSchemaQuery(ctx context.Context, er *query.ExecutionResult) error } return blockedPreds(preds), nil } - result := authorizePreds(ctx, userData, preds, acl.Read) - return result.blocked, nil + result, err := AuthorizePredicates(ctx, preds, acl.Read) + if err != nil { + return nil, err + } + return result.Blocked, nil } // find the predicates which are blocked for the schema query @@ -1115,61 +1132,6 @@ func authorizeSchemaQuery(ctx context.Context, er *query.ExecutionResult) error return nil } -// AuthSuperAdmin authorizes the operations for the users who belong to the guardians -// group in the galaxy namespace. This authorization is used for admin usages like creation and -// deletion of a namespace, resetting passwords across namespaces etc. -// NOTE: The caller should not wrap the error returned. If needed, propagate the GRPC error code. -func AuthSuperAdmin(ctx context.Context) error { - if !x.WorkerConfig.AclEnabled { - return nil - } - ns, err := x.ExtractNamespaceFrom(ctx) - if err != nil { - return errors.Wrap(err, "Authorize guardian of the galaxy, extracting jwt token, error:") - } - if ns != 0 { - return status.Error( - codes.PermissionDenied, "Only superadmin is allowed to do this operation") - } - // AuthorizeGuardians will extract (user, []groups) from the JWT claims and will check if - // any of the group to which the user belongs is "guardians" or not. - if err := AuthorizeGuardians(ctx); err != nil { - s := status.Convert(err) - return status.Error( - s.Code(), "AuthSuperAdmin: failed to authorize guardians. "+s.Message()) - } - glog.V(3).Info("Successfully authorised guardian of the galaxy") - return nil -} - -// AuthorizeGuardians authorizes the operation for users which belong to Guardians group. -// NOTE: The caller should not wrap the error returned. If needed, propagate the GRPC error code. -func AuthorizeGuardians(ctx context.Context) error { - if worker.Config.AclSecretKey == nil { - // the user has not turned on the acl feature - return nil - } - - userData, err := extractUserAndGroups(ctx) - switch { - case err == x.ErrNoJwt: - return status.Error(codes.PermissionDenied, err.Error()) - case err != nil: - return status.Error(codes.Unauthenticated, err.Error()) - default: - userId := userData.userId - groupIds := userData.groupIds - - if !x.IsSuperAdmin(groupIds) { - // Deny access for members of non-guardian groups - return status.Error(codes.PermissionDenied, fmt.Sprintf("Only guardians are "+ - "allowed access. User '%v' is not a member of guardians group.", userId)) - } - } - - return nil -} - /* addUserFilterToQuery applies makes sure that a user can access only its own acl info by applying filter of userid and groupid to acl predicates. A query like diff --git a/edgraph/access_control.go b/edgraph/access_control.go new file mode 100644 index 00000000000..f6424728d56 --- /dev/null +++ b/edgraph/access_control.go @@ -0,0 +1,262 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package edgraph + +import ( + "context" + "strings" + "sync/atomic" + + "github.com/golang/glog" + + "github.com/dgraph-io/dgraph/v25/acl" + "github.com/dgraph-io/dgraph/v25/x" +) + +// Capability is an authority over the cluster or a tenant, as opposed to access to +// particular predicates. Capabilities answer "may this caller perform this class of +// operation at all", and they are deliberately few: an operation that needs a +// finer distinction should be authorized on its predicates instead. +type Capability int + +const ( + // CapClusterAdmin is authority over the whole cluster: creating and dropping + // namespaces, dropping all data, resetting passwords across tenants. Nothing + // scoped to a single tenant should require it. + CapClusterAdmin Capability = iota + + // CapTenantAdmin is administrative authority within a tenant — reading cluster + // state and health, arming an external-snapshot import. + // + // It is a weaker capability than CapClusterAdmin, and under the built-in policy + // the difference is real but narrow: CapClusterAdmin additionally requires the + // caller to be in the root namespace. + CapTenantAdmin + + // CapAssumeTenant is the authority to act in a namespace other than the + // caller's own. The live loader needs it for a galaxy-wide write. + // + // Split out from CapClusterAdmin even though the built-in policy resolves both + // identically, because they are different powers and conflating them costs + // something later: a scoped service account should be able to write across + // tenants without also being able to drop the cluster. + CapAssumeTenant + + // CapLeaseUIDs is the authority to lease a block of UIDs from Zero. + // + // Separate from CapClusterAdmin, which it briefly shared, because the callers + // are not administrators: `dgraph live` allocates UIDs through this RPC on every + // run — xidmap falls back to it whenever it was built without a direct Zero + // connection — so folding it in made the loader require a whitelisted IP or an + // auth token against any cluster running without ACL, where it had needed + // neither. + // + // Tightening it is the same change as fail-closed galaxy-operation, with the same + // cost and the same answer: it wants a flag-gated release rather than to ride + // along. Under ACL it is guardian-of-the-root-namespace, exactly as it was before + // capabilities existed. + CapLeaseUIDs +) + +func (c Capability) String() string { + switch c { + case CapClusterAdmin: + return "cluster-admin" + case CapTenantAdmin: + return "tenant-admin" + case CapAssumeTenant: + return "assume-tenant" + case CapLeaseUIDs: + return "lease-uids" + } + return "unknown-capability" +} + +// AccessController decides what a verified caller may do. +// +// It answers only authorization. Who the caller is has already been established by +// the time a method here runs — x.PrincipalFrom carries the answer — and which +// tenant the request operates in is the TenantResolver's business. Keeping all +// three apart is the point of the surrounding work; an implementation that +// verifies a credential or derives a namespace has taken on someone else's job. +// +// Two methods rather than one, because the two questions have different shapes. +// A capability is a yes or no. Predicate access is a partition: the answer is +// which of the requested predicates are usable, and the caller proceeds with +// those. +type AccessController interface { + // Name identifies the policy, for logs and for the error an unimplemented + // capability produces. + Name() string + + // AuthorizeCapability reports whether the caller on ctx holds c, returning a + // gRPC status error when it does not. + // + // Callers must not wrap the returned error. They may prepend context to its + // message, but the code has to survive — several endpoints distinguish + // Unauthenticated from PermissionDenied, and clients act on that difference. + AuthorizeCapability(ctx context.Context, c Capability) error + + // AuthorizePredicates partitions preds into what the caller may and may not + // access under op. + // + // It deliberately does not return "denied". A query names predicates the caller + // may have no business seeing, and the established behavior is to drop them and + // answer the rest rather than refuse the whole request — so the result is data + // the caller acts on, not a verdict. See PredResult. + // + // The error is for a policy that could not reach an answer at all: the built-in + // one reads a local cache and never fails, but a policy that has to query the + // graph can. An error must not be read as a denial. + // + // Identity comes from ctx, not from a parameter. Handing a policy Dgraph's own + // userData would make every implementation speak ACL. + AuthorizePredicates(ctx context.Context, preds []string, op *acl.Operation) (*PredResult, error) +} + +// PredResult is how a policy partitions the predicates a request named. +type PredResult struct { + // Allowed lists the predicates the caller may access — but a nil Allowed means + // *all* of them, not none. + // + // The sentinel is load-bearing and it predates this interface: a caller with + // blanket access has no enumerable predicate list, and materializing one would + // mean listing every predicate in the namespace on every request. Blocked is + // still honored when Allowed is nil, which is how ACL predicates stay + // unreadable even to a caller that may read everything else. + Allowed []string + + // Blocked is the subset of the requested predicates the caller may not access. + // Empty means none were refused. + Blocked map[string]struct{} +} + +// namespaceOrClaim reports the namespace a request OPERATES IN, preferring the +// resolved tenancy over the token's claim. Its one caller is authorizePreds, and +// that is deliberate. +// +// The distinction this function exists for, learned the hard way: "which tenant's +// data does this request touch" and "who is this caller" are different questions +// with different correct sources. Predicate authorization is the first — it must +// key on the same channel storage keys on, or it authorizes in one namespace while +// executing in another. Everything asking the second — authSuperAdmin's root- +// namespace test, filterTablets, shouldAllowAcls — must read the signed claim, +// because the resolved value comes from md["namespace"], which is client-supplied +// until a resolver overwrites it, and not every path resolves before authorizing. +// +// A divergence is logged rather than rejected: it should be impossible, and turning +// "impossible" into an outage is worse than making it visible. +func namespaceOrClaim(ctx context.Context, claim uint64, site string) uint64 { + resolved, err := x.ExtractNamespace(ctx) + if err != nil { + glog.Warningf("%s: no resolved tenancy on the context (%v); falling back to the "+ + "token's claim %#x", site, err, claim) + return claim + } + if resolved != claim { + glog.Warningf("%s: resolved namespace %#x differs from the token's claim %#x; "+ + "authorizing against the resolved one", site, resolved, claim) + } + return resolved +} + +// CapabilitySource is one way a caller can come to hold a capability. +// +// The built-in policy consults its sources in order and the first grant wins, so +// a deployment can add a path to cluster authority without replacing the whole +// policy. That matters because the alternative — SetAccessController — means +// reimplementing Dgraph's ACL rules in order to add one grant beside them. +// +// A source answers only "does this caller hold c". It must not deny: returning +// false means "not by this route", and the next source still gets asked. +type CapabilitySource interface { + // Name identifies the source in logs and in the denial message, so an + // operator can see which routes were tried. + Name() string + // Grants reports whether the caller holds c by this route. p is the verified + // principal, or nil when the request carried no credential — a source keyed on + // operator-level controls rather than identity still works in that case. + Grants(ctx context.Context, p *x.Principal, c Capability) bool +} + +// capabilitySources are consulted in order. breakGlass is first and always +// present: if cluster authority could only come from an identity provider, an +// outage or a key rotation there would lock an operator out of namespace +// lifecycle and drop-all, which is not an acceptable failure mode for a database. +var capabilitySources = []CapabilitySource{breakGlassSource{}} + +// RegisterCapabilitySource appends a source, consulted after the ones already +// registered. Call it during command setup, before any listener starts serving; +// it is not safe to call concurrently with request handling. +func RegisterCapabilitySource(s CapabilitySource) { + if s == nil { + return + } + capabilitySources = append(capabilitySources, s) +} + +// grantedBySource reports whether any registered source grants c, and names the +// one that did. +func grantedBySource(ctx context.Context, c Capability) (string, bool) { + p := x.PrincipalFrom(ctx) + for _, s := range capabilitySources { + if s.Grants(ctx, p, c) { + return s.Name(), true + } + } + return "", false +} + +// sourceNames lists the routes that were tried, for the denial message. +func sourceNames() string { + names := make([]string, 0, len(capabilitySources)) + for _, s := range capabilitySources { + names = append(names, s.Name()) + } + return strings.Join(names, ", ") +} + +// accessController holds the installed policy, or nil for the built-in one. An +// atomic pointer for the same reason as x.tenantResolver and x.authenticator: one +// word, read per request, written once at startup. +var accessController atomic.Pointer[AccessController] + +// SetAccessController installs a deployment-specific policy, replacing the +// built-in one that authorizes against Dgraph's own ACL. Call it during command +// setup, before any listener starts serving. Passing nil restores the built-in. +func SetAccessController(ac AccessController) { + if ac == nil { + accessController.Store(nil) + return + } + accessController.Store(&ac) +} + +// currentAccessController returns the installed policy, or the built-in one when +// none is installed. +func currentAccessController() AccessController { + if ac := accessController.Load(); ac != nil { + return *ac + } + return predicateACL{} +} + +// AuthorizeCapability authorizes c for the caller on ctx under the installed +// policy. This is the entry point every call site uses; the interface exists so +// what it resolves to can be replaced. +// +// NOTE: do not wrap the returned error. Prepend to the message if needed, and +// propagate the gRPC code. +func AuthorizeCapability(ctx context.Context, c Capability) error { + return currentAccessController().AuthorizeCapability(ctx, c) +} + +// AuthorizePredicates partitions preds for the caller on ctx under the installed +// policy. See AccessController.AuthorizePredicates: an error means the policy could +// not decide, which is not the same as a denial. +func AuthorizePredicates(ctx context.Context, preds []string, op *acl.Operation) (*PredResult, error) { + return currentAccessController().AuthorizePredicates(ctx, preds, op) +} diff --git a/edgraph/access_test.go b/edgraph/access_test.go index 424e3ab8545..1f772b9994b 100644 --- a/edgraph/access_test.go +++ b/edgraph/access_test.go @@ -6,11 +6,13 @@ package edgraph import ( + "context" "testing" "time" "github.com/golang-jwt/jwt/v5" "github.com/stretchr/testify/require" + "google.golang.org/grpc/metadata" "github.com/dgraph-io/dgraph/v25/acl" "github.com/dgraph-io/dgraph/v25/worker" @@ -115,3 +117,122 @@ func TestMain(m *testing.M) { worker.Config.AclSecretKey = x.Sensitive("6ABBAA2014CFF00289D20D20DA296F67") m.Run() } + +// TestExtractUserAndGroupsPrefersThePrincipal is the equivalence proof for the +// fast path. Both routes are run over the same inputs and required to agree, +// rather than the fast path being checked against remembered behavior. +// +// The inputs that must NOT take the fast path are the point of the test. Each one +// is a way the Principal alone is insufficient, and each would be a privilege +// change if admitted. +func TestExtractUserAndGroupsPrefersThePrincipal(t *testing.T) { + // aclAuthenticator reports no identity at all unless ACL is on, and the fast + // path is only reachable when it does. Scoped here rather than in TestMain, + // which other tests in this package read as ACL-off. + prev := x.WorkerConfig.AclEnabled + t.Cleanup(func() { x.WorkerConfig.AclEnabled = prev }) + x.WorkerConfig.AclEnabled = true + + expiry := time.Now().Add(30 * time.Minute).Unix() + + // withPrincipal resolves identity onto the context the way the interceptor + // does, then attaches the token so the slow path stays available. + withPrincipal := func(token string) context.Context { + ctx := metadata.NewIncomingContext(context.Background(), + metadata.Pairs("accessJwt", token)) + return x.WithResolvedIdentity(ctx) + } + + t.Run("both routes agree on a valid token", func(t *testing.T) { + for _, want := range []userData{ + {1234567890, "user1", []string{"701", "702"}}, + {2345678901, "user2", []string{"703", "701"}}, + {0, "groot", []string{"guardians"}}, + {7, "no-groups", nil}, + } { + token := generateJWT(want.namespace, want.userId, want.groupIds, expiry) + + slow, err := validateToken(token) + require.NoError(t, err) + + fast, err := extractUserAndGroups(withPrincipal(token)) + require.NoError(t, err) + + require.Equal(t, slow, fast, "the fast path disagreed with validateToken") + require.Equal(t, want.namespace, fast.namespace) + require.Equal(t, want.userId, fast.userId) + require.Equal(t, want.groupIds, fast.groupIds) + + // And the fast path must actually have been taken, or this proves + // nothing about it. + _, ok := userDataFromPrincipal(withPrincipal(token)) + require.True(t, ok, "expected the fast path to apply") + } + }) + + t.Run("a token with no namespace claim still falls through and is rejected", func(t *testing.T) { + // aclAuthenticator ignores the namespace claim, so a Principal exists here. + // Taking the fast path would accept the request and let the caller keep + // whatever namespace metadata they sent, because aclTenantResolver tolerates + // a token it cannot extract one from. + claims := jwt.MapClaims{"userid": "sneaky", "exp": expiry, "groups": []string{"guardians"}} + token := jwt.NewWithClaims(jwt.SigningMethodHS256, &claims) + signed, err := token.SignedString(x.MaybeKeyToBytes(worker.Config.AclSecretKey)) + require.NoError(t, err) + + ctx := withPrincipal(signed) + require.NotNil(t, x.PrincipalFrom(ctx), "precondition: identity resolved") + + _, ok := userDataFromPrincipal(ctx) + require.False(t, ok, "the fast path must decline a token with no namespace claim") + + _, err = extractUserAndGroups(ctx) + require.Error(t, err, "the request must still be rejected") + require.Contains(t, err.Error(), "namespace in claims is not valid") + }) + + t.Run("an external issuer's groups never take the fast path", func(t *testing.T) { + // The escalation this guards: IsSuperAdmin matches on the group name, so an + // external issuer asserting `guardians` would otherwise become superadmin. + ctx := x.WithPrincipal(context.Background(), &x.Principal{ + Issuer: "https://idp.example/identity", + Subject: "some-agent", + Groups: []string{"guardians"}, + Claims: map[string]any{"namespace": float64(0)}, + Method: x.MethodExternalJWT, + }) + _, ok := userDataFromPrincipal(ctx) + require.False(t, ok, "only Dgraph's own token speaks for Dgraph's own groups") + }) + + t.Run("no principal falls through to the token", func(t *testing.T) { + token := generateJWT(42, "user", []string{"701"}, expiry) + ctx := metadata.NewIncomingContext(context.Background(), + metadata.Pairs("accessJwt", token)) + + _, ok := userDataFromPrincipal(ctx) + require.False(t, ok) + + ud, err := extractUserAndGroups(ctx) + require.NoError(t, err) + require.Equal(t, uint64(42), ud.namespace) + }) + + t.Run("an expired token is rejected on both routes", func(t *testing.T) { + token := generateJWT(1, "user", []string{"701"}, time.Now().Add(-time.Hour).Unix()) + + _, slowErr := validateToken(token) + require.Error(t, slowErr) + + ctx := withPrincipal(token) + require.Nil(t, x.PrincipalFrom(ctx), + "an expired token must not resolve to a Principal") + _, err := extractUserAndGroups(ctx) + require.Error(t, err) + }) + + t.Run("no credential at all is rejected", func(t *testing.T) { + _, err := extractUserAndGroups(x.WithResolvedIdentity(context.Background())) + require.Error(t, err) + }) +} diff --git a/edgraph/capability_test.go b/edgraph/capability_test.go new file mode 100644 index 00000000000..38f8aa5c555 --- /dev/null +++ b/edgraph/capability_test.go @@ -0,0 +1,828 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package edgraph + +import ( + "bytes" + "context" + "go/ast" + "go/parser" + "go/token" + "io/fs" + "net" + "os" + "path/filepath" + "slices" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" + "google.golang.org/grpc/status" + + "github.com/dgraph-io/dgo/v250/protos/api" + "github.com/dgraph-io/dgraph/v25/acl" + "github.com/dgraph-io/dgraph/v25/protos/pb" + "github.com/dgraph-io/dgraph/v25/worker" + "github.com/dgraph-io/dgraph/v25/x" +) + +// capabilitySites is every operation that requires a capability, and which one. +// +// This is a decision table, not a description. Each entry was AuthSuperAdmin or +// AuthorizeGuardians before capabilities existed, and the mapping is where the +// judgement lives: whether an operation is cluster-wide, tenant-scoped, or a +// cross-tenant write. Getting one wrong grants or removes real authority. +// +// TestEveryCapabilitySiteIsDeclared parses the source and requires this table to +// match, in both directions. A silent reclassification is the failure it exists to +// prevent — changing the constant at a call site is a one-word edit that reads as +// harmless in review, and the tests that would catch it are integration tests that +// only cover the paths someone thought to exercise. +var capabilitySites = map[string]Capability{ + // Cluster-wide authority. + "edgraph/server.go:alter": CapClusterAdmin, // drop-all + "edgraph/namespace.go:CreateNamespace": CapClusterAdmin, + "edgraph/namespace.go:DropNamespace": CapClusterAdmin, + "edgraph/namespace.go:ListNamespaces": CapClusterAdmin, + + "graphql/resolve/middlewares.go:resolveGuardianOfTheGalaxyAuth": CapClusterAdmin, + // Leasing UIDs. Not administration: `dgraph live` does it on every run, which is + // why it is not CapClusterAdmin. + "edgraph/zero.go:AllocateIDs": CapLeaseUIDs, + + // Acting in a namespace other than the caller's own. Distinct from + // CapClusterAdmin even though the built-in policy resolves them identically — + // the live loader needs a cross-tenant write and has no business dropping the + // cluster. + "edgraph/server.go:parseSchemaFromAlterOperation": CapAssumeTenant, + "edgraph/server.go:doQuery": CapAssumeTenant, + + // Administrative within a tenant: guardians of any namespace, which is what + // these checked before. + "edgraph/server.go:Health": CapTenantAdmin, + "edgraph/server.go:State": CapTenantAdmin, + "edgraph/server.go:UpdateExtSnapshotStreamingState": CapTenantAdmin, + "edgraph/server.go:StreamExtSnapshot": CapTenantAdmin, + "graphql/resolve/middlewares.go:resolveGuardianAuth": CapTenantAdmin, +} + +// capabilityScanRoot is the repo root, walked in full. +// +// This was a list of directories, which is how it came to miss +// a caller in a package nobody thought to add to the list. A decision table whose +// scan can miss a decision site is worse than no table: it reports a completeness it +// has not checked. Walking everything removes the class, and keeps working for a +// caller added in a package that does not exist yet. +const capabilityScanRoot = ".." + +// capabilityScanSkip are directories with nothing to say about capabilities and a lot +// of files, skipped to keep the walk quick. +var capabilityScanSkip = map[string]bool{ + ".git": true, "vendor": true, "testdata": true, "protos": true, + "compose": true, "contrib": true, ".trunk": true, "systest": true, +} + +// scanCapabilitySites parses the source and returns "dir/file.go:FuncName" for +// every AuthorizeCapability call, mapped to the capability constant it passes. +func scanCapabilitySites(t *testing.T) map[string]string { + t.Helper() + found := make(map[string]string) + var parsed int + + err := filepath.WalkDir(capabilityScanRoot, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + if capabilityScanSkip[d.Name()] { + return fs.SkipDir + } + return nil + } + if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + // Cheap pre-filter: parsing every Go file in the repo is wasteful when almost + // none of them mention the function. + src, readErr := os.ReadFile(path) + if readErr != nil { + return readErr + } + if !bytes.Contains(src, []byte("AuthorizeCapability")) { + return nil + } + parsed++ + + fset := token.NewFileSet() + af, parseErr := parser.ParseFile(fset, path, src, 0) + require.NoError(t, parseErr, "parsing %s", path) + + for _, decl := range af.Decls { + fd, ok := decl.(*ast.FuncDecl) + if !ok || fd.Body == nil { + continue + } + // The dispatcher and the interface method take a capability as a parameter + // rather than naming one, so they are not decision sites. + if fd.Name.Name == "AuthorizeCapability" { + continue + } + ast.Inspect(fd.Body, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok || callee(call.Fun) != "AuthorizeCapability" || len(call.Args) < 2 { + return true + } + key := sitePath(path) + ":" + fd.Name.Name + if prev, dup := found[key]; dup && prev != identName(call.Args[1]) { + t.Errorf("%s gates on two different capabilities (%s and %s); split the "+ + "function or the table cannot describe it", key, prev, identName(call.Args[1])) + } + found[key] = identName(call.Args[1]) + return true + }) + } + return nil + }) + require.NoError(t, err) + require.Positive(t, parsed, "the walk parsed no files mentioning AuthorizeCapability; "+ + "capabilityScanRoot is wrong and this test proves nothing") + return found +} + +func callee(fn ast.Expr) string { + switch f := fn.(type) { + case *ast.Ident: + return f.Name + case *ast.SelectorExpr: + return f.Sel.Name + } + return "" +} + +func identName(e ast.Expr) string { + switch a := e.(type) { + case *ast.Ident: + return a.Name + case *ast.SelectorExpr: + return a.Sel.Name + } + return "" +} + +// sitePath normalizes a walked path to be repo-relative, so the table reads the same +// regardless of where the test runs from. +func sitePath(path string) string { + clean := filepath.ToSlash(filepath.Clean(path)) + return strings.TrimPrefix(clean, "../") +} + +// capabilityNames maps the constant identifiers back to values, so the table can +// be written in terms of the constants while the scan sees only names. +var capabilityNames = map[string]Capability{ + "CapClusterAdmin": CapClusterAdmin, + "CapTenantAdmin": CapTenantAdmin, + "CapAssumeTenant": CapAssumeTenant, + "CapLeaseUIDs": CapLeaseUIDs, +} + +func TestEveryCapabilitySiteIsDeclared(t *testing.T) { + found := scanCapabilitySites(t) + + for site, gotName := range found { + want, declared := capabilitySites[site] + if !declared { + t.Errorf("%s gates on %s but is not in capabilitySites. Add it, and say why that "+ + "capability rather than another — the table is where the decision is recorded.", + site, gotName) + continue + } + got, known := capabilityNames[gotName] + if !known { + t.Errorf("%s passes %s, which is not a known capability constant", site, gotName) + continue + } + if got != want { + t.Errorf("%s gates on %v, but capabilitySites says %v. If the change is intended, "+ + "change the table too — that is the review the constant alone does not get.", + site, got, want) + } + } + + for site := range capabilitySites { + if _, ok := found[site]; !ok { + t.Errorf("capabilitySites declares %s, which no longer gates on a capability. "+ + "Either the check was dropped — which is a privilege change — or it moved and "+ + "the table needs updating.", site) + } + } +} + +// TestCapabilitySiteCoverage guards against the whole table being deleted or the +// scan silently matching nothing, which would make the test above vacuously pass. +func TestCapabilitySiteCoverage(t *testing.T) { + found := scanCapabilitySites(t) + require.NotEmpty(t, found, "the scan found no capability sites at all") + require.Len(t, found, len(capabilitySites)) + + // Every capability must be exercised somewhere, or it is dead surface. + var used []Capability + for _, c := range capabilitySites { + if !slices.Contains(used, c) { + used = append(used, c) + } + } + require.Len(t, used, len(capabilityNames), "some capability has no call site") +} + +// fromIP builds a context that looks like a gRPC request from ip, which is what +// the IP whitelist reads. Every test harness sets whitelist=0.0.0.0/0, so without +// constructing this by hand the deny half of break-glass is never exercised. +func fromIP(t *testing.T, ip string) context.Context { + t.Helper() + addr, err := net.ResolveTCPAddr("tcp", net.JoinHostPort(ip, "50051")) + require.NoError(t, err) + return peer.NewContext(context.Background(), &peer.Peer{Addr: addr}) +} + +// withAuthToken adds the --security token header a caller would present. +func withAuthToken(ctx context.Context, token string) context.Context { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + md = metadata.New(nil) + } else { + md = md.Copy() + } + md.Set("auth-token", token) + return metadata.NewIncomingContext(ctx, md) +} + +// aclCtx builds a request context carrying a signed ACL token, resolved to a +// Principal the way the identity interceptor does. +func aclCtx(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()) + ctx := metadata.NewIncomingContext(context.Background(), metadata.Pairs("accessJwt", token)) + return x.WithResolvedIdentity(ctx) +} + +// TestPredicateACLCapabilities pins the built-in policy's behavior, including the +// error codes and the message text. Both matter beyond this package: several +// endpoints wrap the message and clients distinguish Unauthenticated from +// PermissionDenied. +func TestPredicateACLCapabilities(t *testing.T) { + prevEnabled, prevSecret := x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey + t.Cleanup(func() { + x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey = prevEnabled, prevSecret + }) + + policy := predicateACL{} + + t.Run("with ACL off, cluster authority requires break-glass", func(t *testing.T) { + x.WorkerConfig.AclEnabled = false + worker.Config.AclSecretKey = nil + + // This is the behavior change. Before, an ACL-off cluster granted + // CapClusterAdmin to anyone, which is why CreateNamespace, DropNamespace, + // ListNamespaces, and AllocateIDs were reachable by any client that could + // open a connection. + err := policy.AuthorizeCapability(context.Background(), CapClusterAdmin) + require.Error(t, err, "an unattributable caller must not be a cluster admin") + require.Equal(t, codes.PermissionDenied, status.Code(err)) + require.Contains(t, err.Error(), "break-glass", + "the denial should name the route an operator can actually use") + + // A whitelisted source IP with no auth token configured is the default + // break-glass configuration, and it grants. + require.NoError(t, policy.AuthorizeCapability(fromIP(t, "127.0.0.1"), CapClusterAdmin)) + + // A source IP outside the whitelist does not, which is the half of the rule + // every test cluster hides: the harnesses all set whitelist=0.0.0.0/0. + err = policy.AuthorizeCapability(fromIP(t, "203.0.113.7"), CapClusterAdmin) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + + // The others are unchanged with ACL off, each for its own reason recorded at + // the switch in predicate_acl.go. Asserted here so that changing any of them + // is a deliberate act rather than a side effect. + require.NoError(t, policy.AuthorizeCapability(context.Background(), CapAssumeTenant), + "tightening this is the deferred fail-closed galaxy-operation change") + require.NoError(t, policy.AuthorizeCapability(context.Background(), CapTenantAdmin), + "tightening this would newly gate /state and /health?all") + require.NoError(t, policy.AuthorizeCapability(context.Background(), CapLeaseUIDs), + "`dgraph live` leases UIDs on every run against an ACL-off cluster and has "+ + "never needed a credential; requiring one is a loader-breaking change") + }) + + t.Run("with ACL off, the auth token is required when one is configured", func(t *testing.T) { + x.WorkerConfig.AclEnabled = false + worker.Config.AclSecretKey = nil + prevToken := worker.Config.AuthToken + t.Cleanup(func() { worker.Config.AuthToken = prevToken }) + worker.Config.AuthToken = "operator-token" + + // Whitelisted IP alone is no longer enough once a token exists. + err := policy.AuthorizeCapability(fromIP(t, "127.0.0.1"), CapClusterAdmin) + require.Error(t, err, "a configured auth token must be presented") + require.Equal(t, codes.PermissionDenied, status.Code(err)) + + require.NoError(t, policy.AuthorizeCapability( + withAuthToken(fromIP(t, "127.0.0.1"), "operator-token"), CapClusterAdmin)) + + err = policy.AuthorizeCapability( + withAuthToken(fromIP(t, "127.0.0.1"), "wrong-token"), CapClusterAdmin) + require.Error(t, err, "a wrong token must not grant") + }) + + t.Run("with ACL on, break-glass is not a second route to cluster admin", func(t *testing.T) { + x.WorkerConfig.AclEnabled = true + worker.Config.AclSecretKey = x.Sensitive("6ABBAA2014CFF00289D20D20DA296F67") + prevToken := worker.Config.AuthToken + t.Cleanup(func() { worker.Config.AuthToken = prevToken }) + worker.Config.AuthToken = "operator-token" + + // Satisfying break-glass completely, but presenting no ACL token. Granting + // here would let the --security token administer the cluster with no ACL + // identity at all, which is a loosening rather than the tightening this + // change is for. + ctx := withAuthToken(fromIP(t, "127.0.0.1"), "operator-token") + err := policy.AuthorizeCapability(ctx, CapClusterAdmin) + require.Error(t, err, "with ACL on, guardianship is the only route") + require.NotContains(t, err.Error(), "break-glass", + "the ACL-on denial should come from the ACL rule, not the source list") + }) + + x.WorkerConfig.AclEnabled = true + worker.Config.AclSecretKey = x.Sensitive("6ABBAA2014CFF00289D20D20DA296F67") + + tests := []struct { + name string + ctx func(t *testing.T) context.Context + cap Capability + wantCode codes.Code + wantMsg string + }{ + { + name: "a root-namespace guardian is a cluster admin", + ctx: func(t *testing.T) context.Context { return aclCtx(t, 0, "groot", []string{"guardians"}) }, + cap: CapClusterAdmin, + }, + { + name: "and may assume another tenant", + ctx: func(t *testing.T) context.Context { return aclCtx(t, 0, "groot", []string{"guardians"}) }, + cap: CapAssumeTenant, + }, + { + name: "and is a tenant admin", + ctx: func(t *testing.T) context.Context { return aclCtx(t, 0, "groot", []string{"guardians"}) }, + cap: CapTenantAdmin, + }, + { + name: "a guardian of another namespace is not a cluster admin", + ctx: func(t *testing.T) context.Context { return aclCtx(t, 7, "groot", []string{"guardians"}) }, + cap: CapClusterAdmin, + wantCode: codes.PermissionDenied, + wantMsg: "Only superadmin is allowed to do this operation", + }, + { + name: "nor may it assume another tenant", + ctx: func(t *testing.T) context.Context { return aclCtx(t, 7, "groot", []string{"guardians"}) }, + cap: CapAssumeTenant, + wantCode: codes.PermissionDenied, + wantMsg: "Only superadmin is allowed to do this operation", + }, + { + name: "but it is a tenant admin, which is the distinction", + ctx: func(t *testing.T) context.Context { return aclCtx(t, 7, "groot", []string{"guardians"}) }, + cap: CapTenantAdmin, + }, + { + name: "a non-guardian in the root namespace is denied", + ctx: func(t *testing.T) context.Context { return aclCtx(t, 0, "alice", []string{"dev"}) }, + cap: CapClusterAdmin, + wantCode: codes.PermissionDenied, + wantMsg: "is not a member of guardians group", + }, + { + name: "a non-guardian is not a tenant admin either", + ctx: func(t *testing.T) context.Context { return aclCtx(t, 0, "alice", []string{"dev"}) }, + cap: CapTenantAdmin, + wantCode: codes.PermissionDenied, + wantMsg: "is not a member of guardians group", + }, + { + name: "a caller with no groups at all is denied", + ctx: func(t *testing.T) context.Context { return aclCtx(t, 0, "alice", nil) }, + cap: CapClusterAdmin, + wantCode: codes.PermissionDenied, + wantMsg: "is not a member of guardians group", + }, + { + name: "no token is PermissionDenied for a tenant admin", + ctx: func(t *testing.T) context.Context { return context.Background() }, + cap: CapTenantAdmin, + wantCode: codes.PermissionDenied, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := policy.AuthorizeCapability(tt.ctx(t), tt.cap) + if tt.wantCode == codes.OK { + require.NoError(t, err) + return + } + require.Error(t, err) + require.Equal(t, tt.wantCode, status.Code(err), "err=%v", err) + if tt.wantMsg != "" { + require.Contains(t, err.Error(), tt.wantMsg) + } + }) + } +} + +// TestUnknownCapabilityFailsClosed covers the branch that has no case. A +// capability added without a rule must be denied, because the alternative reading +// leaves every future addition unrestricted until someone notices. +func TestUnknownCapabilityFailsClosed(t *testing.T) { + err := predicateACL{}.AuthorizeCapability(context.Background(), Capability(99)) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + require.Contains(t, err.Error(), "no rule for") + require.Equal(t, "unknown-capability", Capability(99).String()) +} + +// fakeController records what it was asked and grants everything. +type fakeController struct { + asked []Capability + askedPreds [][]string +} + +func (*fakeController) Name() string { return "fake" } + +func (f *fakeController) AuthorizeCapability(_ context.Context, c Capability) error { + f.asked = append(f.asked, c) + return nil +} + +func (f *fakeController) AuthorizePredicates(_ context.Context, preds []string, + _ *acl.Operation) (*PredResult, error) { + f.askedPreds = append(f.askedPreds, preds) + return &PredResult{Blocked: map[string]struct{}{}}, nil +} + +func TestSetAccessController(t *testing.T) { + prevEnabled := x.WorkerConfig.AclEnabled + t.Cleanup(func() { + x.WorkerConfig.AclEnabled = prevEnabled + SetAccessController(nil) + }) + // ACL on, so the built-in policy would deny an unauthenticated caller. If the + // installed one is consulted instead, the call succeeds. + x.WorkerConfig.AclEnabled = true + + require.Equal(t, "predicate-acl", currentAccessController().Name()) + + fake := &fakeController{} + SetAccessController(fake) + require.Equal(t, "fake", currentAccessController().Name()) + require.NoError(t, AuthorizeCapability(context.Background(), CapClusterAdmin)) + require.Equal(t, []Capability{CapClusterAdmin}, fake.asked) + + SetAccessController(nil) + require.Equal(t, "predicate-acl", currentAccessController().Name(), + "nil must restore the built-in policy") +} + +// TestCapabilityStringsAreStable guards the names that appear in the fail-closed +// error, which is what tells an operator which capability to grant. +func TestCapabilityStringsAreStable(t *testing.T) { + require.Equal(t, "cluster-admin", CapClusterAdmin.String()) + require.Equal(t, "tenant-admin", CapTenantAdmin.String()) + require.Equal(t, "assume-tenant", CapAssumeTenant.String()) +} + +// erroringController stands in for a policy that cannot reach an answer — a +// one backed by a remote authorization service whose query fails, say. +type erroringController struct{} + +func (erroringController) Name() string { return "erroring" } +func (erroringController) AuthorizeCapability(context.Context, Capability) error { + return status.Error(codes.Unavailable, "policy unavailable") +} +func (erroringController) AuthorizePredicates(context.Context, []string, + *acl.Operation) (*PredResult, error) { + return nil, status.Error(codes.Unavailable, "policy unavailable") +} + +// TestAuthorizePredicatesGoesThroughThePolicy checks the plumbing rather than +// ACL's rules — those are covered end to end by acl/acl_test.go against a live +// cluster, which is the gate for this change. +func TestAuthorizePredicatesGoesThroughThePolicy(t *testing.T) { + t.Cleanup(func() { SetAccessController(nil) }) + + fake := &fakeController{} + SetAccessController(fake) + + res, err := AuthorizePredicates(context.Background(), []string{"name", "age"}, acl.Read) + require.NoError(t, err) + require.NotNil(t, res) + require.Equal(t, [][]string{{"name", "age"}}, fake.askedPreds) + + SetAccessController(nil) + require.Equal(t, "predicate-acl", currentAccessController().Name()) +} + +// TestPredicateErrorIsNotReadAsPermitted is the property the new error return +// creates a way to get wrong. +// +// authorizePreds could not fail, so every call site treated "no blocked +// predicates" as the only outcome. A policy that has to query the graph can fail, +// and an empty PredResult from a failed call would look exactly like "nothing was +// refused" — which is to say, like permission. Each site must return the error +// instead. +func TestPredicateErrorIsNotReadAsPermitted(t *testing.T) { + prevEnabled, prevSecret := x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey + t.Cleanup(func() { + x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey = prevEnabled, prevSecret + SetAccessController(nil) + }) + x.WorkerConfig.AclEnabled = true + worker.Config.AclSecretKey = x.Sensitive("6ABBAA2014CFF00289D20D20DA296F67") + SetAccessController(erroringController{}) + + // A non-guardian, so the superadmin short-circuit does not fire and the + // predicate decision is actually consulted. + ctx := aclCtx(t, 0, "alice", []string{"dev"}) + + err := authorizeAlter(ctx, &api.Operation{Schema: "name: string ."}) + require.Error(t, err, "a policy that could not decide must not be read as permitting") + require.Equal(t, codes.Unavailable, status.Code(err), + "the policy's own code should survive rather than becoming PermissionDenied") +} + +// TestPredResultSentinel pins the meaning of a nil Allowed, which is the contract +// the interface had to carry over and the one a plain error return could not +// express. Nil means every predicate; an empty slice means none. +func TestPredResultSentinel(t *testing.T) { + all := &PredResult{Allowed: nil, Blocked: map[string]struct{}{"dgraph.xid": {}}} + require.Nil(t, all.Allowed, "nil Allowed is 'all predicates', and Blocked still applies") + require.Len(t, all.Blocked, 1) + + none := &PredResult{Allowed: []string{}, Blocked: map[string]struct{}{}} + require.NotNil(t, none.Allowed, "an empty slice is 'no predicates' and must not be nil") + require.Empty(t, none.Allowed) +} + +// TestClusterAdminDoesNotTrustClientNamespace is the regression test for a +// privilege escalation introduced by the tenancy rekeying. +// +// authSuperAdmin's `ns != 0` test asks whether the CALLER is rooted in the root +// namespace. That is a property of the credential, not of where the request is +// being routed — so it must read the signed claim. Rekeying it onto +// requestNamespace made it prefer x.ExtractNamespace, which reads md["namespace"] +// from incoming metadata, and that is client-controlled server-side. +// +// The four RPCs that gate on CapClusterAdmin and nothing else — CreateNamespace, +// DropNamespace, ListNamespaces, AllocateIDs — call AuthorizeCapability as their +// first statement, with no ResolveTenant before it. So on exactly those paths the +// metadata is whatever the caller sent, and a guardian of any tenant could claim +// namespace 0 and administer the cluster. +func TestClusterAdminDoesNotTrustClientNamespace(t *testing.T) { + prevEnabled, prevSecret := x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey + t.Cleanup(func() { + x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey = prevEnabled, prevSecret + }) + x.WorkerConfig.AclEnabled = true + worker.Config.AclSecretKey = x.Sensitive("6ABBAA2014CFF00289D20D20DA296F67") + + // A real guardian, but of namespace 7 — not a cluster admin. + token := generateJWT(7, "tenant-groot", []string{"guardians"}, time.Now().Add(30*time.Minute).Unix()) + + t.Run("a tenant guardian claiming namespace 0 in metadata is denied", func(t *testing.T) { + // The forgery: a valid ns-7 token, plus metadata asserting the request is in + // namespace 0. No ResolveTenant runs on this path to overwrite it. + ctx := metadata.NewIncomingContext(context.Background(), metadata.Pairs( + "accessJwt", token, + "namespace", "0", + )) + ctx = x.WithResolvedIdentity(ctx) + + err := predicateACL{}.AuthorizeCapability(ctx, CapClusterAdmin) + require.Error(t, err, "client-supplied namespace metadata must not confer cluster admin") + require.Equal(t, codes.PermissionDenied, status.Code(err)) + require.Contains(t, err.Error(), "Only superadmin is allowed to do this operation") + }) + + t.Run("the same token is still denied with no namespace metadata at all", func(t *testing.T) { + ctx := x.WithResolvedIdentity(metadata.NewIncomingContext( + context.Background(), metadata.Pairs("accessJwt", token))) + err := predicateACL{}.AuthorizeCapability(ctx, CapClusterAdmin) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + }) + + t.Run("a genuine root guardian is still allowed", func(t *testing.T) { + rootToken := generateJWT(0, "groot", []string{"guardians"}, time.Now().Add(30*time.Minute).Unix()) + ctx := x.WithResolvedIdentity(metadata.NewIncomingContext( + context.Background(), metadata.Pairs("accessJwt", rootToken))) + require.NoError(t, predicateACL{}.AuthorizeCapability(ctx, CapClusterAdmin)) + }) + + t.Run("a root guardian is allowed even if metadata claims another namespace", func(t *testing.T) { + // The mirror image: the claim governs, so hostile metadata cannot demote + // a legitimate cluster admin either. + rootToken := generateJWT(0, "groot", []string{"guardians"}, time.Now().Add(30*time.Minute).Unix()) + ctx := x.WithResolvedIdentity(metadata.NewIncomingContext(context.Background(), + metadata.Pairs("accessJwt", rootToken, "namespace", "7"))) + require.NoError(t, predicateACL{}.AuthorizeCapability(ctx, CapClusterAdmin)) + }) +} + +// TestFilterTabletsDoesNotTrustClientNamespace is the /state half of the same +// escalation. filterTablets returns early without filtering when the namespace is +// the root one, and State reaches it with no ResolveTenant ahead of it — so reading +// md["namespace"] let a guardian of any tenant ask for namespace 0 and receive +// every tenant's predicates. +func TestFilterTabletsDoesNotTrustClientNamespace(t *testing.T) { + prevEnabled := x.WorkerConfig.AclEnabled + t.Cleanup(func() { x.WorkerConfig.AclEnabled = prevEnabled }) + x.WorkerConfig.AclEnabled = true + + state := func() *pb.MembershipState { + return &pb.MembershipState{Groups: map[uint32]*pb.Group{1: {Tablets: map[string]*pb.Tablet{ + x.NamespaceAttr(7, "mine"): {Predicate: x.NamespaceAttr(7, "mine")}, + x.NamespaceAttr(9, "theirs"): {Predicate: x.NamespaceAttr(9, "theirs")}, + x.NamespaceAttr(0, "rootpred"): {Predicate: x.NamespaceAttr(0, "rootpred")}, + }}}} + } + token := generateJWT(7, "tenant-groot", []string{"guardians"}, time.Now().Add(30*time.Minute).Unix()) + + t.Run("claiming namespace 0 in metadata does not lift the filter", func(t *testing.T) { + ctx := metadata.NewIncomingContext(context.Background(), metadata.Pairs( + "accessJwt", token, "namespace", "0")) + ms := state() + require.NoError(t, filterTablets(ctx, ms)) + + tablets := ms.GetGroups()[1].GetTablets() + require.Contains(t, tablets, "mine", "the caller's own tenant is visible") + require.NotContains(t, tablets, "theirs", "another tenant's predicate must not be disclosed") + require.NotContains(t, tablets, "rootpred", "the root namespace's predicate must not be disclosed") + require.Len(t, tablets, 1) + }) + + t.Run("a genuine root guardian still sees everything", func(t *testing.T) { + rootToken := generateJWT(0, "groot", []string{"guardians"}, time.Now().Add(30*time.Minute).Unix()) + ctx := metadata.NewIncomingContext(context.Background(), + metadata.Pairs("accessJwt", rootToken)) + ms := state() + require.NoError(t, filterTablets(ctx, ms)) + require.Len(t, ms.GetGroups()[1].GetTablets(), 3, "root sees unfiltered state") + }) +} + +// identitySource is a stand-in for an identity-based CapabilitySource, e.g. +// --external-identity cluster-admin-clients. +type identitySource struct { + subject string + asked int +} + +func (*identitySource) Name() string { return "test-identity-source" } +func (s *identitySource) Grants(_ context.Context, p *x.Principal, c Capability) bool { + s.asked++ + return c == CapClusterAdmin && p != nil && p.Subject == s.subject +} + +// TestCapabilitySourcesAreConsultedUnderACL is the regression test for a silent +// misconfiguration: authorizeClusterAdmin returned authSuperAdmin's result directly +// whenever ACL was enabled, so no registered source was ever asked. A deployment +// that configured cluster-admin-clients AND ran ACL would have found the roster +// inert, with nothing in the logs or the error saying so. +func TestCapabilitySourcesAreConsultedUnderACL(t *testing.T) { + prevEnabled, prevSecret := x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey + prevSources := capabilitySources + t.Cleanup(func() { + x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey = prevEnabled, prevSecret + capabilitySources = prevSources + }) + x.WorkerConfig.AclEnabled = true + worker.Config.AclSecretKey = x.Sensitive("6ABBAA2014CFF00289D20D20DA296F67") + + src := &identitySource{subject: "migrator"} + capabilitySources = append(prevSources, src) + + admin := &x.Principal{ + Issuer: "https://idp.example/identity", + Subject: "migrator", + Claims: map[string]any{"user_type": "agent"}, + Method: x.MethodExternalJWT, + } + + t.Run("a source can grant even while ACL is enabled", func(t *testing.T) { + ctx := x.WithPrincipal(context.Background(), admin) + require.NoError(t, predicateACL{}.AuthorizeCapability(ctx, CapClusterAdmin)) + require.Positive(t, src.asked, "the source must actually be consulted") + }) + + t.Run("a caller no source recognizes still gets the ACL error", func(t *testing.T) { + // A real guardian of a non-root namespace, so the ACL rule produces its own + // PermissionDenied. The point is that consulting sources must not replace + // that message with the generic source-list one: an operator debugging an + // ACL deployment needs to be told about guardianship, not about + // --security whitelist. + token := generateJWT(7, "tenant-groot", []string{"guardians"}, + time.Now().Add(30*time.Minute).Unix()) + ctx := x.WithResolvedIdentity(metadata.NewIncomingContext( + context.Background(), metadata.Pairs("accessJwt", token))) + + err := predicateACL{}.AuthorizeCapability(ctx, CapClusterAdmin) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + require.Contains(t, err.Error(), "Only superadmin is allowed to do this operation", + "the ACL rule's own text must survive rather than the generic denial") + require.NotContains(t, err.Error(), "With ACL disabled it comes from") + }) + + t.Run("a source is not asked for a capability it does not grant", func(t *testing.T) { + ctx := x.WithPrincipal(context.Background(), admin) + // CapAssumeTenant has no source route; the ACL rule decides alone. + err := predicateACL{}.AuthorizeCapability(ctx, CapAssumeTenant) + require.Error(t, err, "naming a cluster-admin client must not confer assume-tenant") + }) +} + +// TestBreakGlassSelfExcludesUnderACL pins where the ACL-on restriction now lives. +// It moved from authorizeClusterAdmin into the source so that it applies to the one +// route it was reasoned about — a shared secret plus an IP range — rather than to +// every source anyone adds later. +func TestBreakGlassSelfExcludesUnderACL(t *testing.T) { + prevEnabled, prevToken := x.WorkerConfig.AclEnabled, worker.Config.AuthToken + t.Cleanup(func() { + x.WorkerConfig.AclEnabled, worker.Config.AuthToken = prevEnabled, prevToken + }) + worker.Config.AuthToken = "" + + // Fully satisfying break-glass: a loopback caller with no token configured. + ctx := fromIP(t, "127.0.0.1") + + x.WorkerConfig.AclEnabled = false + require.True(t, breakGlassSource{}.Grants(ctx, nil, CapClusterAdmin), + "with ACL off, break-glass is the route") + + x.WorkerConfig.AclEnabled = true + require.False(t, breakGlassSource{}.Grants(ctx, nil, CapClusterAdmin), + "with ACL on, the --security token must not be a second route to cluster admin") +} + +// TestClusterAdminWithoutATokenIsUnauthenticated pins the status code on the +// credential-missing path. +// +// authSuperAdmin wrapped ExtractNamespaceFrom's plain error, and every caller +// propagates status.Convert(err).Code() — so an absent or expired token on +// CreateNamespace, DropNamespace, ListNamespaces or the GraphQL admin surface arrived +// as codes.Unknown. A client cannot distinguish that from a server fault, and +// authorizeGuardians twelve lines below already classified the same failure as +// Unauthenticated. +func TestClusterAdminWithoutATokenIsUnauthenticated(t *testing.T) { + prevEnabled, prevSecret := x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey + t.Cleanup(func() { + x.WorkerConfig.AclEnabled, worker.Config.AclSecretKey = prevEnabled, prevSecret + }) + x.WorkerConfig.AclEnabled = true + worker.Config.AclSecretKey = x.Sensitive("6ABBAA2014CFF00289D20D20DA296F67") + + for _, tt := range []struct { + name string + ctx context.Context + }{ + {"no metadata at all", context.Background()}, + {"metadata but no token", metadata.NewIncomingContext( + context.Background(), metadata.Pairs("namespace", "0"))}, + {"a token that does not parse", metadata.NewIncomingContext( + context.Background(), metadata.Pairs("accessJwt", "not-a-jwt"))}, + } { + t.Run(tt.name, func(t *testing.T) { + err := predicateACL{}.AuthorizeCapability(tt.ctx, CapClusterAdmin) + require.Error(t, err) + require.Equal(t, codes.Unauthenticated, status.Code(err), + "a credential problem must not reach the client as Unknown") + }) + } + + t.Run("an expired token is also Unauthenticated", func(t *testing.T) { + expired := generateJWT(0, "groot", []string{"guardians"}, + time.Now().Add(-time.Hour).Unix()) + ctx := metadata.NewIncomingContext(context.Background(), + metadata.Pairs("accessJwt", expired)) + err := predicateACL{}.AuthorizeCapability(ctx, CapClusterAdmin) + require.Error(t, err) + require.Equal(t, codes.Unauthenticated, status.Code(err)) + }) +} diff --git a/edgraph/namespace.go b/edgraph/namespace.go index f206d5ff60c..68947a8080c 100644 --- a/edgraph/namespace.go +++ b/edgraph/namespace.go @@ -20,7 +20,7 @@ import ( func (s *Server) CreateNamespace(ctx context.Context, in *api.CreateNamespaceRequest) ( *api.CreateNamespaceResponse, error) { - if err := AuthSuperAdmin(ctx); err != nil { + if err := AuthorizeCapability(ctx, CapClusterAdmin); err != nil { s := status.Convert(err) return nil, status.Error(s.Code(), "Non superadmin user cannot create namespace. "+s.Message()) @@ -39,7 +39,7 @@ func (s *Server) CreateNamespace(ctx context.Context, in *api.CreateNamespaceReq func (s *Server) DropNamespace(ctx context.Context, in *api.DropNamespaceRequest) ( *api.DropNamespaceResponse, error) { - if err := AuthSuperAdmin(ctx); err != nil { + if err := AuthorizeCapability(ctx, CapClusterAdmin); err != nil { s := status.Convert(err) return nil, status.Error(s.Code(), "Non superadmin user cannot drop namespace. "+s.Message()) @@ -65,7 +65,7 @@ func (s *Server) DropNamespace(ctx context.Context, in *api.DropNamespaceRequest func (s *Server) ListNamespaces(ctx context.Context, in *api.ListNamespacesRequest) ( *api.ListNamespacesResponse, error) { - if err := AuthSuperAdmin(ctx); err != nil { + if err := AuthorizeCapability(ctx, CapClusterAdmin); err != nil { s := status.Convert(err) return nil, status.Error(s.Code(), "Non superadmin user cannot list namespaces. "+s.Message()) diff --git a/edgraph/predicate_acl.go b/edgraph/predicate_acl.go new file mode 100644 index 00000000000..63d57c1f69b --- /dev/null +++ b/edgraph/predicate_acl.go @@ -0,0 +1,314 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package edgraph + +import ( + "context" + "fmt" + + "github.com/golang/glog" + "github.com/pkg/errors" + "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" +) + +// predicateACL is the built-in policy: Dgraph's own ACL, and the only one an OSS +// build has. +type predicateACL struct{} + +func (predicateACL) Name() string { return "predicate-acl" } + +func (predicateACL) AuthorizeCapability(ctx context.Context, c Capability) error { + switch c { + case CapClusterAdmin: + return authorizeClusterAdmin(ctx) + case CapAssumeTenant: + // Deliberately still authSuperAdmin, which grants unconditionally when ACL + // is off. Tightening it is the fail-closed galaxy-operation change, and it + // has a cost the cluster-admin case does not: the live and bulk loaders use + // --force_namespace against clusters that may have no ACL, so they would + // need a whitelisted IP or an auth token from wherever they happen to run. + // That wants its own flag-gated release rather than riding along here. + // + // The other CapAssumeTenant site, parseSchemaFromAlterOperation, is reached + // only from alter, which already required break-glass before it got here — + // so the gap is the loader path specifically. + return authSuperAdmin(ctx) + case CapLeaseUIDs: + // authSuperAdmin unchanged, which means open when ACL is off. See the constant + // for why: `dgraph live` leases UIDs through this on every run against an + // ACL-off cluster, and it has never needed a credential to do so. + return authSuperAdmin(ctx) + case CapTenantAdmin: + // Also unchanged, and also not an oversight. Its sites are Health(all), + // State, and the external-snapshot pair; the snapshot pair already adds + // hasPoormansAuth of its own. Requiring break-glass for the first two would + // newly gate /state and /health?all behind the IP whitelist, which is a + // monitoring-visible change with a blast radius of its own. Separate + // decision, separate change. + return authorizeGuardians(ctx) + } + // Fail closed. A capability with no case here is a wiring bug, and reading an + // unhandled capability as "granted" would make every future addition + // unrestricted until someone noticed. + return status.Errorf(codes.PermissionDenied, + "%s: no rule for the %s capability", predicateACL{}.Name(), c) +} + +// authorizeClusterAdmin is the one capability whose ACL-off behavior changed. +// +// With ACL enabled, this is exactly the old rule: a guardian of the root +// namespace. Break-glass is deliberately NOT consulted in that case. ACL already +// provides an identity-based route to cluster authority, and adding a +// secret-and-IP route beside it would let a caller with the --security token +// administer the cluster with no ACL token at all. That is a loosening, and no +// goal here needs it. +// +// With ACL disabled, the old rule granted cluster authority to *everyone*. That +// is the hole: CreateNamespace, DropNamespace, and ListNamespaces gate on this +// capability and nothing else, so on an ACL-off cluster they were reachable by any +// client that could open a connection. Now the same controls alter already +// requires — a whitelisted source IP, plus the --security auth token when one is +// configured — are what grant it. +// +// AllocateIDs is deliberately not in that list. It gated on the same +// AuthSuperAdmin before the split and now gates on CapLeaseUIDs instead +// (Server.AllocateIDs in zero.go), which stays open with ACL off because +// `dgraph live` leases a UID block on every run and has never needed a credential +// to do it. +// +// This is a real behavior change and the only one in this stage. It is defensible +// unflagged because it makes namespace lifecycle no stricter than schema change: +// validateAlterOperation has always called hasAdminAuth, and that is the same +// HasWhitelistedIP-plus-hasPoormansAuth pair break-glass consults — so any +// deployment that alters a schema already has a working whitelist or token +// configuration, and the operations being closed are the more destructive ones. A +// deployment that genuinely wants the old behavior sets the same whitelist it +// already sets for alter. +func authorizeClusterAdmin(ctx context.Context) error { + if x.WorkerConfig.AclEnabled { + // Guardianship of the root namespace is the first route, and the only one an + // OSS build has. If it grants, nothing else needs asking. + aclErr := authSuperAdmin(ctx) + if aclErr == nil { + return nil + } + // Then the registered sources. Consulting them here rather than returning + // aclErr outright is the difference between a configured + // --external-identity cluster-admin-clients working and being silently + // inert on any cluster that also runs ACL. + // + // Which is not the same as re-admitting break-glass alongside ACL: that + // exclusion now lives in breakGlassSource.Grants, where it applies to the + // one source it was reasoned about rather than to every source there will + // ever be. + if name, ok := grantedBySource(ctx, CapClusterAdmin); ok { + glog.V(3).Infof("cluster-admin granted by %s", name) + return nil + } + // The ACL error, not a generic one: its code and message text are what + // callers and tests key on. + return aclErr + } + if name, ok := grantedBySource(ctx, CapClusterAdmin); ok { + glog.V(3).Infof("cluster-admin granted by %s", name) + return nil + } + return status.Errorf(codes.PermissionDenied, + "cluster-admin authority is required and this caller holds none. With ACL disabled it "+ + "comes from: %s. Configure --security whitelist and token, or enable --acl.", + sourceNames()) +} + +// breakGlassSource grants cluster authority to a caller that satisfies the +// operator-level controls rather than an identity: a whitelisted source IP, plus +// the --security auth token when one is configured. +// +// It is the recovery path, and it is why cluster authority never depends solely on +// an external identity provider. An outage there, a rotated key, or a bad config +// would otherwise lock an operator out of namespace lifecycle and drop-all. +type breakGlassSource struct{} + +func (breakGlassSource) Name() string { return "break-glass (--security whitelist/token)" } + +func (breakGlassSource) Grants(ctx context.Context, _ *x.Principal, c Capability) bool { + if c != CapClusterAdmin { + return false + } + // Not a second route while ACL is on. ACL already provides an identity-based + // path to cluster authority, and admitting a shared secret and an IP range + // beside it would let the --security token administer the cluster with no ACL + // token at all. Break-glass exists for the configuration that has no other + // path, which is ACL off. + if x.WorkerConfig.AclEnabled { + return false + } + // The two halves of hasAdminAuth, called directly rather than through it: that + // function logs every call at Info level, which is right for a rare admin RPC + // and wrong for a capability check that may run on any request. + if _, err := x.HasWhitelistedIP(ctx); err != nil { + return false + } + return hasPoormansAuth(ctx) == nil +} + +// authSuperAdmin authorizes a caller in the guardians group of the root namespace. +// It was edgraph.AuthSuperAdmin, called directly from eight places; it is now +// reached only through AuthorizeCapability. +// +// NOTE: the caller should not wrap the error returned. If needed, propagate the +// GRPC error code. +func authSuperAdmin(ctx context.Context) error { + if !x.WorkerConfig.AclEnabled { + return nil + } + // The signed claim, deliberately — not requestNamespace. + // + // This test asks whether the CALLER is rooted in the root namespace, which is a + // property of their credential, not of where the request is being routed. The + // rekeying that moved authorizePreds onto the resolved namespace was right for + // that site — it keys ACL lookups on the same channel storage keys on — and + // wrong here, because requestNamespace prefers x.ExtractNamespace, and that + // reads md["namespace"], which is client-controlled on the server side. + // + // The four RPCs gated on CapClusterAdmin and nothing else — CreateNamespace, + // DropNamespace, ListNamespaces, AllocateIDs — call AuthorizeCapability as their + // first statement, with no ResolveTenant before it to overwrite that metadata. + // So reading it there let a guardian of any tenant claim namespace 0 and + // administer the cluster, and let hostile metadata demote a real cluster admin. + ns, err := x.ExtractNamespaceFrom(ctx) + if err != nil { + // Unauthenticated, not a bare wrapped error. Every caller propagates + // status.Convert(err).Code(), and an unwrapped error converts to Unknown — so a + // missing or expired token on CreateNamespace, DropNamespace, ListNamespaces or + // the GraphQL admin surface looked like a server fault rather than a credential + // problem, and a client could not tell "log in again" from "retry later". + // authorizeGuardians below already classifies the same failure this way. + return status.Error(codes.Unauthenticated, + errors.Wrap(err, "Authorize guardian of the galaxy, extracting jwt token, error:").Error()) + } + if ns != 0 { + return status.Error( + codes.PermissionDenied, "Only superadmin is allowed to do this operation") + } + // authorizeGuardians will extract (user, []groups) from the JWT claims and will check if + // any of the group to which the user belongs is "guardians" or not. + if err := authorizeGuardians(ctx); err != nil { + s := status.Convert(err) + return status.Error( + s.Code(), "AuthSuperAdmin: failed to authorize guardians. "+s.Message()) + } + glog.V(3).Info("Successfully authorised guardian of the galaxy") + return nil +} + +// authorizeGuardians authorizes a caller in the guardians group of any namespace. +// It was edgraph.AuthorizeGuardians. +// +// NOTE: the caller should not wrap the error returned. If needed, propagate the +// GRPC error code. +func authorizeGuardians(ctx context.Context) error { + if worker.Config.AclSecretKey == nil { + // the user has not turned on the acl feature + return nil + } + + userData, err := extractUserAndGroups(ctx) + switch { + case errors.Is(err, x.ErrNoJwt): + return status.Error(codes.PermissionDenied, err.Error()) + case err != nil: + return status.Error(codes.Unauthenticated, err.Error()) + default: + userId := userData.userId + groupIds := userData.groupIds + + if !x.IsSuperAdmin(groupIds) { + // Deny access for members of non-guardian groups + return status.Error(codes.PermissionDenied, fmt.Sprintf("Only guardians are "+ + "allowed access. User '%v' is not a member of guardians group.", userId)) + } + } + + return nil +} + +// AuthorizePredicates is the built-in policy's predicate half: Dgraph's ACL +// rules, read from worker.AclCachePtr. +// +// It derives identity from ctx rather than taking it as a parameter, per the +// interface. The extra extractUserAndGroups is nearly free now that it reads the +// Principal the identity interceptor already resolved, and it keeps the signature +// free of ACL's own types. +func (predicateACL) AuthorizePredicates(ctx context.Context, preds []string, + op *acl.Operation) (*PredResult, error) { + + userData, err := extractUserAndGroups(ctx) + if err != nil { + return nil, status.Error(codes.Unauthenticated, err.Error()) + } + return authorizePreds(ctx, userData, preds, op), nil +} + +func authorizePreds(ctx context.Context, userData *userData, preds []string, + aclOp *acl.Operation) *PredResult { + + if !worker.AclCachePtr.Loaded() { + RefreshACLs(ctx) + } + + userId := userData.userId + groupIds := userData.groupIds + + // Key the ACL lookup on the namespace the storage layer will actually use. + // + // This used to read userData.namespace — the tenant claim inside the access + // token — while every read and write below keys on x.ExtractNamespace(ctx), + // the resolved tenancy. The two agree today only because the tenant resolver + // copies the claim into the context; they are separate channels, and + // authorizing against one while executing against the other is a + // confused-deputy bug waiting for the day a resolver derives the tenant from + // something other than that claim. + // + // Deliberately not a hard failure. If the context carries no resolved + // tenancy, fall back to the claim, which is what this read unconditionally + // before; a divergence is logged rather than rejected, so an unexpected one + // becomes visible without turning into an outage. + ns := namespaceOrClaim(ctx, userData.namespace, "authorizePreds") + + blockedPreds := make(map[string]struct{}) + for _, pred := range preds { + nsPred := x.NamespaceAttr(ns, pred) + if err := worker.AclCachePtr.AuthorizePredicate(groupIds, nsPred, aclOp); err != nil { + logAccess(&accessEntry{ + userId: userId, + groups: groupIds, + preds: preds, + operation: aclOp, + allowed: false, + }) + blockedPreds[pred] = struct{}{} + } + } + if worker.HasAccessToAllPreds(ns, groupIds, aclOp) { + // Setting allowed to nil allows access to all predicates. Note that the access to ACL + // predicates will still be blocked. + return &PredResult{Allowed: nil, Blocked: blockedPreds} + } + // User can have multiple permission for same predicate, add predicate + allowedPreds := make([]string, 0, len(worker.AclCachePtr.GetUserPredPerms(userId))) + // only if the acl.Op is covered in the set of permissions for the user + for predicate, perm := range worker.AclCachePtr.GetUserPredPerms(userId) { + if (perm & aclOp.Code) > 0 { + allowedPreds = append(allowedPreds, predicate) + } + } + return &PredResult{Allowed: allowedPreds, Blocked: blockedPreds} +} diff --git a/edgraph/query.go b/edgraph/query.go index 749ef899a08..7bd01552fcd 100644 --- a/edgraph/query.go +++ b/edgraph/query.go @@ -29,6 +29,11 @@ func (s *Server) RunDQL(ctx context.Context, req *api.RunDQLRequest) (*api.Respo apiReq.CommitNow = true } - ctx = x.AttachJWTNamespace(ctx) + // Entry point: see Server.Query. The namespace the client sent has no standing + // here, and the resolver leaves it in place when it cannot derive one. + ctx, err = x.ResolveTenant(x.ClearIncomingNamespace(ctx)) + if err != nil { + return nil, err + } return (&Server{}).doQuery(ctx, &Request{req: apiReq}) } diff --git a/edgraph/reserved_predicate_guard_test.go b/edgraph/reserved_predicate_guard_test.go new file mode 100644 index 00000000000..6666606f5a1 --- /dev/null +++ b/edgraph/reserved_predicate_guard_test.go @@ -0,0 +1,88 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package edgraph + +import ( + "context" + "testing" + + "github.com/dgraph-io/dgo/v250/protos/api" + "github.com/dgraph-io/dgraph/v25/x" + "github.com/stretchr/testify/require" +) + +type guardTrustKey struct{} + +// guardTestNamespace registers a synthetic reserved namespace covering both lock +// styles: one exact predicate and one dynamic prefix. Registration is global and +// panics on a duplicate, so it runs once. +func init() { + x.RegisterReservedNamespace(x.ReservedNamespace{ + PredicatePrefix: "dgraph.guardtest.rel.", + Predicates: []string{"dgraph.guardtest.xid", "dgraph.guardtest.cfg"}, + ValueLocked: []string{"dgraph.guardtest.cfg"}, + ValueLockedPrefixes: []string{"dgraph.guardtest.rel."}, + TrustMarker: guardTrustKey{}, + }) +} + +// TestReservedPredicateGuardValueLocks covers the mutation-path guard for both +// exact-name and prefix value locks. The prefix case is what protects predicates +// created dynamically at runtime, which cannot be enumerated up front — for +// an authorization store that is every stored grant, so an unguarded prefix means they +// are forgeable through plain /mutate. +func TestReservedPredicateGuardValueLocks(t *testing.T) { + nq := func(pred string) *api.NQuad { + return &api.NQuad{Subject: "0x1", Predicate: pred} + } + + t.Run("untrusted context is denied", func(t *testing.T) { + guard := newReservedPredicateGuard(context.Background()) + + for _, pred := range []string{ + "dgraph.guardtest.cfg", // exact lock + "dgraph.guardtest.rel.document.owner", // prefix lock + "dgraph.guardtest.rel.never_seen.yet", // prefix lock, predicate not yet created + "dgraph.guardtest.REL.Document.Owner", // case-insensitive + } { + require.Errorf(t, guard(nq(pred)), "value-locked predicate %q must be denied", pred) + } + }) + + t.Run("trusted context is allowed", func(t *testing.T) { + ctx := context.WithValue(context.Background(), guardTrustKey{}, true) + guard := newReservedPredicateGuard(ctx) + + // This is the case an owning service's write path depends on: its in-process + // client sets the marker on the context it builds, so + // locking the relation prefix must not break WriteRelationships. + for _, pred := range []string{ + "dgraph.guardtest.cfg", + "dgraph.guardtest.rel.document.owner", + "dgraph.guardtest.rel.never_seen.yet", + } { + require.NoErrorf(t, guard(nq(pred)), "trusted writer must be allowed to write %q", pred) + } + }) + + t.Run("a false marker is not trusted", func(t *testing.T) { + ctx := context.WithValue(context.Background(), guardTrustKey{}, false) + guard := newReservedPredicateGuard(ctx) + require.Error(t, guard(nq("dgraph.guardtest.rel.document.owner"))) + }) + + t.Run("unlocked and unowned predicates pass", func(t *testing.T) { + guard := newReservedPredicateGuard(context.Background()) + + for _, pred := range []string{ + "dgraph.guardtest.xid", // owned but deliberately not locked + "dgraph.guardtest.relx.evil", // near miss outside the locked prefix + "person.name", + } { + require.NoErrorf(t, guard(nq(pred)), "predicate %q must not be value-locked", pred) + } + }) +} diff --git a/edgraph/server.go b/edgraph/server.go index e5e9de7164c..8bcee62de2b 100644 --- a/edgraph/server.go +++ b/edgraph/server.go @@ -113,7 +113,11 @@ type existingGQLSchemaQryResp struct { // If multiple schema nodes were found, it returns an error. func GetGQLSchema(namespace uint64) (uid, graphQLSchema string, err error) { ctx := context.WithValue(context.Background(), Authorize, false) - ctx = x.AttachNamespace(ctx, namespace) + // Trusted attribution: this runs on a hand-built background context with no + // request credential to present, and it crosses the tenant seam via + // QueryNoGrpc. Marking it keeps ResolveTenant from trying to derive a + // namespace that was never going to be there. + ctx = x.AttachTrustedTenant(ctx, namespace) resp, err := (&Server{}).QueryNoGrpc(ctx, &api.Request{ Query: ` @@ -249,7 +253,7 @@ func parseSchemaFromAlterOperation(ctx context.Context, sch string) ( if x.IsRootNsOperation(ctx) { // Only the guardian of the galaxy can do a galaxy wide query/mutation. This operation is // needed by live loader. - if err := AuthSuperAdmin(ctx); err != nil { + if err := AuthorizeCapability(ctx, CapAssumeTenant); err != nil { s := status.Convert(err) return nil, status.Error(s.Code(), "Non superadmin user cannot bypass namespaces. "+s.Message()) @@ -353,7 +357,18 @@ func InsertDropRecord(ctx context.Context, dropOp string) error { // Alter handles requests to change the schema or remove parts or all of the // data. It enforces the admin-IP-whitelist and ACL authorization checks. func (s *Server) Alter(ctx context.Context, op *api.Operation) (*api.Payload, error) { - return s.alter(ctx, op, NeedAuthorize) + // Entry point: drop the namespace the client sent before resolving. + // + // md["namespace"] is client-controlled on the server side, and the built-in + // resolver leaves whatever is there when it cannot derive one from the access + // JWT — so an unattributable request would otherwise proceed on the tenant the + // caller named. Authorization currently catches that, because it reads the signed + // claim, but that is an invariant held by call-site ordering, and relying on + // ordering is what produced the escalations this guards against. + // + // Deliberately here and not in the shared continuation below: alter also serves AlterNoAuth, which the + // in-process callers reach with a context they have already attributed. + return s.alter(x.ClearIncomingNamespace(ctx), op, NeedAuthorize) } // AlterNoAuth is Alter without the admin-IP-whitelist and ACL authorization @@ -376,7 +391,10 @@ func (s *Server) alter(ctx context.Context, op *api.Operation, doAuth AuthMode) ctx, span := otel.Tracer("").Start(ctx, "Server.Alter") defer span.End() - ctx = x.AttachJWTNamespace(ctx) + ctx, err := x.ResolveTenant(ctx) + if err != nil { + return nil, err + } span.AddEvent("Alter operation", trace.WithAttributes(attribute.String("op", op.String()))) // Always print out Alter operations because they are important and rare. @@ -403,7 +421,7 @@ func (s *Server) alter(ctx context.Context, op *api.Operation, doAuth AuthMode) glog.V(2).Info("Blocked drop-all because it is not permitted.") return empty, errors.New("Drop all operation is not permitted.") } - if err := AuthSuperAdmin(ctx); err != nil { + if err := AuthorizeCapability(ctx, CapClusterAdmin); err != nil { s := status.Convert(err) return empty, status.Error(s.Code(), "Drop all can only be called by the guardian of the galaxy. "+s.Message()) @@ -1230,7 +1248,7 @@ func (s *Server) Health(ctx context.Context, all bool) (*api.Response, error) { var healthAll []pb.HealthInfo if all { - if err := AuthorizeGuardians(ctx); err != nil { + if err := AuthorizeCapability(ctx, CapTenantAdmin); err != nil { return nil, err } pool := conn.GetPools().GetAll() @@ -1270,6 +1288,10 @@ func filterTablets(ctx context.Context, ms *pb.MembershipState) error { if !x.WorkerConfig.AclEnabled { return nil } + // The signed claim, not the resolved namespace: this decides which tenant's + // tablets the CALLER may see, and State reaches here with no ResolveTenant + // ahead of it, so md["namespace"] would be whatever the caller sent. Reading it + // let a guardian of any tenant ask for namespace 0 and skip filtering entirely. namespace, err := x.ExtractNamespaceFrom(ctx) if err != nil { return errors.Errorf("Namespace not found in JWT.") @@ -1297,7 +1319,7 @@ func (s *Server) State(ctx context.Context) (*api.Response, error) { return nil, ctx.Err() } - if err := AuthorizeGuardians(ctx); err != nil { + if err := AuthorizeCapability(ctx, CapTenantAdmin); err != nil { return nil, err } @@ -1344,7 +1366,18 @@ func (s *Server) QueryGraphQL(ctx context.Context, req *api.Request, } func (s *Server) Query(ctx context.Context, req *api.Request) (*api.Response, error) { - resp, err := s.QueryNoGrpc(ctx, req) + // Entry point: drop the namespace the client sent before resolving. + // + // md["namespace"] is client-controlled on the server side, and the built-in + // resolver leaves whatever is there when it cannot derive one from the access + // JWT — so an unattributable request would otherwise proceed on the tenant the + // caller named. Authorization currently catches that, because it reads the signed + // claim, but that is an invariant held by call-site ordering, and relying on + // ordering is what produced the escalations this guards against. + // + // Deliberately here and not in the shared continuation below: QueryNoGrpc also serves the two HTTP + // handlers and GetGQLSchema, which attaches its own trusted tenancy. + resp, err := s.QueryNoGrpc(x.ClearIncomingNamespace(ctx), req) if err != nil { return resp, err } @@ -1357,7 +1390,10 @@ func (s *Server) Query(ctx context.Context, req *api.Request) (*api.Response, er // Query handles queries or mutations func (s *Server) QueryNoGrpc(ctx context.Context, req *api.Request) (*api.Response, error) { - ctx = x.AttachJWTNamespace(ctx) + ctx, err := x.ResolveTenant(ctx) + if err != nil { + return nil, err + } if x.WorkerConfig.AclEnabled && req.GetStartTs() != 0 { // A fresh StartTs is assigned if it is 0. ns, err := x.ExtractNamespace(ctx) @@ -1481,7 +1517,7 @@ func (s *Server) doQuery(ctx context.Context, req *Request) (resp *api.Response, 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. - if err := AuthSuperAdmin(ctx); err != nil { + if err := AuthorizeCapability(ctx, CapAssumeTenant); err != nil { s := status.Convert(err) return nil, status.Error(s.Code(), "Non superadmin user cannot bypass namespaces. "+s.Message()) @@ -2063,7 +2099,7 @@ func (s *Server) UpdateExtSnapshotStreamingState(ctx context.Context, // it is protected under ACL and under an --security auth-token. Each gate fails open when // its feature is unconfigured, so the arming requirement on the stream path backstops the // bare-OSS case. - if err := AuthorizeGuardians(ctx); err != nil { + if err := AuthorizeCapability(ctx, CapTenantAdmin); err != nil { return nil, err } if err := hasPoormansAuth(ctx); err != nil { @@ -2090,7 +2126,7 @@ func (s *Server) StreamExtSnapshot(stream api.Dgraph_StreamExtSnapshotServer) er // Authorize at stream start, before any data is consumed. Stream auth metadata rides on the // stream's context, so the same gates used for the unary entry point apply here. - if err := AuthorizeGuardians(stream.Context()); err != nil { + if err := AuthorizeCapability(stream.Context(), CapTenantAdmin); err != nil { return err } if err := hasPoormansAuth(stream.Context()); err != nil { @@ -2118,7 +2154,7 @@ func (s *Server) CommitOrAbort(ctx context.Context, tc *api.TxnContext) (*api.Tx return &api.TxnContext{}, errors.Errorf( "StartTs cannot be zero while committing a transaction") } - if ns, err := x.ExtractNamespaceFrom(ctx); err == nil { + if ns, err := x.ExtractNamespace(ctx); err == nil { annotateNamespace(span, ns) } annotateStartTs(span, tc.StartTs) diff --git a/edgraph/tenancy_entrypoint_test.go b/edgraph/tenancy_entrypoint_test.go new file mode 100644 index 00000000000..d192d9e11c4 --- /dev/null +++ b/edgraph/tenancy_entrypoint_test.go @@ -0,0 +1,117 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package edgraph + +import ( + "context" + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/metadata" + + "github.com/dgraph-io/dgo/v250/protos/api" + "github.com/dgraph-io/dgraph/v25/x" +) + +// errProbeRefused is what the probe resolver returns instead of attributing a +// request. Refusing is what keeps these unit tests: every entry point returns the +// resolver's error immediately, so none of them reach worker, posting, or storage. +var errProbeRefused = errors.New("probe resolver: refusing to attribute") + +// namespaceProbe records what an entry point handed the tenant resolver. +type namespaceProbe struct { + called bool + saw []string +} + +// installNamespaceProbe installs a fail-closed tenant resolver that captures the +// incoming namespace metadata it was given. +// +// The resolver is the seam the entry points already resolve through, so this needs +// no production change: whatever md["namespace"] the resolver observes is exactly +// what a deployment-specific resolver would observe in production, and a resolver +// that must not trust that value can only be safe if it never arrives. +func installNamespaceProbe(t *testing.T) *namespaceProbe { + t.Helper() + p := &namespaceProbe{} + x.SetTenantResolver(func(ctx context.Context) (context.Context, error) { + p.called = true + if md, ok := metadata.FromIncomingContext(ctx); ok { + p.saw = md.Get("namespace") + } + return ctx, errProbeRefused + }) + t.Cleanup(func() { x.SetTenantResolver(nil) }) + return p +} + +// spoofedCtx is a request naming namespace 9 while presenting no credential of any +// kind — the uncredentialed cross-tenant caller the guard exists to stop. +func spoofedCtx() context.Context { + return metadata.NewIncomingContext(context.Background(), + metadata.New(map[string]string{"namespace": "9"})) +} + +// assertGuarded is the whole assertion: the resolver ran, and it was not shown the +// caller's namespace. +func assertGuarded(t *testing.T, p *namespaceProbe, err error) { + t.Helper() + require.ErrorIs(t, err, errProbeRefused, + "the entry point must return the resolver's refusal, not continue past it") + require.True(t, p.called, + "the resolver was never reached; this entry point no longer resolves a tenant "+ + "and the test proves nothing") + require.Empty(t, p.saw, + "the resolver was handed the client's own namespace. On the server side "+ + "md[\"namespace\"] is client-controlled, and the built-in resolver leaves it in "+ + "place when it cannot derive one from a credential — so this is an "+ + "uncredentialed caller acting in a tenant of their choosing. Restore the "+ + "x.ClearIncomingNamespace call at this entry point.") +} + +// TestEntryPointsClearClientNamespace is the regression test for the entry-point +// guards. Deleting any x.ClearIncomingNamespace call left every unit and package +// test in the repo green, which is why these exist. +func TestEntryPointsClearClientNamespace(t *testing.T) { + const q = "{ q(func: uid(0x1)) { uid } }" + + t.Run("Server.Query", func(t *testing.T) { + p := installNamespaceProbe(t) + _, err := (&Server{}).Query(spoofedCtx(), &api.Request{Query: q}) + assertGuarded(t, p, err) + }) + + t.Run("Server.Alter", func(t *testing.T) { + p := installNamespaceProbe(t) + _, err := (&Server{}).Alter(spoofedCtx(), &api.Operation{Schema: "name: string ."}) + assertGuarded(t, p, err) + }) + + t.Run("Server.RunDQL", func(t *testing.T) { + p := installNamespaceProbe(t) + _, err := (&Server{}).RunDQL(spoofedCtx(), &api.RunDQLRequest{DqlQuery: q}) + assertGuarded(t, p, err) + }) +} + +// TestAttributedContinuationsKeepTheirNamespace is the other half of the invariant: +// the guard belongs at entry points only. A continuation that cleared the namespace +// would break the in-process callers that arrive already attributed and hold no +// credential to re-present. +func TestAttributedContinuationsKeepTheirNamespace(t *testing.T) { + p := installNamespaceProbe(t) + // AlterNoAuth is reached through alter() — the same continuation Alter guards + // above — with a context an in-process caller attributed itself. + ctx := metadata.NewIncomingContext(context.Background(), + metadata.New(map[string]string{"namespace": "9"})) + _, err := (&Server{}).AlterNoAuth(ctx, &api.Operation{Schema: "name: string ."}) + require.ErrorIs(t, err, errProbeRefused) + require.True(t, p.called) + require.Equal(t, []string{"9"}, p.saw, + "the guard has moved into the shared continuation: a trusted in-process caller's "+ + "namespace is being stripped") +} diff --git a/edgraph/zero.go b/edgraph/zero.go index 9552d50aae7..ebb749959fb 100644 --- a/edgraph/zero.go +++ b/edgraph/zero.go @@ -21,7 +21,7 @@ func (s *Server) AllocateIDs(ctx context.Context, req *api.AllocateIDsRequest) ( *api.AllocateIDsResponse, error) { // For now, we only allow users in superadmin group to do this operation in v25 - if err := AuthSuperAdmin(ctx); err != nil { + if err := AuthorizeCapability(ctx, CapLeaseUIDs); err != nil { s := status.Convert(err) return nil, status.Error(s.Code(), "v25.AllocateIDs can only be called by the superadmin group. "+s.Message()) diff --git a/graphql/admin/http.go b/graphql/admin/http.go index 680aa8f47a0..c71f07d5027 100644 --- a/graphql/admin/http.go +++ b/graphql/admin/http.go @@ -182,7 +182,10 @@ func (gs *graphqlSubscription) Subscribe( } audit.AuditWebSockets(ctx, req) - namespace := x.ExtractNamespaceHTTP(&http.Request{Header: reqHeader}) + namespace, err := x.ResolveTenantHTTP(&http.Request{Header: reqHeader}) + if err != nil { + return nil, err + } glog.V(2).Infof("namespace: %d. Got GraphQL request over HTTP.", namespace) // first load the schema, then do anything else if err := LazyLoadSchema(namespace); err != nil { @@ -242,11 +245,14 @@ func (gh *graphqlHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - // Pass in PoorMan's auth, ACL and IP information if present. - ctx = x.AttachAccessJwt(ctx, r) - ctx = x.AttachRemoteIP(ctx, r) - ctx = x.AttachAuthToken(ctx, r) - ctx = x.AttachJWTNamespace(ctx) + // Pass in PoorMan's auth, ACL and IP information if present, and resolve the + // caller's identity from it. + ctx = x.AttachRequestIdentity(ctx, r) + ctx, err := x.ResolveTenant(ctx) + if err != nil { + WriteErrorResponse(w, r, err) + return + } var res *schema.Response gqlReq, err := getRequest(r) diff --git a/graphql/resolve/middlewares.go b/graphql/resolve/middlewares.go index f4e40cb44a7..806c66bc6fe 100644 --- a/graphql/resolve/middlewares.go +++ b/graphql/resolve/middlewares.go @@ -114,7 +114,7 @@ func (mws MutationMiddlewares) Then(resolver MutationResolver) MutationResolver // resolveGuardianOfTheGalaxyAuth returns a Resolved with error if the context doesn't contain any // superadmin auth, otherwise it returns nil func resolveGuardianOfTheGalaxyAuth(ctx context.Context, f schema.Field) *Resolved { - if err := edgraph.AuthSuperAdmin(ctx); err != nil { + if err := edgraph.AuthorizeCapability(ctx, edgraph.CapClusterAdmin); err != nil { return EmptyResult(f, err) } return nil @@ -123,7 +123,7 @@ func resolveGuardianOfTheGalaxyAuth(ctx context.Context, f schema.Field) *Resolv // resolveGuardianAuth returns a Resolved with error if the context doesn't contain any Guardian auth, // otherwise it returns nil func resolveGuardianAuth(ctx context.Context, f schema.Field) *Resolved { - if err := edgraph.AuthorizeGuardians(ctx); err != nil { + if err := edgraph.AuthorizeCapability(ctx, edgraph.CapTenantAdmin); err != nil { return EmptyResult(f, err) } return nil diff --git a/graphql/resolve/resolver.go b/graphql/resolve/resolver.go index f973f87d3db..9be21b4af83 100644 --- a/graphql/resolve/resolver.go +++ b/graphql/resolve/resolver.go @@ -501,7 +501,12 @@ func (r *RequestResolver) Resolve(ctx context.Context, gqlReq *schema.Request) ( return } - ctx = x.AttachJWTNamespace(ctx) + ctx, err = x.ResolveTenant(ctx) + if err != nil { + resp.Errors = schema.AsGQLErrors(err) + return + } + op, err := r.schema.Operation(gqlReq) if err != nil { resp.Errors = schema.AsGQLErrors(err) diff --git a/query/expand_edges_test.go b/query/expand_edges_test.go new file mode 100644 index 00000000000..2817241e34e --- /dev/null +++ b/query/expand_edges_test.go @@ -0,0 +1,153 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package query + +import ( + "context" + "testing" + + "github.com/dgraph-io/dgraph/v25/protos/pb" + "github.com/dgraph-io/dgraph/v25/x" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/metadata" +) + +// TestExpandEdgesNamespaceSelection covers which namespace each expanded edge's +// predicate is qualified with. In a galaxy operation the caller puts the target +// namespace on each edge, so edges in one request may land in different +// namespaces; otherwise every edge takes the request's namespace and the +// per-edge value is ignored. +// +// Only the non-star path is exercised here: `S * *` expansion calls +// getNodeTypes, which needs worker plumbing. The namespace *plumbing* for that +// path is covered by TestExpandEdgesDerivesPerEdgeContext below. +func TestExpandEdgesNamespaceSelection(t *testing.T) { + edge := func(ns uint64, attr string) *pb.DirectedEdge { + return &pb.DirectedEdge{ + Entity: 1, + Attr: attr, + Namespace: ns, + Op: pb.DirectedEdge_SET, + } + } + + t.Run("non-galaxy ignores the per-edge namespace", func(t *testing.T) { + ctx := x.AttachNamespace(context.Background(), 7) + m := &pb.Mutations{Edges: []*pb.DirectedEdge{ + edge(0, "name"), + edge(9, "email"), // a stray per-edge namespace must not be honored + }} + + got, err := ExpandEdges(ctx, m) + require.NoError(t, err) + require.Len(t, got, 2) + require.Equal(t, x.NamespaceAttr(7, "name"), got[0].Attr) + require.Equal(t, x.NamespaceAttr(7, "email"), got[1].Attr) + }) + + t.Run("galaxy honors each edge's namespace independently", func(t *testing.T) { + // Set INCOMING metadata: x.AttachRootNsOperation is the client-side + // helper and writes outgoing metadata, while x.IsRootNsOperation reads + // incoming — i.e. what the server sees once the call crosses the wire. + ctx := metadata.NewIncomingContext(context.Background(), + metadata.Pairs("galaxy-operation", "true")) + ctx = x.AttachNamespace(ctx, x.RootNamespace) + m := &pb.Mutations{Edges: []*pb.DirectedEdge{ + edge(3, "name"), + edge(5, "email"), + edge(3, "phone"), + }} + + got, err := ExpandEdges(ctx, m) + require.NoError(t, err) + require.Len(t, got, 3) + // Each edge resolves independently — in particular edge 3 does not leak + // into edge 5, and edge 5 does not persist into the third edge. + require.Equal(t, x.NamespaceAttr(3, "name"), got[0].Attr) + require.Equal(t, x.NamespaceAttr(5, "email"), got[1].Attr) + require.Equal(t, x.NamespaceAttr(3, "phone"), got[2].Attr) + }) + + t.Run("reverse edges are dropped", func(t *testing.T) { + ctx := x.AttachNamespace(context.Background(), 0) + m := &pb.Mutations{Edges: []*pb.DirectedEdge{edge(0, "~friend")}} + + got, err := ExpandEdges(ctx, m) + require.NoError(t, err) + require.Empty(t, got) + }) + + t.Run("a missing namespace is an error, not a silent zero", func(t *testing.T) { + _, err := ExpandEdges(context.Background(), &pb.Mutations{ + Edges: []*pb.DirectedEdge{edge(0, "name")}, + }) + require.Error(t, err) + }) +} + +// TestExpandEdgesDerivesPerEdgeContext pins the fix for the discarded +// x.AttachNamespace return. ExpandEdges used to call it without using the +// returned context — both in the loop and in a deferred "reset" — so ctx was +// never actually re-namespaced. getNodeTypes then read dgraph.type from the +// request's namespace while the predicate list was built for the edge's, +// resolving a galaxy-mode `S * *` delete against the wrong schema. +// +// The assertion is on the mechanism rather than on getNodeTypes: a context +// derived for namespace N must report N, and the caller's context must be left +// alone. Against the previous implementation the derived-context assertion fails. +func TestExpandEdgesDerivesPerEdgeContext(t *testing.T) { + ctx := x.AttachNamespace(context.Background(), 7) + + derived := x.AttachNamespace(ctx, 3) + + gotDerived, err := x.ExtractNamespace(derived) + require.NoError(t, err) + require.Equal(t, uint64(3), gotDerived, "derived context must carry the edge's namespace") + + gotOriginal, err := x.ExtractNamespace(ctx) + require.NoError(t, err) + require.Equal(t, uint64(7), gotOriginal, "deriving must not mutate the caller's context") +} + +// TestExpandEdgesPassesTheEdgeNamespaceToGetNodeTypes is the regression test the +// original fix lacked. +// +// ExpandEdges discarded the context returned by x.AttachNamespace, so getNodeTypes +// read dgraph.type from the REQUEST's namespace while the predicate list was built +// for the EDGE's — a galaxy-mode `S * *` delete resolved its expansion against the +// wrong schema. Nothing observed that: getNodeTypes needs worker plumbing, so the +// test written alongside the fix asserted x.AttachNamespace's own return semantics +// instead and passed against the unfixed code. +func TestExpandEdgesPassesTheEdgeNamespaceToGetNodeTypes(t *testing.T) { + prev := nodeTypesForEdge + t.Cleanup(func() { nodeTypesForEdge = prev }) + + var seen []uint64 + nodeTypesForEdge = func(ctx context.Context, _ *SubGraph) ([]string, error) { + ns, err := x.ExtractNamespace(ctx) + require.NoError(t, err, "the per-edge context must carry a namespace") + seen = append(seen, ns) + return nil, nil + } + + // A galaxy-mode star delete against two different tenants in one mutation, which + // is the shape that made the bug observable: whichever namespace ctx happened to + // carry would have been used for both. + // Incoming metadata, which is what x.IsRootNsOperation reads on the server side. + ctx := metadata.NewIncomingContext(context.Background(), + metadata.Pairs("galaxy-operation", "true")) + ctx = x.AttachNamespace(ctx, x.RootNamespace) + _, err := ExpandEdges(ctx, &pb.Mutations{ + StartTs: 1, + Edges: []*pb.DirectedEdge{ + {Entity: 1, Attr: x.Star, Namespace: 7, Op: pb.DirectedEdge_DEL}, + {Entity: 2, Attr: x.Star, Namespace: 9, Op: pb.DirectedEdge_DEL}, + }, + }) + require.NoError(t, err) + require.Equal(t, []uint64{7, 9}, seen, + "each edge's own namespace must reach getNodeTypes, not the request's") +} diff --git a/query/mutation.go b/query/mutation.go index 3094988d43e..e854ab1ea3a 100644 --- a/query/mutation.go +++ b/query/mutation.go @@ -48,27 +48,38 @@ func ApplyMutations(ctx context.Context, m *pb.Mutations) (*api.TxnContext, erro return tctx, err } +// nodeTypesForEdge is getNodeTypes, indirected so a test can observe which +// namespace the per-edge context actually carries. +// +// That is the only thing the discarded-context fix below changed, and it is +// otherwise reachable only through worker.ProcessTaskOverNetwork — so without this +// the fix had no test that would fail if it were reverted, which was exactly the +// case when it landed. +var nodeTypesForEdge = getNodeTypes + func ExpandEdges(ctx context.Context, m *pb.Mutations) ([]*pb.DirectedEdge, error) { edges := make([]*pb.DirectedEdge, 0, 2*len(m.Edges)) - namespace, err := x.ExtractNamespace(ctx) + reqNamespace, err := x.ExtractNamespace(ctx) if err != nil { return nil, errors.Wrapf(err, "While expanding edges") } isGalaxyQuery := x.IsRootNsOperation(ctx) - // Reset the namespace to the original. - defer func(ns uint64) { - x.AttachNamespace(ctx, ns) - }(namespace) - for _, edge := range m.Edges { x.AssertTrue(edge.Op == pb.DirectedEdge_DEL || edge.Op == pb.DirectedEdge_SET) + + // For a galaxy operation the caller puts the target namespace on each + // edge, so an edge may target a namespace other than the request's. Derive + // a per-edge context rather than mutating ctx: x.AttachNamespace returns a + // new context, so the previous code — which discarded that return, both + // here and in a deferred "reset" — left ctx untouched and getNodeTypes + // below read dgraph.type from the REQUEST's namespace while the predicate + // list was built for the EDGE's. For a galaxy-mode `S * *` delete that + // resolves the expansion against the wrong schema. + namespace, edgeCtx := reqNamespace, ctx if isGalaxyQuery { - // The caller should make sure that the directed edges contain the namespace we want - // to insert into. Now, attach the namespace in the context, so that further query - // proceeds as if made from the user of 'namespace'. namespace = edge.GetNamespace() - x.AttachNamespace(ctx, namespace) + edgeCtx = x.AttachNamespace(ctx, namespace) } var preds []string @@ -78,7 +89,7 @@ func ExpandEdges(ctx context.Context, m *pb.Mutations) ([]*pb.DirectedEdge, erro sg := &SubGraph{} sg.DestUIDs = &pb.List{Uids: []uint64{edge.GetEntity()}} sg.ReadTs = m.StartTs - types, err := getNodeTypes(ctx, sg) + types, err := nodeTypesForEdge(edgeCtx, sg) if err != nil { return nil, err } diff --git a/systest/integration2/cluster_admin_test.go b/systest/integration2/cluster_admin_test.go new file mode 100644 index 00000000000..4c1e3fc5b46 --- /dev/null +++ b/systest/integration2/cluster_admin_test.go @@ -0,0 +1,154 @@ +//go:build integration2 + +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package main + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + + "github.com/dgraph-io/dgraph/v25/dgraphtest" +) + +// aclOffDenial is the message authorizeClusterAdmin produces when ACL is disabled and +// no capability source grants. Asserting on it is what stops these tests passing +// because the RPC failed for some unrelated reason — a bare PermissionDenied could +// come from anywhere. +const aclOffDenial = "cluster-admin authority is required and this caller holds none" + +// TestClusterAdminAclOffRequiresAuthToken covers the token half of break-glass. It is +// the test whose absence let the ACL-off behavior change ship unnoticed: before the +// change, an ACL-off cluster granted cluster authority to anyone who could open a +// connection, so the no-token CreateNamespace below succeeded. +// +// It needs no new harness capability — WithSecurityToken already exists — which is +// why it is the half that can be trusted first. +func TestClusterAdminAclOffRequiresAuthToken(t *testing.T) { + const token = "break-glass-token" + conf := dgraphtest.NewClusterConfig().WithNumAlphas(1).WithNumZeros(1).WithReplicas(1). + WithSecurityToken(token) + c, err := dgraphtest.NewLocalCluster(conf) + require.NoError(t, err) + defer func() { c.Cleanup(t.Failed()) }() + require.NoError(t, c.Start()) + + gc, cleanup, err := c.Client() + require.NoError(t, err) + defer cleanup() + + // The three RPCs that gate on CapClusterAdmin and nothing else. Deliberately + // gRPC, not the admin GraphQL surface: every admin GraphQL op carries + // IpWhitelistingMW and was already gated before the change, so it cannot + // distinguish the old rule from the new one. + _, err = gc.CreateNamespace(context.Background()) + require.Error(t, err, + "an ACL-off cluster must not create namespaces for an unauthenticated caller") + require.Equal(t, codes.PermissionDenied, status.Code(err)) + // Assert the reason, not just the code: this text comes only from + // authorizeClusterAdmin's ACL-off branch, so a PermissionDenied raised anywhere + // else cannot satisfy it. + require.ErrorContains(t, err, aclOffDenial) + + _, err = gc.ListNamespaces(context.Background()) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + + // Assert the reason here too. Namespace 1 does not exist on a fresh cluster, so a + // bare require.Error would pass on "no such namespace" just as readily as on a + // refusal, and prove nothing. + err = gc.DropNamespace(context.Background(), 1) + require.Error(t, err, + "dropping a namespace must not be reachable without a credential either") + require.ErrorContains(t, err, aclOffDenial) + + // The token is the half of break-glass this cluster can supply, and it grants. + adminCtx := metadata.AppendToOutgoingContext(context.Background(), "auth-token", token) + ns, err := gc.CreateNamespace(adminCtx) + require.NoError(t, err) + require.Greater(t, ns, uint64(0)) + + nsList, err := gc.ListNamespaces(adminCtx) + require.NoError(t, err) + require.Contains(t, nsList, ns) + require.NoError(t, gc.DropNamespace(adminCtx, ns)) +} + +// TestClusterAdminAclOffRequiresWhitelistedIP covers the whitelist half — the half no +// integration test could reach before WithWhitelist, because dgraphtest hard-coded +// whitelist=0.0.0.0/0 for every cluster it built. It is the integration-level twin of +// the fromIP("203.0.113.7") case in edgraph/capability_test.go. +// +// One assumption is load-bearing and is asserted rather than assumed: the alpha must +// see this caller's source address as neither loopback nor inside the whitelist, or +// the test would pass for the wrong reason. The control test below is what detects +// that — if the source address were somehow admitted here, it would be admitted there +// too, and both would agree. They cannot both be right. +func TestClusterAdminAclOffRequiresWhitelistedIP(t *testing.T) { + // TEST-NET-1 (RFC 5737). It cannot be the source address of a connection arriving + // through a published Docker port, so this cluster admits no remote admin caller. + conf := dgraphtest.NewClusterConfig().WithNumAlphas(1).WithNumZeros(1).WithReplicas(1). + WithWhitelist("192.0.2.0/24") + c, err := dgraphtest.NewLocalCluster(conf) + require.NoError(t, err) + defer func() { c.Cleanup(t.Failed()) }() + require.NoError(t, c.Start()) + + gc, cleanup, err := c.Client() + require.NoError(t, err) + defer cleanup() + + // No auth token is configured, so the source IP is the only half of break-glass in + // play, and it does not grant. + _, err = gc.CreateNamespace(context.Background()) + require.Error(t, err, "a non-whitelisted caller must not hold cluster authority") + require.Equal(t, codes.PermissionDenied, status.Code(err)) + require.ErrorContains(t, err, aclOffDenial) + + _, err = gc.ListNamespaces(context.Background()) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + + err = gc.DropNamespace(context.Background(), 1) + require.Error(t, err) + require.ErrorContains(t, err, aclOffDenial) +} + +// TestClusterAdminAclOffWhitelistedIPGrants is the control for the test above: same +// ACL-off, no-token configuration, whitelist that admits the caller, and now the same +// three RPCs succeed. The whitelist is spelled out rather than left defaulted so the +// pairing is readable in the diff. +// +// Without this control, TestClusterAdminAclOffRequiresWhitelistedIP could pass because +// the RPCs are broken for some unrelated reason rather than because the whitelist +// refused the caller. +func TestClusterAdminAclOffWhitelistedIPGrants(t *testing.T) { + conf := dgraphtest.NewClusterConfig().WithNumAlphas(1).WithNumZeros(1).WithReplicas(1). + WithWhitelist("0.0.0.0/0") + c, err := dgraphtest.NewLocalCluster(conf) + require.NoError(t, err) + defer func() { c.Cleanup(t.Failed()) }() + require.NoError(t, c.Start()) + + gc, cleanup, err := c.Client() + require.NoError(t, err) + defer cleanup() + + ns, err := gc.CreateNamespace(context.Background()) + require.NoError(t, err, + "a whitelisted caller with no token configured holds cluster authority") + require.Greater(t, ns, uint64(0)) + + nsList, err := gc.ListNamespaces(context.Background()) + require.NoError(t, err) + require.Contains(t, nsList, ns) + require.NoError(t, gc.DropNamespace(context.Background(), ns)) +} diff --git a/worker/graphql_schema.go b/worker/graphql_schema.go index f851796286d..ea93045012e 100644 --- a/worker/graphql_schema.go +++ b/worker/graphql_schema.go @@ -115,7 +115,28 @@ func (w *grpcWorker) UpdateGraphQLSchema(ctx context.Context, return nil, errUpdatingGraphQLSchemaOnNonGroupOneLeader } - ctx = x.AttachJWTNamespace(ctx) + // Re-resolve on the receiving alpha when the forwarded request carries a signed + // access JWT: UpdateGQLSchemaOverNetwork copies the caller's incoming metadata + // onward, and re-deriving from the signed token is stronger than trusting the + // forwarded namespace value. When there is no such token the forwarded value + // stands, which is the case the note below is about. + // + // Deliberately NOT clearing the forwarded namespace here, unlike the public gRPC + // entry points. This function is both a network handler and an in-process + // continuation: UpdateGQLSchemaOverNetwork calls it directly when this alpha is + // already the group-1 leader, passing the caller's context — which edgraph's + // UpdateGQLSchema has attributed, and which carries no separate credential to + // re-derive from. Clearing it broke exactly that path, and the live-loader suite + // caught it with "While updating gql schema: No namespace in the metadata". + // + // The weaker guarantee is also the honest one for this port: the internal worker + // listener has no auth interceptor at all, so a caller able to reach it is inside + // the trust boundary already and the boundary here is network-level rather than + // credential-level. + ctx, err := x.ResolveTenant(ctx) + if err != nil { + return nil, errors.Wrapf(err, "While updating gql schema") + } namespace, err := x.ExtractNamespace(ctx) if err != nil { return nil, errors.Wrapf(err, "While updating gql schema") diff --git a/worker/groups.go b/worker/groups.go index 87bbe52e903..48e9ec57d80 100644 --- a/worker/groups.go +++ b/worker/groups.go @@ -1099,6 +1099,12 @@ func GetFeaturesList() []string { var ee []string if Config.AclSecretKey != nil { ee = append(ee, "acl") + } + // Reported separately from ACL. These have historically been the same bit, + // which is already wrong for --limit shared-instance: that mode disables ACL + // for non-root namespaces while multi-tenancy stays on, yet still advertises + // both off the ACL key. + if x.MultiTenancyEnabled() { ee = append(ee, "multi_tenancy") } if x.WorkerConfig.EncryptionKey != nil { diff --git a/worker/zero_proxy.go b/worker/zero_proxy.go index c9ce5389da9..1c8b95cd082 100644 --- a/worker/zero_proxy.go +++ b/worker/zero_proxy.go @@ -21,12 +21,28 @@ func forwardAssignUidsToZero(ctx context.Context, in *pb.Num) (*pb.AssignedIds, return &pb.AssignedIds{}, errors.Errorf("Cannot lease %s via zero proxy", in.Type.String()) } - if x.WorkerConfig.AclEnabled { - var err error - ctx, err = x.AttachJWTNamespaceOutgoing(ctx) + // This is a gRPC entry point, not a continuation of an already-resolved + // request, so the tenant has to be derived here rather than read off the + // context. Resolving through the seam means an installed resolver governs this + // path too, instead of it being hard-wired to the ACL access JWT. + // + // The namespace the client sent is cleared first, and that is load-bearing. + // md["namespace"] is entirely client-controlled server-side, and the built-in + // resolver leaves it in place when it cannot derive a namespace from the access + // JWT. Reading it back would mean a caller who omits or corrupts their token + // leases UIDs in whatever tenant they asked for — which the pre-seam code + // rejected, because it derived the namespace from the signed JWT and returned + // that error. Clearing it makes an unattributable request fail here instead. + if x.MultiTenancyEnabled() { + rctx, err := x.ResolveTenant(x.ClearIncomingNamespace(ctx)) if err != nil { return &pb.AssignedIds{}, err } + ns, err := x.ExtractNamespace(rctx) + if err != nil { + return &pb.AssignedIds{}, err + } + ctx = x.AttachNamespaceOutgoing(ctx, ns) } pl := groups().Leader(0) diff --git a/worker/zero_proxy_tenancy_test.go b/worker/zero_proxy_tenancy_test.go new file mode 100644 index 00000000000..b74f61309c7 --- /dev/null +++ b/worker/zero_proxy_tenancy_test.go @@ -0,0 +1,61 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package worker + +import ( + "context" + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/metadata" + + "github.com/dgraph-io/dgraph/v25/protos/pb" + "github.com/dgraph-io/dgraph/v25/x" +) + +// TestForwardAssignUidsToZeroClearsClientNamespace is the regression test for the +// privilege escalation this guard fixes: uncredentialed cross-tenant UID leasing. +// +// forwardAssignUidsToZero has no other gate. Before the tenant-resolution seam it +// derived the namespace from the signed access JWT and returned that error on +// failure; after it, the built-in resolver's tolerate-a-bad-token branch left the +// client's own md["namespace"] in place and ExtractNamespace read it back. Deleting +// the x.ClearIncomingNamespace call left every package test in the repo green. +// +// The fail-closed probe resolver is what makes this a unit test: the refusal returns +// before groups().Leader(0), so no Zero connection is needed. +func TestForwardAssignUidsToZeroClearsClientNamespace(t *testing.T) { + errRefused := errors.New("probe resolver: refusing to attribute") + + var called bool + var saw []string + // Installing any resolver also makes x.MultiTenancyEnabled() true, which is what + // gates the block under test. + x.SetTenantResolver(func(ctx context.Context) (context.Context, error) { + called = true + if md, ok := metadata.FromIncomingContext(ctx); ok { + saw = md.Get("namespace") + } + return ctx, errRefused + }) + t.Cleanup(func() { x.SetTenantResolver(nil) }) + require.True(t, x.MultiTenancyEnabled(), "the guarded block would be skipped") + + // A caller asking to lease UIDs in namespace 9 with no credential at all. + ctx := metadata.NewIncomingContext(context.Background(), + metadata.New(map[string]string{"namespace": "9"})) + + _, err := forwardAssignUidsToZero(ctx, &pb.Num{Val: 10, Type: pb.Num_UID}) + require.ErrorIs(t, err, errRefused, + "an unattributable lease request must be refused here, not forwarded to Zero") + require.True(t, called, + "the resolver was never reached; this path no longer resolves a tenant") + require.Empty(t, saw, + "the resolver was handed the client's own namespace, so an uncredentialed caller "+ + "leases UIDs in whichever tenant they name. Restore the "+ + "x.ClearIncomingNamespace call in forwardAssignUidsToZero.") +} diff --git a/x/authn.go b/x/authn.go new file mode 100644 index 00000000000..9400f81714d --- /dev/null +++ b/x/authn.go @@ -0,0 +1,219 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package x + +import ( + "context" + "net/http" + "sync/atomic" + + "github.com/golang/glog" + "github.com/pkg/errors" + "google.golang.org/grpc" +) + +// Authenticator verifies the credential on a request and reports who is calling. +// +// It MUST NOT consult or produce tenancy — that is the TenantResolver's job — and +// it MUST NOT deny a request merely for lacking a credential. Returning +// (nil, nil) for "no credential presented" is the normal case for the endpoints +// that are unauthenticated by design. +type Authenticator interface { + // Name identifies the implementation, for logs and audit. + Name() string + // Authenticate returns the verified caller, (nil, nil) when the request + // carries no credential, or an error when it carries one that does not + // verify. + Authenticate(ctx context.Context) (*Principal, error) +} + +// authenticator holds the installed implementation, or nil for the built-in one. +// An atomic pointer for the same reason as tenantResolver: one word, read per +// request, written once at startup. +var authenticator atomic.Pointer[Authenticator] + +// SetAuthenticator installs a deployment-specific authenticator, replacing the +// built-in one that verifies Dgraph's own ACL access JWT. Call it during command +// setup, before any listener starts serving. Passing nil restores the built-in. +func SetAuthenticator(a Authenticator) { + if a == nil { + authenticator.Store(nil) + return + } + authenticator.Store(&a) +} + +// ACLAuthenticator returns the built-in authenticator, which verifies Dgraph's own +// ACL access token. +// +// 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. +func ACLAuthenticator() Authenticator { return aclAuthenticator{} } + +// 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 +// halves of the seam. +func currentAuthenticator() Authenticator { + if a := authenticator.Load(); a != nil { + return *a + } + return aclAuthenticator{} +} + +// WithResolvedIdentity authenticates the request if it carries a credential and +// returns a context carrying the resulting Principal. +// +// It never rejects. A request with no credential, or one whose credential does +// not verify, proceeds with no Principal attached, and the authorization layer +// rejects it exactly as it does today — with the accurate error and the right +// gRPC code. +// +// That contract is deliberate, and it is what removes the need for an +// unauthenticated-endpoint allow-list. Login, health checks, and CheckVersion +// cannot present a credential: Login is how one is obtained in the first place. +// A rejecting interceptor would need to enumerate them, which is a second policy +// engine sitting in front of the one that already knows which operations require +// authentication. Worse, an incomplete list fails closed on exactly the endpoint +// that would let you notice — the cluster stops being able to log in or report +// health. +// +// Verification failures are logged rather than returned, so a stale or malformed +// credential is still visible to an operator without being fatal here. +func WithResolvedIdentity(ctx context.Context) context.Context { + a := currentAuthenticator() + p, err := a.Authenticate(ctx) + switch { + case err != nil: + glog.V(2).Infof("identity: %s could not verify the presented credential: %v", a.Name(), err) + return ctx + case p == nil: + // No credential presented. Normal for unauthenticated endpoints. + return ctx + } + return WithPrincipal(ctx, p) +} + +// IdentityUnaryInterceptor resolves the caller's identity onto the context for +// every unary RPC. See WithResolvedIdentity for why it never rejects. +// +// Install it ahead of the audit interceptor so audit can record the resolved +// principal instead of parsing the credential a second time. +func IdentityUnaryInterceptor() grpc.UnaryServerInterceptor { + return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, + handler grpc.UnaryHandler) (any, error) { + return handler(WithResolvedIdentity(ctx), req) + } +} + +// IdentityStreamInterceptor is the streaming counterpart of +// IdentityUnaryInterceptor. +func IdentityStreamInterceptor() grpc.StreamServerInterceptor { + return func(srv any, ss grpc.ServerStream, info *grpc.StreamServerInfo, + handler grpc.StreamHandler) error { + return handler(srv, &identityServerStream{ServerStream: ss, ctx: WithResolvedIdentity(ss.Context())}) + } +} + +// identityServerStream overrides Context so the handler sees the resolved +// identity. grpc.ServerStream has no setter for its context. +type identityServerStream struct { + grpc.ServerStream + ctx context.Context +} + +func (s *identityServerStream) Context() context.Context { return s.ctx } + +// AttachRequestIdentity performs the standard HTTP-edge prelude: it moves the +// access token, remote IP, and auth token from the request into gRPC metadata, +// then resolves the caller's identity. It replaces the four-line sequence that +// was duplicated at every HTTP handler. +func AttachRequestIdentity(ctx context.Context, r *http.Request) context.Context { + ctx = AttachAccessJwt(ctx, r) + ctx = AttachRemoteIP(ctx, r) + ctx = AttachAuthToken(ctx, r) + return WithResolvedIdentity(ctx) +} + +// aclAuthenticator is the built-in authenticator, and the only one an OSS build +// has. It verifies Dgraph's own ACL access JWT. +// +// It is edgraph.validateToken minus the namespace claim, which is deliberate: +// identity and tenancy are read by different components now, and the namespace +// belongs to the TenantResolver. A token with no namespace claim still yields a +// valid identity here; the resolver decides separately what that means for +// tenancy. +type aclAuthenticator struct{} + +func (aclAuthenticator) Name() string { return "acl" } + +func (aclAuthenticator) Authenticate(ctx context.Context) (*Principal, error) { + if !WorkerConfig.AclEnabled { + // No ACL configured: there is no credential to verify and no identity to + // report. Authorization fails open in this configuration, as it does today. + return nil, nil + } + + jwtStr, err := ExtractJwt(ctx) + if err != nil { + // ErrNoJwt means no credential presented, which is not a failure. + if errors.Is(err, ErrNoJwt) { + return nil, nil + } + return nil, err + } + + claims, err := ParseJWT(jwtStr) + if err != nil { + return nil, err + } + // ParseJWT already rejects an expired token; this additionally requires the + // claim to be present, because MapClaims treats a missing exp as valid. + // Mirrors edgraph.validateToken. + if exp, expErr := claims.GetExpirationTime(); expErr != nil || exp == nil { + return nil, errors.Errorf("Token is expired") + } + + userID, ok := claims["userid"].(string) + if !ok { + return nil, errors.Errorf("userid in claims is not a string:%v", claims["userid"]) + } + + groups, err := groupsFromClaims(claims["groups"]) + if err != nil { + return nil, err + } + + return &Principal{ + Issuer: "dgraph-acl", + Subject: userID, + Groups: groups, + Claims: claims, + Method: MethodACL, + }, nil +} + +// groupsFromClaims converts the `groups` claim to a string slice. An absent claim +// is not an error — a user may belong to no groups — but a non-string member is, +// matching edgraph.validateToken. +func groupsFromClaims(claim any) ([]string, error) { + raw, ok := claim.([]interface{}) + if !ok { + return nil, nil + } + groups := make([]string, 0, len(raw)) + for _, g := range raw { + s, ok := g.(string) + if !ok { + return nil, errors.Errorf("unable to convert group to string:%v", g) + } + groups = append(groups, s) + } + return groups, nil +} diff --git a/x/authn_test.go b/x/authn_test.go new file mode 100644 index 00000000000..5abc355507d --- /dev/null +++ b/x/authn_test.go @@ -0,0 +1,317 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package x + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/metadata" +) + +// oracleUserData mirrors edgraph.userData, and oracleValidateToken mirrors +// edgraph.validateToken's identity extraction verbatim. aclAuthenticator +// duplicates that logic in package x, so the duplicate is held against a copy of +// the original rather than against remembered behavior — the same discipline the +// tenant seam used. +// +// The one deliberate divergence is the namespace claim: validateToken requires it +// and aclAuthenticator ignores it, because tenancy is the resolver's concern now. +// That divergence is asserted explicitly below rather than left implicit. +type oracleUserData struct { + userId string + groupIds []string +} + +func oracleValidateToken(jwtStr string) (*oracleUserData, error) { + claims, err := ParseJWT(jwtStr) + if err != nil { + return nil, err + } + if exp, err := claims.GetExpirationTime(); err != nil || exp == nil { + return nil, errors.New("Token is expired") + } + userId, ok := claims["userid"].(string) + if !ok { + return nil, errors.New("userid in claims is not a string") + } + groups, ok := claims["groups"].([]interface{}) + var groupIds []string + if ok { + groupIds = make([]string, 0, len(groups)) + for _, group := range groups { + groupId, ok := group.(string) + if !ok { + return nil, errors.New("unable to convert group to string") + } + groupIds = append(groupIds, groupId) + } + } + return &oracleUserData{userId: userId, groupIds: groupIds}, nil +} + +func ctxWithToken(token string) context.Context { + md := metadata.New(nil) + if token != "" { + md.Set("accessJwt", token) + } + return metadata.NewIncomingContext(context.Background(), md) +} + +// TestACLAuthenticatorMatchesValidateToken pins that the identity extraction +// moved into x agrees with edgraph.validateToken on every input either accepts +// or rejects. +func TestACLAuthenticatorMatchesValidateToken(t *testing.T) { + cases := []struct { + name string + claims jwt.MapClaims + }{ + {"userid and groups", jwt.MapClaims{ + "userid": "alice", "groups": []string{"dev", "ops"}, + "namespace": float64(5), "exp": float64(1 << 40)}}, + {"userid, no groups", jwt.MapClaims{ + "userid": "groot", "namespace": float64(0), "exp": float64(1 << 40)}}, + {"empty groups list", jwt.MapClaims{ + "userid": "bob", "groups": []string{}, + "namespace": float64(0), "exp": float64(1 << 40)}}, + {"guardians member", jwt.MapClaims{ + "userid": "groot", "groups": []string{"guardians"}, + "namespace": float64(0), "exp": float64(1 << 40)}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + withACL(t, true) + tok := tokenWith(t, tc.claims) + + want, wantErr := oracleValidateToken(tok) + got, gotErr := aclAuthenticator{}.Authenticate(ctxWithToken(tok)) + + require.NoError(t, wantErr) + require.NoError(t, gotErr) + require.NotNil(t, got) + require.Equal(t, want.userId, got.Subject, "subject must match validateToken's userId") + require.Equal(t, want.groupIds, got.Groups, "groups must match validateToken's groupIds") + require.Equal(t, "acl", got.Method) + require.Equal(t, "dgraph-acl", got.Issuer) + }) + } +} + +// TestACLAuthenticatorRejectsBadCredentials covers the inputs that must produce +// an error — a credential was presented and it did not verify. Each is also +// rejected by the oracle, so the two agree on failure as well as success. +func TestACLAuthenticatorRejectsBadCredentials(t *testing.T) { + withACL(t, true) + + expired := tokenWith(t, jwt.MapClaims{ + "userid": "alice", "namespace": float64(0), "exp": float64(1)}) // 1970 + noExp := tokenWith(t, jwt.MapClaims{"userid": "alice", "namespace": float64(0)}) + wrongKey, err := jwt.NewWithClaims(jwt.SigningMethodHS256, + jwt.MapClaims{"userid": "alice", "exp": float64(1 << 40)}). + SignedString([]byte("a-different-32-byte-hmac-key!!!!!")) + require.NoError(t, err) + + for name, tok := range map[string]string{ + "expired": expired, + "no exp claim": noExp, + "signed with wrong key": wrongKey, + "malformed": "not.a.jwt", + "userid is not a string": tokenWith(t, jwt.MapClaims{"userid": 42, "exp": float64(1 << 40)}), + "group is not a string": tokenWith(t, jwt.MapClaims{ + "userid": "alice", "groups": []any{"dev", 7}, "exp": float64(1 << 40)}), + } { + t.Run(name, func(t *testing.T) { + _, oracleErr := oracleValidateToken(tok) + require.Error(t, oracleErr, "oracle must also reject this, or the comparison is meaningless") + + p, err := aclAuthenticator{}.Authenticate(ctxWithToken(tok)) + require.Error(t, err) + require.Nil(t, p) + }) + } +} + +// TestACLAuthenticatorNoCredential covers the case that must NOT be an error. +// Login, health checks, and CheckVersion present nothing, and returning an error +// for them is what makes a rejecting interceptor deadlock a cluster. +func TestACLAuthenticatorNoCredential(t *testing.T) { + t.Run("acl on, no token", func(t *testing.T) { + withACL(t, true) + p, err := aclAuthenticator{}.Authenticate(ctxWithToken("")) + require.NoError(t, err, "an absent credential is not a verification failure") + require.Nil(t, p) + }) + + t.Run("acl on, no metadata at all", func(t *testing.T) { + withACL(t, true) + p, err := aclAuthenticator{}.Authenticate(context.Background()) + require.NoError(t, err) + require.Nil(t, p) + }) + + t.Run("acl off", func(t *testing.T) { + withACL(t, false) + p, err := aclAuthenticator{}.Authenticate(ctxWithToken(tokenWith(t, jwt.MapClaims{ + "userid": "alice", "exp": float64(1 << 40)}))) + require.NoError(t, err) + require.Nil(t, p, "no ACL configured means no identity to report") + }) +} + +// TestACLAuthenticatorIgnoresNamespaceClaim pins the one deliberate divergence +// from validateToken. Tenancy is the resolver's concern, so a token with no +// namespace claim still authenticates — where validateToken would reject it. +func TestACLAuthenticatorIgnoresNamespaceClaim(t *testing.T) { + withACL(t, true) + tok := tokenWith(t, jwt.MapClaims{"userid": "alice", "exp": float64(1 << 40)}) // no namespace + + _, oracleErr := oracleValidateToken(tok) + require.NoError(t, oracleErr, "the oracle checks identity only; namespace is checked separately") + + p, err := aclAuthenticator{}.Authenticate(ctxWithToken(tok)) + require.NoError(t, err) + require.NotNil(t, p) + require.Equal(t, "alice", p.Subject) +} + +// TestWithResolvedIdentityNeverRejects is the load-bearing property. Every input +// that could fail must yield a usable context, because the interceptor has no way +// to signal rejection and must not acquire one. +func TestWithResolvedIdentityNeverRejects(t *testing.T) { + withACL(t, true) + + for name, tok := range map[string]string{ + "no token": "", + "malformed": "not.a.jwt", + "expired": tokenWith(t, jwt.MapClaims{"userid": "alice", "exp": float64(1)}), + "userid is not a string": tokenWith(t, jwt.MapClaims{"userid": 42, "exp": float64(1 << 40)}), + } { + t.Run(name, func(t *testing.T) { + ctx := WithResolvedIdentity(ctxWithToken(tok)) + require.NotNil(t, ctx) + require.Nil(t, PrincipalFrom(ctx), "a failed or absent credential attaches no Principal") + }) + } + + t.Run("a valid token attaches the principal", func(t *testing.T) { + ctx := WithResolvedIdentity(ctxWithToken(tokenWith(t, jwt.MapClaims{ + "userid": "alice", "groups": []string{"dev"}, "exp": float64(1 << 40)}))) + p := PrincipalFrom(ctx) + require.NotNil(t, p) + require.Equal(t, "alice", p.Subject) + require.Equal(t, []string{"dev"}, p.Groups) + }) + + t.Run("an authenticator that errors does not reject", func(t *testing.T) { + t.Cleanup(func() { SetAuthenticator(nil) }) + SetAuthenticator(erroringAuthenticator{}) + ctx := WithResolvedIdentity(context.Background()) + require.NotNil(t, ctx) + require.Nil(t, PrincipalFrom(ctx)) + }) +} + +type erroringAuthenticator struct{} + +func (erroringAuthenticator) Name() string { return "erroring" } +func (erroringAuthenticator) Authenticate(context.Context) (*Principal, error) { + return nil, errors.New("issuer unreachable") +} + +// TestSetAuthenticator covers installation and restoration of the built-in. +func TestSetAuthenticator(t *testing.T) { + t.Cleanup(func() { SetAuthenticator(nil) }) + withACL(t, true) + + SetAuthenticator(fixedAuthenticator{subject: "service-account"}) + ctx := WithResolvedIdentity(context.Background()) + p := PrincipalFrom(ctx) + require.NotNil(t, p) + require.Equal(t, "service-account", p.Subject) + require.Equal(t, "fixed", p.Method) + + // nil restores the built-in, which reports nothing without a credential. + SetAuthenticator(nil) + require.Nil(t, PrincipalFrom(WithResolvedIdentity(context.Background()))) +} + +type fixedAuthenticator struct{ subject string } + +func (fixedAuthenticator) Name() string { return "fixed" } +func (f fixedAuthenticator) Authenticate(context.Context) (*Principal, error) { + return &Principal{Subject: f.subject, Method: "fixed"}, nil +} + +// TestIdentityInterceptors confirms both interceptors attach the identity and +// pass the call through, including that the streaming one overrides the stream's +// context rather than silently dropping the principal. +func TestIdentityInterceptors(t *testing.T) { + withACL(t, true) + tok := tokenWith(t, jwt.MapClaims{"userid": "alice", "exp": float64(1 << 40)}) + + t.Run("unary", func(t *testing.T) { + var seen *Principal + intc := IdentityUnaryInterceptor() + _, err := intc(ctxWithToken(tok), "req", &grpc.UnaryServerInfo{FullMethod: "/api.Dgraph/Query"}, + func(ctx context.Context, _ any) (any, error) { + seen = PrincipalFrom(ctx) + return nil, nil + }) + require.NoError(t, err) + require.NotNil(t, seen) + require.Equal(t, "alice", seen.Subject) + }) + + t.Run("stream", func(t *testing.T) { + var seen *Principal + intc := IdentityStreamInterceptor() + err := intc(nil, fakeServerStream{ctx: ctxWithToken(tok)}, + &grpc.StreamServerInfo{FullMethod: "/api.Dgraph/StreamExtSnapshot"}, + func(_ any, ss grpc.ServerStream) error { + seen = PrincipalFrom(ss.Context()) + return nil + }) + require.NoError(t, err) + require.NotNil(t, seen, "the stream's context must carry the principal") + require.Equal(t, "alice", seen.Subject) + }) +} + +type fakeServerStream struct { + grpc.ServerStream + ctx context.Context +} + +func (s fakeServerStream) Context() context.Context { return s.ctx } + +// TestAttachRequestIdentity covers the HTTP-edge helper that replaces the +// duplicated prelude, including that it preserves the metadata the individual +// Attach* calls used to set. +func TestAttachRequestIdentity(t *testing.T) { + withACL(t, true) + tok := tokenWith(t, jwt.MapClaims{"userid": "alice", "exp": float64(1 << 40)}) + + r := &http.Request{Header: http.Header{}, RemoteAddr: "10.0.0.7:4242"} + r.Header.Set("X-Dgraph-AccessToken", tok) + r.Header.Set("X-Dgraph-AuthToken", "poor-mans") + + ctx := AttachRequestIdentity(context.Background(), r) + + p := PrincipalFrom(ctx) + require.NotNil(t, p) + require.Equal(t, "alice", p.Subject) + + md, ok := metadata.FromIncomingContext(ctx) + require.True(t, ok) + require.Equal(t, []string{tok}, md.Get("accessJwt")) + require.Equal(t, []string{"poor-mans"}, md.Get("auth-token")) +} diff --git a/x/keys.go b/x/keys.go index 84df6399453..09757abbe43 100644 --- a/x/keys.go +++ b/x/keys.go @@ -674,19 +674,38 @@ type ReservedNamespace struct { // Names are the bare predicate form (no namespace prefix), matched // case-insensitively like Predicates above. ValueLocked []string + // ValueLockedPrefixes locks predicates by prefix rather than by exact name, + // for a namespace whose owned predicates are created dynamically and so + // cannot be enumerated up front. Typically the same value as + // PredicatePrefix, which locks every dynamic predicate the namespace owns. + // + // Without this, a namespace that admits dynamic predicates by prefix has no + // way to protect them: they are creatable via Alter but writable by anyone + // through /mutate, which for a namespace holding authorization data means its + // own API can be bypassed. + ValueLockedPrefixes []string // TrustMarker is a context key the owner's trusted in-process caller sets, // via context.WithValue(ctx, TrustMarker, true), to authorize writing - // ValueLocked predicates. Required (non-nil) when ValueLocked is non-empty; - // RegisterReservedNamespace panics otherwise. + // ValueLocked predicates. Required (non-nil) when ValueLocked or + // ValueLockedPrefixes is non-empty; RegisterReservedNamespace panics + // otherwise. TrustMarker any } +// valueLockedPrefix pairs a locked predicate prefix with the TrustMarker that +// authorizes writing predicates under it. +type valueLockedPrefix struct { + prefix string + marker any +} + var ( - reservedNsMu sync.RWMutex - reservedNsPrefixes []string - reservedNsPredicates = map[string]struct{}{} - reservedNsTypes = map[string]struct{}{} - reservedNsValueLocked = map[string]any{} // lowercased bare predicate -> TrustMarker + reservedNsMu sync.RWMutex + reservedNsPrefixes []string + reservedNsPredicates = map[string]struct{}{} + reservedNsTypes = map[string]struct{}{} + reservedNsValueLocked = map[string]any{} // lowercased bare predicate -> TrustMarker + reservedNsValueLockedPrefixes []valueLockedPrefix ) // RegisterReservedNamespace records a plugin's ownership of names under the @@ -697,7 +716,7 @@ var ( // non-empty ValueLocked needs a TrustMarker. It panics on any of these, so a // misconfiguration trips at startup rather than silently at mutation time. func RegisterReservedNamespace(ns ReservedNamespace) { - if len(ns.ValueLocked) > 0 && ns.TrustMarker == nil { + if (len(ns.ValueLocked) > 0 || len(ns.ValueLockedPrefixes) > 0) && ns.TrustMarker == nil { panic("x.RegisterReservedNamespace: ValueLocked is set but TrustMarker is nil; " + "a value-locked predicate with no TrustMarker is unwritable by everyone, including its owner") } @@ -706,7 +725,9 @@ func RegisterReservedNamespace(ns ReservedNamespace) { // predicate the mutation path passes, so a namespace-qualified registration // would never match — silently leaving a value-locked predicate publicly // writable. Reject it at startup instead. - for _, group := range [][]string{{ns.PredicatePrefix}, ns.Predicates, ns.Types, ns.ValueLocked} { + for _, group := range [][]string{ + {ns.PredicatePrefix}, ns.Predicates, ns.Types, ns.ValueLocked, ns.ValueLockedPrefixes, + } { for _, name := range group { if strings.Contains(name, NsSeparator) { panic(fmt.Sprintf("x.RegisterReservedNamespace: name %q must be bare, "+ @@ -745,6 +766,24 @@ func RegisterReservedNamespace(ns ReservedNamespace) { } reservedNsValueLocked[key] = ns.TrustMarker } + for _, p := range ns.ValueLockedPrefixes { + key := strings.ToLower(p) + if key == "" { + // An empty prefix matches every predicate, locking the whole cluster to + // this namespace's marker. It fails closed rather than open, so it is not + // a bypass — but it is never intended, and PredicatePrefix above already + // rejects the same mistake. + panic("x.RegisterReservedNamespace: value-locked prefix must not be empty") + } + for _, existing := range reservedNsValueLockedPrefixes { + if existing.prefix == key { + panic(fmt.Sprintf("x.RegisterReservedNamespace: value-locked prefix %q already registered", key)) + } + } + reservedNsValueLockedPrefixes = append(reservedNsValueLockedPrefixes, valueLockedPrefix{ + prefix: key, marker: ns.TrustMarker, + }) + } } // IsRegisteredReservedPredicate reports whether pred is owned by a registered @@ -783,11 +822,23 @@ func IsRegisteredReservedType(typ string) bool { // a value lock cannot be bypassed by changing the case of an owned name. Unlike // those lookups it does not ParseAttr: the guard passes the bare predicate (no // namespace separator), matching how IsOtherReservedPredicate is consulted. +// +// Exact names win over prefixes. That matters when two namespaces overlap — one +// owning a prefix, another owning a specific predicate inside it — since a single +// ReservedNamespace has one TrustMarker and cannot express the split itself. func ReservedPredicateValueLock(pred string) (marker any, locked bool) { + p := strings.ToLower(pred) reservedNsMu.RLock() defer reservedNsMu.RUnlock() - marker, locked = reservedNsValueLocked[strings.ToLower(pred)] - return marker, locked + if marker, locked = reservedNsValueLocked[p]; locked { + return marker, true + } + for _, vlp := range reservedNsValueLockedPrefixes { + if strings.HasPrefix(p, vlp.prefix) { + return vlp.marker, true + } + } + return nil, false } // TODO: rename this map to a better suited name as per its properties. It is not just for GraphQL diff --git a/x/principal.go b/x/principal.go new file mode 100644 index 00000000000..6f829325bc8 --- /dev/null +++ b/x/principal.go @@ -0,0 +1,84 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package x + +import "context" + +// Principal is the verified answer to "who is calling". It says nothing about +// what tenant the request operates in, and nothing about what the caller may do. +// Those are the tenant resolver's and the authorizer's jobs respectively. +// +// Deliberately no namespace field. Tenancy already has exactly one home — the +// `namespace` gRPC metadata key, read by ExtractNamespace — and it lives there +// because it has to survive the hop to Zero's UID rate limiter and to the +// group-1 leader. Putting a second copy on the Principal would recreate the +// divergence this whole separation exists to remove: today the tenant is in both +// md["namespace"] and the ACL JWT's claim, and authorization reads the claim +// while storage reads the metadata. +type Principal struct { + // Issuer identifies who vouched for this identity: "dgraph-acl" for a token + // Dgraph minted itself, or the verified `iss` claim for an external issuer. + Issuer string + // Subject is the stable identity of the caller — the ACL userId, or `sub` for + // an external issuer. + Subject string + // Groups are authorization-relevant memberships as asserted by the issuer. + Groups []string + // Claims carries the remaining verified claims, for policy layers that need + // more than identity and groups. + Claims map[string]any + // Method names how the caller was authenticated: one of the Method* constants + // below. Useful for audit, and load-bearing for any policy that must not treat + // an external issuer's assertion as equivalent to one Dgraph made itself. + Method string +} + +// Authentication methods a Principal can carry. +// +// Comparing against these is not bookkeeping. Dgraph's ACL groups are the ones +// x.IsSuperAdmin consults, so a policy that reads Principal.Groups without +// checking Method would let any issuer that can mint a `guardians` group +// membership confer superadmin. Whoever asserted the identity decides what the +// assertion is worth. +const ( + // MethodACL is a token Dgraph minted and verified itself. + MethodACL = "acl" + // MethodExternalJWT is a token from a configured external issuer, verified + // against its published keys. + MethodExternalJWT = "external-jwt" + // MethodPreshared is a shared secret, which identifies a client service rather + // than an end user. + MethodPreshared = "preshared" + // MethodInternal is in-process Dgraph code acting on its own behalf, holding no + // request credential. + MethodInternal = "internal" +) + +// principalKey is the context key under which a verified Principal travels. +// +// A context value, never metadata. Incoming metadata is entirely +// client-controlled, so a metadata-borne principal would be forgeable — and the +// internal worker port has no interceptor chain at all, so anything arriving +// there is unauthenticated by construction. A Principal also never needs to +// cross a process boundary: authorization is decided once, on the edge alpha, +// before any fan-out. +type principalKey struct{} + +// WithPrincipal returns a context carrying p as the verified caller identity. +// Only an authenticator should call this: the point of the type is that its +// presence means a credential was actually verified. +func WithPrincipal(ctx context.Context, p *Principal) context.Context { + return context.WithValue(ctx, principalKey{}, p) +} + +// PrincipalFrom returns the verified caller identity, or nil when the request +// presented no credential or one that did not verify. A nil Principal is the +// normal state for unauthenticated endpoints — Login, health checks, and +// CheckVersion — so callers must treat it as "unknown", not as an error. +func PrincipalFrom(ctx context.Context) *Principal { + p, _ := ctx.Value(principalKey{}).(*Principal) + return p +} diff --git a/x/reserved_namespace_test.go b/x/reserved_namespace_test.go index aa29932c7b7..9e202e0859f 100644 --- a/x/reserved_namespace_test.go +++ b/x/reserved_namespace_test.go @@ -117,3 +117,139 @@ func TestRegisterReservedNamespaceRejectsDuplicate(t *testing.T) { RegisterReservedNamespace(ReservedNamespace{Predicates: []string{"dgraph.duptest.x"}}) }) } + +// TestValueLockedPrefixes covers locking dynamically-named predicates by prefix. +// A namespace whose predicates are created one per (namespace, relation) at +// runtime cannot enumerate them in ValueLocked, so without prefix locking they +// would be creatable via Alter yet writable by anyone through /mutate. +func TestValueLockedPrefixes(t *testing.T) { + RegisterReservedNamespace(ReservedNamespace{ + PredicatePrefix: "dgraph.prefixlock.rel.", + Predicates: []string{"dgraph.prefixlock.xid"}, + ValueLockedPrefixes: []string{"dgraph.prefixlock.rel."}, + TrustMarker: testTrust, + }) + + // Any predicate under the locked prefix, including ones that do not exist + // yet — that is the point of locking by prefix. + for _, p := range []string{ + "dgraph.prefixlock.rel.document.owner", + "dgraph.prefixlock.rel.group.member", + "dgraph.prefixlock.rel.", + "dgraph.prefixlock.REL.Document.Owner", // case-insensitive, like exact names + } { + marker, locked := ReservedPredicateValueLock(p) + require.Truef(t, locked, "prefix lock must cover %q", p) + require.Equal(t, testTrust, marker) + } + + // Owned but deliberately not locked: xid stays writable so admin tooling and + // migrations can create nodes. + _, locked := ReservedPredicateValueLock("dgraph.prefixlock.xid") + require.False(t, locked) + + // A near miss outside the prefix is not locked. + _, locked = ReservedPredicateValueLock("dgraph.prefixlock.relative") + require.False(t, locked) +} + +// TestValueLockedExactWinsOverPrefix pins the precedence: a namespace may lock a +// whole prefix to one marker while pinning an individual predicate under it to +// another, so the exact entry must be consulted first. +// TestValueLockedExactWinsOverPrefix pins that an exact value lock takes precedence +// over a prefix that also matches. +// +// It registers the prefix and the exact name in SEPARATE namespaces with distinct +// markers, which is both the only way the precedence is observable and the only way +// the split is expressible: a ReservedNamespace carries one TrustMarker, so the +// earlier version of this test registered both kinds in one namespace, gave them the +// same marker, and passed whichever table ReservedPredicateValueLock consulted +// first. +func TestValueLockedExactWinsOverPrefix(t *testing.T) { + type prefixTrustKey int + type exactTrustKey int + const ( + prefixTrust prefixTrustKey = 1 + exactTrust exactTrustKey = 2 + ) + + // The namespace that owns the whole sub-namespace by prefix. + RegisterReservedNamespace(ReservedNamespace{ + PredicatePrefix: "dgraph.precedence.rel.", + ValueLockedPrefixes: []string{"dgraph.precedence.rel."}, + TrustMarker: prefixTrust, + }) + // A second namespace pinning one predicate inside it to its own marker. Note + // there is no cross-kind conflict check, so this registration is accepted — the + // precedence rule is what decides the overlap. + RegisterReservedNamespace(ReservedNamespace{ + Predicates: []string{"dgraph.precedence.rel.special"}, + ValueLocked: []string{"dgraph.precedence.rel.special"}, + TrustMarker: exactTrust, + }) + + marker, locked := ReservedPredicateValueLock("dgraph.precedence.rel.special") + require.True(t, locked) + require.Equal(t, exactTrust, marker, + "the exact entry must win over the prefix that also matches") + + marker, locked = ReservedPredicateValueLock("dgraph.precedence.rel.ordinary") + require.True(t, locked) + require.Equal(t, prefixTrust, marker, + "a predicate with no exact entry falls to the prefix owner") +} + +// TestValueLockedPrefixRejectsEmpty covers the guard that PredicatePrefix has and +// value locks did not. An empty prefix matches every predicate in the cluster. +func TestValueLockedPrefixRejectsEmpty(t *testing.T) { + type emptyTrustKey int + require.PanicsWithValue(t, + "x.RegisterReservedNamespace: value-locked prefix must not be empty", + func() { + RegisterReservedNamespace(ReservedNamespace{ + PredicatePrefix: "dgraph.emptylock.", + ValueLockedPrefixes: []string{""}, + TrustMarker: emptyTrustKey(1), + }) + }) +} + +// TestValueLockedPrefixesRequireTrustMarker mirrors the ValueLocked invariant: +// a locked prefix with no marker would be unwritable by everyone, including its +// owner, so it must panic at registration. +func TestValueLockedPrefixesRequireTrustMarker(t *testing.T) { + require.Panics(t, func() { + RegisterReservedNamespace(ReservedNamespace{ + PredicatePrefix: "dgraph.prefixnomarker.rel.", + ValueLockedPrefixes: []string{"dgraph.prefixnomarker.rel."}, + // TrustMarker intentionally left nil. + }) + }) +} + +// TestValueLockedPrefixesRejectQualified confirms a namespace-qualified prefix is +// rejected, for the same reason a qualified exact name is: the guard matches the +// bare predicate, so it would never fire and the predicates would stay writable. +func TestValueLockedPrefixesRejectQualified(t *testing.T) { + require.Panics(t, func() { + RegisterReservedNamespace(ReservedNamespace{ + ValueLockedPrefixes: []string{NamespaceAttr(RootNamespace, "dgraph.qualprefix.rel.")}, + TrustMarker: testTrust, + }) + }) +} + +// TestValueLockedPrefixesRejectDuplicate confirms two namespaces cannot claim the +// same locked prefix, since import order would silently pick the TrustMarker. +func TestValueLockedPrefixesRejectDuplicate(t *testing.T) { + require.Panics(t, func() { + RegisterReservedNamespace(ReservedNamespace{ + ValueLockedPrefixes: []string{"dgraph.dupprefix.rel."}, + TrustMarker: testTrust, + }) + RegisterReservedNamespace(ReservedNamespace{ + ValueLockedPrefixes: []string{"dgraph.dupprefix.rel."}, + TrustMarker: testTrust, + }) + }) +} diff --git a/x/tenancy.go b/x/tenancy.go new file mode 100644 index 00000000000..7652095f1ca --- /dev/null +++ b/x/tenancy.go @@ -0,0 +1,225 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package x + +import ( + "context" + "net/http" + "sync/atomic" + + "github.com/pkg/errors" + "google.golang.org/grpc/metadata" +) + +// TenantResolver attributes an incoming request to a tenant — a Dgraph +// namespace. It returns a context whose tenancy channel names the tenant the +// request acts in (set with AttachNamespace, read by ExtractNamespace), or an +// error if the request cannot be attributed. +// +// A resolver MUST NOT trust the incoming `namespace` metadata value: on the +// server side, incoming metadata is entirely client-controlled. It must either +// overwrite that value from a verified credential or return an error. +// +// The built-in resolver derives the namespace from Dgraph's own ACL access JWT, +// which is why tenancy currently requires ACL: with ACL disabled there is no +// credential to read a namespace from, so every request is the root namespace. +// A deployment that authenticates elsewhere can install its own resolver and +// decouple the two. +type TenantResolver func(ctx context.Context) (context.Context, error) + +// tenantResolver holds the installed resolver, or nil for the built-in one. +// +// An atomic pointer rather than a mutex: this is one word read on every request, +// written once at startup. (The reserved-namespace registry above uses an +// RWMutex because it guards several maps and a slice, not a single pointer.) +var tenantResolver atomic.Pointer[TenantResolver] + +// SetTenantResolver installs a deployment-specific tenant resolver, replacing +// the built-in one that derives the namespace from Dgraph's own ACL access JWT. +// Call it during command setup, after flags are parsed and before any listener +// starts serving. Passing nil restores the built-in resolver. +func SetTenantResolver(r TenantResolver) { + if r == nil { + tenantResolver.Store(nil) + return + } + tenantResolver.Store(&r) +} + +// TenantResolverInstalled reports whether a resolver other than the built-in one +// is installed. See MultiTenancyEnabled. +func TenantResolverInstalled() bool { + return tenantResolver.Load() != nil +} + +// MultiTenancyEnabled reports whether this cluster can serve more than one +// namespace — i.e. whether any request can name a namespace other than +// RootNamespace. True when ACL is on (the ACL access JWT carries the namespace) +// or when a deployment-specific tenant resolver is installed. +// +// Prefer this over reading WorkerConfig.AclEnabled directly wherever the +// question being asked is "is this cluster multi-tenant" rather than "is ACL +// configured". The two have been the same bit historically; they are not the +// same question. +func MultiTenancyEnabled() bool { + return WorkerConfig.AclEnabled || TenantResolverInstalled() +} + +// ResolveTenant attributes ctx to a tenant using the installed resolver. +func ResolveTenant(ctx context.Context) (context.Context, error) { + if isTrustedTenantCtx(ctx) { + // Already attributed by in-process Dgraph code that holds no request + // credential to present. Leave it alone. + return ctx, nil + } + if r := tenantResolver.Load(); r != nil { + rctx, err := (*r)(ctx) + if rctx == nil { + // A resolver that fails closed will naturally return only an error, and + // returning that nil onward is worse than the rejection it meant: a caller + // that logs the error and proceeds, or that drops it, panics on first use + // of the context instead. Hand back the original so the failure is always + // an error rather than sometimes a crash. + return ctx, err + } + return rctx, err + } + return aclTenantResolver(ctx) +} + +// ClearIncomingNamespace removes any namespace the client put on the request. +// +// Call it at a gRPC entry point, before ResolveTenant. On the server side +// md["namespace"] is entirely client-controlled, and the built-in resolver leaves +// whatever is there when it cannot derive a namespace from the access JWT — so +// without this, a caller who omits or corrupts their credential still gets the +// tenant they asked for. Clearing it first turns that case into ExtractNamespace's +// "No namespace in the metadata" error, which is a rejection. +// +// Only for entry points. A continuation of an already-resolved request must keep +// the namespace it was attributed to. +func ClearIncomingNamespace(ctx context.Context) context.Context { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return ctx + } + md = md.Copy() + md.Delete("namespace") + return metadata.NewIncomingContext(ctx, md) +} + +// aclTenantResolver is the built-in resolver, and the only one an OSS build has. +// It is the pre-seam body of AttachJWTNamespace, unchanged: same branches, same +// returns, and the error is always nil. +func aclTenantResolver(ctx context.Context) (context.Context, error) { + if !WorkerConfig.AclEnabled { + // Single-tenant cluster: everything is the root namespace. + return AttachNamespace(ctx, RootNamespace), nil + } + + ns, err := ExtractNamespaceFrom(ctx) + if err != nil { + // Tolerate the failure and leave whatever namespace the context already + // carries. Under ACL the request is rejected downstream by + // authorizeRequest, which needs the same JWT this just failed to parse, + // so nothing reaches storage on an unattributed context. + return ctx, nil + } + return AttachNamespace(ctx, ns), nil +} + +// ResolveTenantHTTP attributes an incoming HTTP request to a tenant, so the HTTP +// surface derives a namespace through the same resolver as the gRPC surface +// rather than reading the ACL JWT claim directly. It replaces the former +// ExtractNamespaceHTTP. +// +// It deliberately keeps that function's fail-open behavior: a request whose +// tenant cannot be determined resolves to the root namespace. The error return +// carries only a resolver's own failure, which the built-in resolver never +// produces. +// +// Failing open looks wrong until you enumerate the callers, which have +// materially different requirements — a blanket rejection breaks most of them: +// +// - /admin sets resolver=0 unconditionally and uses this value only for +// LazyLoadSchema, so the root namespace is the correct answer there. It also +// serves the login mutation and the test harness's health check. +// - /probe/graphql is unauthenticated by construction. +// - audit must never reject; it only records the event. +// - only /graphql routes by the resolved namespace, and it uses +// ResolveTenantHTTPStrict instead. +// +// Rejecting a credential-less request on the first two deadlocks the cluster: +// login stops working, so nothing can ever obtain the token that would have +// satisfied the check. +func ResolveTenantHTTP(r *http.Request) (uint64, error) { + ctx, err := ResolveTenant(AttachAccessJwt(context.Background(), r)) + if err != nil { + return 0, err + } + ns, nsErr := ExtractNamespace(ctx) + if nsErr != nil { + return RootNamespace, nil + } + return ns, nil +} + +// ResolveTenantHTTPStrict is ResolveTenantHTTP for the one caller that routes by +// the resolved namespace: the /graphql handler, which uses it to choose whose +// GraphQL schema serves the request. +// +// It differs on exactly one input. A request presenting an access token that +// cannot be resolved is rejected, rather than quietly served the root namespace's +// schema — leaking the shape of the root namespace's public API to a caller whose +// own tenant could not be determined. +// +// A request presenting NO token still resolves to the root namespace. That +// distinction is the whole design: /admin serves the login mutation and the +// harness health check, and health probes are unauthenticated by construction, so +// rejecting a credential-less request deadlocks the cluster — login stops working, +// which is how a token would have been obtained. Only /graphql needs the stricter +// rule, and only /graphql gets it. +func ResolveTenantHTTPStrict(r *http.Request) (uint64, error) { + ctx := AttachAccessJwt(context.Background(), r) + resolved, err := ResolveTenant(ctx) + if err != nil { + return 0, err + } + if ns, nsErr := ExtractNamespace(resolved); nsErr == nil { + return ns, nil + } + + // Nothing resolved, so distinguish "no credential" from "unusable + // credential". Asking ExtractJwt rather than re-reading the header name keeps + // this on the same accessor AttachAccessJwt feeds — two copies of that name + // could drift apart, and the failure would be silent: the strict rule would + // simply never fire. + if _, jwtErr := ExtractJwt(ctx); jwtErr == nil { + return 0, errors.Errorf("unable to determine the namespace from the supplied access token") + } + return RootNamespace, nil +} + +// trustedTenantKey marks a context whose tenancy was set by trusted in-process +// code rather than derived from a request credential. +type trustedTenantKey struct{} + +// AttachTrustedTenant attributes ctx to ns on behalf of in-process Dgraph code +// that holds no request credential — schema bootstrap, ACL upserts, GetGQLSchema +// — and marks it so ResolveTenant leaves the attribution alone. +// +// The marker is a context value, never metadata, so it cannot arrive over the +// wire: a network client has no way to claim it. That is what makes it safe for +// a resolver to otherwise distrust the incoming namespace entirely. +func AttachTrustedTenant(ctx context.Context, ns uint64) context.Context { + return context.WithValue(AttachNamespace(ctx, ns), trustedTenantKey{}, true) +} + +// isTrustedTenantCtx reports whether ctx was attributed by AttachTrustedTenant. +func isTrustedTenantCtx(ctx context.Context) bool { + trusted, _ := ctx.Value(trustedTenantKey{}).(bool) + return trusted +} diff --git a/x/tenancy_entrypoints_test.go b/x/tenancy_entrypoints_test.go new file mode 100644 index 00000000000..831826c34a2 --- /dev/null +++ b/x/tenancy_entrypoints_test.go @@ -0,0 +1,164 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package x + +import ( + "bytes" + "go/ast" + "go/parser" + "go/token" + "io/fs" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// clearNamespaceSites is every function that strips the client-supplied namespace, +// and why that function is the right place for it. +// +// This is a decision table, not a description. ClearIncomingNamespace belongs at an +// entry point and nowhere else: md["namespace"] is client-controlled server-side and +// the built-in resolver leaves it in place when it cannot derive one from a +// credential, so an entry point that skips the call attributes the request to +// whatever tenant the caller named. A shared continuation that makes the call is the +// mirror-image bug — it discards the namespace an already-attributed in-process +// caller was given. +// +// The behavioral tests in edgraph and worker prove each of these four rejects an +// uncredentialed request. This table adds what behavior cannot see: that the call +// sits in *this* function rather than having drifted down into the continuation +// below it, where it would still look correct from the outside. +var clearNamespaceSites = map[string]string{ + "edgraph/query.go:RunDQL": "gRPC entry point; resolves its own tenant", + "edgraph/server.go:Alter": "entry point, not alter(), which also serves AlterNoAuth " + + "with a context an in-process caller has already attributed", + "edgraph/server.go:Query": "entry point, not QueryNoGrpc(), which also serves the two " + + "HTTP handlers and GetGQLSchema's trusted tenancy", + "worker/zero_proxy.go:forwardAssignUidsToZero": "gRPC entry point for UID leasing, and " + + "the only gate on it — without this an uncredentialed caller leases UIDs in any tenant", +} + +// clearNamespaceScanRoot is the repo root, walked in full: a scan that can miss a +// call site cannot report completeness. See capabilityScanRoot in edgraph. +const clearNamespaceScanRoot = ".." + +var clearNamespaceScanSkip = map[string]bool{ + ".git": true, "vendor": true, "testdata": true, "protos": true, + "compose": true, "contrib": true, ".trunk": true, "systest": true, + // A nested checkout under the repo root would be walked as if it were part of + // this one, so the same function is found twice and the count assertion fails. + // Agent worktrees land here by convention. + ".worktrees": true, +} + +// scanClearNamespaceSites returns "dir/file.go:FuncName" for every function whose +// body calls ClearIncomingNamespace. +func scanClearNamespaceSites(t *testing.T) map[string]bool { + t.Helper() + found := make(map[string]bool) + var parsed int + + err := filepath.WalkDir(clearNamespaceScanRoot, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + if clearNamespaceScanSkip[d.Name()] { + return fs.SkipDir + } + return nil + } + if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + src, readErr := os.ReadFile(path) + if readErr != nil { + return readErr + } + if !bytes.Contains(src, []byte("ClearIncomingNamespace")) { + return nil + } + parsed++ + + fset := token.NewFileSet() + af, parseErr := parser.ParseFile(fset, path, src, 0) + require.NoError(t, parseErr, "parsing %s", path) + + for _, decl := range af.Decls { + fd, ok := decl.(*ast.FuncDecl) + if !ok || fd.Body == nil { + continue + } + // The declaration itself is not a call site. + if fd.Name.Name == "ClearIncomingNamespace" { + continue + } + ast.Inspect(fd.Body, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok || clearNamespaceCallee(call.Fun) != "ClearIncomingNamespace" { + return true + } + found[clearNamespaceSitePath(path)+":"+fd.Name.Name] = true + return true + }) + } + return nil + }) + require.NoError(t, err) + require.Positive(t, parsed, "the walk parsed no files mentioning ClearIncomingNamespace; "+ + "clearNamespaceScanRoot is wrong and this test proves nothing") + return found +} + +func clearNamespaceCallee(fn ast.Expr) string { + switch f := fn.(type) { + case *ast.Ident: + return f.Name + case *ast.SelectorExpr: + return f.Sel.Name + } + return "" +} + +func clearNamespaceSitePath(path string) string { + clean := filepath.ToSlash(filepath.Clean(path)) + return strings.TrimPrefix(clean, "../") +} + +// TestEveryEntryPointClearsIncomingNamespace pins the guard's call sites in both +// directions. A deleted call is a privilege escalation; a call added to a shared +// continuation silently strips tenancy from trusted in-process callers. +func TestEveryEntryPointClearsIncomingNamespace(t *testing.T) { + found := scanClearNamespaceSites(t) + + for site := range found { + if _, declared := clearNamespaceSites[site]; !declared { + t.Errorf("%s clears the incoming namespace but is not in clearNamespaceSites. "+ + "If it is an entry point, add it and say so. If it is a continuation of an "+ + "already-attributed request, the call is wrong: it discards the namespace an "+ + "in-process caller supplied.", site) + } + } + + for site, why := range clearNamespaceSites { + if !found[site] { + t.Errorf("clearNamespaceSites declares %s (%s), which no longer clears the "+ + "incoming namespace. Deleting that call lets an uncredentialed caller act in "+ + "whichever tenant they name in md[\"namespace\"].", site, why) + } + } +} + +// TestClearNamespaceSiteCoverage guards against the table or the scan going empty, +// which would make the test above vacuously pass. +func TestClearNamespaceSiteCoverage(t *testing.T) { + found := scanClearNamespaceSites(t) + require.NotEmpty(t, found, "the scan found no entry-point guards at all") + require.Len(t, found, len(clearNamespaceSites)) +} diff --git a/x/tenancy_test.go b/x/tenancy_test.go new file mode 100644 index 00000000000..5e17061ea79 --- /dev/null +++ b/x/tenancy_test.go @@ -0,0 +1,451 @@ +/* + * SPDX-FileCopyrightText: © 2017-2026 Istari Digital, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package x + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/golang-jwt/jwt/v5" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/metadata" +) + +// oldAttachJWTNamespace is the pre-seam body of AttachJWTNamespace, preserved +// verbatim as the oracle for TestResolveTenantIsBitIdentical. It is the thing +// this stage promises not to change; asserting against a copy rather than +// against remembered behavior is what makes that promise checkable. +func oldAttachJWTNamespace(ctx context.Context) context.Context { + if !WorkerConfig.AclEnabled { + return AttachNamespace(ctx, RootNamespace) + } + + ns, err := ExtractNamespaceFrom(ctx) + if err == nil { + ctx = AttachNamespace(ctx, ns) + } + return ctx +} + +// hmacKey is 32 bytes: the FIPS provider rejects an HMAC key shorter than the +// digest size, and this package's tests run in the FIPS build too. +var hmacKey = []byte("tenancy-test-hmac-key-32-bytes!!") + +// tokenWith mints a signed ACL-style token from the given claims. +func tokenWith(t *testing.T, claims jwt.MapClaims) string { + t.Helper() + signed, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString(hmacKey) + require.NoError(t, err) + return signed +} + +// withACL configures WorkerConfig for HS256 ACL tokens and restores it after the +// test. WorkerConfig is global, so every mutation here has to be undone. +func withACL(t *testing.T, enabled bool) { + t.Helper() + prevEnabled, prevAlg, prevKey := WorkerConfig.AclEnabled, WorkerConfig.AclJwtAlg, WorkerConfig.AclPublicKey + t.Cleanup(func() { + WorkerConfig.AclEnabled, WorkerConfig.AclJwtAlg, WorkerConfig.AclPublicKey = prevEnabled, prevAlg, prevKey + }) + WorkerConfig.AclEnabled = enabled + WorkerConfig.AclJwtAlg = jwt.GetSigningMethod("HS256") + WorkerConfig.AclPublicKey = hmacKey +} + +// accessToken mints an ACL-style access JWT carrying the given namespace claim. +// The claim is a float64 on the wire, matching what getAccessJwt produces. +func accessToken(t *testing.T, ns uint64) string { + t.Helper() + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ + "userid": "alice", + "groups": []string{"dev"}, + "namespace": float64(ns), + "exp": float64(1 << 40), // far future + }) + signed, err := tok.SignedString(hmacKey) + require.NoError(t, err) + return signed +} + +// nsOf reports the namespace metadata value on ctx, or "" when absent, so the +// two implementations can be compared on the one channel that matters. +func nsOf(ctx context.Context) string { + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return "" + } + if v := md.Get("namespace"); len(v) > 0 { + return v[0] + } + return "" +} + +// TestResolveTenantIsBitIdentical is the Stage 1 gate. ResolveTenant must agree +// with the pre-seam implementation on every input, so introducing the seam is +// provably behavior-preserving rather than believed to be. +func TestResolveTenantIsBitIdentical(t *testing.T) { + require.False(t, TenantResolverInstalled(), "no resolver may be installed for this comparison") + + valid := func(t *testing.T) string { return accessToken(t, 5) } + + cases := []struct { + name string + aclEnabled bool + token func(*testing.T) string // "" for no token + preAttach *uint64 // namespace already on the context + }{ + {name: "acl off, no token", aclEnabled: false}, + {name: "acl off, valid token", aclEnabled: false, token: valid}, + {name: "acl off, namespace pre-attached", aclEnabled: false, preAttach: ptr(uint64(9))}, + {name: "acl off, malformed token and pre-attached", aclEnabled: false, + token: func(*testing.T) string { return "not.a.jwt" }, preAttach: ptr(uint64(9))}, + + {name: "acl on, no token", aclEnabled: true}, + {name: "acl on, no token but namespace pre-attached", aclEnabled: true, preAttach: ptr(uint64(9))}, + {name: "acl on, valid token", aclEnabled: true, token: valid}, + {name: "acl on, valid token overrides pre-attached", aclEnabled: true, token: valid, preAttach: ptr(uint64(9))}, + {name: "acl on, malformed token", aclEnabled: true, + token: func(*testing.T) string { return "not.a.jwt" }}, + {name: "acl on, malformed token with pre-attached", aclEnabled: true, + token: func(*testing.T) string { return "not.a.jwt" }, preAttach: ptr(uint64(9))}, + {name: "acl on, token signed with the wrong key", aclEnabled: true, token: func(t *testing.T) string { + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{"namespace": float64(5)}) + s, err := tok.SignedString([]byte("a-different-32-byte-hmac-key!!!!!")) + require.NoError(t, err) + return s + }}, + {name: "acl on, token with no namespace claim", aclEnabled: true, token: func(t *testing.T) string { + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{"userid": "alice"}) + s, err := tok.SignedString(hmacKey) + require.NoError(t, err) + return s + }}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + withACL(t, tc.aclEnabled) + + build := func() context.Context { + md := metadata.New(nil) + if tc.token != nil { + md.Set("accessJwt", tc.token(t)) + } + ctx := metadata.NewIncomingContext(context.Background(), md) + if tc.preAttach != nil { + ctx = AttachNamespace(ctx, *tc.preAttach) + } + return ctx + } + + // Separate contexts so neither implementation observes the other's + // mutations to the shared metadata map. + wantCtx := oldAttachJWTNamespace(build()) + gotCtx, err := ResolveTenant(build()) + + require.NoError(t, err, "the built-in resolver never returns an error") + require.Equal(t, nsOf(wantCtx), nsOf(gotCtx), "namespace metadata must match the pre-seam behavior") + }) + } +} + +// TestAttachTrustedTenantIsHonored covers the one input the pre-seam code had no +// concept of. In-process callers that hold no request credential mark their +// context, and ResolveTenant must leave that attribution alone — otherwise a +// fail-closed resolver would break schema bootstrap and the in-process Alter path. +func TestAttachTrustedTenantIsHonored(t *testing.T) { + withACL(t, true) + + // A trusted context naming namespace 9, with a token for namespace 5 also + // present: the marker wins, so the resolver cannot override a deliberate + // in-process attribution. + md := metadata.Pairs("accessJwt", accessToken(t, 5)) + ctx := AttachTrustedTenant(metadata.NewIncomingContext(context.Background(), md), 9) + + got, err := ResolveTenant(ctx) + require.NoError(t, err) + require.Equal(t, "9", nsOf(got)) + + // And an installed resolver is not consulted at all for a trusted context. + t.Cleanup(func() { SetTenantResolver(nil) }) + SetTenantResolver(func(context.Context) (context.Context, error) { + t.Error("resolver must not run for a trusted context") + return nil, errors.New("unreachable") + }) + got, err = ResolveTenant(ctx) + require.NoError(t, err) + require.Equal(t, "9", nsOf(got)) +} + +// TestSetTenantResolver covers installation, delegation, error propagation, and +// restoring the built-in. +func TestSetTenantResolver(t *testing.T) { + withACL(t, false) // built-in would force RootNamespace; the resolver must win + t.Cleanup(func() { SetTenantResolver(nil) }) + + require.False(t, TenantResolverInstalled()) + + SetTenantResolver(func(ctx context.Context) (context.Context, error) { + return AttachNamespace(ctx, 42), nil + }) + require.True(t, TenantResolverInstalled()) + + got, err := ResolveTenant(metadata.NewIncomingContext(context.Background(), metadata.New(nil))) + require.NoError(t, err) + require.Equal(t, "42", nsOf(got), "installed resolver must take precedence over the built-in") + + // An error is propagated rather than swallowed. Every call site takes it and + // rejects, which is the whole reason ResolveTenant replaced the old + // context-in/context-out signature. + sentinel := errors.New("cannot attribute request") + SetTenantResolver(func(context.Context) (context.Context, error) { return nil, sentinel }) + _, err = ResolveTenant(context.Background()) + require.ErrorIs(t, err, sentinel) + + SetTenantResolver(nil) + require.False(t, TenantResolverInstalled()) + got, err = ResolveTenant(context.Background()) + require.NoError(t, err) + require.Equal(t, "0", nsOf(got), "built-in resolver restored: ACL off means root namespace") +} + +// TestMultiTenancyEnabled pins the truth table. A stock OSS build reports false, +// which is what keeps the Stage 3 substitutions behavior-preserving there. +func TestMultiTenancyEnabled(t *testing.T) { + t.Cleanup(func() { SetTenantResolver(nil) }) + + withACL(t, false) + require.False(t, MultiTenancyEnabled(), "stock build: ACL off, no resolver") + + withACL(t, true) + require.True(t, MultiTenancyEnabled(), "ACL carries the namespace claim") + + withACL(t, false) + SetTenantResolver(func(ctx context.Context) (context.Context, error) { return ctx, nil }) + require.True(t, MultiTenancyEnabled(), "an installed resolver can name a tenant without ACL") +} + +// TestResolveTenantHTTP covers the HTTP surface, which previously swallowed the +// parse error and returned namespace 0. That mattered because these call sites +// choose which namespace's GraphQL schema serves a request — so a malformed +// token silently routed the caller to the root namespace's schema instead of +// being rejected. +func TestResolveTenantHTTP(t *testing.T) { + req := func(token string) *http.Request { + r := &http.Request{Header: http.Header{}} + if token != "" { + r.Header.Set("X-Dgraph-AccessToken", token) + } + return r + } + + t.Run("acl off resolves to the root namespace", func(t *testing.T) { + withACL(t, false) + ns, err := ResolveTenantHTTP(req("")) + require.NoError(t, err) + require.Equal(t, RootNamespace, ns) + }) + + t.Run("acl on derives the namespace from the token", func(t *testing.T) { + withACL(t, true) + ns, err := ResolveTenantHTTP(req(accessToken(t, 5))) + require.NoError(t, err) + require.Equal(t, uint64(5), ns) + }) + + // Fail-open is preserved deliberately — see the ResolveTenantHTTP doc comment. + // Two attempts at making this reject taught the same lesson twice: /admin + // forces resolver=0 and only needs this for LazyLoadSchema, health probes and + // the login mutation cannot carry a token at all, and rejecting them deadlocks + // the cluster because login is how a token is obtained in the first place. + t.Run("an unresolvable request falls back to the root namespace", func(t *testing.T) { + withACL(t, true) + for name, token := range map[string]string{ + "no token": "", + "malformed": "not.a.jwt", + "signed with wrong key": func() string { + tok := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{"namespace": float64(5)}) + s, err := tok.SignedString([]byte("a-different-32-byte-hmac-key!!!!!")) + require.NoError(t, err) + return s + }(), + } { + t.Run(name, func(t *testing.T) { + ns, err := ResolveTenantHTTP(req(token)) + require.NoError(t, err) + require.Equal(t, RootNamespace, ns) + }) + } + }) + + t.Run("an installed resolver's error propagates", func(t *testing.T) { + withACL(t, false) + t.Cleanup(func() { SetTenantResolver(nil) }) + sentinel := errors.New("cannot attribute request") + SetTenantResolver(func(context.Context) (context.Context, error) { return nil, sentinel }) + + _, err := ResolveTenantHTTP(req("")) + require.ErrorIs(t, err, sentinel) + }) +} + +// TestResolveTenantHTTPStrict covers the /graphql-only variant. The pair of +// assertions is the whole point: an unusable token is rejected, an absent one is +// not. Two earlier attempts at this hardening failed by conflating them — the +// first rejected both, which broke login and deadlocked the cluster; the second +// still applied the rule to /admin, where the root namespace is the right answer +// and the test harness's own health check presents a token. +func TestResolveTenantHTTPStrict(t *testing.T) { + req := func(token string) *http.Request { + r := &http.Request{Header: http.Header{}} + if token != "" { + r.Header.Set("X-Dgraph-AccessToken", token) + } + return r + } + + t.Run("no token resolves to root, never rejects", func(t *testing.T) { + for _, acl := range []bool{false, true} { + withACL(t, acl) + ns, err := ResolveTenantHTTPStrict(req("")) + require.NoErrorf(t, err, "acl=%v: login and probes carry no token by design", acl) + require.Equal(t, RootNamespace, ns) + } + }) + + t.Run("valid token resolves to its namespace", func(t *testing.T) { + withACL(t, true) + ns, err := ResolveTenantHTTPStrict(req(accessToken(t, 5))) + require.NoError(t, err) + require.Equal(t, uint64(5), ns) + }) + + t.Run("unusable token is rejected", func(t *testing.T) { + withACL(t, true) + wrongKey, err := jwt.NewWithClaims(jwt.SigningMethodHS256, + jwt.MapClaims{"namespace": float64(5), "exp": float64(1 << 40)}). + SignedString([]byte("a-different-32-byte-hmac-key!!!!!")) + require.NoError(t, err) + + for name, token := range map[string]string{ + "malformed": "not.a.jwt", + "signed with wrong key": wrongKey, + "expired": tokenWith(t, jwt.MapClaims{ + "userid": "alice", "namespace": float64(5), "exp": float64(1)}), + "no namespace claim": tokenWith(t, jwt.MapClaims{ + "userid": "alice", "exp": float64(1 << 40)}), + } { + t.Run(name, func(t *testing.T) { + _, err := ResolveTenantHTTPStrict(req(token)) + require.Error(t, err, + "must not serve the root namespace's schema to a caller whose tenant is unknown") + }) + } + }) + + // With ACL off there is no credential to verify, so even a token that would + // otherwise be unusable resolves to the root namespace — the single-tenant + // answer. Strictness must not invent a failure where tenancy cannot vary. + t.Run("acl off ignores an unusable token", func(t *testing.T) { + withACL(t, false) + ns, err := ResolveTenantHTTPStrict(req("not.a.jwt")) + require.NoError(t, err) + require.Equal(t, RootNamespace, ns) + }) + + t.Run("lenient variant still admits what strict rejects", func(t *testing.T) { + withACL(t, true) + ns, err := ResolveTenantHTTP(req("not.a.jwt")) + require.NoError(t, err, "/admin and probes must keep working") + require.Equal(t, RootNamespace, ns) + }) +} + +func ptr[T any](v T) *T { return &v } + +// TestResolveTenantNeverReturnsNilContext pins the guard against a resolver that +// fails closed by returning only an error. +// +// The hazard is not hypothetical: the deprecated AttachJWTNamespace shim did +// `ctx, _ = ResolveTenant(ctx)`, so a nil context would have reached every caller +// that shim served and panicked on first use — a crash where a rejection was +// intended. The shim is gone, but the guard belongs on the seam rather than on the +// discipline of each caller. +func TestResolveTenantNeverReturnsNilContext(t *testing.T) { + t.Cleanup(func() { SetTenantResolver(nil) }) + + wantErr := errors.New("no credential; refusing to attribute") + SetTenantResolver(func(context.Context) (context.Context, error) { + return nil, wantErr + }) + + base := context.Background() + got, err := ResolveTenant(base) + require.ErrorIs(t, err, wantErr, "the resolver's error must survive") + require.NotNil(t, got, "a nil context must never be handed to a caller") + // Usable, not merely non-nil. + require.NoError(t, got.Err()) +} + +// TestClearIncomingNamespaceClosesTheEntryPointHole is the regression test for a +// privilege escalation introduced when the zero-proxy UID lease path was converted +// onto the seam. +// +// Before the conversion the path derived the namespace from the signed access JWT +// and returned that error on failure. After it, the built-in resolver's +// tolerate-a-bad-token branch left the client's own md["namespace"] in place and +// ExtractNamespace read it back, so a caller with no usable credential leased UIDs +// in whichever tenant they named. +func TestClearIncomingNamespaceClosesTheEntryPointHole(t *testing.T) { + withACL(t, true) + + // A caller asking for namespace 9 while presenting no access token at all. + md := metadata.New(map[string]string{"namespace": "9"}) + ctx := metadata.NewIncomingContext(context.Background(), md) + + t.Run("without clearing, the client's namespace survives", func(t *testing.T) { + rctx, err := ResolveTenant(ctx) + require.NoError(t, err, "the built-in resolver tolerates a missing token") + ns, err := ExtractNamespace(rctx) + require.NoError(t, err) + require.Equal(t, uint64(9), ns, + "documents the hazard: the value came from the caller, not a credential") + }) + + t.Run("clearing first turns it into a rejection", func(t *testing.T) { + rctx, err := ResolveTenant(ClearIncomingNamespace(ctx)) + require.NoError(t, err) + _, err = ExtractNamespace(rctx) + require.Error(t, err, + "an unattributable entry-point request must not resolve to any tenant") + }) + + t.Run("a valid token still resolves, and the claim wins over the client's value", func(t *testing.T) { + token := accessToken(t, 7) + withTok := metadata.NewIncomingContext(context.Background(), + metadata.New(map[string]string{"namespace": "9", "accessJwt": token})) + rctx, err := ResolveTenant(ClearIncomingNamespace(withTok)) + require.NoError(t, err) + ns, err := ExtractNamespace(rctx) + require.NoError(t, err) + require.Equal(t, uint64(7), ns, "the namespace must come from the signed claim") + }) + + t.Run("clearing leaves other metadata alone", func(t *testing.T) { + in := metadata.NewIncomingContext(context.Background(), + metadata.New(map[string]string{"namespace": "9", "accessJwt": "keep-me"})) + out, ok := metadata.FromIncomingContext(ClearIncomingNamespace(in)) + require.True(t, ok) + require.Empty(t, out.Get("namespace")) + require.Equal(t, []string{"keep-me"}, out.Get("accessJwt")) + }) + + t.Run("no metadata at all is not a crash", func(t *testing.T) { + require.NotNil(t, ClearIncomingNamespace(context.Background())) + }) +} diff --git a/x/x.go b/x/x.go index 135194e7661..73659303f82 100644 --- a/x/x.go +++ b/x/x.go @@ -249,14 +249,6 @@ func GqlErrorf(message string, args ...interface{}) *GqlError { } } -// ExtractNamespaceHTTP parses the namespace value from the incoming HTTP request. -func ExtractNamespaceHTTP(r *http.Request) uint64 { - ctx := AttachAccessJwt(context.Background(), r) - // Ignoring error because the default value is zero anyways. - namespace, _ := ExtractNamespaceFrom(ctx) - return namespace -} - // ExtractNamespace parses the namespace value from the incoming gRPC context. For the non-ACL mode, // it is caller's responsibility to set the galaxy namespace. func ExtractNamespace(ctx context.Context) (uint64, error) { @@ -445,24 +437,6 @@ func ParseRequest(w http.ResponseWriter, r *http.Request, data interface{}) bool return true } -// AttachJWTNamespace attaches the namespace in the JWT claims to the context if present, otherwise -// it attaches the galaxy namespace. -func AttachJWTNamespace(ctx context.Context) context.Context { - if !WorkerConfig.AclEnabled { - return AttachNamespace(ctx, RootNamespace) - } - - ns, err := ExtractNamespaceFrom(ctx) - if err == nil { - // Attach the namespace only if we got one from JWT. - // This preserves any namespace directly present in the context which is needed for - // requests originating from dgraph internal code like server.go::GetGQLSchema() where - // context is created by hand. - ctx = AttachNamespace(ctx, ns) - } - return ctx -} - // AttachNamespace adds given namespace to the metadata of the context. func AttachNamespace(ctx context.Context, namespace uint64) context.Context { md, ok := metadata.FromIncomingContext(ctx) @@ -474,19 +448,6 @@ func AttachNamespace(ctx context.Context, namespace uint64) context.Context { return metadata.NewIncomingContext(ctx, md) } -// AttachJWTNamespaceOutgoing attaches the namespace in the JWT claims to the outgoing metadata of -// the context. -func AttachJWTNamespaceOutgoing(ctx context.Context) (context.Context, error) { - if !WorkerConfig.AclEnabled { - return AttachNamespaceOutgoing(ctx, RootNamespace), nil - } - ns, err := ExtractNamespaceFrom(ctx) - if err != nil { - return ctx, err - } - return AttachNamespaceOutgoing(ctx, ns), nil -} - // AttachNamespaceOutgoing adds given namespace in the outgoing metadata of the context. func AttachNamespaceOutgoing(ctx context.Context, namespace uint64) context.Context { md, ok := metadata.FromOutgoingContext(ctx)