Skip to content

Commit

Permalink
Merge pull request #525 from Microsoft/authCookieLaw
Browse files Browse the repository at this point in the history
Do not store authId in the cookie
  • Loading branch information
Kamil Szostak authored Sep 28, 2017
2 parents c5cbccf + 8d0c8a9 commit f071969
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 15 deletions.
6 changes: 4 additions & 2 deletions API-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,11 @@ You don't usually have to use this, as it happens automatically on window closin
<a name="setAuthenticatedUserContext"></a>
### setAuthenticatedUserContext

setAuthenticatedUserContext(authenticatedUserId: string, accountId?: string)
setAuthenticatedUserContext(authenticatedUserId: string, accountId?: string, storeInCookie = false)

Set the authenticated user id and the account id in this session. Use this when you have identified a specific signed-in user. Parameters must not contain spaces or ,;=|
Set the authenticated user id and the account id. Use this when you have identified a specific signed-in user. Parameters must not contain spaces or ,;=|

The method will only set the `authenticatedUserId` and `accountId` for all events in the current page view. To set them for all events within the whole session, you should either call this method on every page view or set `storeInCookie = true`.

Parameter | Description
---|---
Expand Down
28 changes: 23 additions & 5 deletions JavaScript/JavaScriptSDK.Tests/CheckinTests/Context/User.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,24 @@ class UserContextTests extends TestClass {
}
});

this.testCase({
name: "setAuthenticatedUserContext: auth id and account id is set (not in the cookie)",
test: () => {
// setup
var authAndAccountId = ['bla@bla.com', 'contoso'];
var user = new Microsoft.ApplicationInsights.Context.User(this.getEmptyConfig());
var cookieStub = this.sandbox.stub(Microsoft.ApplicationInsights.Util, "setCookie");

// act
user.setAuthenticatedUserContext(authAndAccountId[0], authAndAccountId[1]);

// verify
Assert.equal('bla@bla.com', user.authenticatedId, "user auth id was not set");
Assert.equal('contoso', user.accountId, "user account id was not set");
Assert.equal(cookieStub.notCalled, true, "cookie was not set");
}
});

this.testCase({
name: "setAuthenticatedUserContext: auth user set in cookie without account id",
test: () => {
Expand All @@ -175,7 +193,7 @@ class UserContextTests extends TestClass {
var user = new Microsoft.ApplicationInsights.Context.User(this.getEmptyConfig());

// act
user.setAuthenticatedUserContext(authAndAccountId[0]);
user.setAuthenticatedUserContext(authAndAccountId[0], null, true);

// verify
Assert.equal(authAndAccountId[0], user.authenticatedId, "user auth id was set");
Expand All @@ -192,7 +210,7 @@ class UserContextTests extends TestClass {
var user = new Microsoft.ApplicationInsights.Context.User(this.getEmptyConfig());

// act
user.setAuthenticatedUserContext(authAndAccountId[0], authAndAccountId[1]);
user.setAuthenticatedUserContext(authAndAccountId[0], authAndAccountId[1], true);

// verify
Assert.equal(authAndAccountId[0], user.authenticatedId, "user auth id was set");
Expand All @@ -209,7 +227,7 @@ class UserContextTests extends TestClass {
var user = new Microsoft.ApplicationInsights.Context.User(this.getEmptyConfig());

// act
user.setAuthenticatedUserContext(authAndAccountId[0]);
user.setAuthenticatedUserContext(authAndAccountId[0], null, true);

// verify
Assert.equal(authAndAccountId[0], user.authenticatedId, "user auth id was set");
Expand Down Expand Up @@ -291,7 +309,7 @@ class UserContextTests extends TestClass {
var loggingStub = this.sandbox.stub(Microsoft.ApplicationInsights._InternalLogging, "throwInternal");

// act
user.setAuthenticatedUserContext(authAndAccountId[0], authAndAccountId[1]);
user.setAuthenticatedUserContext(authAndAccountId[0], authAndAccountId[1], true);

// verify
Assert.equal(undefined, user.authenticatedId, "user auth id was not set");
Expand Down Expand Up @@ -332,7 +350,7 @@ class UserContextTests extends TestClass {
var loggingStub = this.sandbox.stub(Microsoft.ApplicationInsights._InternalLogging, "throwInternal");

// act
user.setAuthenticatedUserContext(authAndAccountId[0], authAndAccountId[1]);
user.setAuthenticatedUserContext(authAndAccountId[0], authAndAccountId[1], true);

// verify
Assert.equal(authAndAccountId[0], user.authenticatedId, "user auth id was set");
Expand Down
15 changes: 15 additions & 0 deletions JavaScript/JavaScriptSDK.Tests/CheckinTests/appInsights.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,21 @@ class AppInsightsTests extends TestClass {
}
});

this.testCase({
name: "AppInsightsTests: set authenticatedId does not set the cookie by default",
test: () => {
let appInsights = new Microsoft.ApplicationInsights.AppInsights(this.getAppInsightsSnippet());
let setAuthStub = this.sandbox.stub(appInsights.context.user, "setAuthenticatedUserContext");

appInsights.setAuthenticatedUserContext("10001");

Assert.equal(true, setAuthStub.calledOnce);
Assert.equal(false, setAuthStub.calledWithExactly("10001", null, false));

setAuthStub.reset();
}
});

this.testCase({
name: "AppInsightsTests: trackPageView sends user-specified duration when passed",
test: () => {
Expand Down
10 changes: 7 additions & 3 deletions JavaScript/JavaScriptSDK/AppInsights.ts
Original file line number Diff line number Diff line change
Expand Up @@ -421,15 +421,19 @@ module Microsoft.ApplicationInsights {
}

/**
* Sets the autheticated user id and the account id in this session.
* Sets the authenticated user id and the account id.
* User auth id and account id should be of type string. They should not contain commas, semi-colons, equal signs, spaces, or vertical-bars.
*
* By default the method will only set the authUserID and accountId for all events in this page view. To add them to all events within
* the whole session, you should either call this method on every page view or set `storeInCookie = true`.
*
* @param authenticatedUserId {string} - The authenticated user id. A unique and persistent string that represents each authenticated user in the service.
* @param accountId {string} - An optional string to represent the account associated with the authenticated user.
* @param storeInCookie {boolean} - AuthenticateUserID will be stored in a cookie and added to all events within this session.
*/
public setAuthenticatedUserContext(authenticatedUserId: string, accountId?: string) {
public setAuthenticatedUserContext(authenticatedUserId: string, accountId?: string, storeInCookie = false) {
try {
this.context.user.setAuthenticatedUserContext(authenticatedUserId, accountId);
this.context.user.setAuthenticatedUserContext(authenticatedUserId, accountId, storeInCookie);
} catch (e) {
_InternalLogging.throwInternal(LoggingSeverity.WARNING,
_InternalMessageId.SetAuthContextFailed,
Expand Down
12 changes: 7 additions & 5 deletions JavaScript/JavaScriptSDK/Context/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ module Microsoft.ApplicationInsights.Context {
public storeRegion: string;

/**
* Sets the autheticated user id and the account id in this session.
* Sets the authenticated user id and the account id in this session.
*
* @param authenticatedUserId {string} - The authenticated user id. A unique and persistent string that represents each authenticated user in the service.
* @param accountId {string} - An optional string to represent the account associated with the authenticated user.
*/
public setAuthenticatedUserContext(authenticatedUserId: string, accountId?: string) {
public setAuthenticatedUserContext(authenticatedUserId: string, accountId?: string, storeInCookie = false) {

// Validate inputs to ensure no cookie control characters.
var isInvalidInput = !this.validateUserInput(authenticatedUserId) || (accountId && !this.validateUserInput(accountId));
Expand All @@ -74,9 +74,11 @@ module Microsoft.ApplicationInsights.Context {
authCookie = [this.authenticatedId, this.accountId].join(User.cookieSeparator);
}

// Set the cookie. No expiration date because this is a session cookie (expires when browser closed).
// Encoding the cookie to handle unexpected unicode characters.
Util.setCookie(User.authUserCookieName, encodeURI(authCookie), this.config.cookieDomain());
if (storeInCookie) {
// Set the cookie. No expiration date because this is a session cookie (expires when browser closed).
// Encoding the cookie to handle unexpected unicode characters.
Util.setCookie(User.authUserCookieName, encodeURI(authCookie), this.config.cookieDomain());
}
}

/**
Expand Down

0 comments on commit f071969

Please sign in to comment.