Add auth support to [Reddit] badges (#10790)

* Add auth support to [Reddit] badges

* Simplify token response schema

.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 }}'

.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 }}'

.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 }}'

.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 }}

.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 }}'

.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 }}'

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'

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: ...

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: '...'

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(),

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                                                |

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`)

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}`,
+        },
+      },
+    }
+  }
+}

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',

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',
+  })

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',
       },

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',
+  })