From 4a75cf09a786c5e93232ae699af985e5bc89440e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Bigourdan <10694593+PyvesB@users.noreply.github.com> Date: Sat, 18 Jan 2025 16:13:53 +0100 Subject: [PATCH] Add auth support to [Reddit] badges (#10790) * Add auth support to [Reddit] badges * Simplify token response schema --- .github/actions/service-tests/action.yml | 2 + .github/workflows/coveralls-code-coverage.yml | 2 + .github/workflows/daily-tests.yml | 2 + .github/workflows/deploy-review-app.yml | 2 + .github/workflows/test-services-22.yml | 2 + .github/workflows/test-services.yml | 2 + config/custom-environment-variables.yml | 2 + .../local-shields-io-production.template.yml | 2 + config/local.template.yml | 2 + core/server/server.js | 2 + doc/production-hosting.md | 1 + doc/server-secrets.md | 11 +++ services/reddit/reddit-base.js | 89 +++++++++++++++++++ .../reddit/subreddit-subscribers.service.js | 14 +-- .../reddit/subreddit-subscribers.tester.js | 21 ++++- services/reddit/user-karma.service.js | 14 +-- services/reddit/user-karma.tester.js | 72 ++++++++++++++- 17 files changed, 223 insertions(+), 19 deletions(-) create mode 100644 services/reddit/reddit-base.js diff --git a/.github/actions/service-tests/action.yml b/.github/actions/service-tests/action.yml index ca10f539cb..ae60b44ab5 100644 --- a/.github/actions/service-tests/action.yml +++ b/.github/actions/service-tests/action.yml @@ -67,6 +67,8 @@ runs: OBS_USER: '${{ inputs.obs-user }}' OBS_PASS: '${{ inputs.obs-pass }}' PEPY_KEY: '${{ inputs.pepy-key }}' + REDDIT_CLIENT_ID: '${{ inputs.reddit-client-id }}' + REDDIT_CLIENT_SECRET: '${{ inputs.reddit-client-secret }}' SL_INSIGHT_USER_UUID: '${{ inputs.sl-insight-user-uuid }}' SL_INSIGHT_API_TOKEN: '${{ inputs.sl-insight-api-token }}' TWITCH_CLIENT_ID: '${{ inputs.twitch-client-id }}' diff --git a/.github/workflows/coveralls-code-coverage.yml b/.github/workflows/coveralls-code-coverage.yml index 99a78376de..70e51b2438 100644 --- a/.github/workflows/coveralls-code-coverage.yml +++ b/.github/workflows/coveralls-code-coverage.yml @@ -60,6 +60,8 @@ jobs: OBS_USER: '${{ secrets.SERVICETESTS_OBS_USER }}' OBS_PASS: '${{ secrets.SERVICETESTS_OBS_PASS }}' PEPY_KEY: '${{ secrets.SERVICETESTS_PEPY_KEY }}' + REDDIT_CLIENT_ID: '${{ secrets.SERVICETESTS_REDDIT_CLIENT_ID }}' + REDDIT_CLIENT_SECRET: '${{ secrets.SERVICETESTS_REDDIT_CLIENT_SECRET }}' SL_INSIGHT_USER_UUID: '${{ secrets.SERVICETESTS_SL_INSIGHT_USER_UUID }}' SL_INSIGHT_API_TOKEN: '${{ secrets.SERVICETESTS_SL_INSIGHT_API_TOKEN }}' TWITCH_CLIENT_ID: '${{ secrets.SERVICETESTS_TWITCH_CLIENT_ID }}' diff --git a/.github/workflows/daily-tests.yml b/.github/workflows/daily-tests.yml index 995fbb1e5d..0af24d80ba 100644 --- a/.github/workflows/daily-tests.yml +++ b/.github/workflows/daily-tests.yml @@ -57,6 +57,8 @@ jobs: OBS_USER: '${{ secrets.SERVICETESTS_OBS_USER }}' OBS_PASS: '${{ secrets.SERVICETESTS_OBS_PASS }}' PEPY_KEY: '${{ secrets.SERVICETESTS_PEPY_KEY }}' + REDDIT_CLIENT_ID: '${{ secrets.SERVICETESTS_REDDIT_CLIENT_ID }}' + REDDIT_CLIENT_SECRET: '${{ secrets.SERVICETESTS_REDDIT_CLIENT_SECRET }}' SL_INSIGHT_USER_UUID: '${{ secrets.SERVICETESTS_SL_INSIGHT_USER_UUID }}' SL_INSIGHT_API_TOKEN: '${{ secrets.SERVICETESTS_SL_INSIGHT_API_TOKEN }}' TWITCH_CLIENT_ID: '${{ secrets.SERVICETESTS_TWITCH_CLIENT_ID }}' diff --git a/.github/workflows/deploy-review-app.yml b/.github/workflows/deploy-review-app.yml index 8b2a4bfb3e..afb6b7081d 100644 --- a/.github/workflows/deploy-review-app.yml +++ b/.github/workflows/deploy-review-app.yml @@ -36,6 +36,8 @@ jobs: OBS_USER=${{ secrets.SERVICETESTS_OBS_USER }} OBS_PASS=${{ secrets.SERVICETESTS_OBS_PASS }} PEPY_KEY=${{ secrets.SERVICETESTS_PEPY_KEY }} + REDDIT_CLIENT_ID=${{ secrets.SERVICETESTS_REDDIT_CLIENT_ID }} + REDDIT_CLIENT_SECRET=${{ secrets.SERVICETESTS_REDDIT_CLIENT_SECRET }} SL_INSIGHT_API_TOKEN=${{ secrets.SERVICETESTS_SL_INSIGHT_USER_UUID }} SL_INSIGHT_USER_UUID=${{ secrets.SERVICETESTS_SL_INSIGHT_API_TOKEN }} TWITCH_CLIENT_ID=${{ secrets.SERVICETESTS_TWITCH_CLIENT_ID }} diff --git a/.github/workflows/test-services-22.yml b/.github/workflows/test-services-22.yml index bcbe3e09d3..5f47dc86c9 100644 --- a/.github/workflows/test-services-22.yml +++ b/.github/workflows/test-services-22.yml @@ -30,6 +30,8 @@ jobs: obs-user: '${{ secrets.SERVICETESTS_OBS_USER }}' obs-pass: '${{ secrets.SERVICETESTS_OBS_PASS }}' pepy-key: '${{ secrets.SERVICETESTS_PEPY_KEY }}' + reddit-client-id: '${{ secrets.SERVICETESTS_REDDIT_CLIENT_ID }}' + reddit-client-secret: '${{ secrets.SERVICETESTS_REDDIT_CLIENT_SECRET }}' sl-insight-user-uuid: '${{ secrets.SERVICETESTS_SL_INSIGHT_USER_UUID }}' sl-insight-api-token: '${{ secrets.SERVICETESTS_SL_INSIGHT_API_TOKEN }}' twitch-client-id: '${{ secrets.SERVICETESTS_TWITCH_CLIENT_ID }}' diff --git a/.github/workflows/test-services.yml b/.github/workflows/test-services.yml index 6fbf9bcba4..a44021266e 100644 --- a/.github/workflows/test-services.yml +++ b/.github/workflows/test-services.yml @@ -28,6 +28,8 @@ jobs: obs-user: '${{ secrets.SERVICETESTS_OBS_USER }}' obs-pass: '${{ secrets.SERVICETESTS_OBS_PASS }}' pepy-key: '${{ secrets.SERVICETESTS_PEPY_KEY }}' + reddit-client-id: '${{ secrets.SERVICETESTS_REDDIT_CLIENT_ID }}' + reddit-client-secret: '${{ secrets.SERVICETESTS_REDDIT_CLIENT_SECRET }}' sl-insight-user-uuid: '${{ secrets.SERVICETESTS_SL_INSIGHT_USER_UUID }}' sl-insight-api-token: '${{ secrets.SERVICETESTS_SL_INSIGHT_API_TOKEN }}' twitch-client-id: '${{ secrets.SERVICETESTS_TWITCH_CLIENT_ID }}' diff --git a/config/custom-environment-variables.yml b/config/custom-environment-variables.yml index bbe982eadf..6be5b1b114 100644 --- a/config/custom-environment-variables.yml +++ b/config/custom-environment-variables.yml @@ -105,6 +105,8 @@ private: opencollective_token: 'OPENCOLLECTIVE_TOKEN' pepy_key: 'PEPY_KEY' postgres_url: 'POSTGRES_URL' + reddit_client_id: 'REDDIT_CLIENT_ID' + reddit_client_secret: 'REDDIT_CLIENT_SECRET' sentry_dsn: 'SENTRY_DSN' sl_insight_userUuid: 'SL_INSIGHT_USER_UUID' sl_insight_apiToken: 'SL_INSIGHT_API_TOKEN' diff --git a/config/local-shields-io-production.template.yml b/config/local-shields-io-production.template.yml index 6f2ad3b86b..d4d302e451 100644 --- a/config/local-shields-io-production.template.yml +++ b/config/local-shields-io-production.template.yml @@ -5,6 +5,8 @@ private: gh_client_id: ... gh_client_secret: ... gitlab_token: ... + reddit_client_id: ... + reddit_client_secret: ... sentry_dsn: ... shields_secret: ... sl_insight_userUuid: ... diff --git a/config/local.template.yml b/config/local.template.yml index f5cc116578..9d2f66bfc9 100644 --- a/config/local.template.yml +++ b/config/local.template.yml @@ -9,6 +9,8 @@ private: gitlab_token: '...' obs_user: '...' obs_pass: '...' + reddit_client_id: '...' + reddit_client_secret: '...' twitch_client_id: '...' twitch_client_secret: '...' weblate_api_key: '...' diff --git a/core/server/server.js b/core/server/server.js index e50ae7392b..9cf6f3ca55 100644 --- a/core/server/server.js +++ b/core/server/server.js @@ -194,6 +194,8 @@ const privateConfigSchema = Joi.object({ opencollective_token: Joi.string(), pepy_key: Joi.string(), postgres_url: Joi.string().uri({ scheme: 'postgresql' }), + reddit_client_id: Joi.string(), + reddit_client_secret: Joi.string(), sentry_dsn: Joi.string(), sl_insight_userUuid: Joi.string(), sl_insight_apiToken: Joi.string(), diff --git a/doc/production-hosting.md b/doc/production-hosting.md index 42f3439bb0..4750349635 100644 --- a/doc/production-hosting.md +++ b/doc/production-hosting.md @@ -24,6 +24,7 @@ Production hosting is managed by the Shields ops team: | Cloudflare (CDN) | Access management | @espadrine | | Cloudflare (CDN) | Admin access | @calebcartwright, @chris48s, @espadrine, @paulmelnikow, @PyvesB | | Twitch | OAuth app | @PyvesB | +| Reddit | OAuth app | @chris48s, @PyvesB | | Discord | OAuth app | @PyvesB | | YouTube | Account owner | @PyvesB | | GitLab | Account owner | @calebcartwright | diff --git a/doc/server-secrets.md b/doc/server-secrets.md index 1f94579324..3c238c2794 100644 --- a/doc/server-secrets.md +++ b/doc/server-secrets.md @@ -290,6 +290,17 @@ Create an account, sign in and obtain generate a key on your `PYPI_URL` can be used to optionally send all the PyPI requests to a Self-hosted Pypi registry, users can also override this by query parameter `pypiBaseUrl`. +### Reddit + +Using a token for Reddit is optional but will allow higher API rates. + +- `REDDIT_CLIENT_ID` (yml: `private.reddit_client_id`) +- `REDDIT_CLIENT_SECRET` (yml: `private.reddit_client_secret`) + +Register to use the API using [this form](https://support.reddithelp.com/hc/en-us/requests/new?ticket_form_id=14868593862164) +and create an app in the [Reddit preferences page](https://www.reddit.com/prefs/apps) +in order to obtain a client id and a client secret for making Reddit API calls. + ### SymfonyInsight (formerly Sensiolabs) - `SL_INSIGHT_USER_UUID` (yml: `private.sl_insight_userUuid`) diff --git a/services/reddit/reddit-base.js b/services/reddit/reddit-base.js new file mode 100644 index 0000000000..992f2f4dcb --- /dev/null +++ b/services/reddit/reddit-base.js @@ -0,0 +1,89 @@ +import Joi from 'joi' +import { BaseJsonService } from '../index.js' + +const tokenSchema = Joi.object({ + access_token: Joi.string().required(), + expires_in: Joi.number(), +}) + +// Abstract class for Reddit badges +// Authorization flow based on https://github.com/reddit-archive/reddit/wiki/OAuth2#application-only-oauth. +export default class RedditBase extends BaseJsonService { + static category = 'social' + + static auth = { + userKey: 'reddit_client_id', + passKey: 'reddit_client_secret', + authorizedOrigins: ['https://www.reddit.com'], + isRequired: false, + } + + constructor(...args) { + super(...args) + if (!RedditBase._redditToken && this.authHelper.isConfigured) { + RedditBase._redditToken = this._getNewToken() + } + } + + async _getNewToken() { + const tokenRes = await super._requestJson( + this.authHelper.withBasicAuth({ + schema: tokenSchema, + url: 'https://www.reddit.com/api/v1/access_token', + options: { + method: 'POST', + body: 'grant_type=client_credentials', + }, + httpErrors: { + 401: 'invalid token', + }, + }), + ) + + // replace the token when we are 80% near the expire time + // 2147483647 is the max 32-bit value that is accepted by setTimeout(), it's about 24.9 days + const replaceTokenMs = Math.min( + tokenRes.expires_in * 1000 * 0.8, + 2147483647, + ) + const timeout = setTimeout(() => { + RedditBase._redditToken = this._getNewToken() + }, replaceTokenMs) + + // do not block program exit + timeout.unref() + + return tokenRes.access_token + } + + async _requestJson(request) { + if (!this.authHelper.isConfigured) { + return super._requestJson(request) + } + + request = await this.addBearerAuthHeader(request) + try { + return await super._requestJson(request) + } catch (err) { + if (err.response && err.response.statusCode === 401) { + // if the token is expired or has been revoked, retry once + RedditBase._redditToken = this._getNewToken() + request = await this.addBearerAuthHeader(request) + return super._requestJson(request) + } + // cannot recover + throw err + } + } + + async addBearerAuthHeader(request) { + return { + ...request, + options: { + headers: { + Authorization: `Bearer ${await RedditBase._redditToken}`, + }, + }, + } + } +} diff --git a/services/reddit/subreddit-subscribers.service.js b/services/reddit/subreddit-subscribers.service.js index 6cbf0456ec..2a505536b8 100644 --- a/services/reddit/subreddit-subscribers.service.js +++ b/services/reddit/subreddit-subscribers.service.js @@ -1,7 +1,8 @@ import Joi from 'joi' import { optionalNonNegativeInteger } from '../validators.js' import { metric } from '../text-formatters.js' -import { BaseJsonService, NotFound, pathParams } from '../index.js' +import { NotFound, pathParams } from '../index.js' +import RedditBase from './reddit-base.js' const schema = Joi.object({ data: Joi.object({ @@ -9,9 +10,7 @@ const schema = Joi.object({ }).required(), }).required() -export default class RedditSubredditSubscribers extends BaseJsonService { - static category = 'social' - +export default class RedditSubredditSubscribers extends RedditBase { static route = { base: 'reddit/subreddit-subscribers', pattern: ':subreddit', @@ -29,8 +28,6 @@ export default class RedditSubredditSubscribers extends BaseJsonService { }, } - static _cacheLength = 7200 - static defaultBadgeData = { label: 'reddit', namedLogo: 'reddit', @@ -49,7 +46,10 @@ export default class RedditSubredditSubscribers extends BaseJsonService { async fetch({ subreddit }) { return this._requestJson({ schema, - url: `https://www.reddit.com/r/${subreddit}/about.json`, + // API requests with a bearer token should be made to https://oauth.reddit.com, NOT www.reddit.com. + url: this.authHelper.isConfigured + ? `https://oauth.reddit.com/r/${subreddit}/about.json` + : `https://www.reddit.com/r/${subreddit}/about.json`, httpErrors: { 404: 'subreddit not found', 403: 'subreddit is private', diff --git a/services/reddit/subreddit-subscribers.tester.js b/services/reddit/subreddit-subscribers.tester.js index 1a9f5518b4..b993551c02 100644 --- a/services/reddit/subreddit-subscribers.tester.js +++ b/services/reddit/subreddit-subscribers.tester.js @@ -1,6 +1,10 @@ +import { noToken } from '../test-helpers.js' import { isMetric } from '../test-validators.js' import { createServiceTester } from '../tester.js' +import _serviceClass from './subreddit-subscribers.service.js' export const t = await createServiceTester() +const noRedditToken = noToken(_serviceClass) +const hasRedditToken = () => !noRedditToken() t.create('subreddit-subscribers (valid subreddit)') .get('/drums.json') @@ -30,7 +34,8 @@ t.create('subreddit-subscribers (private sub)') message: 'subreddit is private', }) -t.create('subreddit-subscribers (private sub)') +t.create('subreddit-subscribers (private sub, without token)') + .skipWhen(hasRedditToken) .get('/centuryclub.json') .intercept(nock => nock('https://www.reddit.com/r') @@ -41,3 +46,17 @@ t.create('subreddit-subscribers (private sub)') label: 'reddit', message: 'subreddit not found', }) + +t.create('subreddit-subscribers (private sub, with token)') + .skipWhen(noRedditToken) + .get('/centuryclub.json') + .intercept(nock => + nock('https://oauth.reddit.com/r') + .get('/centuryclub/about.json') + .reply(200, { kind: 't5', data: {} }), + ) + .networkOn() // API /access_token may or may not be called depending on whether another test ran before and cached the token. Rather than conditionally intercepting it, let it go through and only mock the API call we're validating specific behaviour against. + .expectBadge({ + label: 'reddit', + message: 'subreddit not found', + }) diff --git a/services/reddit/user-karma.service.js b/services/reddit/user-karma.service.js index ae0accb41c..460ae2d771 100644 --- a/services/reddit/user-karma.service.js +++ b/services/reddit/user-karma.service.js @@ -1,7 +1,8 @@ import Joi from 'joi' import { anyInteger } from '../validators.js' import { metric } from '../text-formatters.js' -import { BaseJsonService, pathParams } from '../index.js' +import { pathParams } from '../index.js' +import RedditBase from './reddit-base.js' const schema = Joi.object({ data: Joi.object({ @@ -10,9 +11,7 @@ const schema = Joi.object({ }).required(), }).required() -export default class RedditUserKarma extends BaseJsonService { - static category = 'social' - +export default class RedditUserKarma extends RedditBase { static route = { base: 'reddit/user-karma', pattern: ':variant(link|comment|combined)/:user', @@ -37,8 +36,6 @@ export default class RedditUserKarma extends BaseJsonService { }, } - static _cacheLength = 7200 - static defaultBadgeData = { label: 'reddit karma', namedLogo: 'reddit', @@ -61,7 +58,10 @@ export default class RedditUserKarma extends BaseJsonService { async fetch({ user }) { return this._requestJson({ schema, - url: `https://www.reddit.com/u/${user}/about.json`, + // API requests with a bearer token should be made to https://oauth.reddit.com, NOT www.reddit.com. + url: this.authHelper.isConfigured + ? `https://oauth.reddit.com/u/${user}/about.json` + : `https://www.reddit.com/u/${user}/about.json`, httpErrors: { 404: 'user not found', }, diff --git a/services/reddit/user-karma.tester.js b/services/reddit/user-karma.tester.js index 4bf9b828d9..418d1533f2 100644 --- a/services/reddit/user-karma.tester.js +++ b/services/reddit/user-karma.tester.js @@ -1,6 +1,10 @@ +import { noToken } from '../test-helpers.js' import { isMetricAllowNegative } from '../test-validators.js' import { createServiceTester } from '../tester.js' +import _serviceClass from './subreddit-subscribers.service.js' export const t = await createServiceTester() +const noRedditToken = noToken(_serviceClass) +const hasRedditToken = () => !noRedditToken() t.create('user-karma (valid - link)') .get('/link/user_simulator.json') @@ -30,7 +34,8 @@ t.create('user-karma (non-existing user)') message: 'user not found', }) -t.create('user-karma (link - math check)') +t.create('user-karma (link - math check, without token)') + .skipWhen(hasRedditToken) .get('/link/user_simulator.json') .intercept(nock => nock('https://www.reddit.com/u') @@ -42,7 +47,22 @@ t.create('user-karma (link - math check)') message: '20', }) -t.create('user-karma (comment - math check)') +t.create('user-karma (link - math check, with token)') + .skipWhen(noRedditToken) + .get('/link/user_simulator.json') + .intercept(nock => + nock('https://oauth.reddit.com/u') + .get('/user_simulator/about.json') + .reply(200, { kind: 't2', data: { link_karma: 20, comment_karma: 80 } }), + ) + .networkOn() // API /access_token may or may not be called depending on whether another test ran before and cached the token. Rather than conditionally intercepting it, let it go through and only mock the API call we're validating specific behaviour against. + .expectBadge({ + label: 'u/user_simulator karma (link)', + message: '20', + }) + +t.create('user-karma (comment - math check, without token)') + .skipWhen(hasRedditToken) .get('/comment/user_simulator.json') .intercept(nock => nock('https://www.reddit.com/u') @@ -54,7 +74,22 @@ t.create('user-karma (comment - math check)') message: '80', }) -t.create('user-karma (combined - math check)') +t.create('user-karma (comment - math check, with token)') + .skipWhen(noRedditToken) + .get('/comment/user_simulator.json') + .intercept(nock => + nock('https://oauth.reddit.com/u') + .get('/user_simulator/about.json') + .reply(200, { kind: 't2', data: { link_karma: 20, comment_karma: 80 } }), + ) + .networkOn() // API /access_token may or may not be called depending on whether another test ran before and cached the token. Rather than conditionally intercepting it, let it go through and only mock the API call we're validating specific behaviour against. + .expectBadge({ + label: 'u/user_simulator karma (comment)', + message: '80', + }) + +t.create('user-karma (combined - math check, without token)') + .skipWhen(hasRedditToken) .get('/combined/user_simulator.json') .intercept(nock => nock('https://www.reddit.com/u') @@ -66,7 +101,22 @@ t.create('user-karma (combined - math check)') message: '100', }) -t.create('user-karma (combined - missing data)') +t.create('user-karma (combined - math check, with token)') + .skipWhen(noRedditToken) + .get('/combined/user_simulator.json') + .intercept(nock => + nock('https://oauth.reddit.com/u') + .get('/user_simulator/about.json') + .reply(200, { kind: 't2', data: { link_karma: 20, comment_karma: 80 } }), + ) + .networkOn() // API /access_token may or may not be called depending on whether another test ran before and cached the token. Rather than conditionally intercepting it, let it go through and only mock the API call we're validating specific behaviour against. + .expectBadge({ + label: 'u/user_simulator karma', + message: '100', + }) + +t.create('user-karma (combined - missing data, without token)') + .skipWhen(hasRedditToken) .get('/combined/user_simulator.json') .intercept(nock => nock('https://www.reddit.com/u') @@ -77,3 +127,17 @@ t.create('user-karma (combined - missing data)') label: 'reddit karma', message: 'invalid response data', }) + +t.create('user-karma (combined - missing data, with token)') + .skipWhen(noRedditToken) + .get('/combined/user_simulator.json') + .intercept(nock => + nock('https://oauth.reddit.com/u') + .get('/user_simulator/about.json') + .reply(200, { kind: 't2', data: { link_karma: 20 } }), + ) + .networkOn() // API /access_token may or may not be called depending on whether another test ran before and cached the token. Rather than conditionally intercepting it, let it go through and only mock the API call we're validating specific behaviour against. + .expectBadge({ + label: 'reddit karma', + message: 'invalid response data', + })