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

ai_user cookie should use userCookiePostfix for user cookie storage #1587

Merged
merged 6 commits into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,35 @@ export class SessionManagerTests extends AITestClass {
}
});

this.testCase({
name: 'Session uses sessionCookiePostfix over namePrefix for cookie storage if both are configured.',
test: () => {

var sessionPrefix = newId();
var config = {
namePrefix: () => sessionPrefix,
sessionCookiePostfix: () => "testSessionCookieNamePostfix",
sessionExpirationMs: () => undefined,
sessionRenewalMs: () => undefined,
cookieDomain: () => undefined

};
// Setup
let cookie = "";
const cookieStub: SinonStub = this.sandbox.stub(this.core.getCookieMgr(), 'set').callsFake((cookieName, value, maxAge, domain, path) => {
cookie = cookieName;
});

// Act
const sessionManager = new _SessionManager(config, this.core);
sessionManager.update();

// Test
Assert.ok(cookieStub.called, 'cookie set');
Assert.equal('ai_session' + 'testSessionCookieNamePostfix', cookie, 'Correct cookie name when session cookie postfix is provided - [' + cookie + ']');
}
});

this.testCase({
name: 'Validate Session default re-hydration within expiry period',
useFakeTimers: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import PropertiesPlugin from "../../src/PropertiesPlugin";
import { ITelemetryConfig } from "../../src/Interfaces/ITelemetryConfig";
import { TelemetryContext } from "../../src/TelemetryContext";
import { TelemetryTrace } from "../../src/Context/TelemetryTrace";
import { IConfig } from "@microsoft/applicationinsights-common";

export class PropertiesTests extends AITestClass {
private properties: PropertiesPlugin;
Expand Down Expand Up @@ -196,6 +197,30 @@ export class PropertiesTests extends AITestClass {
}
});

this.testCase({
name: "ai_user cookie uses userCookiePostfix for cookie storage",
test: () => {
// setup
var actualCookieName: string;
var actualCookieValue: string;

var newIdStub = this.sandbox.stub(this as any, "_getNewId").callsFake(() => "newId");
var getCookieStub = this.sandbox.stub(this as any, "_getCookie").callsFake(() =>"");
var setCookieStub = this.sandbox.stub(this as any, "_setCookie").callsFake((cookieName, cookieValue) => {
actualCookieName = cookieName;
actualCookieValue = cookieValue;
});

// act
let config: IConfig & IConfiguration = this.getEmptyConfig();
config.userCookiePostfix = 'testUserCookieNamePostfix';
this.properties.initialize(config, this.core, []);

// verify
Assert.equal("ai_usertestUserCookieNamePostfix", actualCookieName, "ai_user cookie is set");
}
});

this.testCase({
name: "Ctor: auth and account id initialize from cookie",
test: () => {
Expand Down Expand Up @@ -631,6 +656,8 @@ export class PropertiesTests extends AITestClass {
isBrowserLinkTrackingEnabled: () => true,
appId: () => "",
namePrefix: () => "",
sessionCookiePostfix: () => "",
userCookiePostfix: () => "",
idLength: () => 22,
getNewId: () => this._getNewId
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface ISessionConfig {
sessionRenewalMs?: () => number;
sessionExpirationMs?: () => number;
namePrefix?: () => string;
sessionCookiePostfix?: () => string;
idLength?: () => number;
getNewId?: () => (idLength?: number) => string;

Expand Down Expand Up @@ -73,7 +74,12 @@ export class _SessionManager {
}

_self.config = config;
_storageNamePrefix = () => _self.config.namePrefix && _self.config.namePrefix() ? cookieNameConst + _self.config.namePrefix() : cookieNameConst;
// sessionCookiePostfix takes the preference if it is configured, otherwise takes namePrefix if configured.
const sessionCookiePostfix = (_self.config.sessionCookiePostfix && _self.config.sessionCookiePostfix()) ?
_self.config.sessionCookiePostfix() :
((_self.config.namePrefix && _self.config.namePrefix()) ? _self.config.namePrefix() : "");

_storageNamePrefix = () => cookieNameConst + sessionCookiePostfix;

_self.automaticSession = new Session();

Expand Down
11 changes: 7 additions & 4 deletions extensions/applicationinsights-properties-js/src/Context/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,15 @@ export class User implements IUserContext {
constructor(config: ITelemetryConfig, core: IAppInsightsCore) {
let _logger = safeGetLogger(core);
let _cookieManager: ICookieMgr = safeGetCookieMgr(core);
let _storageNamePrefix: () => string;

dynamicProto(User, this, (_self) => {
_self.config = config;
xiao-lix marked this conversation as resolved.
Show resolved Hide resolved
const userCookiePostfix = (_self.config.userCookiePostfix && _self.config.userCookiePostfix()) ? _self.config.userCookiePostfix() : "";
_storageNamePrefix = () => User.userCookieName + userCookiePostfix;
xiao-lix marked this conversation as resolved.
Show resolved Hide resolved

// get userId or create new one if none exists
const cookie = _cookieManager.get(User.userCookieName);
const cookie = _cookieManager.get(_storageNamePrefix());
if (cookie) {
_self.isNewUser = false;
const params = cookie.split(User.cookieSeparator);
Expand All @@ -73,8 +78,6 @@ export class User implements IUserContext {
}
}

_self.config = config;

if (!_self.id) {
let theConfig = (config || {}) as ITelemetryConfig;
let getNewId = (theConfig.getNewId ? theConfig.getNewId() : null) || newId;
Expand All @@ -88,7 +91,7 @@ export class User implements IUserContext {
_self.isNewUser = true;
const newCookie = [_self.id, acqStr];

_cookieManager.set(User.userCookieName, newCookie.join(User.cookieSeparator), oneYear);
_cookieManager.set(_storageNamePrefix(), newCookie.join(User.cookieSeparator), oneYear);

// If we have an config.namePrefix() + ai_session in local storage this means the user actively removed our cookies.
// We should respect their wishes and clear ourselves from local storage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ export interface ITelemetryConfig {
isBrowserLinkTrackingEnabled: () => boolean;
appId: () => string;
namePrefix: () => string;
sessionCookiePostfix: () => string;
userCookiePostfix: () => string;
idLength: () => number;
getNewId: () => (idLength?: number) => string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export default class PropertiesPlugin extends BaseTelemetryPlugin implements IPr
isBrowserLinkTrackingEnabled: () => false,
appId: () => null,
namePrefix: () => undefined,
sessionCookiePostfix: () => undefined,
userCookiePostfix: () => undefined,
idLength: () => 22,
getNewId: () => null
};
Expand Down
16 changes: 16 additions & 0 deletions shared/AppInsightsCommon/src/Interfaces/IConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,22 @@ export interface IConfig {
*/
namePrefix?: string;

/**
* @description An optional value that will be used as name postfix for session cookie name.
* @type {string}
* @memberof IConfig
* @defaultValue null
*/
sessionCookiePostfix?: string;
xiao-lix marked this conversation as resolved.
Show resolved Hide resolved

/**
* @description An optional value that will be used as name postfix for user cookie name.
* @type {string}
* @memberof IConfig
* @defaultValue null
*/
userCookiePostfix?: string;

/**
* @description An optional value that will track Request Header through trackDependency function.
* @type {boolean}
Expand Down