Skip to content

feat(user-management): add “Add existing account” dialog and provisio… #53114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
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
1 change: 1 addition & 0 deletions apps/provisioning_api/appinfo/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
// Users
['root' => '/cloud', 'name' => 'Users#getUsers', 'url' => '/users', 'verb' => 'GET'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you run composer run cs:fix locally?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Thanks for the catch!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmpf the reformating is still there

['root' => '/cloud', 'name' => 'Users#getUsersDetails', 'url' => '/users/details', 'verb' => 'GET'],
['root' => '/cloud', 'name' => 'Users#searchAllUsers', 'url' => '/users/search', 'verb' => 'GET'],
['root' => '/cloud', 'name' => 'Users#getDisabledUsersDetails', 'url' => '/users/disabled', 'verb' => 'GET'],
['root' => '/cloud', 'name' => 'Users#getLastLoggedInUsers', 'url' => '/users/recent', 'verb' => 'GET'],
['root' => '/cloud', 'name' => 'Users#searchByPhoneNumbers', 'url' => '/users/search/by-phone', 'verb' => 'POST'],
Expand Down
39 changes: 39 additions & 0 deletions apps/provisioning_api/lib/Controller/UsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,45 @@ public function getLastLoggedInUsers(string $search = '',
}


/**
* Search all users by id or display name
*
* Allows subadmins to look up existing users that are not yet part of
* their groups so they can add them.
*
* @param string $search Text to search for
* @param ?int $limit Limit the amount of users returned
* @param int $offset Offset for searching for users
* @return DataResponse<Http::STATUS_OK, array{users: array<string, string>}, array{}>
*
* 200: Users returned
*/
#[NoAdminRequired]
public function searchAllUsers(string $search = '', ?int $limit = null, int $offset = 0): DataResponse {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not exactly what getUsers does? It also has a search param

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah there is no need for a new endpoint, the frontend should use the existing ones unless there’s a really good reason.
I think getUsersDetails is the one to use as displaynames are needed, I suppose this is what the rest of the frontend uses (I did not check).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about this! Changed this and did a much simpler version of what I had before, let me know what you think now!

$currentUser = $this->userSession->getUser();

$uid = $currentUser->getUID();
$subAdminManager = $this->groupManager->getSubAdmin();
$isAdmin = $this->groupManager->isAdmin($uid);
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($uid);

if ($isAdmin || $isDelegatedAdmin || $subAdminManager->isSubAdmin($currentUser)) {
$users = $this->userManager->searchDisplayName($search, $limit, $offset);
$result = [];
foreach ($users as $user) {
/** @var IUser $user */
$result[$user->getUID()] = $user->getDisplayName();
}

return new DataResponse([
'users' => $result,
]);
}

throw new OCSForbiddenException();
}



/**
* @NoSubAdminRequired
Expand Down
44 changes: 32 additions & 12 deletions apps/settings/src/components/UserList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@
- SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
- SPDX-License-Identifier: AGPL-3.0-or-later
-->

<template>
<Fragment>

<NewUserDialog v-if="showConfig.showNewUserForm"
:loading="loading"
:new-user="newUser"
:quota-options="quotaOptions"
@reset="resetForm"
@closing="closeDialog" />
:loading="loading"

Check failure on line 9 in apps/settings/src/components/UserList.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Expected "\t" character, but found " " character
:new-user="newUser"

Check failure on line 10 in apps/settings/src/components/UserList.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Expected "\t" character, but found " " character
:quota-options="quotaOptions"

Check failure on line 11 in apps/settings/src/components/UserList.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Expected "\t" character, but found " " character
@reset="resetForm"

Check failure on line 12 in apps/settings/src/components/UserList.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Expected "\t" character, but found " " character
@closing="closeDialog" />

Check failure on line 13 in apps/settings/src/components/UserList.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Expected "\t" character, but found " " character
<AddExistingUserDialog v-if="showConfig.showAddExistingUserForm"
:group="currentGroup"

Check failure on line 15 in apps/settings/src/components/UserList.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Expected indentation of 3 tabs but found 4 tabs
@closing="closeAddExistingDialog" />

Check failure on line 16 in apps/settings/src/components/UserList.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Expected indentation of 3 tabs but found 4 tabs

<NcEmptyContent v-if="filteredUsers.length === 0"
class="empty"
Expand Down Expand Up @@ -70,6 +73,7 @@

import VirtualList from './Users/VirtualList.vue'
import NewUserDialog from './Users/NewUserDialog.vue'
import AddExistingUserDialog from './Users/AddExistingUserDialog.vue'
import UserListFooter from './Users/UserListFooter.vue'
import UserListHeader from './Users/UserListHeader.vue'
import UserRow from './Users/UserRow.vue'
Expand Down Expand Up @@ -101,6 +105,7 @@
NcIconSvgWrapper,
NcLoadingIcon,
NewUserDialog,
AddExistingUserDialog,
UserListFooter,
UserListHeader,
VirtualList,
Expand Down Expand Up @@ -171,8 +176,16 @@
},

groups() {
return this.$store.getters.getSortedGroups
.filter(group => group.id !== '__nc_internal_recent' && group.id !== 'disabled')
return this.$store.getters.getSortedGroups

Check failure on line 179 in apps/settings/src/components/UserList.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Expected indentation of 3 tabs but found 24 spaces
.filter(group => group.id !== '__nc_internal_recent' && group.id !== 'disabled')

Check failure on line 180 in apps/settings/src/components/UserList.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Expected indentation of 4 tabs but found 32 spaces
},

currentGroup() {
const gid = this.selectedGroup

Check failure on line 184 in apps/settings/src/components/UserList.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Expected indentation of 3 tabs but found 4
if (!gid) {
return null
}
return this.groups.find(g => g.id === gid) || null
},

quotaOptions() {
Expand Down Expand Up @@ -309,10 +322,17 @@
},

closeDialog() {
this.$store.commit('setShowConfig', {
key: 'showNewUserForm',
value: false,
})
this.$store.commit('setShowConfig', {
key: 'showNewUserForm',
value: false,
})
},

closeAddExistingDialog() {
this.$store.commit('setShowConfig', {
key: 'showAddExistingUserForm',
value: false,
})
},

async search({ query }) {
Expand Down
178 changes: 178 additions & 0 deletions apps/settings/src/components/Users/AddExistingUserDialog.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
<!--
- SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
- SPDX-License-Identifier: AGPL-3.0-or-later
-->
<template>
<NcDialog
class="dialog"
size="small"
:name="t('settings', 'Add existing account')"
out-transition
v-on="$listeners"
>
<form
id="add-existing-user-form"
class="dialog__form"
@submit.prevent="submit"
>
<NcSelect
v-model="selectedUser"
class="dialog__select"
:options="possibleUsers"
:placeholder="t('settings', 'Search accounts')"
:user-select="true"
:dropdown-should-open="dropdownShouldOpen"
label="displayname"
@search="searchUsers"
/>
<NcSelect
v-model="selectedGroup"
class="dialog__select"
:options="availableGroups"
:placeholder="t('settings', 'Select group')"
label="name"
:disabled="preselectedGroup"
:clearable="false"
/>
</form>
<template #actions>
<NcButton
class="dialog__submit"
form="add-existing-user-form"
type="primary"
native-type="submit"
>
{{ t('settings', 'Add to group') }}
</NcButton>
</template>
</NcDialog>
</template>

<script>
import { t } from '@nextcloud/l10n'
import NcDialog from '@nextcloud/vue/components/NcDialog'
import NcSelect from '@nextcloud/vue/components/NcSelect'
import NcButton from '@nextcloud/vue/components/NcButton'
import { showError } from '@nextcloud/dialogs'
import { confirmPassword } from '@nextcloud/password-confirmation'
import logger from '../../logger.ts'

export default {
name: 'AddExistingUserDialog',
components: { NcDialog, NcSelect, NcButton },
props: {
group: {
type: Object,
default: null,
},
},
data() {
return {
possibleUsers: [],
selectedUser: null,
availableGroups: [],
selectedGroup: null,
preselectedGroup: false,
promise: null,
}
},
mounted() {
// only subadmins see a filtered list
this.availableGroups = this.$store.getters.getSubAdminGroups
if (this.group) {
this.selectedGroup = this.group
this.preselectedGroup = true
}
},
methods: {
t,
async searchUsers(query) {
if (this.promise) {
this.promise.cancel?.()
}
try {
this.promise = this.$store.dispatch('searchUsers', {
offset: 0,
limit: 10,
search: query,
})
const resp = await this.promise
this.possibleUsers = resp?.data
? Object.values(resp.data.ocs.data.users)
: []
} catch (error) {
logger.error(t('settings', 'Failed to search accounts'), { error })
} finally {
this.promise = null
}
},
/**
* Only open the dropdown once there's some text in the search-box.
* @param {object} vm the vue-select instance
* @returns {boolean}
*/
dropdownShouldOpen(vm) {
return vm.search && vm.search.length > 0;
},
async submit() {
if (!this.selectedUser || !this.selectedGroup) {
return
}

// require the admin to re-enter their password
try {
await confirmPassword()
} catch {
showError(t('settings', 'Password confirmation is required'))
return
}

try {
// now that we've confirmed, call the action (it will include the
// proper header automatically)
await this.$store.dispatch('addUserGroup', {
userid: this.selectedUser.id,
gid: this.selectedGroup.id,
})
// if this user wasn’t in the list yet, fetch their details
if (
!this.$store.getters
.getUsers.find((u) => u.id === this.selectedUser.id)
) {
const resp = await this.$store.dispatch(
'getUser',
this.selectedUser.id
)
if (resp) {
this.$store.commit('addUserData', resp)
}
}
this.$emit('closing')
} catch (error) {
logger.error(t('settings', 'Failed to add user to group'), { error })
showError(t('settings', 'Failed to add user to group'))
}
},
},
}
</script>

<style scoped lang="scss">
.dialog {
&__form {
display: flex;
flex-direction: column;
align-items: center;
padding: 0 8px;
gap: 4px 0;
}
&__select {
width: 100%;
}
&__submit {
margin-top: 4px;
margin-bottom: 8px;
}
}
</style>

38 changes: 32 additions & 6 deletions apps/settings/src/store/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
* SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

import { getBuilder } from '@nextcloud/browser-storage'
import { getCapabilities } from '@nextcloud/capabilities'
import { parseFileSize } from '@nextcloud/files'
Expand Down Expand Up @@ -53,6 +52,7 @@ const state = {
showFirstLogin: localStorage.getItem('account_settings__showFirstLogin') === 'true',
showLastLogin: localStorage.getItem('account_settings__showLastLogin') === 'true',
showNewUserForm: localStorage.getItem('account_settings__showNewUserForm') === 'true',
showAddExistingUserForm: localStorage.getItem('account_settings__showAddExistingUserForm') === 'true',
showLanguages: localStorage.getItem('account_settings__showLanguages') === 'true',
},
}
Expand Down Expand Up @@ -106,6 +106,13 @@ const mutations = {
addUserGroup(state, { userid, gid }) {
const group = state.groups.find(groupSearch => groupSearch.id === gid)
const user = state.users.find(user => user.id === userid)

// Safety check: ensure user exists
if (!user) {
console.warn(`Cannot add user ${userid} to group ${gid}: user not found in store`)
return
}

// increase count if user is enabled
if (group && user.enabled && state.userCount > 0) {
group.usercount++
Expand Down Expand Up @@ -331,13 +338,32 @@ const actions = {
searchUsers(context, { offset, limit, search }) {
search = typeof search === 'string' ? search : ''

return api.get(generateOcsUrl('cloud/users/details?offset={offset}&limit={limit}&search={search}', { offset, limit, search })).catch((error) => {
if (!axios.isCancel(error)) {
context.commit('API_FAILURE', error)
}
})
const isAdmin = usersSettings.isAdmin || usersSettings.isDelegatedAdmin
const endpoint = isAdmin ? 'cloud/users/details' : 'cloud/users/search'

return api.get(generateOcsUrl(`${endpoint}?offset={offset}&limit={limit}&search={search}`, { offset, limit, search }))
.then((response) => {
if (!isAdmin) {
const normalized = {}
const users = response.data.ocs.data.users
Object.keys(users).forEach((id) => {
normalized[id] = {
id,
displayname: users[id],
}
})
response.data.ocs.data.users = normalized
}
return response
})
.catch((error) => {
if (!axios.isCancel(error)) {
context.commit('API_FAILURE', error)
}
})
},


/**
* Get user details
*
Expand Down
Loading