Merge branch 'tusooa/1362-no-create-app-on-first-visit' into 'develop'

Create an app only when needed

See merge request pleroma/pleroma-fe!2073

build/msw_plugin.js

@@ -0,0 +1,28 @@
+import { resolve } from 'node:path'
+import { readFile } from 'node:fs/promises'
+
+const target = 'node_modules/msw/lib/mockServiceWorker.js'
+
+const mswPlugin = () => {
+  let projectRoot
+  return {
+    name: 'msw-plugin',
+    apply: 'serve',
+    configResolved (conf) {
+      projectRoot = conf.root
+    },
+    configureServer (server) {
+      server.middlewares.use(async (req, res, next) => {
+        if (req.path === '/mockServiceWorker.js') {
+          const file = await readFile(resolve(projectRoot, target))
+          res.set('Content-Type', 'text/javascript')
+          res.send(file)
+        } else {
+          next()
+        }
+      })
+    }
+  }
+}
+
+export default mswPlugin

changelog.d/no-create-app-on-first-visit.fix

@@ -0,0 +1 @@
+Create an OAuth app only when needed

package.json

@@ -88,6 +88,7 @@
     "http-proxy-middleware": "3.0.3",
     "iso-639-1": "3.1.5",
     "lodash": "4.17.21",
+    "msw": "2.7.3",
     "nightwatch": "3.11.1",
     "opn": "5.5.0",
     "ora": "0.4.1",

src/boot/after_store.js

@@ -11,7 +11,6 @@ import routes from './routes'
 import VBodyScrollLock from 'src/directives/body_scroll_lock'
 
 import { windowWidth, windowHeight } from '../services/window_utils/window_utils'
-import { getOrCreateApp, getClientToken } from '../services/new_api/oauth.js'
 import backendInteractorService from '../services/backend_interactor_service/backend_interactor_service.js'
 import { applyConfig } from '../services/style_setter/style_setter.js'
 import FaviconService from '../services/favicon_service/favicon_service.js'
@@ -228,17 +227,8 @@ const getStickers = async ({ store }) => {
 }
 
 const getAppSecret = async ({ store }) => {
-  const { state, commit } = store
+  if (store.state.oauth.userToken) {
-  const { oauth, instance } = state
+    store.commit('setBackendInteractor', backendInteractorService(store.getters.getToken()))
-  if (oauth.userToken) {
-    commit('setBackendInteractor', backendInteractorService(store.getters.getToken()))
-  } else {
-    return getOrCreateApp({ ...oauth, instance: instance.server, commit })
-      .then((app) => getClientToken({ ...app, instance: instance.server }))
-      .then((token) => {
-        commit('setAppToken', token.access_token)
-        commit('setBackendInteractor', backendInteractorService(store.getters.getToken()))
-      })
   }
 }
 

src/components/login_form/login_form.js

@@ -34,16 +34,21 @@ const LoginForm = {
       this.isTokenAuth ? this.submitToken() : this.submitPassword()
     },
     submitToken () {
-      const { clientId, clientSecret } = this.oauth
       const data = {
-        clientId,
-        clientSecret,
         instance: this.instance.server,
         commit: this.$store.commit
       }
 
-      oauthApi.getOrCreateApp(data)
+      // NOTE: we do not really need the app token, but obtaining a token and
-        .then((app) => { oauthApi.login({ ...app, ...data }) })
+      // calling verify_credentials is the only way to ensure the app still works.
+      this.$store.dispatch('ensureAppToken')
+        .then(() => {
+          const app = {
+            clientId: this.oauth.clientId,
+            clientSecret: this.oauth.clientSecret,
+          }
+          oauthApi.login({ ...app, ...data })
+        })
     },
     submitPassword () {
       const { clientId } = this.oauth
@@ -55,7 +60,14 @@ const LoginForm = {
       }
       this.error = false
 
-      oauthApi.getOrCreateApp(data).then((app) => {
+      // NOTE: we do not really need the app token, but obtaining a token and
+      // calling verify_credentials is the only way to ensure the app still works.
+      this.$store.dispatch('ensureAppToken').then(() => {
+        const app = {
+          clientId: this.oauth.clientId,
+          clientSecret: this.oauth.clientSecret,
+        }
+
         oauthApi.getTokenWithCredentials(
           {
             ...app,

src/modules/oauth.js

@@ -1,5 +1,27 @@
+import { createApp, getClientToken, verifyAppToken } from 'src/services/new_api/oauth.js'
+
+// status codes about verifyAppToken (GET /api/v1/apps/verify_credentials)
+const isAppTokenRejected = error => (
+  // Pleroma API docs say it returns 422 when unauthorized
+  error.statusCode === 422 ||
+  // but it actually returns 400 (as of 2.9.0)
+  // NOTE: don't try to match against the error message, it is translatable
+  error.statusCode === 400 ||
+  // and Mastodon docs say it returns 401
+  error.statusCode === 401
+)
+
+// status codes about getAppToken (GET /oauth/token)
+const isClientDataRejected = error => (
+  // Mastodon docs say it returns 401
+  error.statusCode === 401 ||
+  // but Pleroma actually returns 400 (as of 2.9.0)
+  // NOTE: don't try to match against the error message, it is translatable
+  error.statusCode === 400
+)
+
 const oauth = {
-  state: {
+  state: () => ({
     clientId: false,
     clientSecret: false,
     /* App token is authentication for app without any user, used mostly for
@@ -11,7 +33,7 @@ const oauth = {
      * that need authorized user to be successful (i.e. posting, liking etc)
      */
     userToken: false
-  },
+  }),
   mutations: {
     setClientData (state, { clientId, clientSecret }) {
       state.clientId = clientId
@@ -41,6 +63,88 @@ const oauth = {
       // added here for smoother transition, otherwise user will be logged out
       return state.userToken || state.token
     }
+  },
+  actions: {
+    async createApp ({ rootState, commit }) {
+      const instance = rootState.instance.server
+      const app = await createApp(instance)
+      commit('setClientData', app)
+      return app
+    },
+    /// Use this if you want to get the client id and secret but are not interested
+    /// in whether they are valid.
+    /// @return {{ clientId: string, clientSecret: string }} An object representing the app
+    ensureApp ({ state, dispatch }) {
+      if (state.clientId && state.clientSecret) {
+        return {
+          clientId: state.clientId,
+          clientSecret: state.clientSecret
+        }
+      } else {
+        return dispatch('createApp')
+      }
+    },
+    async getAppToken ({ state, rootState, commit }) {
+      const res = await getClientToken({
+        clientId: state.clientId,
+        clientSecret: state.clientSecret,
+        instance: rootState.instance.server
+      })
+      commit('setAppToken', res.access_token)
+      return res.access_token
+    },
+    /// Use this if you want to ensure the app is still valid to use.
+    /// @return {string} The access token to the app (not attached to any user)
+    async ensureAppToken ({ state, rootState, dispatch, commit }) {
+      if (state.appToken) {
+        try {
+          await verifyAppToken({
+            instance: rootState.instance.server,
+            appToken: state.appToken
+          })
+          return state.appToken
+        } catch (e) {
+          if (!isAppTokenRejected(e)) {
+            // The server did not reject our token, but we encountered other problems. Maybe the server is down.
+            throw e
+          } else {
+            // The app token is rejected, so it is no longer useful.
+            commit('setAppToken', false)
+          }
+        }
+      }
+
+      // appToken is not available, or is rejected: try to get a new one.
+      // First, make sure the client id and client secret are filled.
+      try {
+        await dispatch('ensureApp')
+      } catch (e) {
+        console.error('Cannot create app', e)
+        throw e
+      }
+
+      // Note that at this step, the client id and secret may be invalid
+      // (because the backend may have already deleted the app due to no user login)
+      try {
+        return await dispatch('getAppToken')
+      } catch (e) {
+        if (!isClientDataRejected(e)) {
+          // Non-credentials problem, fail fast
+          console.error('Cannot get app token', e)
+          throw e
+        } else {
+          // the client id and secret are invalid, so we should clear them
+          // and re-create our app
+          commit('setClientData', {
+            clientId: false,
+            clientSecret: false
+          })
+          await dispatch('createApp')
+          // try once again to get the token
+          return await dispatch('getAppToken')
+        }
+      }
+    }
   }
 }
 

src/modules/users.js

@@ -1,5 +1,6 @@
 import backendInteractorService from '../services/backend_interactor_service/backend_interactor_service.js'
 import { windowWidth, windowHeight } from '../services/window_utils/window_utils'
+import apiService from '../services/api/api.service.js'
 import oauthApi from '../services/new_api/oauth.js'
 import { compact, map, each, mergeWith, last, concat, uniq, isArray } from 'lodash'
 import { registerPushNotifications, unregisterPushNotifications } from '../services/sw/sw.js'
@@ -527,11 +528,10 @@ const users = {
     async signUp (store, userInfo) {
       store.commit('signUpPending')
 
-      const rootState = store.rootState
-
       try {
-        const data = await rootState.api.backendInteractor.register(
+        const token = await store.dispatch('ensureAppToken')
-          { params: { ...userInfo } }
+        const data = await apiService.register(
+          { credentials: token, params: { ...userInfo } }
         )
 
         if (data.access_token) {
@@ -562,7 +562,9 @@ const users = {
         instance: instance.server
       }
 
-      return oauthApi.getOrCreateApp(data)
+      // NOTE: No need to verify the app still exists, because if it doesn't,
+      // the token will be invalid too
+      return store.dispatch('ensureApp')
         .then((app) => {
           const params = {
             app,

src/services/new_api/oauth.js

@@ -1,12 +1,20 @@
 import { reduce } from 'lodash'
+import { StatusCodeError } from 'src/services/errors/errors.js'
 
 const REDIRECT_URI = `${window.location.origin}/oauth-callback`
 
-export const getOrCreateApp = ({ clientId, clientSecret, instance, commit }) => {
+export const getJsonOrError = async (response) => {
-  if (clientId && clientSecret) {
+  if (response.ok) {
-    return Promise.resolve({ clientId, clientSecret })
+    return response.json()
+      .catch((error) => {
+        throw new StatusCodeError(response.status, error, {}, response)
+      })
+  } else {
+    throw new StatusCodeError(response.status, await response.text(), {}, response)
   }
+}
 
+export const createApp = (instance) => {
   const url = `${instance}/api/v1/apps`
   const form = new window.FormData()
 
@@ -19,9 +27,16 @@ export const getOrCreateApp = ({ clientId, clientSecret, instance, commit }) =>
     method: 'POST',
     body: form
   })
-    .then((data) => data.json())
+    .then(getJsonOrError)
     .then((app) => ({ clientId: app.client_id, clientSecret: app.client_secret }))
-    .then((app) => commit('setClientData', app) || app)
+}
+
+export const verifyAppToken = ({ instance, appToken }) => {
+  return window.fetch(`${instance}/api/v1/apps/verify_credentials`, {
+    method: 'GET',
+    headers: { Authorization: `Bearer ${appToken}` }
+  })
+    .then(getJsonOrError)
 }
 
 const login = ({ instance, clientId }) => {
@@ -92,7 +107,7 @@ export const getClientToken = ({ clientId, clientSecret, instance }) => {
   return window.fetch(url, {
     method: 'POST',
     body: form
-  }).then((data) => data.json())
+  }).then(getJsonOrError)
 }
 const verifyOTPCode = ({ app, instance, mfaToken, code }) => {
   const url = `${instance}/oauth/mfa/challenge`
@@ -144,7 +159,6 @@ const oauth = {
   login,
   getToken,
   getTokenWithCredentials,
-  getOrCreateApp,
   verifyOTPCode,
   verifyRecoveryCode,
   revokeToken

test/fixtures/mock_api.js

@@ -0,0 +1,62 @@
+import { test as testBase } from 'vitest'
+import { setupWorker } from 'msw/browser'
+import { http, HttpResponse } from 'msw'
+
+// https://mswjs.io/docs/recipes/vitest-browser-mode
+export const injectMswToTest = (defaultHandlers) => {
+  const worker = setupWorker(...defaultHandlers)
+
+  return testBase.extend({
+    worker: [
+      async ({}, use) => {
+        await worker.start()
+
+        await use(worker)
+
+        worker.resetHandlers()
+        worker.stop()
+      },
+      {
+        auto: true
+      }
+    ],
+  })
+}
+
+export const testServer = 'https://test.server.example'
+
+export const authApis = [
+  http.post(`${testServer}/api/v1/apps`, () => {
+    return HttpResponse.json({
+      client_id: 'test-id',
+      client_secret: 'test-secret'
+    })
+  }),
+  http.get(`${testServer}/api/v1/apps/verify_credentials`, ({ request }) => {
+    const authHeader = request.headers.get('Authorization')
+    if (authHeader === 'Bearer test-app-token' ||
+        authHeader === 'Bearer also-good-app-token') {
+      return HttpResponse.json({})
+    } else {
+      // Pleroma 2.9.0 gives the following respoonse upon error
+      return HttpResponse.json({ error: { detail: 'Internal server error' } }, {
+        status: 400
+      })
+    }
+  }),
+  http.post(`${testServer}/oauth/token`, async ({ request }) => {
+    const data = await request.formData()
+
+    if (data.get('client_id') === 'test-id' &&
+       data.get('client_secret') === 'test-secret' &&
+       data.get('grant_type') === 'client_credentials' &&
+       data.has('redirect_uri')) {
+      return HttpResponse.json({ access_token: 'test-app-token' })
+    } else {
+      // Pleroma 2.9.0 gives the following respoonse upon error
+      return HttpResponse.json({ error: 'Invalid credentials' }, {
+        status: 400
+      })
+    }
+  })
+]

test/unit/specs/modules/oauth.spec.js

@@ -0,0 +1,199 @@
+import { createStore } from 'vuex'
+import { http, HttpResponse } from 'msw'
+import oauth from 'src/modules/oauth.js'
+import { injectMswToTest, authApis, testServer } from '/test/fixtures/mock_api.js'
+
+const test = injectMswToTest(authApis)
+
+const getStore = (defaultStateInjection) => {
+  const stateFunction = defaultStateInjection ? () => {
+    return {
+      ...oauth.state(),
+      ...defaultStateInjection
+    }
+  } : oauth.state
+
+  return createStore({
+    modules: {
+      instance: {
+        state: () => ({ server: testServer })
+      },
+      oauth: {
+        ...oauth,
+        state: stateFunction
+      }
+    }
+  })
+}
+
+
+describe('createApp', () => {
+  test('it should use create an app and record client id and secret', async () => {
+    const store = getStore()
+    const app = await store.dispatch('createApp')
+    expect(store.state.oauth.clientId).to.eql('test-id')
+    expect(store.state.oauth.clientSecret).to.eql('test-secret')
+    expect(app.clientId).to.eql('test-id')
+    expect(app.clientSecret).to.eql('test-secret')
+  })
+
+  test('it should throw and not update if failed', async ({ worker }) => {
+    worker.use(
+      http.post(`${testServer}/api/v1/apps`, () => {
+        return HttpResponse.text('Throttled', { status: 429 })
+      })
+    )
+
+    const store = getStore()
+    const res = store.dispatch('createApp')
+    await expect(res).rejects.toThrowError('Throttled')
+    expect(store.state.oauth.clientId).to.eql(false)
+    expect(store.state.oauth.clientSecret).to.eql(false)
+  })
+})
+
+describe('ensureApp', () => {
+  test('it should create an app if it does not exist', async () => {
+    const store = getStore()
+    const app = await store.dispatch('ensureApp')
+    expect(store.state.oauth.clientId).to.eql('test-id')
+    expect(store.state.oauth.clientSecret).to.eql('test-secret')
+    expect(app.clientId).to.eql('test-id')
+    expect(app.clientSecret).to.eql('test-secret')
+  })
+
+  test('it should not create an app if it exists', async ({ worker }) => {
+    worker.use(
+      http.post(`${testServer}/api/v1/apps`, () => {
+        return HttpResponse.text('Should not call this API', { status: 400 })
+      })
+    )
+
+    const store = getStore({
+      clientId: 'another-id',
+      clientSecret: 'another-secret'
+    })
+    const app = await store.dispatch('ensureApp')
+    expect(store.state.oauth.clientId).to.eql('another-id')
+    expect(store.state.oauth.clientSecret).to.eql('another-secret')
+    expect(app.clientId).to.eql('another-id')
+    expect(app.clientSecret).to.eql('another-secret')
+  })
+})
+
+describe('getAppToken', () => {
+  test('it should get app token and set it in state', async () => {
+    const store = getStore({
+      clientId: 'test-id',
+      clientSecret: 'test-secret'
+    })
+    const token = await store.dispatch('getAppToken')
+    expect(token).to.eql('test-app-token')
+    expect(store.state.oauth.appToken).to.eql('test-app-token')
+  })
+
+  test('it should throw and not set state if it cannot get app token', async () => {
+    const store = getStore({
+      clientId: 'bad-id',
+      clientSecret: 'bad-secret'
+    })
+    await expect(store.dispatch('getAppToken')).rejects.toThrowError('400')
+    expect(store.state.oauth.appToken).to.eql(false)
+  })
+})
+
+describe('ensureAppToken', () => {
+  test('it should work if the state is empty', async () => {
+    const store = getStore()
+    const token = await store.dispatch('ensureAppToken')
+    expect(token).to.eql('test-app-token')
+    expect(store.state.oauth.appToken).to.eql('test-app-token')
+  })
+
+  test('it should work if we already have a working token', async () => {
+    const store = getStore({
+      appToken: 'also-good-app-token'
+    })
+    const token = await store.dispatch('ensureAppToken')
+    expect(token).to.eql('also-good-app-token')
+    expect(store.state.oauth.appToken).to.eql('also-good-app-token')
+  })
+
+  test('it should work if we have a bad token but good app credentials', async ({ worker }) => {
+    worker.use(
+      http.post(`${testServer}/api/v1/apps`, () => {
+        return HttpResponse.text('Should not call this API', { status: 400 })
+      })
+    )
+    const store = getStore({
+      appToken: 'bad-app-token',
+      clientId: 'test-id',
+      clientSecret: 'test-secret'
+    })
+    const token = await store.dispatch('ensureAppToken')
+    expect(token).to.eql('test-app-token')
+    expect(store.state.oauth.appToken).to.eql('test-app-token')
+  })
+
+  test('it should work if we have no token but good app credentials', async ({ worker }) => {
+    worker.use(
+      http.post(`${testServer}/api/v1/apps`, () => {
+        return HttpResponse.text('Should not call this API', { status: 400 })
+      })
+    )
+    const store = getStore({
+      clientId: 'test-id',
+      clientSecret: 'test-secret'
+    })
+    const token = await store.dispatch('ensureAppToken')
+    expect(token).to.eql('test-app-token')
+    expect(store.state.oauth.appToken).to.eql('test-app-token')
+  })
+
+  test('it should work if we have no token and bad app credentials', async () => {
+    const store = getStore({
+      clientId: 'bad-id',
+      clientSecret: 'bad-secret'
+    })
+    const token = await store.dispatch('ensureAppToken')
+    expect(token).to.eql('test-app-token')
+    expect(store.state.oauth.appToken).to.eql('test-app-token')
+    expect(store.state.oauth.clientId).to.eql('test-id')
+    expect(store.state.oauth.clientSecret).to.eql('test-secret')
+  })
+
+  test('it should work if we have bad token and bad app credentials', async () => {
+    const store = getStore({
+      appToken: 'bad-app-token',
+      clientId: 'bad-id',
+      clientSecret: 'bad-secret'
+    })
+    const token = await store.dispatch('ensureAppToken')
+    expect(token).to.eql('test-app-token')
+    expect(store.state.oauth.appToken).to.eql('test-app-token')
+    expect(store.state.oauth.clientId).to.eql('test-id')
+    expect(store.state.oauth.clientSecret).to.eql('test-secret')
+  })
+
+  test('it should throw if we cannot create an app', async ({ worker }) => {
+    worker.use(
+      http.post(`${testServer}/api/v1/apps`, () => {
+        return HttpResponse.text('Throttled', { status: 429 })
+      })
+    )
+
+    const store = getStore()
+    await expect(store.dispatch('ensureAppToken')).rejects.toThrowError('Throttled')
+  })
+
+  test('it should throw if we cannot obtain app token', async ({ worker }) => {
+    worker.use(
+      http.post(`${testServer}/oauth/token`, () => {
+        return HttpResponse.text('Throttled', { status: 429 })
+      })
+    )
+
+    const store = getStore()
+    await expect(store.dispatch('ensureAppToken')).rejects.toThrowError('Throttled')
+  })
+})

vite.config.js

@@ -9,6 +9,7 @@ import emojisPlugin from './build/emojis_plugin.js'
 import { devSwPlugin, buildSwPlugin, swMessagesPlugin } from './build/sw_plugin.js'
 import copyPlugin from './build/copy_plugin.js'
 import { getCommitHash } from './build/commit_hash.js'
+import mswPlugin from './build/msw_plugin.js'
 
 const localConfigPath = '<projectRoot>/config/local.json'
 const getLocalDevSettings = async () => {
@@ -119,7 +120,8 @@ export default defineConfig(async ({ mode, command }) => {
         lintInWorker: true,
         lintOnStart: true,
         cacheLocation: resolve(projectRoot, 'node_modules/.cache/stylelintcache')
-      })
+      }),
+      ...(mode === 'test' ? [mswPlugin()] : [])
     ],
     optimizeDeps: {
       // For unknown reasons, during vitest, vite will re-optimize the following

yarn.lock

@@ -5856,7 +5856,7 @@ ms@2.1.3, ms@^2.1.1, ms@^2.1.3:
   resolved "https://registry.yarnpkg.com/ms/-/ms-2.1.3.tgz#574c8138ce1d2b5861f0b44579dbadd60c6615b2"
   integrity sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==
 
-msw@^2.7.3:
+msw@2.7.3, msw@^2.7.3:
   version "2.7.3"
   resolved "https://registry.yarnpkg.com/msw/-/msw-2.7.3.tgz#5ad569fae7c7cdb8be2eeba3d041c185600b25a0"
   integrity sha512-+mycXv8l2fEAjFZ5sjrtjJDmm2ceKGjrNbBr1durRg6VkU9fNUE/gsmQ51hWbHqs+l35W1iM+ZsmOD9Fd6lspw==