Add auth support to [Reddit] badges

[PyvesB]

Fixes #9256.

[github-actions]

	Warnings
⚠️	This PR modified the server but none of its tests. That's okay so long as it's refactoring existing code.

	Messages
📖	✨ Thanks for your contribution to Shields, @PyvesB!
📖	Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS against 5a5e06e

[PyvesB]

This is partly based on https://github.com/badges/shields/blob/master/services/twitch/twitch-base.js. However:

• there are some annoying differences compared to Twitch's OAuth flow.
• the logic in the Twitch code is partly broken. For example, the loop here does not refresh the request object, so we're querying new access tokens but still using the old one in our retries, which defeats the whole purpose of the retry loop. Also, I noticed whilst experimenting that err.response could be undefined in some cases, making the conditional blow up.

I decided to just try to make things work for Reddit as a fist iteration, and will came back to rework things at a later point.

[chris48s]

Yeah agreed. Last time I looked at the code for twitch I thought there are some issues that want fixing. Lets not copy the approach.

[PyvesB]

I reused the account that had been created alongside #9817 (comment) and shared the login details in Keybase.

[PyvesB]

The credentials are not being picked up as this is from a fork, so we're getting the existing state from the master branch, i.e. tests failing. They pass locally when credentials are configured, I guess we can simply merge this once the review is completed, and see what happens.

[chris48s]

Couple of questions on the token:

1. Do you know what rate limit we get if we authenticate with a token?

2. Have you made separate credentials for CI/staging/prod or have you set the same ones on all of them. Where possible, I think its preferable to have different credentials for each environment. Can we set up 3 OAuth apps under one account?

[chris48s]

Related to the question about what rate limit we get with authentication, are we confident that we'd have enough rate limit to get away with completely ditching this?

[chris48s]

The expires_in value is used. The scope and token_type values are discarded. Suggest

const tokenSchema = Joi.object({
  access_token: Joi.string().required(),
  expires_in: Joi.number().required(),
}).required()

here.

[chris48s]

This approach using setTimeout() isn't wrong, and I'm not saying we have to change it.
That said, just as a data point: We implemented something quite similar to this in the JWT auth helper.

shields/core/base-service/auth-helper.js

Lines 256 to 318 in 41d072e

	static _isJwtValid(expiry) {
	// we consider the token valid if the expiry
	// datetime is later than (now + 1 minute)
	return dayjs.unix(expiry).isAfter(dayjs().add(1, 'minutes'))
	}
	async _getJwt(loginEndpoint) {
	const { _user: username, _pass: password } = this
	// attempt to get JWT from cache
	if (
	jwtCache?.[loginEndpoint]?.[username]?.token &&
	jwtCache?.[loginEndpoint]?.[username]?.expiry &&
	this.constructor._isJwtValid(jwtCache[loginEndpoint][username].expiry)
	) {
	// cache hit
	return jwtCache[loginEndpoint][username].token
	}
	// cache miss - request a new JWT
	const originViolation = !this.isAllowedOrigin(loginEndpoint)
	if (originViolation) {
	throw new InvalidParameter({
	prettyMessage: 'requested origin not authorized',
	})
	}
	const { buffer } = await checkErrorResponse({})(
	await fetch(loginEndpoint, {
	method: 'POST',
	form: { username, password },
	}),
	)
	const json = validate(
	{
	ErrorClass: InvalidResponse,
	prettyErrorMessage: 'invalid response data from auth endpoint',
	},
	parseJson(buffer),
	Joi.object({ token: Joi.string().required() }).required(),
	)
	const token = json.token
	const expiry = this.constructor._getJwtExpiry(token)
	// store in the cache
	if (!(loginEndpoint in jwtCache)) {
	jwtCache[loginEndpoint] = {}
	}
	jwtCache[loginEndpoint][username] = { token, expiry }
	return token
	}
	async _getJwtAuthHeader(loginEndpoint) {
	if (!this.isConfigured) {
	return undefined
	}
	const token = await this._getJwt(loginEndpoint)
	return { Authorization: `Bearer ${token}` }
	}

One thing that is quite nice about that approach is that the control flow is a bit more linear.
Dunno if you reckon that is any better/worse? If you just want to roll with this then I think it is ok.