[Enhancement]: Centralize response creation

[jonbarrow]

Checked Existing

• I have checked the repository for duplicate issues.

What enhancement would you like to see?

Create a helper function to centralize the creation of responses in some services. This is mainly aimed at NNAS but might be useful in other services as well.

The motivation behind this is the fact that our response creation is very clunky in a lot of places. Lots of repeated code, some areas missing things, lots of duplicate calls to set headers, etc.

I noticed this when looking at how #111 adds in a warning comment for the OAuth endpoint. This comment is supposed to warn users that sharing details could compromise their data like their device certificate (we know of at least 1 case of a user being tricked into giving up their device certificate this way, which was the motivation for the comment). I wanted to add this warning to other endpoints, since basically every NNAS endpoint needs it, and realized how much duplicate code that would be.

Any other details to share? (OPTIONAL)

I'm thinking maybe something like this?

import express from 'express';
import xmlbuilder from 'xmlbuilder';

type CreateNNASErrorResponseOptions = {
	status?: number;
	error: {
		cause?: string;
		code?: string;
		message?: string;
	}
}

type CreateNNASResponseOptions = {
	status?: number;
	body: Record<string, any>;
}

const router = express.Router();

router.post('/access_token/generate', async (request: express.Request, response: express.Response): Promise<void> => {
	// Call createNNASErrorResponse if error, otherwise createNNASResponse
});

function createNNASErrorResponse(response: express.Response, options: CreateNNASErrorResponseOptions): void {
	createNNASResponse(response, {
		status: options.status || 400,
		body: {
			error: options.error
		}
	});
}

function createNNASResponse(response: express.Response, options: CreateNNASResponseOptions): void {
	response.set('Content-Type', 'text/xml');
	response.set('Server', 'Nintendo 3DS (http)');
	response.set('X-Nintendo-Date', new Date().getTime().toString());

	const body = xmlbuilder
		.create(options.body)
		.commentBefore('WARNING! DO NOT SHARE ANYTHING IN THIS REQUEST OR RESPONSE WITH UNTRUSTED USERS! IT CAN BE USED TO IMPERSONATE YOU AND YOUR CONSOLE, POTENTIALLY GETTING YOU BANNED!').end();

	response.status(options.status || 200).send(body);
}

This way we can easily just call the helper functions and have them set the headers and warning and all that

It should be noted that I'm unsure how exactly to handle situations where a NNAS error expects a format like:

<error>
	<cause>grant_type</cause>
	<code>0004</code>
	<message>Invalid Grant Type</message>
</error>

Rather than:

<errors>
	<error>
		<code>0108</code>
		<message>Account has been banned</message>
	</error>
</errors>

It seems like sometimes the outer <errors> element is expected, and other times it is not. I could be mistaken about this though? @DaniElectra @shutterbug2000 @InternalLoss can we confirm if this is the case?

[ssdrive-flaktra]

If NNAS sometimes expects an outer <errors> element, maybe there could be an option in createNNASErrorResponse which would toggle this element on?

[jonbarrow]

The thing is, I'm not actually sure if it's optional or not. The longer I think about it, the more a part of me is convinced that it always expects the outer element and any cases in our implementation that are missing it are just wrong

That's why I tagged some people to help verify this. I forgot to tag @ashquarky though so I'll do that now

[jonbarrow]

Upon investigation, the outer element is always required