Skip to content

Commit

Permalink
Instrumentation Key validation. (#667)
Browse files Browse the repository at this point in the history
* ikeyu validation initial commit with changes in test files

* updating regex to match breeze ikey validation

* updating warn message to pass ikey

* Removed external link to breeze repo
  • Loading branch information
kryalama committed Aug 11, 2020
1 parent 53b440b commit b23988f
Show file tree
Hide file tree
Showing 20 changed files with 117 additions and 70 deletions.
25 changes: 25 additions & 0 deletions Library/Config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import CorrelationIdManager = require('./CorrelationIdManager');
import ConnectionStringParser = require('./ConnectionStringParser');
import Logging = require('./Logging');
import Constants = require('../Declarations/Constants');
import http = require('http');
import https = require('https');
Expand Down Expand Up @@ -71,6 +72,10 @@ class Config {
: setupString; // CS was invalid, so it must be an ikey

this.instrumentationKey = csCode.instrumentationkey || iKeyCode /* === instrumentationKey */ || csEnv.instrumentationkey || Config._getInstrumentationKey();
// validate ikey. If fails throw a warning
if(!Config._validateInstrumentationKey(this.instrumentationKey)) {
Logging.warn("An invalid instrumentation key was provided. There may be resulting telemetry loss", this.instrumentationKey);
}

this.endpointUrl = `${csCode.ingestionendpoint || csEnv.ingestionendpoint || this.endpointBase}/v2/track`;
this.maxBatchSize = 250;
Expand Down Expand Up @@ -133,6 +138,26 @@ class Config {

return iKey;
}

/**
* Validate UUID Format
* Specs taken from breeze repo
* The definition of a VALID instrumentation key is as follows:
* Not none
* Not empty
* Every character is a hex character [0-9a-f]
* 32 characters are separated into 5 sections via 4 dashes
* First section has 8 characters
* Second section has 4 characters
* Third section has 4 characters
* Fourth section has 4 characters
* Fifth section has 12 characters
*/
private static _validateInstrumentationKey(iKey:string): boolean {
const UUID_Regex = '^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$';
const regexp = new RegExp(UUID_Regex);
return regexp.test(iKey);
}
}

export = Config;
4 changes: 2 additions & 2 deletions Tests/AutoCollection/Console.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe("AutoCollection/Console", () => {
describe("#init and #dispose()", () => {
it("init should enable and dispose should stop console autocollection", () => {

var appInsights = AppInsights.setup("key").setAutoCollectConsole(true);
var appInsights = AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333").setAutoCollectConsole(true);
var enableConsoleRequestsSpy = sinon.spy(Console.INSTANCE, "enable");
appInsights.start();

Expand All @@ -29,7 +29,7 @@ describe("AutoCollection/Console", () => {

describe("#log and #error()", () => {
it("should call trackException for errors and trackTrace for logs", () => {
var appInsights = AppInsights.setup("key");
var appInsights = AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
appInsights.start();

const trackExceptionStub = sinon.stub(AppInsights.defaultClient, "trackException");
Expand Down
2 changes: 1 addition & 1 deletion Tests/AutoCollection/Exceptions.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe("AutoCollection/Exceptions", () => {
var processOnSpy = sinon.spy(global.process, "on");
var processRemoveListenerSpy = sinon.spy(global.process, "removeListener");

AppInsights.setup("key").setAutoCollectExceptions(true).start();
AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333").setAutoCollectExceptions(true).start();

if (AutoCollectionExceptions["_canUseUncaughtExceptionMonitor"]) {
assert.equal(processOnSpy.callCount, 1, "After enabling exception autocollection, there should be 1 call to processOnSpy");
Expand Down
4 changes: 2 additions & 2 deletions Tests/AutoCollection/Heartbeat.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import TelemetryClient = require("../../Library/TelemetryClient");
import Context = require("../../Library/Context");

describe("AutoCollection/HeartBeat", () => {
const client = new TelemetryClient("key");
const client = new TelemetryClient("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
client.config.correlationId = "testicd";

afterEach(() => {
Expand All @@ -19,7 +19,7 @@ describe("AutoCollection/HeartBeat", () => {
it("init should enable and dispose should stop autocollection interval", () => {
var setIntervalSpy = sinon.spy(global, "setInterval");
var clearIntervalSpy = sinon.spy(global, "clearInterval");
AppInsights.setup("key")
AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333")
.setAutoCollectPerformance(false, false)
.setAutoCollectHeartbeat(true)
.start();
Expand Down
6 changes: 3 additions & 3 deletions Tests/AutoCollection/HttpDependencies.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe("AutoCollection/HttpDependencies", () => {
describe("#init and #dispose()", () => {
it("init should enable and dispose should stop dependencies autocollection", () => {

var appInsights = AppInsights.setup("key").setAutoCollectDependencies(true);
var appInsights = AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333").setAutoCollectDependencies(true);
var enableHttpDependenciesSpy = sinon.spy(HttpDependencies.INSTANCE, "enable");
appInsights.start();

Expand All @@ -38,7 +38,7 @@ describe("AutoCollection/HttpDependencies", () => {
telemetry.request.clearHeaders();
});
it("should accept string request-context", () => {
var appInsights = AppInsights.setup("key").setAutoCollectDependencies(true);
var appInsights = AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333").setAutoCollectDependencies(true);
AppInsights.defaultClient.config.correlationId = "abcdefg";
appInsights.start();

Expand All @@ -47,7 +47,7 @@ describe("AutoCollection/HttpDependencies", () => {
});

it ("should accept nonstring request-context", () => {
var appInsights = AppInsights.setup("key").setAutoCollectDependencies(true);
var appInsights = AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333").setAutoCollectDependencies(true);
AppInsights.defaultClient.config.correlationId = "abcdefg";
appInsights.start();

Expand Down
6 changes: 3 additions & 3 deletions Tests/AutoCollection/HttpRequests.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe("AutoCollection/HttpRequests", () => {
});
describe("#init and #dispose()", () => {
it("init should enable and dispose should stop server requests autocollection", () => {
var appInsights = AppInsights.setup("key").setAutoCollectRequests(true);
var appInsights = AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333").setAutoCollectRequests(true);
var enableHttpRequestsSpy = sinon.spy(HttpRequests.INSTANCE, "enable");
appInsights.start();

Expand All @@ -36,7 +36,7 @@ describe("AutoCollection/HttpRequests", () => {
});

it("should accept string request-context", () => {
var appInsights = AppInsights.setup("key").setAutoCollectRequests(true);
var appInsights = AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333").setAutoCollectRequests(true);
AppInsights.defaultClient.config.correlationId = "abcdefg";
appInsights.start();

Expand All @@ -45,7 +45,7 @@ describe("AutoCollection/HttpRequests", () => {
});

it("should accept nonstring request-context", () => {
var appInsights = AppInsights.setup("key").setAutoCollectDependencies(true);
var appInsights = AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333").setAutoCollectDependencies(true);
AppInsights.defaultClient.config.correlationId = "abcdefg";
appInsights.start();

Expand Down
6 changes: 3 additions & 3 deletions Tests/AutoCollection/NativePerformance.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe("AutoCollection/NativePerformance", () => {
var setIntervalSpy = sinon.spy(global, "setInterval");
var clearIntervalSpy = sinon.spy(global, "clearInterval");

AppInsights.setup("key")
AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333")
.setAutoCollectHeartbeat(false)
.setAutoCollectPerformance(false, true)
.start();
Expand All @@ -36,7 +36,7 @@ describe("AutoCollection/NativePerformance", () => {
});

it("constructor should be safe to call multiple times", () => {
var client = new TelemetryClient("key");
var client = new TelemetryClient("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
var native = new AutoCollectNativePerformance(client);
var disposeSpy = sinon.spy(AutoCollectNativePerformance.INSTANCE, "dispose");

Expand All @@ -48,7 +48,7 @@ describe("AutoCollection/NativePerformance", () => {
});

it("Calling enable when metrics are not available should fail gracefully", () => {
var client = new TelemetryClient("key");
var client = new TelemetryClient("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
var native = new AutoCollectNativePerformance(client);

AutoCollectNativePerformance["_metricsAvailable"] = false;
Expand Down
8 changes: 4 additions & 4 deletions Tests/AutoCollection/Performance.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe("AutoCollection/Performance", () => {
it("init should enable and dispose should stop autocollection interval", () => {
var setIntervalSpy = sinon.spy(global, "setInterval");
var clearIntervalSpy = sinon.spy(global, "clearInterval");
AppInsights.setup("key")
AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333")
.setAutoCollectHeartbeat(false)
.setAutoCollectPerformance(true, false)
.start();
Expand All @@ -30,9 +30,9 @@ describe("AutoCollection/Performance", () => {
it("should not produce incorrect metrics because of multiple instances of Performance class", (done) => {
const setIntervalStub = sinon.stub(global, "setInterval", () => ({ unref: () => {}}));
const clearIntervalSpy = sinon.spy(global, "clearInterval");
const appInsights = AppInsights.setup("key").setAutoCollectPerformance(false).start();
const performance1 = new Performance(new TelemetryClient("key"), 1234, false);
const performance2 = new Performance(new TelemetryClient("key"), 4321, true);
const appInsights = AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333").setAutoCollectPerformance(false).start();
const performance1 = new Performance(new TelemetryClient("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333"), 1234, false);
const performance2 = new Performance(new TelemetryClient("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333"), 4321, true);
performance1.enable(true);
performance2.enable(true);
Performance.INSTANCE.enable(true);
Expand Down
2 changes: 1 addition & 1 deletion Tests/AutoCollection/bunyan.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe("diagnostic-channel/bunyan", () => {
disable();
});
it("should call trackException for errors, trackTrace for logs", () => {
AppInsights.setup("key");
AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
AppInsights.start();

const trackExceptionStub = sinon.stub(AppInsights.defaultClient, "trackException");
Expand Down
2 changes: 1 addition & 1 deletion Tests/AutoCollection/winston.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe("diagnostic-channel/winston", () => {
disable();
});
it("should call trackException for errors, trackTrace for logs", () => {
AppInsights.setup("key");
AppInsights.setup("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
AppInsights.start();

const trackExceptionStub = sinon.stub(AppInsights.defaultClient, "trackException");
Expand Down
6 changes: 3 additions & 3 deletions Tests/Bootstrap/Default.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ describe("#setupAndStart()", () => {
// Test
const Default = require("../../Bootstrap/Default") as typeof DefaultTypes;
Default.setLogger(new DiagnosticLogger(logger));
const instance1 = Default.setupAndStart("abc");
const instance1 = Default.setupAndStart("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert.ok(instance1.defaultClient);
const instance2 = Default.setupAndStart("abc");
const instance2 = Default.setupAndStart("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert.deepEqual(instance1.defaultClient, instance2.defaultClient);
assert.deepEqual(instance1.defaultClient["_telemetryProcessors"].length, 2)
assert.deepEqual(instance2.defaultClient["_telemetryProcessors"].length, 2)
Expand All @@ -68,7 +68,7 @@ describe("#setupAndStart()", () => {
// Test
const Default = require("../../Bootstrap/Default") as typeof DefaultTypes;
Default.setLogger(new DiagnosticLogger(logger));
const instance = Default.setupAndStart("abc");
const instance = Default.setupAndStart("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert.deepEqual(instance, appInsights);

// Cleanup
Expand Down
10 changes: 5 additions & 5 deletions Tests/Library/Client.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import EnvelopeFactory = require("../../Library/EnvelopeFactory");

describe("Library/TelemetryClient", () => {

var iKey = "Instrumentation-Key-12345-6789A";
var iKey = "1aa11111-bbbb-1ccc-8ddd-eeeeffff3333";
var appId = "Application-Key-12345-6789A";
var name = "name";
var value = 3;
Expand Down Expand Up @@ -61,24 +61,24 @@ describe("Library/TelemetryClient", () => {

describe("#constructor()", () => {
it("should initialize config", () => {
var client = new Client("key");
var client = new Client("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert.ok(client.config);
assert.ok(client.config.instrumentationKey);
});

it("should initialize context", () => {
var client = new Client("key");
var client = new Client("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert.ok(client.context);
assert.ok(client.context.tags);
});

it("should initialize common properties", () => {
var client = new Client("key");
var client = new Client("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert.ok(client.commonProperties);
});

it("should initialize channel", () => {
var client = new Client("key");
var client = new Client("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert.ok(client.channel);
});
});
Expand Down
32 changes: 27 additions & 5 deletions Tests/Library/Config.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import Constants = require("../../Declarations/Constants");

describe("Library/Config", () => {

var iKey = "iKey";
var iKey = "1aa11111-bbbb-1ccc-8ddd-eeeeffff3333";
var appVer = "appVer";

describe("#constructor", () => {
Expand Down Expand Up @@ -98,7 +98,7 @@ describe("Library/Config", () => {
});

it("should initialize valid values", () => {
var config = new Config("iKey");
var config = new Config("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert(typeof config.instrumentationKey === "string");
assert(typeof config.endpointUrl === "string");
assert(typeof config.maxBatchSize === "number");
Expand All @@ -111,7 +111,7 @@ describe("Library/Config", () => {
});

it("should initialize values that we claim in README", () => {
var config = new Config("iKey");
var config = new Config("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert(config.maxBatchSize === 250);
assert(config.maxBatchIntervalMs === 15000);
assert(config.disableAppInsights === false);
Expand All @@ -126,15 +126,37 @@ describe("Library/Config", () => {
it("should initialize values that we claim in README (2)", () => {
process.env.http_proxy = "test";
process.env.https_proxy = "test2";
var config = new Config("iKey");
var config = new Config("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert(config.proxyHttpUrl === "test");
assert(config.proxyHttpsUrl === "test2");
});

it("should add azure domain to excluded list", () => {
var config = new Config("iKey");
var config = new Config("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert.equal(config.correlationHeaderExcludedDomains[0].toString(), "*.core.windows.net");
});

it("instrumentation key validation-valid key passed", () => {
var warnStub = sinon.stub(console, "warn");
var config = new Config("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
assert.ok(warnStub.notCalled, "warning was not raised");
warnStub.restore();
});

it("instrumentation key validation-invalid key passed", () => {
var warnStub = sinon.stub(console, "warn");
var config = new Config("1aa11111bbbb1ccc8dddeeeeffff3333");
assert.ok(warnStub.calledOn, "warning was raised");
warnStub.restore();
});

it("instrumentation key validation-invalid key passed", () => {
var warnStub = sinon.stub(console, "warn");
var config = new Config("abc");
assert.ok(warnStub.calledOn, "warning was raised");
warnStub.restore();
});

});
});
});
8 changes: 4 additions & 4 deletions Tests/Library/ConnectionStringParser.tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe("ConnectionStringParser", () => {
describe("#parse()", () => {
it("should parse all valid fields", () => {
const authorization = "ikey"
const instrumentationKey = "instr_key";
const instrumentationKey = "1aa11111-bbbb-1ccc-8ddd-eeeeffff3333";
const ingestionEndpoint = "ingest";
const liveEndpoint = "live";
const connectionString = `Authorization=${authorization};InstrumentationKey=${instrumentationKey};IngestionEndpoint=${ingestionEndpoint};LiveEndpoint=${liveEndpoint}`;
Expand All @@ -23,7 +23,7 @@ describe("ConnectionStringParser", () => {

it("should ignore invalid fields", () => {
const authorization = "ikey"
const instrumentationKey = "ikey";
const instrumentationKey = "1aa11111-bbbb-1ccc-8ddd-eeeeffff3333";
const ingestionEndpoint = "ingest";
const liveEndpoint = "live";
const connectionString = `Autho.rization=${authorization};Instrume.ntationKey=${instrumentationKey};Ingestion.Endpoint=${ingestionEndpoint};LiveEnd.point=${liveEndpoint}`;
Expand Down Expand Up @@ -53,9 +53,9 @@ describe("ConnectionStringParser", () => {

it("should use correct default endpoints", () => {
runTest({
connectionString: "InstrumentationKey=00000000-0000-0000-0000-000000000000",
connectionString: "InstrumentationKey=1aa11111-bbbb-1ccc-8ddd-eeeeffff3333",
expectedAuthorization: undefined,
expectedInstrumentationKey: "00000000-0000-0000-0000-000000000000",
expectedInstrumentationKey: "1aa11111-bbbb-1ccc-8ddd-eeeeffff3333",
expectedBreezeEndpoint: Constants.DEFAULT_BREEZE_ENDPOINT,
expectedLiveMetricsEndpoint: Constants.DEFAULT_LIVEMETRICS_ENDPOINT
});
Expand Down
4 changes: 2 additions & 2 deletions Tests/Library/EnvelopeFactoryTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe("Library/EnvelopeFactory", () => {
describe("#createEnvelope()", () => {
var commonproperties: { [key: string]: string } = { common1: "common1", common2: "common2", common: "common" };
it("should assign common properties to the data", () => {
var client1 = new Client("key");
var client1 = new Client("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
client1.commonProperties = commonproperties;
client1.config.samplingPercentage = 99;
var eventTelemetry = <Contracts.EventTelemetry>{name:"name"};
Expand All @@ -40,7 +40,7 @@ describe("Library/EnvelopeFactory", () => {

it("should allow tags to be overwritten", () => {

var client = new Client("key");
var client = new Client("1aa11111-bbbb-1ccc-8ddd-eeeeffff3333");
var env = EnvelopeFactory.createEnvelope(<Contracts.EventTelemetry>{name:"name"}, Contracts.TelemetryType.Event, commonproperties, client.context, client.config);
assert.deepEqual(env.tags, client.context.tags, "tags are set by default");
var customTag = <{ [id: string]: string }>{ "ai.cloud.roleInstance": "override" };
Expand Down

0 comments on commit b23988f

Please sign in to comment.