Skip to content

Commit

Permalink
feat: Remove the perspective feature (closes #749)
Browse files Browse the repository at this point in the history
  • Loading branch information
claustres committed Aug 22, 2023
1 parent 2a4b06b commit 70631f7
Show file tree
Hide file tree
Showing 13 changed files with 84 additions and 97 deletions.
9 changes: 9 additions & 0 deletions core/api/hooks/hooks.authentication.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
41 changes: 0 additions & 41 deletions core/api/hooks/hooks.model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 0 additions & 5 deletions core/api/hooks/hooks.query.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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]
}
Expand Down
14 changes: 14 additions & 0 deletions core/api/hooks/hooks.users.js
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
11 changes: 4 additions & 7 deletions core/api/services/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import _ from 'lodash'
import path from 'path'
import makeDebug from 'debug'
import { fileURLToPath } from 'url'
Expand Down Expand Up @@ -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 => {
Expand All @@ -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')
Expand Down
23 changes: 11 additions & 12 deletions core/api/services/users/users.hooks.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
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: [],
find: [],
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' }
Expand Down Expand Up @@ -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: [],
Expand Down
9 changes: 6 additions & 3 deletions core/client/components/account/KProfile.vue
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@
<!-- Avatar -->
<div class="row justify-center">
<KAvatar
:object="User"
:object="avatar"
size="7rem"
/>
</div>
<!-- Information -->
<div class="column">
<div class="row justify-center text-subtitle1 text-weight-bold">
{{ User.name }}
{{ name }}
</div>
<div class="row justify-center">
{{ User.description }}
{{ description }}
</div>
</div>
</div>
Expand Down Expand Up @@ -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) {
Expand Down
11 changes: 8 additions & 3 deletions core/client/components/team/KMemberCard.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
:header="header"
:actions="itemActions"
:bind-actions="false"
:dense="dense">
:dense="dense"
:options="{ nameField: 'profile.name', descriptionField: 'profile.description' }">
<!--
Card avater
Card avatar
-->
<template v-slot:card-avatar>
<KAvatar :object="item" size="3.2rem" />
<KAvatar :object="item" size="3.2rem"
:options="{ nameField: 'profile.name', avatarField: 'profile.avatar' }"/>
</template>
<!--
Card descriptions
Expand Down Expand Up @@ -104,6 +106,9 @@ export default {
const components = _.filter(this.itemActions, { scope: 'header' })
components.splice(0, 0, { id: 'role-badge', component: 'QBadge', label: this.$t(this.roleLabel(this.role)), color: 'grey-7' }, { component: 'QSpace' })
return components
},
avatar () {
},
role () {
const role = getRoleForOrganisation(this.item, this.contextId)
Expand Down
12 changes: 12 additions & 0 deletions core/client/mixins/mixin.base-editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,18 @@ export const baseEditor = {
type: String,
default: undefined
},
perspective: {
type: String,
default: ''
},
// Indicates if the stored object in-memory is only the perspective part (default)
// or the full structure, ie { perspective: { xxx } }
// Note: the full structure is always retrieved/sent from/to the service anyway but sometimes
// it is easier to manipulate a full-object and edit a nested property seen as a perspective on the front side
perspectiveAsObject: {
type: Boolean,
default: true
},
clearButton: {
type: String,
default: ''
Expand Down
14 changes: 14 additions & 0 deletions core/client/mixins/mixin.base-viewer.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@
export const baseViewer = {
props: {
perspective: {
type: String,
default: ''
},
clearButton: {
type: String,
default: ''
},
resetButton: {
type: String,
default: ''
}
},
computed: {
viewerTitle () {
// Retuns the schema title
Expand Down
23 changes: 2 additions & 21 deletions core/client/mixins/mixin.object-proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,6 @@ export const objectProxy = {
objectId: {
type: String,
default: ''
},
perspective: {
type: String,
default: ''
},
// Indicates if the stored object in-memory is only the perspective part (default)
// or the full structure, ie { perspective: { xxx = } }
// Note: the full structure is always retrieved/sent from/to the service anyway but sometimes
// it is easier to manipulate a full-object and edit a nested property seen as a perspective on the front side
perspectiveAsObject: {
type: Boolean,
default: true
}
},
data () {
Expand All @@ -32,24 +20,17 @@ export const objectProxy = {
getObjectId () {
return this.object ? this.object._id : ''
},
hasPerspective (perspective) {
return this.object ? _.has(this.object, perspective) : false
},
loadObject () {
if (!this.objectId) {
this.object = null
return Promise.resolve(null)
}
// Create a new mixin promise if required
const objectChanged = (this.getObjectId() !== this.objectId) || !this.hasPerspective(this.perspective)
const objectChanged = (this.getObjectId() !== this.objectId)
if (!this.objectPromise || objectChanged) {
this.objectPromise = createQuerablePromise((resolve, reject) => {
let params = {}
if (this.perspective && this.perspectiveAsObject) {
params = { query: { $select: ['_id', this.perspective] } }
}
this.getService()
.get(this.objectId, params)
.get(this.objectId)
.then(object => {
this.object = object
resolve(object)
Expand Down
2 changes: 1 addition & 1 deletion core/common/schemas/users.update-profile.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"$schema": "http://json-schema.org/draft-07/schema#",
"$id": "http://kalisio.xyz/schemas/users.update-profile.json#",
"title": "schemas.OBJECT_NAME",
"description": "User profile perspective",
"description": "User profile",
"type": "object",
"properties": {
"name": {
Expand Down
7 changes: 3 additions & 4 deletions test/api/core/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@ describe('core:services', () => {
permissions.defineAbilities.registerHook(permissions.defineUserAbilities)

app = kdk()
// Register perspective hook
// Register hooks
app.hooks({
before: { all: hooks.authorise },
after: { all: hooks.processPerspectives },
error: { all: hooks.log }
})
port = app.get('port')
Expand Down Expand Up @@ -151,7 +150,6 @@ describe('core:services', () => {
})
.then(users => {
expect(users.data.length > 0).beTrue()
// By default no perspective
expect(users.data[0].name).toExist()
expect(users.data[0].description).toExist()
expect(users.data[0].email).toExist()
Expand Down Expand Up @@ -239,9 +237,10 @@ describe('core:services', () => {
})
})

it('get a user perspective', () => {
it('get user profile', () => {
return userService.find({ query: { $select: ['profile'] } })
.then(users => {
expect(users.data[0].name).beUndefined()
expect(users.data[0].profile.name).toExist()
expect(users.data[0].profile.description).toExist()
expect(users.data[0].profile.phone).toExist()
Expand Down

0 comments on commit 70631f7

Please sign in to comment.