feat: [OCISDEV-249] add MFA capability

We've added a capability to check if MFA is enabled.
If the capability is enabled, we will require MFA when accessing
the admin settings page.

Author: LukasHirt

changelog/unreleased/enhancement-add-mfa-capability.md

@@ -0,0 +1,6 @@
+Enhancement: Add MFA capability
+
+We've added a capability to check if MFA is enabled.
+If the capability is enabled, we will require MFA when accessing the admin settings page.
+
+https://github.com/owncloud/web/pull/12925

dev/docker/ocis.web.config.json

@@ -5,7 +5,8 @@
     "metadata_url": "https://host.docker.internal:9200/.well-known/openid-configuration",
     "authority": "https://host.docker.internal:9200",
     "client_id": "web",
-    "response_type": "code"
+    "response_type": "code",
+    "scope": "openid profile email acr"
   },
   "options": {
     "contextHelpersReadMore": true

packages/web-app-admin-settings/src/index.ts

@@ -10,6 +10,8 @@ import {
   AppNavigationItem,
   defineWebApplication,
   useAbility,
+  useAuthService,
+  useCapabilityStore,
   useUserStore
 } from '@ownclouders/web-pkg'
 import { RouteRecordRaw } from 'vue-router'
@@ -22,7 +24,7 @@ function $gettext(msg: string) {
 
 const appId = 'admin-settings'
 
-function getAvailableRoute(ability: Ability) {
+function getNextAvailableRoute(ability: Ability) {
   if (ability.can('read-all', 'Setting')) {
     return { name: 'admin-settings-general' }
   }
@@ -42,6 +44,16 @@ function getAvailableRoute(ability: Ability) {
   throw Error('Insufficient permissions')
 }
 
+async function requireAcr(redirectUrl: string) {
+  const capabilityStore = useCapabilityStore()
+  if (!capabilityStore.authMfaEnabled) {
+    return
+  }
+
+  const authService = useAuthService()
+  await authService.requireAcr(capabilityStore.authMfaRequiredLevelname, redirectUrl)
+}
+
 export const routes = ({ $ability }: { $ability: Ability }): RouteRecordRaw[] => [
   {
     path: '/',
@@ -53,10 +65,13 @@ export const routes = ({ $ability }: { $ability: Ability }): RouteRecordRaw[] =>
     path: '/general',
     name: 'admin-settings-general',
     component: General,
-    beforeEnter: (to, from, next) => {
+    beforeEnter: async (to, from, next) => {
       if (!$ability.can('read-all', 'Setting')) {
-        return next(getAvailableRoute($ability))
+        return next(getNextAvailableRoute($ability))
       }
+
+      await requireAcr(to.fullPath)
+
       next()
     },
     meta: {
@@ -68,10 +83,13 @@ export const routes = ({ $ability }: { $ability: Ability }): RouteRecordRaw[] =>
     path: '/users',
     name: 'admin-settings-users',
     component: Users,
-    beforeEnter: (to, from, next) => {
+    beforeEnter: async (to, from, next) => {
       if (!$ability.can('read-all', 'Account')) {
-        return next(getAvailableRoute($ability))
+        return next(getNextAvailableRoute($ability))
       }
+
+      await requireAcr(to.fullPath)
+
       next()
     },
     meta: {
@@ -83,10 +101,13 @@ export const routes = ({ $ability }: { $ability: Ability }): RouteRecordRaw[] =>
     path: '/groups',
     name: 'admin-settings-groups',
     component: Groups,
-    beforeEnter: (to, from, next) => {
+    beforeEnter: async (to, from, next) => {
       if (!$ability.can('read-all', 'Group')) {
-        return next(getAvailableRoute($ability))
+        return next(getNextAvailableRoute($ability))
       }
+
+      await requireAcr(to.fullPath)
+
       next()
     },
     meta: {
@@ -98,10 +119,13 @@ export const routes = ({ $ability }: { $ability: Ability }): RouteRecordRaw[] =>
     path: '/spaces',
     name: 'admin-settings-spaces',
     component: Spaces,
-    beforeEnter: (to, from, next) => {
+    beforeEnter: async (to, from, next) => {
       if (!$ability.can('read-all', 'Drive')) {
-        return next(getAvailableRoute($ability))
+        return next(getNextAvailableRoute($ability))
       }
+
+      await requireAcr(to.fullPath)
+
       next()
     },
     meta: {

packages/web-app-admin-settings/tests/unit/index.spec.ts

@@ -1,10 +1,26 @@
 import { navItems, routes } from '../../src/index'
 import { Ability } from '@ownclouders/web-client'
+import { createTestingPinia } from '@ownclouders/web-test-helpers'
 import { mock } from 'vitest-mock-extended'
+import * as pkg from '@ownclouders/web-pkg'
+import { AuthService } from '../../../web-runtime/src/services/auth/authService'
+
+const mockRequireAcr = vi.fn()
+vi.spyOn(pkg, 'useAuthService').mockReturnValue(mock<AuthService>({ requireAcr: mockRequireAcr }))
 
 const getAbilityMock = (hasPermission: boolean) => mock<Ability>({ can: () => hasPermission })
 
 describe('admin settings index', () => {
+  beforeEach(() => {
+    createTestingPinia({
+      initialState: {
+        capabilities: {
+          capabilities: { auth: { mfa: { enabled: true, levelnames: ['advanced'] } } }
+        }
+      }
+    })
+  })
+
   describe('navItems', () => {
     describe('general', () => {
       it.each([true, false])('should be enabled according to the permissions', (enabled) => {
@@ -64,11 +80,11 @@ describe('admin settings index', () => {
         }),
         redirect: { name: 'admin-settings-groups' }
       }
-    ])('redirects "/general" with sufficient permissions', ({ can, redirect }) => {
+    ])('redirects "/general" with sufficient permissions', async ({ can, redirect }) => {
       const ability = mock<Ability>({ can })
       const route = routes({ $ability: ability }).find((n) => n.path === '/general')
       const nextMock = vi.fn()
-      ;(route.beforeEnter as any)({}, {}, nextMock)
+      await (route.beforeEnter as any)({}, {}, nextMock)
       const args = [...(redirect ? [redirect] : [])]
       expect(nextMock).toHaveBeenCalledWith(...args)
     })
@@ -84,11 +100,11 @@ describe('admin settings index', () => {
         }),
         redirect: { name: 'admin-settings-spaces' }
       }
-    ])('redirects "/users" with sufficient permissions', ({ can, redirect }) => {
+    ])('redirects "/users" with sufficient permissions', async ({ can, redirect }) => {
       const ability = mock<Ability>({ can })
       const route = routes({ $ability: ability }).find((n) => n.path === '/users')
       const nextMock = vi.fn()
-      ;(route.beforeEnter as any)({}, {}, nextMock)
+      await (route.beforeEnter as any)({}, {}, nextMock)
       const args = [...(redirect ? [redirect] : [])]
       expect(nextMock).toHaveBeenCalledWith(...args)
     })
@@ -104,11 +120,11 @@ describe('admin settings index', () => {
         }),
         redirect: { name: 'admin-settings-general' }
       }
-    ])('redirects "/groups" with sufficient permissions', ({ can, redirect }) => {
+    ])('redirects "/groups" with sufficient permissions', async ({ can, redirect }) => {
       const ability = mock<Ability>({ can })
       const route = routes({ $ability: ability }).find((n) => n.path === '/groups')
       const nextMock = vi.fn()
-      ;(route.beforeEnter as any)({}, {}, nextMock)
+      await (route.beforeEnter as any)({}, {}, nextMock)
       const args = [...(redirect ? [redirect] : [])]
       expect(nextMock).toHaveBeenCalledWith(...args)
     })
@@ -124,24 +140,56 @@ describe('admin settings index', () => {
         }),
         redirect: { name: 'admin-settings-users' }
       }
-    ])('redirects "/spaces" with sufficient permissions', ({ can, redirect }) => {
+    ])('redirects "/spaces" with sufficient permissions', async ({ can, redirect }) => {
       const ability = mock<Ability>({ can })
       const route = routes({ $ability: ability }).find((n) => n.path === '/spaces')
       const nextMock = vi.fn()
-      ;(route.beforeEnter as any)({}, {}, nextMock)
+      await (route.beforeEnter as any)({}, {}, nextMock)
       const args = [...(redirect ? [redirect] : [])]
       expect(nextMock).toHaveBeenCalledWith(...args)
     })
     it.each(['/general', '/users', '/groups', '/spaces'])(
       'should throw an error if permissions are insufficient',
-      (path) => {
+      async (path) => {
         const ability = mock<Ability>({ can: vi.fn(() => false) })
         const route = routes({ $ability: ability }).find((n) => n.path === path)
         const nextMock = vi.fn()
-        expect(() => {
-          ;(route.beforeEnter as any)({}, {}, nextMock)
-        }).toThrowError('Insufficient permissions')
+        await expect(() => (route.beforeEnter as any)({}, {}, nextMock)).rejects.toThrowError(
+          'Insufficient permissions'
+        )
       }
     )
+
+    describe('requireAcr', () => {
+      it.each(['/general', '/users', '/groups', '/spaces'])(
+        'should call requireAcr if MFA is enabled when path is %s',
+        async (path) => {
+          const ability = mock<Ability>({ can: vi.fn(() => true) })
+          const route = routes({ $ability: ability }).find((n) => n.path === path)
+          await (route.beforeEnter as any)({ fullPath: path }, {}, vi.fn())
+          expect(mockRequireAcr).toHaveBeenCalledWith('advanced', path)
+        }
+      )
+    })
+
+    describe('requireAcr', () => {
+      it.each(['/general', '/users', '/groups', '/spaces'])(
+        'should not call requireAcr if MFA is disabled when path is %s',
+        async (path) => {
+          createTestingPinia({
+            initialState: {
+              capabilities: {
+                capabilities: { auth: { mfa: { enabled: false, levelnames: ['advanced'] } } }
+              }
+            }
+          })
+
+          const ability = mock<Ability>({ can: vi.fn(() => true) })
+          const route = routes({ $ability: ability }).find((n) => n.path === path)
+          await (route.beforeEnter as any)({}, {}, vi.fn())
+          expect(mockRequireAcr).not.toHaveBeenCalled()
+        }
+      )
+    })
   })
 })

packages/web-client/src/ocs/capabilities.ts

@@ -50,8 +50,16 @@ export interface ArchiverCapability {
   max_size?: string
 }
 
+interface AuthCapability {
+  mfa: {
+    enabled?: boolean
+    levelnames?: string[]
+  }
+}
+
 export interface Capabilities {
   capabilities: {
+    auth: AuthCapability
     checksums?: {
       preferredUploadType?: string
       supportedTypes?: string[]

packages/web-pkg/src/composables/authContext/useAuthService.ts

@@ -6,6 +6,7 @@ export interface AuthServiceInterface {
   signinSilent(): Promise<unknown>
   logoutUser(): Promise<void | NavigationFailure>
   getRefreshToken(): Promise<string>
+  requireAcr(acrValue: string, redirectUrl: string): Promise<void>
 }
 
 export const useAuthService = (): AuthServiceInterface => {

packages/web-pkg/src/composables/piniaStores/capabilities.ts

@@ -5,6 +5,12 @@ import merge from 'lodash-es/merge'
 import { SharePermissionBit } from '@ownclouders/web-client'
 
 const defaultValues = {
+  auth: {
+    mfa: {
+      enabled: false,
+      levelnames: ['advanced']
+    }
+  },
   core: {
     'support-sse': false,
     'support-url-signing': false
@@ -145,6 +151,9 @@ export const useCapabilityStore = defineStore('capabilities', () => {
   const searchMediaType = computed(() => unref(capabilities).search.property?.mediatype)
   const searchContent = computed(() => unref(capabilities).search.property?.content)
 
+  const authMfaEnabled = computed(() => unref(capabilities).auth.mfa.enabled)
+  const authMfaRequiredLevelname = computed(() => unref(capabilities).auth.mfa.levelnames.at(0))
+
   return {
     isInitialized,
     capabilities,
@@ -191,7 +200,9 @@ export const useCapabilityStore = defineStore('capabilities', () => {
     passwordPolicy,
     searchLastMofifiedDate,
     searchMediaType,
-    searchContent
+    searchContent,
+    authMfaEnabled,
+    authMfaRequiredLevelname
   }
 })
 

packages/web-runtime/src/services/auth/authService.ts

@@ -384,6 +384,30 @@ export class AuthService implements AuthServiceInterface {
     console.debug('[authService:handleDelegatedTokenUpdate] - going to update the access_token')
     return this.userManager.updateContext(event.data, false)
   }
+
+  /**
+   * Redirects to the login page if the user is not authenticated or if the ACR value is not the one required.
+   *
+   * @param acrValue - The ACR value to require.
+   * @param redirectUrl - The URL to redirect to after login.
+   *
+   * @throws {Error} In cases of wrong authentication.
+   */
+  public async requireAcr(acrValue: string, redirectUrl: string) {
+    const user = await this.userManager.getUser()
+    if (!user || user.expired) {
+      this.userManager.setPostLoginRedirectUrl(redirectUrl)
+      return this.userManager.signinRedirect({ acr_values: acrValue })
+    }
+
+    const { acr } = user.profile
+    if (acr === acrValue) {
+      return
+    }
+
+    this.userManager.setPostLoginRedirectUrl(redirectUrl)
+    return this.userManager.signinRedirect({ acr_values: acrValue })
+  }
 }
 
 export const authService = new AuthService()

packages/web-runtime/src/services/auth/userManager.ts

@@ -73,7 +73,10 @@ export class UserManager extends OidcUserManager {
       client_id: '',
 
       // we trigger the token renewal manually via a timer running in a web worker
-      automaticSilentRenew: false
+      automaticSilentRenew: false,
+
+      // do not filter acr
+      filterProtocolClaims: ['nbf', 'jti', 'auth_time', 'nonce', 'amr', 'azp', 'at_hash']
     }
 
     if (options.configStore.isOIDC) {