fix(/commit): protect the commit endpoint via acl

[NamanJain8]

/commit endpoint was not ACL protected. In a multi-tenant system, it could be disastrous where a malicious user can commit or abort the transactions of any namespace. This PR fixes this.

Fixes DGRAPH-3129

---

This change is 

[NamanJain8]

Reviewable status: 0 of 7 files reviewed, 1 unresolved discussion (waiting on @manishrjain, @martinmr, and @vvbalaji-dgraph)

---

edgraph/server.go, line 1471 at r1 (raw file):

	txn := posting.Oracle().GetTxn(startTs)
	if txn == nil {

TODO: Check if this nil check is necessary. Currently, added it as a failsafe.

[aman-bansal]

Reviewed 2 of 7 files at r1.
Reviewable status: 2 of 7 files reviewed, 2 unresolved discussions (waiting on @manishrjain, @martinmr, @NamanJain8, and @vvbalaji-dgraph)

---

edgraph/server.go, line 1490 at r1 (raw file):

	}

	if x.WorkerConfig.AclEnabled {

I believe this wont work. you need to wait for all the raft entries to catchup.
see this if err := posting.Oracle().WaitForTs(ctx, m.StartTs); err != nil { return err }

then only you can ensure that txn would be there

[manishrjain]

Do address @aman-bansal 's review comments.

Reviewed 7 of 7 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @martinmr, @NamanJain8, and @vvbalaji-dgraph)

---

dgraph/cmd/alpha/http.go, line 507 at r1 (raw file):

	tctx, err := (&edgraph.Server{}).CommitOrAbort(ctx, tc)
	switch {
	case tctx.Aborted == true:

no need to say == true.

---

edgraph/server.go, line 1476 at r1 (raw file):

	if txn.Namespace != ns {
		return errors.New("Please log in into correct namespace.")

Mention the namespace that the user has. Don't mention the namespace that the txn has.

[manishrjain]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @aman-bansal, @martinmr, @NamanJain8, and @vvbalaji-dgraph)

---

edgraph/server.go, line 1471 at r1 (raw file):

Previously, NamanJain8 (Naman Jain) wrote…

TODO: Check if this nil check is necessary. Currently, added it as a failsafe.

we can return an error, that txn is not there.

---

edgraph/server.go, line 1490 at r1 (raw file):

Previously, aman-bansal (aman bansal) wrote…

I believe this wont work. you need to wait for all the raft entries to catchup.
see this if err := posting.Oracle().WaitForTs(ctx, m.StartTs); err != nil { return err }

then only you can ensure that txn would be there

That won't help. Because StartTs is assigned before mutations are applied. So, a server could be already at startTs, but not have the mutations.

Typically, the server which does the mutations, also gets the commit request. That way, the txn would be in the cache.

[ahsanbarkati]

Looks good

The changes requested are no longer valid. Discussed the PR with Manish and Ahsan, its good to merge.