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

Cannot modify the request headers and cookies when using a custom endpoint #1545

Closed
llatinov opened this issue Apr 23, 2021 · 6 comments
Closed
Milestone

Comments

@llatinov
Copy link

I am using the AppInsights tracking in a React application. This is my setup

  1. I cannot expose the connectionString as a configuration in the React application, because this means everybody reading my React code will have my connections string.
  2. I have created a separate proxying microservice and I send the tracking data to it
const appInsights = new ApplicationInsights({
	config: {
		instrumentationKey: 'TO_BE_HANDLED_BY_BACKEND',
		disableInstrumentationKeyValidation: true,
		endpointUrl: configApi.msApplicationInsigts.trackUrl
	}
})
  1. I still want to secure my microservice and prevent misuse. I am limited to company internal rules, so in the requests, I am always sending:
    3.1. Secure authentication cookie
    3.2. Special header config value that complements the cookie
  2. The above setup works fine when I use fetch() method.

With the current setup defined, my Application Insights tracking is not working, because I am not able to modify the outgoing request. I have found addTelemetryInitializer which gives me access to the request payload but I need to be able to configure the outgoing request, similar to how I am able to modify the fetch:

const options: RequestInit = {
	credentials: 'include',
	mode: 'cors',
	headers: {
		'X-Custom-Header': 'value'
	}
}

return fetch(url, options)
@MSNev
Copy link
Collaborator

MSNev commented Apr 23, 2021

Hi, there are a couple of things here

  • There is now security risk in exposing connection string and instrumentationKey as this information does not provide any potential attacker access to your collected data, it only provides the ability to "upload" telemetry. So the only risk here is that someone could send bogus/excessive telemetry to your endpoint.
  • Currently the sender only supports XDR (for IE9 or lower) and XHR (XMLHttpRequest), we have an issue open to add an option to use fetch Investigate and add a sender that uses fetch when XMLHttpRequest is not available #1268, but that is not currently scheduled
  • Looking at the code I can't identify and "easy" and supportable way within the SDK today to allow adding additional headers to the sender 😞 , I'll add this as part of the work required for Investigate and add a sender that uses fetch when XMLHttpRequest is not available #1268 as that will require a bunch of work anyway.

So having said above, let me provide an approach that you might be able to implement as we do understand the concerns you are talking about.

  • One immediate option that comes to mind would be to effectively "patch" the XHR request (as the SDK does), by overriding the XHR implementation or just "hooking" the XHR send function using the same mechanism that the SDK does.
    Simplistically, we provide an InstrumentProto() helper that you could use to provide a callback that would allow you to add the additional headers.
InstrumentProto(XMLHttpRequest, "send", {
    req: (args:IInstrumentCallDetails, context?: Document | BodyInit | null) => {
        let xhr = args.inst as XMLHttpRequest;
        xhr.setRequestHeader("yourheader", "yourvalue");
    }
});

You don't need to call the base function as these helpers "wrap" the calls so the "real" function is still called after the hooked request.

The implementations are here https://github.com/microsoft/ApplicationInsights-JS/blob/master/shared/AppInsightsCore/src/JavaScriptSDK/InstrumentHooks.ts
I couldn't find any documentation, but the internal usage is located here
https://github.com/microsoft/ApplicationInsights-JS/blob/master/extensions/applicationinsights-dependencies-js/src/ajax.ts#L469

@MSNev
Copy link
Collaborator

MSNev commented Apr 23, 2021

I should add that the above will hook "ALL" XHR requests, so you should check the outbound URL to ensure that it's the endpoint you want to add the headers too.

@llatinov
Copy link
Author

llatinov commented Apr 24, 2021

Thanks a lot for the fast and accurate response. The suggested change works perfectly. Just to complement the code, I've added the import with import { InstrumentProto, IInstrumentCallDetails } from '@microsoft/applicationinsights-core-js' and sending the auth cookies with xhr.withCredentials = true.

Maybe you can add one paragraph in the README about this? If you consider it not important to add it, then the issue can be closed.

@xiao-lix
Copy link
Contributor

xiao-lix commented Apr 29, 2021

Added getSender method and addHeader method. And provided customHeaders configuration.

@MSNev
Copy link
Collaborator

MSNev commented Jun 10, 2021

v2.6.3 is now fully deployed

@MSNev MSNev closed this as completed Jun 10, 2021
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants