Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions apps/settings/lib/Controller/AuthSettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use BadMethodCallException;
use OC\Authentication\Exceptions\InvalidTokenException as OcInvalidTokenException;
use OC\Authentication\Exceptions\PasswordlessTokenException;
use OC\Authentication\Token\GrantableApps;
use OC\Authentication\Token\INamedToken;
use OC\Authentication\Token\IProvider;
use OC\Authentication\Token\RemoteWipe;
Expand Down Expand Up @@ -54,6 +55,7 @@ public function __construct(
private IConfig $serverConfig,
private IL10N $l,
private TokenScopes $tokenScopes,
private GrantableApps $grantableApps,
) {
parent::__construct($appName, $request);
}
Expand Down Expand Up @@ -113,9 +115,9 @@ public function create(string $name = '', bool $qrcodeLogin = false, ?array $sco
} else {
$type = IToken::PERMANENT_TOKEN;
try {
$scope = $scopes === null ? null : $this->tokenScopes->updateScope([], [TokenScopes::KEY => $scopes]);
} catch (\InvalidArgumentException) {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
$scope = $scopes === null ? null : $this->tokenScopes->updateScope([], [TokenScopes::KEY => $scopes], $this->grantableApps->get());
} catch (\InvalidArgumentException $e) {
return new JSONResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
}
}

Expand Down Expand Up @@ -255,9 +257,9 @@ public function update(int $id, array $scope, string $name): JSONResponse {
$currentScope = $token->getScopeAsArray();

try {
$newScope = $this->tokenScopes->updateScope($currentScope, $scope);
} catch (\InvalidArgumentException) {
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
$newScope = $this->tokenScopes->updateScope($currentScope, $scope, $this->grantableApps->get());
} catch (\InvalidArgumentException $e) {
return new JSONResponse(['message' => $e->getMessage()], Http::STATUS_BAD_REQUEST);
}

if ($newScope !== $currentScope) {
Expand Down
4 changes: 4 additions & 0 deletions apps/settings/lib/Settings/Personal/Security/Authtokens.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace OCA\Settings\Settings\Personal\Security;

use OC\Authentication\Token\GrantableApps;
use OC\Authentication\Token\INamedToken;
use OC\Authentication\Token\IProvider as IAuthTokenProvider;
use OC\Authentication\Token\IToken;
Expand All @@ -33,6 +34,7 @@ public function __construct(
private IInitialState $initialState,
private IConfig $serverConfig,
private ?string $userId,
private GrantableApps $grantableApps,
) {
}

Expand All @@ -49,6 +51,8 @@ public function getForm(): TemplateResponse {
&& $this->serverConfig->getSystemValueBool('auth_can_create_app_token', true)
);

$this->initialState->provideInitialState('grantable_apps', $this->grantableApps->entries());

Util::addStyle(Application::APP_ID, 'personal-security-authtokens');
Util::addScript(Application::APP_ID, 'personal-security-authtokens');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { computed, useTemplateRef, watch } from 'vue'
import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwitch'
import NcNoteCard from '@nextcloud/vue/components/NcNoteCard'
import NcRadioGroup from '@nextcloud/vue/components/NcRadioGroup'
import { AREA_OPTIONS, AREAS, areaState, GRANT_LABELS, PRESET_GRANTS, SERVICES, setGrant } from '../utils/accessLevel.ts'
import { appLabel, AREA_OPTIONS, AREAS, areaState, GRANT_LABELS, GRANTABLE_APPS, PRESET_GRANTS, SERVICES, setGrant, toScopes } from '../utils/accessLevel.ts'

/** What the app password can reach */
const selection = defineModel<Selection>({ required: true })
Expand All @@ -23,27 +23,40 @@ const props = defineProps<{

const areas = useTemplateRef('areas')

const acrossApps = GRANTABLE_APPS.filter((app) => app.acrossApps)
const plainApps = GRANTABLE_APPS.filter((app) => !app.acrossApps)
const filesDescription = t('settings', 'Most apps need this for attachments and other file features.')

const preset = computed({
get: () => selection.value.preset,
set(value: string) {
const name = value as Preset
const grants = PRESET_GRANTS[name as keyof typeof PRESET_GRANTS] ?? selection.value.grants
selection.value = { preset: name, grants }
const grants = PRESET_GRANTS[name as keyof typeof PRESET_GRANTS]
selection.value = grants ? { preset: name, grants, apps: [] } : { ...selection.value, preset: name }
},
})

// Custom needs something to save, a ticked box or the kept scopes
watch([() => selection.value, areas], () => {
const empty = selection.value.grants.length === 0 && props.kept.length === 0
areas.value?.querySelector('input')?.setCustomValidity(empty ? t('settings', 'Choose at least one area') : '')
const empty = toScopes(selection.value, props.kept)?.length === 0
areas.value?.querySelector('input')?.setCustomValidity(empty ? t('settings', 'Choose at least one option') : '')
}, { flush: 'post', immediate: true })

/**
* @param grant Area or sub-option to change
* @param on Whether to tick it
*/
function toggle(grant: Grant, on: boolean) {
selection.value = { preset: 'custom', grants: setGrant(selection.value.grants, grant, on) }
selection.value = { ...selection.value, preset: 'custom', grants: setGrant(selection.value.grants, grant, on) }
}

/**
* @param app App id to change
* @param on Whether to tick it
*/
function toggleApp(app: string, on: boolean) {
const apps = selection.value.apps.filter((id) => id !== app)
selection.value = { ...selection.value, preset: 'custom', apps: on ? [...apps, app] : apps }
}
</script>

Expand Down Expand Up @@ -75,6 +88,7 @@ function toggle(grant: Grant, on: boolean) {
<NcCheckboxRadioSwitch
:modelValue="areaState(selection.grants, area) === 'all'"
:indeterminate="areaState(selection.grants, area) === 'partial'"
:description="area === 'files' ? filesDescription : undefined"
@update:modelValue="toggle(area, $event)">
{{ GRANT_LABELS[area] }}
</NcCheckboxRadioSwitch>
Expand All @@ -95,6 +109,24 @@ function toggle(grant: Grant, on: boolean) {
@update:modelValue="toggle(service, $event)">
{{ GRANT_LABELS[service] }}
</NcCheckboxRadioSwitch>
<NcCheckboxRadioSwitch
v-for="app in acrossApps"
:key="app.id"
:modelValue="selection.apps.includes(app.id)"
@update:modelValue="toggleApp(app.id, $event)">
{{ appLabel(app) }}
</NcCheckboxRadioSwitch>
<hr v-if="plainApps.length > 0" class="access-picker__divider">
<NcCheckboxRadioSwitch
v-for="app in plainApps"
:key="app.id"
:modelValue="selection.apps.includes(app.id)"
@update:modelValue="toggleApp(app.id, $event)">
<span class="access-picker__app">
<img class="access-picker__app-icon" :src="app.icon" alt="">
{{ app.name }}
</span>
</NcCheckboxRadioSwitch>
</div>
</template>

Expand All @@ -117,5 +149,26 @@ function toggle(grant: Grant, on: boolean) {
&__options {
padding-inline-start: calc(8 * var(--default-grid-baseline));
}

&__divider {
// Same inset as the checkboxes, which NcCheckboxRadioSwitch centers in a clickable area
margin-block: calc(2 * var(--default-grid-baseline));
margin-inline: calc((var(--default-clickable-area) - 20px) / 2);
border: none;
border-top: 1px solid var(--color-border);
}

&__app {
display: flex;
align-items: center;
gap: calc(2 * var(--default-grid-baseline));
}

&__app-icon {
width: calc(4 * var(--default-grid-baseline));
height: calc(4 * var(--default-grid-baseline));
// Navigation icons are white, for the header
filter: var(--background-invert-if-bright);
}
}
</style>
40 changes: 29 additions & 11 deletions apps/settings/src/authtokens/utils/accessLevel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,47 @@

import type { Selection } from './accessLevel.ts'

import { describe, expect, it } from 'vitest'
import { describe, expect, it, vi } from 'vitest'
vi.mock('@nextcloud/initial-state', () => ({
loadState: () => [
{ id: 'spreed', name: 'Talk', icon: '', acrossApps: false },
{ id: 'activity', name: 'Activity', icon: '', acrossApps: true },
],
}))

import { accessLabel, areaState, fromScopes, setGrant, toScopes } from './accessLevel.ts'

const files = ['files:files:read', 'files:files:write', 'files:dav:read', 'files:dav:write', 'files_sharing:shares:write']
const calendarContacts = ['dav:calendar:read', 'dav:calendar:write', 'dav:contacts:read', 'dav:contacts:write']

describe('accessLevel', () => {
it.each<[string, Selection]>([
['files', { preset: 'files', grants: ['files', 'filesShare'] }],
['files with trash and versions', { preset: 'custom', grants: ['files', 'filesShare', 'filesTrash', 'filesVersions'] }],
['calendar and contacts', { preset: 'calendarContacts', grants: ['calendar', 'calendarWrite', 'contacts', 'contactsWrite'] }],
['files without sharing', { preset: 'custom', grants: ['files'] }],
['read-only calendar', { preset: 'custom', grants: ['calendar'] }],
['files with search', { preset: 'custom', grants: ['files', 'filesShare', 'search'] }],
['people lookup alone', { preset: 'custom', grants: ['people'] }],
['files', { preset: 'files', grants: ['files', 'filesShare'], apps: [] }],
['files with trash and versions', { preset: 'custom', grants: ['files', 'filesShare', 'filesTrash', 'filesVersions'], apps: [] }],
['calendar and contacts', { preset: 'calendarContacts', grants: ['calendar', 'calendarWrite', 'contacts', 'contactsWrite'], apps: [] }],
['files without sharing', { preset: 'custom', grants: ['files'], apps: [] }],
['read-only calendar', { preset: 'custom', grants: ['calendar'], apps: [] }],
['files with search', { preset: 'custom', grants: ['files', 'filesShare', 'search'], apps: [] }],
['people lookup alone', { preset: 'custom', grants: ['people'], apps: [] }],
])('round-trips %s', (_, selection) => {
const scopes = toScopes(selection, [])!
expect(fromScopes(scopes)).toEqual({ selection, kept: [] })
})

it('reads app ids as ticked apps and keeps unknown ones', () => {
const { selection, kept } = fromScopes(['spreed', 'deck'])
expect(selection).toEqual({ preset: 'custom', grants: [], apps: ['spreed'] })
expect(kept).toEqual(['deck'])
expect(toScopes(selection, kept)).toEqual(['spreed', 'deck'])
})

it('labels apps, with the ones that show other apps\' data marked', () => {
expect(accessLabel({ filesystem: true, scopes: [...files, 'spreed', 'activity'] })).toBe('Files, Talk, and Activity across all apps')
})

it('reads a token without scopes as all access', () => {
expect(fromScopes(undefined).selection.preset).toBe('all')
expect(toScopes({ preset: 'all', grants: ['files'] }, [])).toBeNull()
expect(toScopes({ preset: 'all', grants: ['files'], apps: [] }, [])).toBeNull()
})

it('matches presets regardless of order', () => {
Expand All @@ -38,9 +56,9 @@ describe('accessLevel', () => {
const { selection, kept } = fromScopes([...files, 'app:deck', 'dav:calendar:write'])

// Write without read is possible through the API, the picker never creates it
expect(selection).toEqual({ preset: 'custom', grants: ['files', 'filesShare'] })
expect(selection).toEqual({ preset: 'custom', grants: ['files', 'filesShare'], apps: [] })
expect(kept).toEqual(['app:deck', 'dav:calendar:write'])
expect(toScopes({ preset: 'custom', grants: ['contacts'] }, kept))
expect(toScopes({ preset: 'custom', grants: ['contacts'], apps: [] }, kept))
.toEqual(['dav:contacts:read', 'app:deck', 'dav:calendar:write'])
})

Expand Down
33 changes: 28 additions & 5 deletions apps/settings/src/authtokens/utils/accessLevel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import type { ITokenScope } from '../store/authtoken.ts'

import { loadState } from '@nextcloud/initial-state'
import { getLanguage, t } from '@nextcloud/l10n'

export type Area = 'files' | 'calendar' | 'contacts'
Expand All @@ -15,8 +16,20 @@ export type Grant = Area | Service | 'filesShare' | 'filesTrash' | 'filesVersion
export type Preset = 'all' | 'files' | 'calendarContacts' | 'custom'
type AreaState = 'all' | 'partial' | 'none'

export interface IGrantableApp {
id: string
name: string
icon: string
/** Whether the app shows other apps' data */
acrossApps: boolean
}

export const GRANTABLE_APPS = loadState<IGrantableApp[]>('settings', 'grantable_apps', [])

export interface Selection {
preset: Preset
/** Ticked apps, stored as their app id */
apps: string[]
/** Ticked grants, only used when the preset is not `all` */
grants: Grant[]
}
Expand Down Expand Up @@ -71,17 +84,18 @@ const GRANTS = Object.keys(GRANT_SCOPES) as Grant[]
*/
export function fromScopes(scopes?: string[]): { selection: Selection, kept: string[] } {
if (scopes === undefined) {
return { selection: { preset: 'all', grants: [] }, kept: [] }
return { selection: { preset: 'all', grants: [], apps: [] }, kept: [] }
}

const held = GRANTS.filter((grant) => GRANT_SCOPES[grant].every((scope) => scopes.includes(scope)))
const grants = held.filter((grant) => held.includes(parentOf(grant)))
const shown = grants.flatMap((grant) => GRANT_SCOPES[grant])
const apps = GRANTABLE_APPS.map((app) => app.id).filter((id) => scopes.includes(id))
const shown = [...grants.flatMap((grant) => GRANT_SCOPES[grant]), ...apps]
const kept = scopes.filter((scope) => !shown.includes(scope))

const presets = Object.keys(PRESET_GRANTS) as (keyof typeof PRESET_GRANTS)[]
const preset = kept.length === 0 ? presets.find((name) => sameItems(PRESET_GRANTS[name], grants)) : undefined
return { selection: { preset: preset ?? 'custom', grants }, kept }
const preset = kept.length === 0 && apps.length === 0 ? presets.find((name) => sameItems(PRESET_GRANTS[name], grants)) : undefined
return { selection: { preset: preset ?? 'custom', grants, apps }, kept }
}

/**
Expand All @@ -94,7 +108,7 @@ export function toScopes(selection: Selection, kept: string[]): string[] | null
if (selection.preset === 'all') {
return null
}
return [...new Set([...selection.grants.flatMap((grant) => GRANT_SCOPES[grant]), ...kept])]
return [...new Set([...selection.grants.flatMap((grant) => GRANT_SCOPES[grant]), ...selection.apps, ...kept])]
}

/**
Expand Down Expand Up @@ -149,6 +163,7 @@ export function accessLabel(scope: ITokenScope): string {
return [(state === 'partial' && readOnly[area]) || GRANT_LABELS[area]]
})
parts.push(...SERVICES.filter((service) => selection.grants.includes(service)).map((service) => GRANT_LABELS[service]))
parts.push(...GRANTABLE_APPS.filter((app) => selection.apps.includes(app.id)).map(appLabel))
if (kept.length > 0) {
// TRANSLATORS Last item of a list such as "Files and other access"
parts.push(parts.length === 0 ? t('settings', 'Other access') : t('settings', 'other access'))
Expand All @@ -159,6 +174,14 @@ export function accessLabel(scope: ITokenScope): string {
return new Intl.ListFormat(getLanguage(), { type: 'conjunction' }).format(parts)
}

/**
* @param app An app the app password can be limited to
*/
export function appLabel(app: IGrantableApp): string {
// TRANSLATORS {app} is an app name, e.g. "Activity". The app password then reads that app's data from all other apps
return app.acrossApps ? t('settings', '{app} across all apps', { app: app.name }) : app.name
}

/**
* @param grant An area, sub-option or service
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use OC\AppFramework\Http;
use OC\Authentication\Exceptions\ExpiredTokenException;
use OC\Authentication\Exceptions\InvalidTokenException;
use OC\Authentication\Token\GrantableApps;
use OC\Authentication\Token\IProvider;
use OC\Authentication\Token\IToken;
use OC\Authentication\Token\IWipeableToken;
Expand Down Expand Up @@ -49,6 +50,7 @@ class AuthSettingsControllerTest extends TestCase {
private RemoteWipe&MockObject $remoteWipe;
private IConfig&MockObject $serverConfig;
private IL10N&MockObject $l;
private GrantableApps&MockObject $grantableApps;
private string $uid = 'jane';
private AuthSettingsController $controller;

Expand All @@ -68,6 +70,8 @@ protected function setUp(): void {
$logger = $this->createMock(LoggerInterface::class);
$this->l = $this->createMock(IL10N::class);

$this->grantableApps = $this->createMock(GrantableApps::class);
$this->grantableApps->method('get')->willReturn(['deck']);
$this->controller = new AuthSettingsController(
'core',
$this->request,
Expand All @@ -83,6 +87,7 @@ protected function setUp(): void {
$this->serverConfig,
$this->l,
new TokenScopes(),
$this->grantableApps,
);
}

Expand Down Expand Up @@ -144,6 +149,7 @@ public static function dataCreateWithScopes(): array {
return [
'files' => [$files, [TokenScopes::KEY => $sorted, IToken::SCOPE_FILESYSTEM => true]],
'calendar' => [['dav:calendar:read'], [TokenScopes::KEY => ['dav:calendar:read'], IToken::SCOPE_FILESYSTEM => false]],
'whole app' => [['deck'], [TokenScopes::KEY => ['deck'], IToken::SCOPE_FILESYSTEM => false]],
];
}

Expand All @@ -168,6 +174,7 @@ public function testCreateInvalidScopes(): void {

$response = $this->controller->create('Nexus 4', false, [FilesTokenScopes::FILES_WRITE]);
$this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus());
$this->assertSame(['message' => 'Files scopes require ' . FilesTokenScopes::FILES_READ], $response->getData());
}

public function testCreateDisabledBySystemConfig(): void {
Expand Down
Loading
Loading