Skip to content
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.

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

feat: ActiveDirectoryPassword and sqlpassword authentication support #707

Merged
merged 103 commits into from
Nov 21, 2018
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
103 commits
Select commit Hold shift + click to select a range
6e2d073
feat: aad authentication support
Feb 8, 2018
8c971f8
chore: add linebreak
Feb 8, 2018
b4441fd
chore: lint fix
Feb 8, 2018
6cfa3cd
chore: fix else block indentations
Feb 13, 2018
4d91c4f
refactor: use error instead of assert
Feb 13, 2018
511338d
chore: merge branch master
Mar 1, 2018
1cad62c
refactor: remove authentication from connection class configs
Mar 12, 2018
9f691c9
test: add feature ext parser test
Mar 12, 2018
de5e3b8
refactor: export login7paylod by name
Mar 13, 2018
8a9b876
refactor: pass parsed data to callback
Mar 13, 2018
dd2bbc9
fix: add featureextack event to catch errors
Mar 13, 2018
4dd2b1f
test: add error cases for invalid aad auth
Mar 13, 2018
fc24867
test: add unit test for fedauth info parser
Mar 13, 2018
12a902f
chore: removed required flow
Mar 13, 2018
fe0122b
chore: rename Pos to offset
Mar 14, 2018
ce3eceb
test: writeuint32atoffset unittest
Mar 14, 2018
ce13bdc
chore: rename writeuin32leatpos to offset
Mar 15, 2018
c233c74
chore: edit the error messages
Mar 16, 2018
2cd3240
Merge branch 'master' into federated-auth
May 25, 2018
e36869a
ci: enable azure active directory testing in ci
May 25, 2018
036433d
ci: update travis configuration
arthurschreiber May 26, 2018
f6772bf
ci: fix a typo in the travis configuration
arthurschreiber May 26, 2018
1ad29ed
ci: tweak travis configuration a bit
arthurschreiber May 26, 2018
eba1623
ci: fix variable substitution
arthurschreiber May 26, 2018
49558e2
ci: don't run azure tests for pull requests
arthurschreiber May 26, 2018
eca2499
ci: update travis config to use the correct variables
arthurschreiber May 26, 2018
62a24fd
refactor: remove async times
Jun 5, 2018
ee2714b
fix: parse multiple featues
Jun 5, 2018
3e07782
refactor: test multiple features
Jun 5, 2018
8df7254
Merge branch master
Jun 5, 2018
0702b77
fix: store acks in map
Jun 7, 2018
eb71906
fix: check fedauth ack before transition to login7
Jun 14, 2018
800e9a4
chore: rename featureextack and preloginrequired
Jun 14, 2018
d63c358
fix: check for feature ext ack before login
Jun 14, 2018
244c05a
chore: fix linter issues
Jun 15, 2018
c7663fb
fix: connecting with sqlpassword and non ad user
Jun 15, 2018
80a9313
fix: undefined property error
Jul 13, 2018
6b4a86d
chore: Merge branch master
Jul 13, 2018
cc71fe9
Merge branch 'master' of github.com:tediousjs/tedious into federated-…
arthurschreiber Jul 14, 2018
aa4437e
Merge branch 'master' of github.com:pekim/tedious into federated-auth
arthurschreiber Jul 24, 2018
b2645cc
Merge branch 'master' of github.com:pekim/tedious into federated-auth
arthurschreiber Jul 24, 2018
c2168cd
test: update `Login7Payload` test cases
arthurschreiber Jul 24, 2018
ea96250
fix: fix failure using regular authentication
arthurschreiber Jul 24, 2018
1257d5b
ci: remove `SqlPassword` authentication type from Travis config
arthurschreiber Jul 25, 2018
d42004e
refactor: start working on a new `authentication` config option
arthurschreiber Jul 25, 2018
ca3aba5
Merge branch 'federated-auth' into federated-auth
Aug 22, 2018
78e9460
chore: merge branch master
Sep 24, 2018
0a28dd1
fix: config validations
Sep 27, 2018
ef3860d
chore: Merge branch 'master'
Sep 27, 2018
9759b65
fix: travis config
Sep 27, 2018
c183a1b
chore: fix linter issue
Sep 27, 2018
3f62dbd
fix: flow type
Sep 28, 2018
2da13de
chore: add missing fedauthinfo flag
Sep 28, 2018
7e9f341
fix: add removed fedauthreq flag
Sep 28, 2018
59e5386
chore: fix merge conflicts
Oct 23, 2018
d8c8f97
fix: buffer allocation, error handling
Oct 24, 2018
b212b29
chore: linter
Oct 24, 2018
8029edc
Merge branch 'master' of github.com:tediousjs/tedious into federated-…
arthurschreiber Oct 26, 2018
851630e
ci: update Travis config for Azure AD Auth
arthurschreiber Oct 26, 2018
16c8ae6
fix: authentication type check
arthurschreiber Nov 3, 2018
60e11a7
fix: check for authentication type
arthurschreiber Nov 3, 2018
5ec11aa
Merge branch 'master' of github.com:pekim/tedious into federated-auth
arthurschreiber Nov 3, 2018
34321da
chore: update `package-lock.json`
arthurschreiber Nov 3, 2018
6066810
refactor: remove unused property
arthurschreiber Nov 3, 2018
fc923e4
refactor: remove unused code
arthurschreiber Nov 3, 2018
4515d4c
refactor: remove `fedAuthLibrary` property
arthurschreiber Nov 3, 2018
2325bdb
refactor: small code cleanups
arthurschreiber Nov 3, 2018
79ab731
refactor: always signal federated authentication support
arthurschreiber Nov 3, 2018
db5dd73
refactor: move logic into the state
arthurschreiber Nov 3, 2018
078f384
refactor: remove `featureExtAckPending` property
arthurschreiber Nov 3, 2018
9b453c1
refactor: remove `requiredPreLoginResponse` property
arthurschreiber Nov 3, 2018
df4570e
refactor: get rid of `fedAuthInfo` property
arthurschreiber Nov 3, 2018
64324d5
refactor: simplify prelogin payload processing
arthurschreiber Nov 3, 2018
d658927
test: skip connection retry tests when authenticating via Azure AD
arthurschreiber Nov 3, 2018
e76668b
test: fix bad credential tests to support Azure AD authentication
arthurschreiber Nov 3, 2018
42dadb1
refactor: simplify offset tracking when writing to a buffer
arthurschreiber Nov 3, 2018
263cde1
fix: use `Buffer.alloc`
arthurschreiber Nov 3, 2018
3b65260
refactor: remove `return` statements
arthurschreiber Nov 3, 2018
c77cd55
refactor: remove unused code
arthurschreiber Nov 3, 2018
14d3d22
style: remove empty line
arthurschreiber Nov 3, 2018
7f602ab
test: fix checking for Azure AD authentication
arthurschreiber Nov 3, 2018
d393e7d
test: update prelogin payload tests
arthurschreiber Nov 3, 2018
60b64dd
test: drop tests for dropped function
arthurschreiber Nov 3, 2018
45921fc
refactor: simplify the FEDAUTHINFO token parser code
arthurschreiber Nov 3, 2018
f242402
fix: do not use the global ADAL authentication cache
arthurschreiber Nov 3, 2018
75e8ff4
refactor: simplify the feature ext ack parser code
arthurschreiber Nov 3, 2018
532b969
test: handle missing `CREATE XML SCHEMA COLLECTION` permissions
arthurschreiber Nov 3, 2018
544f2e1
test: assert connection was established correctly
arthurschreiber Nov 3, 2018
9c9ffa5
test: update feature ext parser tests
arthurschreiber Nov 3, 2018
cd388dc
test: update login7 payload tests
arthurschreiber Nov 3, 2018
fffa06a
test: skip TVP test cases when missing permissions
arthurschreiber Nov 4, 2018
a10055f
fix: use the global ADAL cache
arthurschreiber Nov 4, 2018
e7b1bb5
test: skip TVP tests on older TDS versions
arthurschreiber Nov 4, 2018
d39dfc1
test: fix skipping of tests
arthurschreiber Nov 4, 2018
2acfa02
test: use temporary procedures
arthurschreiber Nov 5, 2018
231962e
test: use temporary procedure
arthurschreiber Nov 5, 2018
2837053
test: handle setup failures more gracefully
arthurschreiber Nov 5, 2018
19be9f6
test: increase timeouts
arthurschreiber Nov 5, 2018
4face04
fix: rename `azure-active-directory` to `active-directory-password`
arthurschreiber Nov 18, 2018
e3a5a89
Merge branch 'master' of github.com:tediousjs/tedious into federated-…
arthurschreiber Nov 18, 2018
8a597f9
text: update tests for authentication type rename
arthurschreiber Nov 18, 2018
01a36f8
fix: rename config option back
arthurschreiber Nov 19, 2018
572e97b
Merge branch 'master' into federated-auth
arthurschreiber Nov 21, 2018
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
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,14 @@
"tag": "next"
},
"dependencies": {
"adal-node": "^0.1.22",
"babel-runtime": "^6.26.0",
"big-number": "0.3.1",
"bl": "^1.2.0",
"commitlint": "^6.1.0",
"depd": "^1.1.2",
"iconv-lite": "^0.4.11",
"punycode": "^2.1.0",
"readable-stream": "^2.2.6",
"sprintf-js": "^1.1.1",
"punycode": "^2.1.0"
Expand Down
170 changes: 161 additions & 9 deletions src/connection.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
const deprecate = require('depd')('tedious');

const BulkLoad = require('./bulk-load');
const Debug = require('./debug');
const EventEmitter = require('events').EventEmitter;
const InstanceLookup = require('./instance-lookup').InstanceLookup;
const TransientErrorLookup = require('./transient-error-lookup.js').TransientErrorLookup;
const TYPE = require('./packet').TYPE;
const PreloginPayload = require('./prelogin-payload');
const Login7Payload = require('./login7-payload');
const Login7Payload = require('./login7-payload').Login7Payload;
const FEDAUTH_OPTIONS = require('./login7-payload').FEDAUTH_OPTIONS;
const NTLMResponsePayload = require('./ntlm-payload');
const Request = require('./request');
const RpcRequestPayload = require('./rpcrequest-payload');
Expand All @@ -20,6 +20,7 @@ const crypto = require('crypto');
const ConnectionError = require('./errors').ConnectionError;
const RequestError = require('./errors').RequestError;
const Connector = require('./connector').Connector;
const AuthenticationContext = require('adal-node').AuthenticationContext;

// A rather basic state machine for managing a connection.
// Implements something approximating s3.2.1.
Expand Down Expand Up @@ -79,6 +80,20 @@ class Connection extends EventEmitter {
throw new TypeError('Invalid server: ' + config.server);
}

this.fedAuthInfo = {
ValidFedAuthEnum: {
SqlPassword: 'SQLPASSWORD',
Copy link
Member

Choose a reason for hiding this comment

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

SqlPassword authentication seems to fail, can you remove it?

ActiveDirectoryPassword: 'ACTIVEDIRECTORYPASSWORD',
// TODO: ActiveDirectoryIntegrated
},
method: undefined,
fedAuthLibrary: undefined,
requiredPreLoginResponse: false,
fedAuthInfoRequested: false,
responsePending: false,
token: undefined
};

if (config.domain != undefined) {
deprecateNonStringConfigValue('domain', config.domain);
}
Expand Down Expand Up @@ -481,7 +496,25 @@ class Connection extends EventEmitter {
deprecateNonBooleanConfigValue('options.useUTC', config.options.useUTC);
this.config.options.useUTC = config.options.useUTC;
}
deprecateNullConfigValue('options.useUTC', config.options.useUTC);

// Whenever authentication property is used to specify an authentication method,
// the client will request encryption (the default value of the Encrypt property will be true)
// and it will validate the server certificate (regardless of the encryption setting), unless TrustServerCertificate = true
if (config.options.authentication != undefined) {
// check for valid options
if (!(config.options.authentication.toUpperCase() === this.fedAuthInfo.ValidFedAuthEnum.SqlPassword ||
config.options.authentication.toUpperCase() === this.fedAuthInfo.ValidFedAuthEnum.ActiveDirectoryPassword)) {
throw new Error('An invalid authentication method is specified');
}
if (this.config.options.tdsVersion < '7_4') {
throw new Error(`Azure Active Directory authentication is not supported in the TDS version ${this.config.options.tdsVersion}`);
}
this.config.options.encrypt = true;
this.fedAuthInfo.method = config.options.authentication;
if (!config.options.trustServerCertificate) {
Copy link
Member

Choose a reason for hiding this comment

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

trustServerCertificate by default is set to true for regular cases here, but it is set to false for AAD. Can we stick with the existing default behaviour here?

Copy link
Author

@Hadis-Knj Hadis-Knj Jun 15, 2018

Choose a reason for hiding this comment

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

Based on MSFT doc "the server certificate will be validated irrespective of the encryption setting unless TrustServerCertificate is set to true". the default in Tedious is true so the mechanism to meet the criteria is to check whether it is set to true explicitly by user or not, and if the user has not set it, change the default to false. the default trustServerCertificate in ADO and JDBC is false and it doesn't change unless it is set in connection string 😺

this.config.options.trustServerCertificate = false;
}
}
}

this.reset = this.reset.bind(this);
Expand All @@ -498,6 +531,7 @@ class Connection extends EventEmitter {
this.transactionDescriptors = [new Buffer([0, 0, 0, 0, 0, 0, 0, 0])];
this.transitionTo(this.STATE.CONNECTING);


if (this.config.options.tdsVersion < '7_2') {
// 'beginTransaction', 'commitTransaction' and 'rollbackTransaction'
// events are utilized to maintain inTransaction property state which in
Expand Down Expand Up @@ -607,6 +641,44 @@ class Connection extends EventEmitter {
this.emit('charsetChange', token.newValue);
});

this.tokenStreamParser.on('fedAuthInfo', (token) => {
const clientId = '7f98cb04-cd1e-40df-9140-3bf7e2cea4db';
if (token.fedAuthInfoData.stsurl && token.fedAuthInfoData.spn) {
this.fedAuthInfo.responsePending = true;
var context = new AuthenticationContext(token.fedAuthInfoData.stsurl);
context.acquireTokenWithUsernamePassword(token.fedAuthInfoData.spn, this.config.userName, this.config.password, clientId, (err, tokenResponse) => {
if (err) {
this.fedAuthInfo.responsePending = false;
this.loginError = ConnectionError('Security token could not be authenticated or authorized.', 'EFEDAUTH');
} else {
this.fedAuthInfo.responsePending = false;
this.fedAuthInfo.token = tokenResponse;
}
});
}
});

this.tokenStreamParser.on('featureExtAck', (token) => {
switch (token.featureId) {
case FEDAUTH_OPTIONS.FEATURE_ID:
if (!this.fedAuthInfo.fedAuthInfoRequested) {
throw new Error('Did not request federated authentication, but received the acknowledgment');
}
switch (this.fedAuthInfo.fedAuthLibrary) {
case FEDAUTH_OPTIONS.LIBRARY_ADAL:
if (0 !== token.featureAckDataLen) {
throw new Error(`Federated authentication acknowledgment for ${this.fedAuthInfo.method} authentication method includes extra data`);
}
break;
default:
throw new Error('Attempting to use unknown federated authentication library');
}
break;
default:
throw new Error('Received acknowledgement for unknown feature');
}
});

this.tokenStreamParser.on('loginack', (token) => {
if (!token.tdsVersion) {
// unsupported TDS version
Expand Down Expand Up @@ -973,7 +1045,8 @@ class Connection extends EventEmitter {

sendPreLogin() {
const payload = new PreloginPayload({
encrypt: this.config.options.encrypt
encrypt: this.config.options.encrypt,
fedAuthRequested: (this.fedAuthInfo.method !== undefined)
});
this.messageIo.sendMessage(TYPE.PRELOGIN, payload.data);
this.debug.payload(function() {
Expand All @@ -994,7 +1067,13 @@ class Connection extends EventEmitter {
this.debug.payload(function() {
return preloginPayload.toString(' ');
});

if (this.fedAuthInfo.method != undefined) {
if (0 !== preloginPayload.fedAuthRequired && 1 !== preloginPayload.fedAuthRequired) {
this.emit('connect', ConnectionError(`Server sent an unexpected response for federated authentication value during negotiation. Value was ${preloginPayload.fedAuthRequired}`, 'EFEDAUTH'));
return this.close();
}
this.fedAuthInfo.requiredPreLoginResponse = (preloginPayload.fedAuthRequired == 1);
}
if (preloginPayload.encryptionString === 'ON' || preloginPayload.encryptionString === 'REQ') {
if (!this.config.options.encrypt) {
this.emit('connect', ConnectionError("Server requires encryption, set 'encrypt' config option to true.", 'EENCRYPT'));
Expand All @@ -1021,7 +1100,8 @@ class Connection extends EventEmitter {
initDbFatal: !this.config.options.fallbackToDefaultDb,
readOnlyIntent: this.config.options.readOnlyIntent,
sspiBlob: clientResponse,
language: this.config.options.language
language: this.config.options.language,
fedAuthInfo: this.fedAuthInfo
});

this.routingData = undefined;
Expand Down Expand Up @@ -1058,6 +1138,22 @@ class Connection extends EventEmitter {
process.nextTick(boundTransitionTo, this.STATE.SENT_NTLM_RESPONSE);
}

sendFedAuthResponsePacket() {
const accessTokenLen = Buffer.byteLength(this.fedAuthInfo.token.accessToken, 'ucs2');
const data = new Buffer(8 + accessTokenLen);
Copy link
Member

Choose a reason for hiding this comment

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

Can you replace Buffer() with Buffer.alloc()?

let offset = 0;
data.writeUInt32LE(accessTokenLen + 4, offset);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Buffer#writeUInt32LE returns the new offset after writing to the buffer, so you could simplify this like:

let offset = 0;
offset = data.writeUInt32LE(accessTokenLen + 4, offset);
offset = data.writeUInt32LE(accessTokenLen, offset);
data.write(this.fedAuthInfo.token.accessToken, offset, 'ucs2');

offset += 4;
data.writeUInt32LE(accessTokenLen, offset);
offset += 4;
data.write(this.fedAuthInfo.token.accessToken, offset, 'ucs2');
this.messageIo.sendMessage(TYPE.FEDAUTH_TOKEN, data);
// sent the fedAuth token message, the rest is similar to standard login 7
process.nextTick(() => {
this.transitionTo(this.STATE.SENT_LOGIN7_WITH_STANDARD_LOGIN);
});
}

// Returns false to apply backpressure.
sendDataToTokenStreamParser(data) {
return this.tokenStreamParser.addBuffer(data);
Expand Down Expand Up @@ -1216,6 +1312,19 @@ class Connection extends EventEmitter {
}
}

processLogin7FedAuthResponse() {
if (this.fedAuthInfo.fedAuthInfoRequested && !this.loginError) {
return this.dispatchEvent('receivedFedAuthInfo');
} else {
if (this.loginError) {
this.emit('connect', this.loginError);
} else {
this.emit('connect', ConnectionError('Login failed.', 'ELOGIN'));
}
return this.dispatchEvent('loginFailed');
}
}

execSqlBatch(request) {
this.makeRequest(request, TYPE.SQL_BATCH, new SqlBatchPayload(request.sqlTextOrProcedure, this.currentTransactionDescriptor(), this.config.options));
}
Expand Down Expand Up @@ -1621,10 +1730,12 @@ Connection.prototype.STATE = {
message: function() {
if (this.messageIo.tlsNegotiationComplete) {
this.sendLogin7Packet(() => {
if (this.config.domain) {
this.transitionTo(this.STATE.SENT_LOGIN7_WITH_NTLM);
if (this.fedAuthInfo.requiredPreLoginResponse) {
return this.transitionTo(this.STATE.SENT_LOGIN7_WITH_FEDAUTH);
} else if (this.config.domain) {
return this.transitionTo(this.STATE.SENT_LOGIN7_WITH_NTLM);
} else {
this.transitionTo(this.STATE.SENT_LOGIN7_WITH_STANDARD_LOGIN);
return this.transitionTo(this.STATE.SENT_LOGIN7_WITH_STANDARD_LOGIN);
Suraiya-Hameed marked this conversation as resolved.
Show resolved Hide resolved
}
});
}
Expand Down Expand Up @@ -1706,6 +1817,47 @@ Connection.prototype.STATE = {
}
}
},
SENT_LOGIN7_WITH_FEDAUTH: {
name: 'SentLogin7Withfedauth',
Copy link
Member

Choose a reason for hiding this comment

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

Can you split this into 2 states, as per 3.2.1 Abstract Data Model and name them to be inline with Sent LOGIN7 Record with Complete Authentication Token State(3.2.5.3) and Sent LOGIN7 Record with Federated Authentication Information Request State(3.2.5.5)?

Copy link
Author

Choose a reason for hiding this comment

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

Sent LOGIN7 Record with Complete Authentication Token State 3.2.5.3 is a replacement to Sent Standard login. it should be implemented in a separate PR over master, created #757 to track it

events: {
socketError: function() {
return this.transitionTo(this.STATE.FINAL);
},
connectTimeout: function() {
return this.transitionTo(this.STATE.FINAL);
},
data: function(data) {
if (this.fedAuthInfo.responsePending) {
// We got data from the server while we're waiting for adal authentication context
// call to complete on the client. We cannot process server data
// until this call completes as the state can change on completion of
// the call. Queue it for later.
const boundDispatchEvent = this.dispatchEvent.bind(this);
return setImmediate(boundDispatchEvent, 'data', data);
} else {
return this.sendDataToTokenStreamParser(data);
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need this if condition in data event. As adal library will be called after fedAuthInfo event gets triggered (which happens after SENT_LOGIN7_WITH_FEDAUTH.data event), it makes sense to have this condition just in message event.

}
},
receivedFedAuthInfo: function() {
return this.sendFedAuthResponsePacket();
},
loginFailed: function() {
return this.transitionTo(this.STATE.FINAL);
},
message: function() {
if (this.fedAuthInfo.responsePending) {
// We got data from the server while we're waiting for adal authentication context
// call to complete on the client. We cannot process server data
// until this call completes as the state can change on completion of
// the call. Queue it for later.
const boundDispatchEvent = this.dispatchEvent.bind(this);
return setImmediate(boundDispatchEvent, 'message');
} else {
return this.processLogin7FedAuthResponse();
}
}
}
},
LOGGED_IN_SENDING_INITIAL_SQL: {
name: 'LoggedInSendingInitialSql',
enter: function() {
Expand Down
Loading