diff --git a/core/api/hooks/hooks.authentication.js b/core/api/hooks/hooks.authentication.js index 9e8956c45..656d926aa 100644 --- a/core/api/hooks/hooks.authentication.js +++ b/core/api/hooks/hooks.authentication.js @@ -61,3 +61,12 @@ export async function consentGuest (hook) { return hook } + +export function discardAuthenticationProviders (hook) { + const providers = hook.app.authenticationProviders || [] + + // Iterate through known providers + for (const provider of providers) { + discard(provider)(hook) + } +} \ No newline at end of file diff --git a/core/api/hooks/hooks.model.js b/core/api/hooks/hooks.model.js index a37860ca7..f56a8d9e9 100644 --- a/core/api/hooks/hooks.model.js +++ b/core/api/hooks/hooks.model.js @@ -36,47 +36,6 @@ export function unprocessTimes (properties) { } } -export function processPerspectives (hook) { - const params = hook.params - const query = params.query - const service = hook.service - - // Test if some perspectives are defined on the model - if (!service.options || !service.options.perspectives) return - - // Iterate through known perspectives of the model - service.options.perspectives.forEach(perspective => { - // Only discard if not explicitely asked by $select - let filterPerspective = true - if (!_.isNil(query) && !_.isNil(query.$select)) { - // Transform to array to unify processing - const selectedFields = (typeof query.$select === 'string' ? [query.$select] : query.$select) - if (Array.isArray(selectedFields)) { - selectedFields.forEach(field => { - // Take care that we might only ask for a subset of perspective fields like ['perspective.fieldName'] - if ((field === perspective) || field.startsWith(perspective + '.')) { - filterPerspective = false - } - }) - } - } - if (filterPerspective) { - discard(perspective)(hook) - } - }) -} - -// When perspectives are present we disallow update in order to avoid erase them. -// Indeed when requesting an object they are not retrieved by default -export function preventUpdatePerspectives (hook) { - const service = hook.service - - // Test if some perspectives are defined on the model - if (!service.options || !service.options.perspectives) return - - disallow()(hook) -} - // The hook serialize allows to copy/move some properties within the objects holded by the hook // It applies an array of rules defined by: // - source: the path to the property to be copied diff --git a/core/api/hooks/hooks.query.js b/core/api/hooks/hooks.query.js index b9face600..5eadbde94 100644 --- a/core/api/hooks/hooks.query.js +++ b/core/api/hooks/hooks.query.js @@ -97,9 +97,6 @@ export function populateObject (options) { if (options.throwOnNotFound) throw new Error(`Cannot find the ${options.idField} to dynamically populate.`) else return hook } - // Then the perspective if any - const perspective = _.get(data, options.perspectiveField) || _.get(query, options.perspectiveField) - debug(`Populating ${idProperty} with ID ${id}`) // Set the retrieved service on the same field or given one in hook params _.set(params, serviceProperty, service) @@ -110,11 +107,9 @@ export function populateObject (options) { try { // Get by ID or name ? if (ObjectID.isValid(id)) { - if (perspective) Object.assign(args, { query: { $select: [perspective] } }) object = await service.get(id.toString(), args) } else { Object.assign(args, { query: { name: id.toString() }, paginate: false }) - if (perspective) Object.assign(args.query, { $select: [perspective] }) const results = await service.find(args) if (results.length >= 0) object = results[0] } diff --git a/core/api/hooks/hooks.users.js b/core/api/hooks/hooks.users.js index 4dc917fa5..42037681d 100644 --- a/core/api/hooks/hooks.users.js +++ b/core/api/hooks/hooks.users.js @@ -12,6 +12,20 @@ const verifyHooks = authManagement.hooks const debug = makeDebug('kdk:core:users:hooks') +// Helper functions to be used in iff hooks +export function disallowRegistration (hook) { + return _.get(hook.app.get('authentication'), 'disallowRegistration') +} +export function allowLocalAuthentication (hook) { + return _.get(hook.app.get('authentication'), 'authStrategies', []).includes('local') +} +export function isNotMe (hook) { + const userId = _.get(hook.params, 'user._id', '') + const item = getItems(hook) + const targetId = _.get(item, '_id', '') + return userId.toString() !== targetId.toString() +} + export function enforcePasswordPolicy (options = {}) { return async function (hook) { if (hook.type !== 'before') { diff --git a/core/api/services/index.js b/core/api/services/index.js index 1c62becbc..8789dc815 100644 --- a/core/api/services/index.js +++ b/core/api/services/index.js @@ -1,3 +1,4 @@ +import _ from 'lodash' import path from 'path' import makeDebug from 'debug' import { fileURLToPath } from 'url' @@ -64,7 +65,8 @@ export async function createOrganisationService (options = {}) { usersService.on('patched', user => { // Patching profile should not trigger abilities update since // it is a perspective and permissions are not available in this case - if (user.organisations || user.groups) authorisationsService.updateAbilities(user) + // Updating abilities in this case will result in loosing permissions for orgs/groups as none are available + if (_.has(user, 'organisations') || _.has(user, 'groups')) authorisationsService.updateAbilities(user) }) // Ensure org services are correctly distributed when replicated orgsService.on('created', organisation => { @@ -90,12 +92,7 @@ export default async function () { const authConfig = app.get('authentication') if (authConfig) { - await app.createService('users', { - modelsPath, - servicesPath, - // Add required OAuth2 provider perspectives to avoid leaking data - perspectives: app.authenticationProviders - }) + await app.createService('users', { modelsPath, servicesPath }) debug('\'users\' service created') await app.createService('authorisations', { servicesPath }) debug('\'authorisations\' service created') diff --git a/core/api/services/users/users.hooks.js b/core/api/services/users/users.hooks.js index ae7c7d42f..219837931 100644 --- a/core/api/services/users/users.hooks.js +++ b/core/api/services/users/users.hooks.js @@ -1,14 +1,10 @@ import _ from 'lodash' import { - serialize, updateAbilities, populatePreviousObject, hashPassword, - enforcePasswordPolicy, storePreviousPassword, sendNewSubscriptionEmail + serialize, updateAbilities, populatePreviousObject, hashPassword, disallowRegistration, allowLocalAuthentication, + discardAuthenticationProviders, enforcePasswordPolicy, storePreviousPassword, sendNewSubscriptionEmail } from '../../hooks/index.js' import commonHooks from 'feathers-hooks-common' -// Helper functions -const disallowRegistration = (hook) => _.get(hook.app.get('authentication'), 'disallowRegistration') -const allowLocalAuthentication = (hook) => _.get(hook.app.get('authentication'), 'authStrategies', []).includes('local') - export default { before: { all: [], @@ -16,6 +12,7 @@ export default { get: [], create: [ commonHooks.when(disallowRegistration, commonHooks.disallow('external')), + // Initialize a profile from base user information serialize([ { source: 'name', target: 'profile.name', delete: true }, { source: 'email', target: 'profile.description' } @@ -49,12 +46,14 @@ export default { after: { all: [ - commonHooks.when(hook => hook.params.provider, commonHooks.discard('password'), commonHooks.discard('previousPasswords')), - serialize([ - { source: 'profile.name', target: 'name' }, - { source: 'profile.avatar', target: 'avatar' }, - { source: 'profile.description', target: 'description' } - ]) + commonHooks.when(hook => hook.params.provider, + commonHooks.discard('password'), + commonHooks.discard('previousPasswords'), + discardAuthenticationProviders), + // Hide profile for external user as it may contain personal information + // However, this causes an issue: https://github.com/feathersjs-ecosystem/feathers-reactive/issues/214 + // So let the application decide what to do + //commonHooks.when(isNotMe, commonHooks.discard('profile')) ], find: [], get: [], diff --git a/core/client/components/account/KProfile.vue b/core/client/components/account/KProfile.vue index 31228ce4f..a63e632ef 100644 --- a/core/client/components/account/KProfile.vue +++ b/core/client/components/account/KProfile.vue @@ -11,17 +11,17 @@
- {{ User.name }} + {{ name }}
- {{ User.description }} + {{ description }}
@@ -98,6 +98,9 @@ const header = computed(() => { } return actions }) +const name = computed(() => _.get(User.value, 'profile.name', '')) +const description = computed(() => _.get(User.value, 'profile.description', '')) +const avatar = computed(() => _.get(User.value, 'profile', {})) // Functions function onTriggered (args) { diff --git a/core/client/components/team/KMemberCard.vue b/core/client/components/team/KMemberCard.vue index 01a865771..359f416a2 100644 --- a/core/client/components/team/KMemberCard.vue +++ b/core/client/components/team/KMemberCard.vue @@ -4,12 +4,14 @@ :header="header" :actions="itemActions" :bind-actions="false" - :dense="dense"> + :dense="dense" + :options="{ nameField: 'profile.name', descriptionField: 'profile.description' }">