Skip to content

Properly handle multiple trusted certs #123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
oskarhane merged 2 commits into from
Sep 5, 2016
Merged

Properly handle multiple trusted certs #123

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6ae929
Properly handle multiple trusted certs
pontusmelke Aug 26, 2016
adc1727
Fixes from code review.
pontusmelke Aug 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/v1/internal/ch-node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ const TrustStrategy = {
}

let tlsOpts = {
ca: opts.trustedCertificates.map(fs.readFileSync),
ca: opts.trustedCertificates.map((f) => fs.readFileSync(f)),
// Because we manually check for this in the connect callback, to give
// a more helpful error to the user
rejectUnauthorized: false
Expand Down
20 changes: 19 additions & 1 deletion test/internal/tls.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var fs = require("fs");
var path = require('path');
var hasFeature = require("../../lib/v1/internal/features");

describe('trust-signed-certificates', function() {
fdescribe('trust-signed-certificates', function() {

var driver;

Expand Down Expand Up @@ -65,6 +65,24 @@ describe('trust-signed-certificates', function() {
driver.session().run( "RETURN 1").then( done );
});

it('should handle multiple certificates', function(done) {
// Assuming we only run this test on NodeJS with TOFU support
if( !NodeChannel.available ) {
done();
return;
}

// Given
driver = neo4j.driver("bolt://localhost", neo4j.auth.basic("neo4j", "neo4j"), {
encrypted: true,
trust: "TRUST_SIGNED_CERTIFICATES",
trustedCertificates: ["build/neo4j/certificates/neo4j.cert", "build/neo4j/certificates/neo4j.cert"]
Copy link

@swist swist Aug 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this matters, but you are passing in the same certificate path twice. Also the logic in this PR shouldn't change syntactically, the before/after change code means essentially the same?

Copy link
Contributor Author

@pontusmelke pontusmelke Aug 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was mostly being lazy, I just wanted to test that you can supply multiple certificates, but you're right it does make the test slightly confusing. I'll fix it.

No this PR should not change anything, only fixing the bad usage of map

});

// When
driver.session().run( "RETURN 1").then( done );
});

afterEach(function(){
if( driver ) {
driver.close();
Expand Down