Change Encrypt to not re-encrypt password values.

[srfrog]

This PR adds a check on passwords sent to Encrypt() so that already encrypted values are returned as-is. This allows for exported passwords to be imported.

Closes #2765

---

This change is 

[manishrjain]

Can you please add tests to ensure that this works?

Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion (waiting on @srfrog and @manishrjain)

---

rdf/parse.go, line 133 at r1 (raw file):

			}
			// if this is a password value dont re-encrypt.
			if t == types.PasswordID {

No need for this special casing. types.Convert below should already take care of this.

[srfrog]

Reviewable status: 0 of 8 files reviewed, 1 unresolved discussion (waiting on @manishrjain)

---

rdf/parse.go, line 133 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

No need for this special casing. types.Convert below should already take care of this.

Convert will try to make the string value into a password, which is why it's encrypting twice. Here, we know it is a password so we dont need the conversion. Either we tell convert to treat this as a password or we skip convert.

[manishrjain]

Almost there. A few comments.

Reviewable status: 0 of 9 files reviewed, 1 unresolved discussion (waiting on @srfrog)

---

systest/live_pw_test.go, line 34 at r2 (raw file):

	tmpDir := filepath.Join(topDir, "test1")
	check(t, os.Mkdir(tmpDir, 0700))
	cluster := NewDgraphCluster(tmpDir)

I've been working towards getting rid of NewDgraphCluster in many test paths over the past months. test.sh script ensures that the cluster is already running, and available for you to use at port 9180. You can run Alter dropall, before running live loader.

So, do not run with NewDgraphCluster.

[srfrog]

Reviewable status: 0 of 7 files reviewed, 1 unresolved discussion (waiting on @manishrjain)

---

systest/live_pw_test.go, line 34 at r2 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

I've been working towards getting rid of NewDgraphCluster in many test paths over the past months. test.sh script ensures that the cluster is already running, and available for you to use at port 9180. You can run Alter dropall, before running live loader.

So, do not run with NewDgraphCluster.

Done.

[gitlw]

Reviewable status: 0 of 7 files reviewed, 2 unresolved discussions (waiting on @manishrjain and @srfrog)

---

systest/live_pw_test.go, line 70 at r3 (raw file):

Quoted 8 lines of code…

	// x.UpdateHealthStatus(true)
	// resp, err := http.Get("http://127.0.0.1:8180/admin/export")
	// require.NoError(t, err)
 
	// b, err := ioutil.ReadAll(resp.Body)
	// require.NoError(t, err)
	// defer resp.Body.Close()
	// require.Contains(t, string(b), "Success", "export failed: %v", string(b))

Remove these commented out lines. Otherwise LGTM. Thanks!

[gitlw]

Reviewed 3 of 4 files at r1, 3 of 6 files at r2, 1 of 3 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain and @srfrog)

[srfrog]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain and @gitlw)

---

systest/live_pw_test.go, line 70 at r3 (raw file):

Previously, gitlw (Lucas Wang) wrote…

	// x.UpdateHealthStatus(true)
	// resp, err := http.Get("http://127.0.0.1:8180/admin/export")
	// require.NoError(t, err)
 
	// b, err := ioutil.ReadAll(resp.Body)
	// require.NoError(t, err)
	// defer resp.Body.Close()
	// require.Contains(t, string(b), "Success", "export failed: %v", string(b))

Remove these commented out lines.

Thanks

[srfrog]

Reviewable status: 6 of 7 files reviewed, 2 unresolved discussions (waiting on @gitlw and @manishrjain)

---

systest/live_pw_test.go, line 70 at r3 (raw file):

Previously, srfrog (Gus) wrote…

Thanks

Done.

[gitlw]

Reviewed 1 of 1 files at r4.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @manishrjain and @gitlw)

[manishrjain]

Got a few comments. Address those and feel free to merge. Thanks for fixing this issue!

Reviewed 3 of 4 files at r1, 3 of 6 files at r2, 1 of 1 files at r4.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @manishrjain, @gitlw, and @srfrog)

---

systest/live_pw_test.go, line 66 at r4 (raw file):

	{
	  q(func: uid(`+assigned.Uids["uid2"]+`)) {
			secret: checkpwd(secret, "password2")

Check other two as well? Also, check a couple failures?

---

systest/live_pw_test.go, line 82 at r4 (raw file):

		CommitNow: true,
		SetNquads: []byte(`
			<_:uid1> <secret> "$2a$10$0Cv9uJBUhG2FstnCUNw2/.GNH7M89M.yaXn3//Zp8a0.s6zVIJFz6"^^<xs:password> .

Add a comment saying that this could break in future versions of bcrypt library.

[manishrjain]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @manishrjain, @gitlw, and @srfrog)

[srfrog]

Reviewable status: 6 of 7 files reviewed, 4 unresolved discussions (waiting on @gitlw and @manishrjain)

---

systest/live_pw_test.go, line 66 at r4 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Check other two as well? Also, check a couple failures?

Done.

---

systest/live_pw_test.go, line 82 at r4 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Add a comment saying that this could break in future versions of bcrypt library.

Done.