Skip to content

Commit

Permalink
Revert "fix: fixing audit logs for websocket connections (#8048) (#8053
Browse files Browse the repository at this point in the history
…) (#8572)"

This reverts commit 8c5ce65.
  • Loading branch information
all-seeing-code authored Jan 24, 2023
1 parent 8c5ce65 commit 79e287b
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 93 deletions.
2 changes: 1 addition & 1 deletion edgraph/access_ee.go
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,7 @@ func AuthGuardianOfTheGalaxy(ctx context.Context) error {
if !x.WorkerConfig.AclEnabled {
return nil
}
ns, err := x.ExtractNamespaceFrom(ctx)
ns, err := x.ExtractJWTNamespace(ctx)
if err != nil {
return status.Error(codes.Unauthenticated,
"AuthGuardianOfTheGalaxy: extracting jwt token, error: "+err.Error())
Expand Down
6 changes: 3 additions & 3 deletions edgraph/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ func filterTablets(ctx context.Context, ms *pb.MembershipState) error {
if !x.WorkerConfig.AclEnabled {
return nil
}
namespace, err := x.ExtractNamespaceFrom(ctx)
namespace, err := x.ExtractJWTNamespace(ctx)
if err != nil {
return errors.Errorf("Namespace not found in JWT.")
}
Expand Down Expand Up @@ -1547,7 +1547,7 @@ func validateNamespace(ctx context.Context, tc *api.TxnContext) error {
return nil
}

ns, err := x.ExtractNamespaceFrom(ctx)
ns, err := x.ExtractJWTNamespace(ctx)
if err != nil {
return err
}
Expand All @@ -1571,7 +1571,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.ExtractJWTNamespace(ctx); err == nil {
annotateNamespace(span, ns)
}
annotateStartTs(span, tc.StartTs)
Expand Down
1 change: 0 additions & 1 deletion ee/audit/audit_ee.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ const (
PoorManAuth = "PoorManAuth"
Grpc = "Grpc"
Http = "Http"
WebSocket = "Websocket"
)

var auditor = &auditLogger{}
Expand Down
8 changes: 1 addition & 7 deletions ee/audit/interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// +build oss

/*
* Copyright 2023 Dgraph Labs, Inc. and Contributors
* Copyright 2022 Dgraph Labs, Inc. and Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,8 +23,6 @@ import (
"context"
"net/http"

"github.com/dgraph-io/dgraph/graphql/schema"

"google.golang.org/grpc"
)

Expand All @@ -38,7 +36,3 @@ func AuditRequestHttp(next http.Handler) http.Handler {
next.ServeHTTP(w, r)
})
}

func AuditWebSockets(ctx context.Context, req *schema.Request) {
return
}
66 changes: 6 additions & 60 deletions ee/audit/interceptor_ee.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// +build !oss

/*
* Copyright 2023 Dgraph Labs, Inc. and Contributors
* Copyright 2022 Dgraph Labs, Inc. and Contributors
*
* Licensed under the Dgraph Community License (the "License"); you
* may not use this file except in compliance with the License. You
Expand All @@ -20,25 +20,23 @@ import (
"fmt"
"io"
"io/ioutil"
"net"
"net/http"
"regexp"
"strconv"
"strings"
"sync/atomic"

"github.com/dgraph-io/dgraph/graphql/schema"
"github.com/dgraph-io/dgraph/x"
"github.com/dgraph-io/gqlparser/v2/ast"
"github.com/dgraph-io/gqlparser/v2/parser"

"github.com/golang/glog"
"github.com/gorilla/websocket"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/metadata"
"google.golang.org/grpc/peer"
"google.golang.org/grpc/status"

"github.com/dgraph-io/dgraph/graphql/schema"
"github.com/dgraph-io/dgraph/x"
"github.com/dgraph-io/gqlparser/v2/ast"
"github.com/dgraph-io/gqlparser/v2/parser"
)

const (
Expand Down Expand Up @@ -94,19 +92,6 @@ func AuditRequestHttp(next http.Handler) http.Handler {
next.ServeHTTP(w, r)
return
}

// Websocket connection in graphQl happens differently. We only get access tokens and
// metadata in payload later once the connection is upgraded to correct protocol.
// Doc: https://github.com/apollographql/subscriptions-transport-ws/blob/v0.9.4/PROTOCOL.md
//
// Auditing for websocket connections will be handled by graphql/admin/http.go:154#Subscribe
for _, subprotocol := range websocket.Subprotocols(r) {
if subprotocol == "graphql-ws" {
next.ServeHTTP(w, r)
return
}
}

rw := NewResponseWriter(w)
var buf bytes.Buffer
tee := io.TeeReader(r.Body, &buf)
Expand All @@ -117,45 +102,6 @@ func AuditRequestHttp(next http.Handler) http.Handler {
})
}

func AuditWebSockets(ctx context.Context, req *schema.Request) {
if atomic.LoadUint32(&auditEnabled) == 0 {
return
}

namespace := uint64(0)
var err error
var user string
// TODO(anurag): X-Dgraph-AccessToken should be exported as a constant
if token := req.Header.Get("X-Dgraph-AccessToken"); token != "" {
user = getUser(token, false)
namespace, err = x.ExtractNamespaceFromJwt(token)
if err != nil {
glog.Warningf("Error while auditing websockets: %s", err)
}
} else if token := req.Header.Get("X-Dgraph-AuthToken"); token != "" {
user = getUser(token, true)
} else {
user = getUser("", false)
}

ip := ""
if peerInfo, ok := peer.FromContext(ctx); ok {
ip, _, _ = net.SplitHostPort(peerInfo.Addr.String())
}

auditor.Audit(&AuditEvent{
User: user,
Namespace: namespace,
ServerHost: x.WorkerConfig.MyAddr,
ClientHost: ip,
Endpoint: "/graphql",
ReqType: WebSocket,
Req: truncate(req.Query, maxReqLength),
Status: http.StatusText(http.StatusOK),
QueryParams: nil,
})
}

func auditGrpc(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo) {
clientHost := ""
if p, ok := peer.FromContext(ctx); ok {
Expand Down
5 changes: 1 addition & 4 deletions graphql/admin/http.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 Dgraph Labs, Inc. and Contributors
* Copyright 2022 Dgraph Labs, Inc. and Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -33,7 +33,6 @@ import (
"go.opencensus.io/trace"

"github.com/dgraph-io/dgraph/edgraph"
"github.com/dgraph-io/dgraph/ee/audit"
"github.com/dgraph-io/dgraph/graphql/api"
"github.com/dgraph-io/dgraph/graphql/resolve"
"github.com/dgraph-io/dgraph/graphql/schema"
Expand Down Expand Up @@ -195,8 +194,6 @@ func (gs *graphqlSubscription) Subscribe(
Variables: variableValues,
Header: reqHeader,
}

audit.AuditWebSockets(ctx, req)
namespace := x.ExtractNamespaceHTTP(&http.Request{Header: reqHeader})
glog.Infof("namespace: %d. Got GraphQL request over websocket.", namespace)
// first load the schema, then do anything else
Expand Down
23 changes: 9 additions & 14 deletions x/jwt_helper.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2017-2023 Dgraph Labs, Inc. and Contributors
* Copyright 2017-2022 Dgraph Labs, Inc. and Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,7 +18,6 @@ package x

import (
"context"
"fmt"

"github.com/dgrijalva/jwt-go"
"github.com/pkg/errors"
Expand Down Expand Up @@ -57,23 +56,19 @@ func ExtractUserName(jwtToken string) (string, error) {
return userId, nil
}

func ExtractNamespaceFromJwt(jwtToken string) (uint64, error) {
claims, err := ParseJWT(jwtToken)
func ExtractJWTNamespace(ctx context.Context) (uint64, error) {
jwtString, err := ExtractJwt(ctx)
if err != nil {
return 0, err
}
claims, err := ParseJWT(jwtString)
if err != nil {
return 0, errors.Wrap(err, "extracting namespace from JWT")
return 0, err
}

namespace, ok := claims["namespace"].(float64)
if !ok {
return 0, errors.Errorf("namespace in claims is not valid:%v", namespace)
}
return uint64(namespace), nil
}

func ExtractNamespaceFrom(ctx context.Context) (uint64, error) {
jwtString, err := ExtractJwt(ctx)

if err != nil {
return 0, fmt.Errorf("extracting namespace from JWT %w", err)
}
return ExtractNamespaceFromJwt(jwtString)
}
6 changes: 3 additions & 3 deletions x/x.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func GqlErrorf(message string, args ...interface{}) *GqlError {
func ExtractNamespaceHTTP(r *http.Request) uint64 {
ctx := AttachAccessJwt(context.Background(), r)
// Ignoring error because the default value is zero anyways.
namespace, _ := ExtractNamespaceFrom(ctx)
namespace, _ := ExtractJWTNamespace(ctx)
return namespace
}

Expand Down Expand Up @@ -441,7 +441,7 @@ func AttachJWTNamespace(ctx context.Context) context.Context {
return AttachNamespace(ctx, GalaxyNamespace)
}

ns, err := ExtractNamespaceFrom(ctx)
ns, err := ExtractJWTNamespace(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
Expand Down Expand Up @@ -469,7 +469,7 @@ func AttachJWTNamespaceOutgoing(ctx context.Context) (context.Context, error) {
if !WorkerConfig.AclEnabled {
return AttachNamespaceOutgoing(ctx, GalaxyNamespace), nil
}
ns, err := ExtractNamespaceFrom(ctx)
ns, err := ExtractJWTNamespace(ctx)
if err != nil {
return ctx, err
}
Expand Down

0 comments on commit 79e287b

Please sign in to comment.