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 all 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
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,9 @@ Most configuration fields are named such that they can be defaulted to falsey. A
| isBrowserLinkTrackingEnabled | boolean | false | Default is false. If true, the SDK will track all [Browser Link](https://docs.microsoft.com/en-us/aspnet/core/client-side/using-browserlink) requests. |
| appId | string | null | AppId is used for the correlation between AJAX dependencies happening on the client-side with the server-side requests. When Beacon API is enabled, it cannot be used automatically, but can be set manually in the configuration. Default is null |
| enableCorsCorrelation | boolean | false | If true, the SDK will add two headers ('Request-Id' and 'Request-Context') to all CORS requests to correlate outgoing AJAX dependencies with corresponding requests on the server side. Default is false |
| namePrefix | string | undefined | An optional value that will be used as name postfix for localStorage and cookie name.
| namePrefix | string | undefined | An optional value that will be used as name postfix for localStorage and session cookie name.
| sessionCookiePostfix | string | undefined | An optional value that will be used as name postfix for session cookie name. If undefined, namePrefix is used as name postfix for session cookie name.
| userCookiePostfix | string | undefined | An optional value that will be used as name postfix for user cookie name. If undefined, no postfix is added on user cookie name.
| enableAutoRouteTracking | boolean | false | Automatically track route changes in Single Page Applications (SPA). If true, each route change will send a new Pageview to Application Insights. Hash route changes changes (`example.com/foo#bar`) are also recorded as new page views.
| enableRequestHeaderTracking | boolean | false | If true, AJAX & Fetch request headers is tracked, default is false. If ignoreHeaders is not configured, Authorization and X-API-Key headers are not logged.
| enableResponseHeaderTracking | boolean | false | If true, AJAX & Fetch request's response headers is tracked, default is false. If ignoreHeaders is not configured, WWW-Authenticate header is not logged.
Expand Down
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
18 changes: 17 additions & 1 deletion shared/AppInsightsCommon/src/Interfaces/IConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,13 +304,29 @@ export interface IConfig {
enableCorsCorrelation?: boolean;

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

/**
* @description An optional value that will be used as name postfix for session cookie name. If undefined, namePrefix is 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. If undefined, no postfix is added on 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