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

Ability to add request headers to GQL request on file upload #61

Open
dominik-myszkowski opened this issue Nov 29, 2022 · 9 comments
Open
Labels

Comments

@dominik-myszkowski
Copy link

dominik-myszkowski commented Nov 29, 2022

🚀 Feature Request

Description

  • Whenever a file upload via apollo-federation-file-upload occurs, there are set request headers and one can not add anything to that header (request from Gateway to subgraph). willSendRequest method will execute but all the headers added in that method (in case of file upload) will be ignored. There are many issues arising from that problem, for example I can not pass authentication token from Gateway to subgraph for authenticating user.

Implementation details

  • There should be an option (object props) to pass file upload headers OR file upload request should add headers added in willSendRequest

Potential caveats

  • none

Acceptance criteria

Additional context and visual reference

@superlevure
Copy link

I'm having the same issue, I can't figure out why the headers set in willSendRequest are ignored. Any clue?

@superlevure
Copy link

superlevure commented Jan 9, 2023

Actually, this was fixed by 962583d but not released yet ?

@BobbiSixkiller
Copy link

Same problem here. I can see that the gateway sets the headers based on the context via the willSendRequest method but the headers don't propagate to the corresponding service with subgraph. Any solutions pls? Right now I can't authenticate user accessing upload resolver implemeted by the microservice :/

@BobbiSixkiller
Copy link

@superlevure please have you somehow managed to get it to set the headers? If so could you please provide a workaround?

@BobbiSixkiller
Copy link

BobbiSixkiller commented Jan 22, 2023

For anyone having this issue, my workaround for now is simply constructing the headers object based on context value received via GraphQLDataSourceProcessOptions args. Somehow the request?.http?.headers returns undefined so I relied on the context, where my authenticated user is set. So basically I tweaked the processFiles method like this

private async processFiles(
	args: GraphQLDataSourceProcessOptions,
	fileVariables: FileVariablesTuple[]
): ProcessResult {
	const { context, request } = args;
	const form = new FormData();

	const variables = cloneDeep(request.variables || {});
	fileVariables.forEach(([variableName]: FileVariablesTuple): void => {
		set(variables, variableName, null);
	});

	const operations = JSON.stringify({
		query: request.query,
		variables,
	});

	form.append("operations", operations);

	const fileMap: { [key: string]: string[] } = {};

	const resolvedFiles: FileUpload[] = await Promise.all(
		fileVariables.map(
			async (
				[variableName, file]: FileVariablesTuple,
				i: number
			): Promise<FileUpload> => {
				const fileUpload: FileUpload = await file;
				fileMap[i] = [`variables.${variableName}`];
				return fileUpload;
			}
		)
	);

	// This must come before the file contents append bellow
	form.append("map", JSON.stringify(fileMap));
	await this.addDataHandler(form, resolvedFiles);

	// This must happen before constructing the request headers
	// otherwise any custom headers set in willSendRequest are ignored
	if (this.willSendRequest) {
		await this.willSendRequest(args);
	}

	console.log(context.user, context.locale);

	const headers = {
		// ...Object.fromEntries(request?.http?.headers || []),
		...Object.fromEntries([["user", JSON.stringify(context[user])]]),
		...form.getHeaders(),
	};

	console.log(headers);

	Object.assign(headers, form.getHeaders() || {});

	const httpRequest = {
		headers,
		method: "POST",
		url: this.url,
	};

	const options = {
		...httpRequest,
		body: form,
	};

	// NOTE: there is currently a type mismatch related to Headers in @apollo/gateway and apollo-server-env:
	//
	//  >> you should ensure that you pass "plain" objects rather than Headers or Request objects,
	//  >> as the newer version has slightly different logic about how to recognize Headers and
	//  >> Request objects.
	//
	// see:
	// - https://github.com/apollographql/federation/pull/1906
	// - https://github.com/profusion/apollo-federation-file-upload/issues/52#issuecomment-1148946002
	request.http = httpRequest as Exclude<typeof request.http, undefined>;

	let httpResponse: Response | undefined;

	try {
		httpResponse = await this.fetcher(this.url, options);

		const body = await this.parseBody(httpResponse);

		if (!isObject(body)) {
			throw new Error(`Expected JSON response body, but received: ${body}`);
		}
		const response = {
			...body,
			http: httpResponse,
		};

		if (typeof this.didReceiveResponse === "function") {
			return this.didReceiveResponse({ context, request, response });
		}

		return response;
	} catch (error) {
		this.didEncounterError(error, options, httpResponse);
		throw error;
	}
}`

@wabrit
Copy link

wabrit commented Jan 24, 2023

Ran into this problem too; it seems like the code here is the culprit as it never passes the headers down to the Request constructor. This will cause lots of issues with subgraphs that need access to e.g. Authorization or apollo-require-preflight headers to function correctly.

As mentioned above there seems to be a commit that fixes this - can this be released soon?

           request.http = {
                headers,
                method: 'POST',
                url: this.url,
            };
            if (this.willSendRequest) {
                yield this.willSendRequest(args);
            }
            const options = Object.assign(Object.assign({}, request.http), { 
                // Apollo types are not up-to-date, make TS happy
                body: form });
            const httpRequest = new apollo_server_env_1.Request(request.http.url, options);

@DDDKnightmare DDDKnightmare added this to the Sprint24 milestone Jan 31, 2023
@paulbijancoch
Copy link

will this be released soon? 💯

@oliveirarleo oliveirarleo removed this from the Sprint25 milestone Feb 16, 2023
@ghost
Copy link

ghost commented Jun 26, 2023

Any idea, I got stucked on this.
Upload works fine, yet headers are not overwritten.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants