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

🌱 Add sendFingerprint option to signIn #82

Merged
merged 3 commits into from
Jan 17, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Read our [contributing guidelines](./CONTRIBUTING.md) if you wish to contribute.
* [unlockAccount](#unlockaccountoptions)
* [verifyRecoveryToken](#verifyrecoverytokenoptions)
* [webfinger](#webfingeroptions)
* [fingerprint] (#fingerprintoptions)
* [fingerprint](#fingerprintoptions)
* [tx.resume](#txresume)
* [tx.exists](#txexists)
* [transaction.status](#transactionstatus)
Expand Down Expand Up @@ -132,6 +132,7 @@ The goal of an authentication flow is to [set an Okta session cookie on the user

- `username` - User’s non-qualified short-name (e.g. dade.murphy) or unique fully-qualified login (e.g [email protected])
- `password` - The password of the user
- `sendFingerprint` - Enabling this will send a `X-Device-Fingerprint` header. Defaults to `false`

Choose a reason for hiding this comment

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

it should probably be noted that this is only honored with certain features. We would be able to get a fingerprint without any features but from a usability standpoint, it is only used in conjunction with certain FFs. In fact I think we ignore the header if the flags are not enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We occasionally include features behind FF (OIDC is an example). As long as sending the header doesn't break anything, I'm not too worried about it.

Choose a reason for hiding this comment

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

nit: I believe there is an extra space in - Enabling


```javascript
authClient.signIn({
Expand Down
18 changes: 17 additions & 1 deletion lib/clientBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,23 @@ proto.features.isTokenVerifySupported = function() {

// { username, password, (relayState), (context) }
proto.signIn = function (opts) {
return tx.postToTransaction(this, '/api/v1/authn', opts);
var sdk = this;
opts = util.clone(opts || {});
function postToTransaction(options) {
delete opts.sendFingerprint;
return tx.postToTransaction(sdk, '/api/v1/authn', opts, options);
}
if (!opts.sendFingerprint) {
return postToTransaction();
}
return sdk.fingerprint()
.then(function(fingerprint) {
return postToTransaction({
headers: {
'X-Device-Fingerprint': fingerprint
}
});
});
};

proto.signOut = function () {
Expand Down
4 changes: 2 additions & 2 deletions lib/tx.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ function transactionExists(sdk) {
return !!sdk.tx.exists._getCookie(config.STATE_TOKEN_COOKIE_NAME);
}

function postToTransaction(sdk, url, options) {
return http.post(sdk, url, options)
function postToTransaction(sdk, url, body, options) {

Choose a reason for hiding this comment

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

nit: we could name it args instead of body just for sake of consistency with http.post signature

return http.post(sdk, url, body, options)
.then(function(res) {
return new AuthTransaction(sdk, res);
});
Expand Down
63 changes: 55 additions & 8 deletions test/spec/fingerprint.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
define(function(require) {
var OktaAuth = require('OktaAuth');
var util = require('../util/util');
var packageJson = require('../../package.json');

describe('fingerprint', function() {
function setup(options) {
Expand Down Expand Up @@ -59,17 +60,17 @@ define(function(require) {
});
});

var authClient = new OktaAuth({
var authClient = options.authClient || new OktaAuth({
url: 'http://example.okta.com'
});
if (typeof options.userAgent !== 'undefined') {
util.mockUserAgent(authClient, options.userAgent);
}
return authClient.fingerprint({ timeout: options.timeout });
return authClient;
}

it('iframe is created with the right src and it is hidden', function (done) {
return setup()
return setup().fingerprint()
.catch(function(err) {
expect(err).toBeUndefined();
})
Expand All @@ -88,7 +89,7 @@ define(function(require) {
});

it('allows non-Okta postMessages', function (done) {
return setup({ sendOtherMessage: true })
return setup({ sendOtherMessage: true }).fingerprint()
.catch(function(err) {
expect(err).toBeUndefined();
})
Expand All @@ -101,7 +102,7 @@ define(function(require) {
});

it('fails if the iframe sends invalid message content', function (done) {
return setup({ firstMessage: 'invalidMessageContent' })
return setup({ firstMessage: 'invalidMessageContent' }).fingerprint()
.then(function() {
done.fail('Fingerprint promise should have been rejected');
})
Expand All @@ -112,7 +113,7 @@ define(function(require) {
});

it('fails if user agent is not defined', function (done) {
return setup({ userAgent: '' })
return setup({ userAgent: '' }).fingerprint()
.then(function() {
done.fail('Fingerprint promise should have been rejected');
})
Expand All @@ -125,7 +126,7 @@ define(function(require) {
it('fails if it is called from a Windows phone', function (done) {
return setup({
userAgent: 'Mozilla/5.0 (compatible; MSIE 9.0; Windows Phone OS 7.5; Trident/5.0;)'
})
}).fingerprint()
.then(function() {
done.fail('Fingerprint promise should have been rejected');
})
Expand All @@ -136,7 +137,7 @@ define(function(require) {
});

it('fails after a timeout period', function (done) {
return setup({ timeout: 5 })
return setup({ timeout: true }).fingerprint({ timeout: 5 })
.then(function() {
done.fail('Fingerprint promise should have been rejected');
})
Expand All @@ -145,5 +146,51 @@ define(function(require) {
done();
});
});

util.itMakesCorrectRequestResponse({
title: 'attaches fingerprint to signIn requests if addFingerprint is true',
setup: {
uri: 'http://example.okta.com',
calls: [
{
request: {
method: 'post',
uri: '/api/v1/authn',
data: { username: 'not', password: 'real' },
headers: {
'Accept': 'application/json',
'Content-Type': 'application/json',
'X-Okta-User-Agent-Extended': 'okta-auth-js-' + packageJson.version,
'X-Device-Fingerprint': 'ABCD'
}
},
response: 'success'
}
]
},
execute: function (test) {
return setup({ authClient: test.oa }).signIn({
username: 'not',
password: 'real',
sendFingerprint: true

Choose a reason for hiding this comment

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

not sure if we have one test already, but if not maybe we should add one with sendFingerprint: false or sendFingerptint: undefined

Copy link
Contributor Author

@lboyette-okta lboyette-okta Jan 16, 2018

Choose a reason for hiding this comment

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

We have tests for signIn that don't pass sendFingerprint, but none that send false

});
}
});

it('fails signIn request if fingerprinting fails', function(done) {
return setup({ firstMessage: 'invalidMessageContent' })
.signIn({
username: 'not',
password: 'real',
sendFingerprint: true
})
.then(function() {
done.fail('signIn promise should have been rejected');
})
.catch(function(err) {
util.assertAuthSdkError(err, 'Unable to parse iframe response');
done();
});
});
});
});