From 8490fc1267b036bd7971434374f81324b53536fc Mon Sep 17 00:00:00 2001 From: NGPixel Date: Sat, 5 Sep 2020 18:33:15 -0400 Subject: [PATCH] feat: handle disabled auth strategies --- client/components/admin/admin-auth.vue | 137 +++++++++++--------- client/components/login.vue | 2 +- server/db/migrations/2.5.1.js | 10 +- server/db/migrations/2.5.108.js | 14 ++ server/graph/resolvers/authentication.js | 10 +- server/graph/schemas/authentication.graphql | 7 +- server/models/users.js | 4 + 7 files changed, 113 insertions(+), 71 deletions(-) create mode 100644 server/db/migrations/2.5.108.js diff --git a/client/components/admin/admin-auth.vue b/client/components/admin/admin-auth.vue index df8212b6..0836dcf2 100644 --- a/client/components/admin/admin-auth.vue +++ b/client/components/admin/admin-auth.vue @@ -77,64 +77,80 @@ .admin-providerlogo img(:src='strategy.strategy.logo', :alt='strategy.strategy.title') v-card-text - .overline.mb-5 {{$t('admin:auth.strategyConfiguration')}} - v-text-field.mb-3( - outlined - label='Display Name' - v-model='strategy.displayName' - prepend-icon='mdi-format-title' - hint='The title shown to the end user for this authentication strategy.' - persistent-hint - ) - template(v-for='cfg in strategy.config') - v-select.mb-3( - v-if='cfg.value.type === "string" && cfg.value.enum' - outlined - :items='cfg.value.enum' - :key='cfg.key' - :label='cfg.value.title' - v-model='cfg.value.value' - prepend-icon='mdi-cog-box' - :hint='cfg.value.hint ? cfg.value.hint : ""' - persistent-hint - :class='cfg.value.hint ? "mb-2" : ""' - :style='cfg.value.maxWidth > 0 ? `max-width:` + cfg.value.maxWidth + `px;` : ``' - ) - v-switch.mb-6( - v-else-if='cfg.value.type === "boolean"' - :key='cfg.key' - :label='cfg.value.title' - v-model='cfg.value.value' - color='primary' - prepend-icon='mdi-cog-box' - :hint='cfg.value.hint ? cfg.value.hint : ""' - persistent-hint - inset - ) - v-textarea.mb-3( - v-else-if='cfg.value.type === "string" && cfg.value.multiline' - outlined - :key='cfg.key' - :label='cfg.value.title' - v-model='cfg.value.value' - prepend-icon='mdi-cog-box' - :hint='cfg.value.hint ? cfg.value.hint : ""' - persistent-hint - :class='cfg.value.hint ? "mb-2" : ""' - ) - v-text-field.mb-3( - v-else - outlined - :key='cfg.key' - :label='cfg.value.title' - v-model='cfg.value.value' - prepend-icon='mdi-cog-box' - :hint='cfg.value.hint ? cfg.value.hint : ""' - persistent-hint - :class='cfg.value.hint ? "mb-2" : ""' - :style='cfg.value.maxWidth > 0 ? `max-width:` + cfg.value.maxWidth + `px;` : ``' - ) - v-divider.mt-3 + .row + .col-8 + v-text-field( + outlined + :label='$t(`admin:auth.displayName`)' + v-model='strategy.displayName' + prepend-icon='mdi-format-title' + :hint='$t(`admin:auth.displayNameHint`)' + persistent-hint + ) + .col-4 + v-switch.mt-1( + :label='$t(`admin:auth.strategyIsEnabled`)' + v-model='strategy.isEnabled' + color='primary' + prepend-icon='mdi-power' + :hint='$t(`admin:auth.strategyIsEnabledHint`)' + persistent-hint + inset + :disabled='strategy.key === `local`' + ) + template(v-if='strategy.config && Object.keys(strategy.config).length > 0') + v-divider + .overline.my-5 {{$t('admin:auth.strategyConfiguration')}} + .pr-3 + template(v-for='cfg in strategy.config') + v-select.mb-3( + v-if='cfg.value.type === "string" && cfg.value.enum' + outlined + :items='cfg.value.enum' + :key='cfg.key' + :label='cfg.value.title' + v-model='cfg.value.value' + prepend-icon='mdi-cog-box' + :hint='cfg.value.hint ? cfg.value.hint : ""' + persistent-hint + :class='cfg.value.hint ? "mb-2" : ""' + :style='cfg.value.maxWidth > 0 ? `max-width:` + cfg.value.maxWidth + `px;` : ``' + ) + v-switch.mb-6( + v-else-if='cfg.value.type === "boolean"' + :key='cfg.key' + :label='cfg.value.title' + v-model='cfg.value.value' + color='primary' + prepend-icon='mdi-cog-box' + :hint='cfg.value.hint ? cfg.value.hint : ""' + persistent-hint + inset + ) + v-textarea.mb-3( + v-else-if='cfg.value.type === "string" && cfg.value.multiline' + outlined + :key='cfg.key' + :label='cfg.value.title' + v-model='cfg.value.value' + prepend-icon='mdi-cog-box' + :hint='cfg.value.hint ? cfg.value.hint : ""' + persistent-hint + :class='cfg.value.hint ? "mb-2" : ""' + ) + v-text-field.mb-3( + v-else + outlined + :key='cfg.key' + :label='cfg.value.title' + v-model='cfg.value.value' + prepend-icon='mdi-cog-box' + :hint='cfg.value.hint ? cfg.value.hint : ""' + persistent-hint + :class='cfg.value.hint ? "mb-2" : ""' + :style='cfg.value.maxWidth > 0 ? `max-width:` + cfg.value.maxWidth + `px;` : ``' + ) + v-divider .overline.my-5 {{$t('admin:auth.registration')}} .pr-3 v-switch.ml-3( @@ -145,7 +161,7 @@ persistent-hint inset ) - v-combobox.ml-3.mt-3( + v-combobox.ml-3.mt-5( :label='$t(`admin:auth.domainsWhitelist`)' v-model='strategy.domainWhitelist' prepend-icon='mdi-email-check-outline' @@ -272,6 +288,7 @@ export default { } })), order: this.activeStrategies.length, + isEnabled: true, displayName: str.title, selfRegistration: false, domainWhitelist: [], @@ -309,6 +326,7 @@ export default { strategyKey: str.strategy.key, displayName: str.displayName, order: str.order, + isEnabled: str.isEnabled, config: str.config.map(cfg => ({...cfg, value: JSON.stringify({ v: cfg.value.value })})), selfRegistration: str.selfRegistration, domainWhitelist: str.domainWhitelist, @@ -384,6 +402,7 @@ export default { value } order + isEnabled displayName selfRegistration domainWhitelist diff --git a/client/components/login.vue b/client/components/login.vue index 0b3c27b2..202881dd 100644 --- a/client/components/login.vue +++ b/client/components/login.vue @@ -661,7 +661,7 @@ export default { query: gql` { authentication { - activeStrategies { + activeStrategies(enabledOnly: true) { key strategy { key diff --git a/server/db/migrations/2.5.1.js b/server/db/migrations/2.5.1.js index 8cf75a6c..1e907974 100644 --- a/server/db/migrations/2.5.1.js +++ b/server/db/migrations/2.5.1.js @@ -1,9 +1,15 @@ exports.up = async knex => { - await knex('authentication').where('isEnabled', false).del() + // Check for users using disabled strategies + const disabledStrategies = await knex('authentication').where('isEnabled', false) + const incompatibleUsers = await knex('users').distinct('providerKey').whereIn('providerKey', disabledStrategies.map(s => s.key)) + const protectedStrategies = (incompatibleUsers && incompatibleUsers.length > 0) ? incompatibleUsers.map(u => u.providerKey) : [] + // Delete disabled strategies + await knex('authentication').whereNotIn('key', protectedStrategies).andWhere('isEnabled', false).del() + + // Update table schema await knex.schema .alterTable('authentication', table => { - table.dropColumn('isEnabled') table.integer('order').unsigned().notNullable().defaultTo(0) table.string('strategyKey').notNullable().defaultTo('') table.string('displayName').notNullable().defaultTo('') diff --git a/server/db/migrations/2.5.108.js b/server/db/migrations/2.5.108.js new file mode 100644 index 00000000..34240fcc --- /dev/null +++ b/server/db/migrations/2.5.108.js @@ -0,0 +1,14 @@ +const has = require('lodash/has') + +exports.up = async knex => { + // -> Fix 2.5.1 added isEnabled columns for beta users + const localStrategy = await knex('authentication').where('key', 'local') + if (!has(localStrategy, 'isEnabled')) { + await knex.schema + .alterTable('authentication', table => { + table.boolean('isEnabled').notNullable().defaultTo(true) + }) + } +} + +exports.down = knex => { } diff --git a/server/graph/resolvers/authentication.js b/server/graph/resolvers/authentication.js index d2ed5d9d..ce7a152a 100644 --- a/server/graph/resolvers/authentication.js +++ b/server/graph/resolvers/authentication.js @@ -70,7 +70,7 @@ module.exports = { }, []), 'key') } }) - return strategies + return args.enabledOnly ? _.filter(strategies, 'isEnabled') : strategies } }, AuthenticationMutation: { @@ -199,18 +199,12 @@ module.exports = { */ async updateStrategies (obj, args, context) { try { - // WIKI.config.auth = { - // audience: _.get(args, 'config.audience', WIKI.config.auth.audience), - // tokenExpiration: _.get(args, 'config.tokenExpiration', WIKI.config.auth.tokenExpiration), - // tokenRenewal: _.get(args, 'config.tokenRenewal', WIKI.config.auth.tokenRenewal) - // } - // await WIKI.configSvc.saveToDb(['auth']) - const previousStrategies = await WIKI.models.authentication.getStrategies() for (const str of args.strategies) { const newStr = { displayName: str.displayName, order: str.order, + isEnabled: str.isEnabled, config: _.reduce(str.config, (result, value, key) => { _.set(result, `${value.key}`, _.get(JSON.parse(value.value), 'v', null)) return result diff --git a/server/graph/schemas/authentication.graphql b/server/graph/schemas/authentication.graphql index 202e82a5..de023844 100644 --- a/server/graph/schemas/authentication.graphql +++ b/server/graph/schemas/authentication.graphql @@ -20,7 +20,10 @@ type AuthenticationQuery { apiState: Boolean! @auth(requires: ["manage:system", "manage:api"]) strategies: [AuthenticationStrategy] @auth(requires: ["manage:system"]) - activeStrategies: [AuthenticationActiveStrategy] + + activeStrategies( + enabledOnly: Boolean + ): [AuthenticationActiveStrategy] } # ----------------------------------------------- @@ -102,6 +105,7 @@ type AuthenticationActiveStrategy { strategy: AuthenticationStrategy! displayName: String! order: Int! + isEnabled: Boolean! config: [KeyValuePair] @auth(requires: ["manage:system"]) selfRegistration: Boolean! domainWhitelist: [String]! @auth(requires: ["manage:system"]) @@ -130,6 +134,7 @@ input AuthenticationStrategyInput { config: [KeyValuePairInput] displayName: String! order: Int! + isEnabled: Boolean! selfRegistration: Boolean! domainWhitelist: [String]! autoEnrollGroups: [Int]! diff --git a/server/models/users.js b/server/models/users.js index f86b51dc..2a3ee313 100644 --- a/server/models/users.js +++ b/server/models/users.js @@ -277,6 +277,10 @@ module.exports = class User extends Model { static async login (opts, context) { if (_.has(WIKI.auth.strategies, opts.strategy)) { const selStrategy = _.get(WIKI.auth.strategies, opts.strategy) + if (!selStrategy.isEnabled) { + throw new WIKI.Error.AuthProviderInvalid() + } + const strInfo = _.find(WIKI.data.authentication, ['key', selStrategy.strategyKey]) // Inject form user/pass