From e282142d3fcf3d483cbea8432c85e216fed6ca72 Mon Sep 17 00:00:00 2001 From: advplyr Date: Sun, 24 Sep 2023 15:36:35 -0500 Subject: [PATCH] Add authentication page in config, add /auth-settings GET endpoint, remove authOpenIDCallbackURL server setting --- client/components/app/ConfigSideNav.vue | 5 + client/pages/config.vue | 1 + client/pages/config/authentication.vue | 138 ++++++++++++++++++++++ client/store/index.js | 2 +- client/strings/en-us.json | 1 + server/Auth.js | 46 ++++---- server/Server.js | 2 - server/controllers/MiscController.js | 23 +++- server/objects/settings/ServerSettings.js | 51 ++++++-- server/routers/ApiRouter.js | 1 + 10 files changed, 225 insertions(+), 45 deletions(-) create mode 100644 client/pages/config/authentication.vue diff --git a/client/components/app/ConfigSideNav.vue b/client/components/app/ConfigSideNav.vue index 50e440d7..677aba70 100644 --- a/client/components/app/ConfigSideNav.vue +++ b/client/components/app/ConfigSideNav.vue @@ -104,6 +104,11 @@ export default { id: 'config-rss-feeds', title: this.$strings.HeaderRSSFeeds, path: '/config/rss-feeds' + }, + { + id: 'config-authentication', + title: this.$strings.HeaderAuthentication, + path: '/config/authentication' } ] diff --git a/client/pages/config.vue b/client/pages/config.vue index 542b7f2c..fdbd7150 100644 --- a/client/pages/config.vue +++ b/client/pages/config.vue @@ -57,6 +57,7 @@ export default { else if (pageName === 'item-metadata-utils') return this.$strings.HeaderItemMetadataUtils else if (pageName === 'rss-feeds') return this.$strings.HeaderRSSFeeds else if (pageName === 'email') return this.$strings.HeaderEmail + else if (pageName === 'authentication') return this.$strings.HeaderAuthentication } return this.$strings.HeaderSettings } diff --git a/client/pages/config/authentication.vue b/client/pages/config/authentication.vue new file mode 100644 index 00000000..acc0ac13 --- /dev/null +++ b/client/pages/config/authentication.vue @@ -0,0 +1,138 @@ + + + + diff --git a/client/store/index.js b/client/store/index.js index 2f8201c1..ed7c35b6 100644 --- a/client/store/index.js +++ b/client/store/index.js @@ -66,7 +66,7 @@ export const getters = { export const actions = { updateServerSettings({ commit }, payload) { - var updatePayload = { + const updatePayload = { ...payload } return this.$axios.$patch('/api/settings', updatePayload).then((result) => { diff --git a/client/strings/en-us.json b/client/strings/en-us.json index 75606da2..47cfe448 100644 --- a/client/strings/en-us.json +++ b/client/strings/en-us.json @@ -88,6 +88,7 @@ "HeaderAppriseNotificationSettings": "Apprise Notification Settings", "HeaderAudiobookTools": "Audiobook File Management Tools", "HeaderAudioTracks": "Audio Tracks", + "HeaderAuthentication": "Authentication", "HeaderBackups": "Backups", "HeaderChangePassword": "Change Password", "HeaderChapters": "Chapters", diff --git a/server/Auth.js b/server/Auth.js index 0041fbed..d6d67d49 100644 --- a/server/Auth.js +++ b/server/Auth.js @@ -57,24 +57,23 @@ class Auth { userInfoURL: global.ServerSettings.authOpenIDUserInfoURL, clientID: global.ServerSettings.authOpenIDClientID, clientSecret: global.ServerSettings.authOpenIDClientSecret, - callbackURL: global.ServerSettings.authOpenIDCallbackURL, + callbackURL: '/auth/openid/callback', scope: ["openid", "email", "profile"], skipUserProfile: false - }, - (async function (issuer, profile, done) { - // TODO: do we want to create the users which does not exist? + }, async (issuer, profile, done) => { + // TODO: do we want to create the users which does not exist? - const user = await Database.userModel.getUserByUsername(profile.username) + const user = await Database.userModel.getUserByUsername(profile.username) - if (!user?.isActive) { - // deny login - done(null, null) - return - } + if (!user?.isActive) { + // deny login + done(null, null) + return + } - // permit login - return done(null, user) - }).bind(this))) + // permit login + return done(null, user) + })) } // Load the JwtStrategy (always) -> for bearer token auth @@ -111,14 +110,13 @@ class Auth { * @param {import('express').Response} res */ paramsToCookies(req, res) { - if (req.query.isRest && req.query.isRest.toLowerCase() == "true") { + if (req.query.isRest?.toLowerCase() == "true") { // store the isRest flag to the is_rest cookie res.cookie('is_rest', req.query.isRest.toLowerCase(), { maxAge: 120000, // 2 min httpOnly: true }) - } - else { + } else { // no isRest-flag set -> set is_rest cookie to false res.cookie('is_rest', "false", { maxAge: 120000, // 2 min @@ -126,7 +124,7 @@ class Auth { }) // check if we are missing a callback parameter - we need one if isRest=false - if (!req.query.callback || req.query.callback === "") { + if (!req.query.callback) { res.status(400).send({ message: 'No callback parameter' }) @@ -151,19 +149,17 @@ class Auth { // get userLogin json (information about the user, server and the session) const data_json = await this.getUserLoginResponsePayload(req.user) - if (req.cookies.is_rest && req.cookies.is_rest === "true") { + if (req.cookies.is_rest === 'true') { // REST request - send data res.json(data_json) - } - else { + } else { // UI request -> check if we have a callback url // TODO: do we want to somehow limit the values for auth_cb? - if (req.cookies.auth_cb && req.cookies.auth_cb.startsWith("http")) { + if (req.cookies.auth_cb?.startsWith('http')) { // UI request -> redirect to auth_cb url and send the jwt token as parameter res.redirect(302, `${req.cookies.auth_cb}?setToken=${data_json.user.token}`) - } - else { - res.status(400).send("No callback or already expired") + } else { + res.status(400).send('No callback or already expired') } } } @@ -205,7 +201,7 @@ class Auth { // openid strategy callback route (this receives the token from the configured openid login provider) router.get('/auth/openid/callback', - passport.authenticate('openidconnect'), + passport.authenticate('openidconnect', { failureRedirect: '/login', failureMessage: true }), // on a successfull login: read the cookies and react like the client requested (callback or json) this.handleLoginSuccessBasedOnCookie.bind(this) ) diff --git a/server/Server.js b/server/Server.js index dbb9bddf..08f4b8d9 100644 --- a/server/Server.js +++ b/server/Server.js @@ -163,8 +163,6 @@ class Server { this.server = http.createServer(app) - - router.use(fileUpload({ defCharset: 'utf8', defParamCharset: 'utf8', diff --git a/server/controllers/MiscController.js b/server/controllers/MiscController.js index 0fa1c62f..d1f3686b 100644 --- a/server/controllers/MiscController.js +++ b/server/controllers/MiscController.js @@ -117,8 +117,9 @@ class MiscController { /** * PATCH: /api/settings * Update server settings - * @param {*} req - * @param {*} res + * + * @param {import('express').Request} req + * @param {import('express').Response} res */ async updateServerSettings(req, res) { if (!req.user.isAdminOrUp) { @@ -246,8 +247,8 @@ class MiscController { * POST: /api/authorize * Used to authorize an API token * - * @param {*} req - * @param {*} res + * @param {import('express').Request} req + * @param {import('express').Response} res */ async authorize(req, res) { if (!req.user) { @@ -539,5 +540,19 @@ class MiscController { res.status(400).send(error.message) } } + + /** + * GET: api/auth-settings (admin only) + * + * @param {import('express').Request} req + * @param {import('express').Response} res + */ + getAuthSettings(req, res) { + if (!req.user.isAdminOrUp) { + Logger.error(`[MiscController] Non-admin user "${req.user.username}" attempted to get auth settings`) + return res.sendStatus(403) + } + return res.json(Database.serverSettings.authenticationSettings) + } } module.exports = new MiscController() \ No newline at end of file diff --git a/server/objects/settings/ServerSettings.js b/server/objects/settings/ServerSettings.js index 71358a00..9348d691 100644 --- a/server/objects/settings/ServerSettings.js +++ b/server/objects/settings/ServerSettings.js @@ -64,14 +64,13 @@ class ServerSettings { this.authGoogleOauth20ClientSecret = '' this.authGoogleOauth20CallbackURL = '' - // generic-oauth20 settings + // openid settings this.authOpenIDIssuerURL = '' this.authOpenIDAuthorizationURL = '' this.authOpenIDTokenURL = '' this.authOpenIDUserInfoURL = '' this.authOpenIDClientID = '' this.authOpenIDClientSecret = '' - this.authOpenIDCallbackURL = '' if (settings) { this.construct(settings) @@ -126,7 +125,6 @@ class ServerSettings { this.authOpenIDUserInfoURL = settings.authOpenIDUserInfoURL || '' this.authOpenIDClientID = settings.authOpenIDClientID || '' this.authOpenIDClientSecret = settings.authOpenIDClientSecret || '' - this.authOpenIDCallbackURL = settings.authOpenIDCallbackURL || '' if (!Array.isArray(this.authActiveAuthMethods)) { this.authActiveAuthMethods = ['local'] @@ -144,16 +142,15 @@ class ServerSettings { // remove uninitialized methods // OpenID - if (this.authActiveAuthMethods.includes('generic-oauth20') && ( + if (this.authActiveAuthMethods.includes('openid') && ( this.authOpenIDIssuerURL === '' || this.authOpenIDAuthorizationURL === '' || this.authOpenIDTokenURL === '' || this.authOpenIDUserInfoURL === '' || this.authOpenIDClientID === '' || - this.authOpenIDClientSecret === '' || - this.authOpenIDCallbackURL === '' + this.authOpenIDClientSecret === '' )) { - this.authActiveAuthMethods.splice(this.authActiveAuthMethods.indexOf('generic-oauth20', 0), 1) + this.authActiveAuthMethods.splice(this.authActiveAuthMethods.indexOf('openid', 0), 1) } // fallback to local @@ -228,8 +225,7 @@ class ServerSettings { authOpenIDTokenURL: this.authOpenIDTokenURL, authOpenIDUserInfoURL: this.authOpenIDUserInfoURL, authOpenIDClientID: this.authOpenIDClientID, // Do not return to client - authOpenIDClientSecret: this.authOpenIDClientSecret, // Do not return to client - authOpenIDCallbackURL: this.authOpenIDCallbackURL + authOpenIDClientSecret: this.authOpenIDClientSecret // Do not return to client } } @@ -243,13 +239,42 @@ class ServerSettings { return json } + get authenticationSettings() { + return { + authActiveAuthMethods: this.authActiveAuthMethods, + authGoogleOauth20ClientID: this.authGoogleOauth20ClientID, // Do not return to client + authGoogleOauth20ClientSecret: this.authGoogleOauth20ClientSecret, // Do not return to client + authGoogleOauth20CallbackURL: this.authGoogleOauth20CallbackURL, + authOpenIDIssuerURL: this.authOpenIDIssuerURL, + authOpenIDAuthorizationURL: this.authOpenIDAuthorizationURL, + authOpenIDTokenURL: this.authOpenIDTokenURL, + authOpenIDUserInfoURL: this.authOpenIDUserInfoURL, + authOpenIDClientID: this.authOpenIDClientID, // Do not return to client + authOpenIDClientSecret: this.authOpenIDClientSecret // Do not return to client + } + } + + /** + * Update server settings + * + * @param {Object} payload + * @returns {boolean} true if updates were made + */ update(payload) { let hasUpdates = false for (const key in payload) { - if (key === 'sortingPrefixes' && payload[key] && payload[key].length) { - const prefixesCleaned = payload[key].filter(prefix => !!prefix).map(prefix => prefix.toLowerCase()) - if (prefixesCleaned.join(',') !== this[key].join(',')) { - this[key] = [...prefixesCleaned] + if (key === 'sortingPrefixes') { + // Sorting prefixes are updated with the /api/sorting-prefixes endpoint + continue + } else if (key === 'authActiveAuthMethods') { + if (!payload[key]?.length) { + Logger.error(`[ServerSettings] Invalid authActiveAuthMethods`, payload[key]) + continue + } + this.authActiveAuthMethods.sort() + payload[key].sort() + if (payload[key].join() !== this.authActiveAuthMethods.join()) { + this.authActiveAuthMethods = payload[key] hasUpdates = true } } else if (this[key] !== payload[key]) { diff --git a/server/routers/ApiRouter.js b/server/routers/ApiRouter.js index 71d9429e..d91c9312 100644 --- a/server/routers/ApiRouter.js +++ b/server/routers/ApiRouter.js @@ -306,6 +306,7 @@ class ApiRouter { this.router.post('/genres/rename', MiscController.renameGenre.bind(this)) this.router.delete('/genres/:genre', MiscController.deleteGenre.bind(this)) this.router.post('/validate-cron', MiscController.validateCronExpression.bind(this)) + this.router.get('/auth-settings', MiscController.getAuthSettings.bind(this)) } async getDirectories(dir, relpath, excludedDirs, level = 0) {