diff --git a/client/pages/config/authentication.vue b/client/pages/config/authentication.vue index 1f934c88..ba4df4c3 100644 --- a/client/pages/config/authentication.vue +++ b/client/pages/config/authentication.vue @@ -64,6 +64,20 @@

+

+
+ +
+
+

{{ $strings.LabelWebRedirectURLsDescription }}

+

+ {{ webCallbackURL }} +
+ {{ mobileAppCallbackURL }} +

+
+
+
@@ -164,6 +178,27 @@ export default { value: 'username' } ] + }, + subfolderOptions() { + const options = [ + { + text: 'None', + value: '' + } + ] + if (this.$config.routerBasePath) { + options.push({ + text: this.$config.routerBasePath, + value: this.$config.routerBasePath + }) + } + return options + }, + webCallbackURL() { + return `https://${this.newAuthSettings.authOpenIDSubfolderForRedirectURLs ? this.newAuthSettings.authOpenIDSubfolderForRedirectURLs : ''}/auth/openid/callback` + }, + mobileAppCallbackURL() { + return `https://${this.newAuthSettings.authOpenIDSubfolderForRedirectURLs ? this.newAuthSettings.authOpenIDSubfolderForRedirectURLs : ''}/auth/openid/mobile-redirect` } }, methods: { @@ -325,7 +360,8 @@ export default { }, init() { this.newAuthSettings = { - ...this.authSettings + ...this.authSettings, + authOpenIDSubfolderForRedirectURLs: this.authSettings.authOpenIDSubfolderForRedirectURLs === undefined ? this.$config.routerBasePath : this.authSettings.authOpenIDSubfolderForRedirectURLs } this.enableLocalAuth = this.authMethods.includes('local') this.enableOpenIDAuth = this.authMethods.includes('openid') diff --git a/client/strings/en-us.json b/client/strings/en-us.json index 0c077ed6..8a91686c 100644 --- a/client/strings/en-us.json +++ b/client/strings/en-us.json @@ -679,6 +679,8 @@ "LabelViewPlayerSettings": "View player settings", "LabelViewQueue": "View player queue", "LabelVolume": "Volume", + "LabelWebRedirectURLsSubfolder": "Subfolder for Redirect URLs", + "LabelWebRedirectURLsDescription": "Authorize these URLs in your OAuth provider to allow redirection back to the web app after login:", "LabelWeekdaysToRun": "Weekdays to run", "LabelXBooks": "{0} books", "LabelXItems": "{0} items", diff --git a/server/Auth.js b/server/Auth.js index b0046799..74b767f5 100644 --- a/server/Auth.js +++ b/server/Auth.js @@ -131,7 +131,7 @@ class Auth { { client: openIdClient, params: { - redirect_uri: '/auth/openid/callback', + redirect_uri: `${global.ServerSettings.authOpenIDSubfolderForRedirectURLs}/auth/openid/callback`, scope: 'openid profile email' } }, @@ -480,9 +480,9 @@ class Auth { // for the request to mobile-redirect and as such the session is not shared this.openIdAuthSession.set(state, { mobile_redirect_uri: req.query.redirect_uri }) - redirectUri = new URL('/auth/openid/mobile-redirect', hostUrl).toString() + redirectUri = new URL(`${global.ServerSettings.authOpenIDSubfolderForRedirectURLs}/auth/openid/mobile-redirect`, hostUrl).toString() } else { - redirectUri = new URL('/auth/openid/callback', hostUrl).toString() + redirectUri = new URL(`${global.ServerSettings.authOpenIDSubfolderForRedirectURLs}/auth/openid/callback`, hostUrl).toString() if (req.query.state) { Logger.debug(`[Auth] Invalid state - not allowed on web openid flow`) @@ -733,7 +733,7 @@ class Auth { const host = req.get('host') // TODO: ABS does currently not support subfolders for installation // If we want to support it we need to include a config for the serverurl - postLogoutRedirectUri = `${protocol}://${host}/login` + postLogoutRedirectUri = `${protocol}://${host}${global.RouterBasePath}/login` } // else for openid-mobile we keep postLogoutRedirectUri on null // nice would be to redirect to the app here, but for example Authentik does not implement diff --git a/server/controllers/MiscController.js b/server/controllers/MiscController.js index cf901bea..2a87f2fe 100644 --- a/server/controllers/MiscController.js +++ b/server/controllers/MiscController.js @@ -679,9 +679,9 @@ class MiscController { continue } let updatedValue = settingsUpdate[key] - if (updatedValue === '') updatedValue = null + if (updatedValue === '' && key != 'authOpenIDSubfolderForRedirectURLs') updatedValue = null let currentValue = currentAuthenticationSettings[key] - if (currentValue === '') currentValue = null + if (currentValue === '' && key != 'authOpenIDSubfolderForRedirectURLs') currentValue = null if (updatedValue !== currentValue) { Logger.debug(`[MiscController] Updating auth settings key "${key}" from "${currentValue}" to "${updatedValue}"`) diff --git a/server/migrations/changelog.md b/server/migrations/changelog.md index 8960ade2..8ba4fad0 100644 --- a/server/migrations/changelog.md +++ b/server/migrations/changelog.md @@ -2,9 +2,10 @@ Please add a record of every database migration that you create to this file. This will help us keep track of changes to the database schema over time. -| Server Version | Migration Script Name | Description | -| -------------- | ---------------------------- | ------------------------------------------------------------------------------------ | -| v2.15.0 | v2.15.0-series-column-unique | Series must have unique names in the same library | -| v2.15.1 | v2.15.1-reindex-nocase | Fix potential db corruption issues due to bad sqlite extension introduced in v2.12.0 | -| v2.15.2 | v2.15.2-index-creation | Creates author, series, and podcast episode indexes | -| v2.17.0 | v2.17.0-uuid-replacement | Changes the data type of columns with UUIDv4 to UUID matching the associated model | +| Server Version | Migration Script Name | Description | +| -------------- | -------------------------------------------- | ------------------------------------------------------------------------------------ | +| v2.15.0 | v2.15.0-series-column-unique | Series must have unique names in the same library | +| v2.15.1 | v2.15.1-reindex-nocase | Fix potential db corruption issues due to bad sqlite extension introduced in v2.12.0 | +| v2.15.2 | v2.15.2-index-creation | Creates author, series, and podcast episode indexes | +| v2.17.0 | v2.17.0-uuid-replacement | Changes the data type of columns with UUIDv4 to UUID matching the associated model | +| v2.17.3 | v2.17.3-use-subfolder-for-oidc-redirect-uris | Save subfolder to OIDC redirect URIs to support existing installations | diff --git a/server/migrations/v2.17.3-use-subfolder-for-oidc-redirect-uris.js b/server/migrations/v2.17.3-use-subfolder-for-oidc-redirect-uris.js new file mode 100644 index 00000000..d03783cd --- /dev/null +++ b/server/migrations/v2.17.3-use-subfolder-for-oidc-redirect-uris.js @@ -0,0 +1,84 @@ +/** + * @typedef MigrationContext + * @property {import('sequelize').QueryInterface} queryInterface - a suquelize QueryInterface object. + * @property {import('../Logger')} logger - a Logger object. + * + * @typedef MigrationOptions + * @property {MigrationContext} context - an object containing the migration context. + */ + +/** + * This upward migration adds an subfolder setting for OIDC redirect URIs. + * It updates existing OIDC setups to set this option to None (empty subfolder), so they continue to work as before. + * IF OIDC is not enabled, no action is taken (i.e. the subfolder is left undefined), + * so that future OIDC setups will use the default subfolder. + * + * @param {MigrationOptions} options - an object containing the migration context. + * @returns {Promise} - A promise that resolves when the migration is complete. + */ +async function up({ context: { queryInterface, logger } }) { + // Upwards migration script + logger.info('[2.17.3 migration] UPGRADE BEGIN: 2.17.3-use-subfolder-for-oidc-redirect-uris') + + const serverSettings = await getServerSettings(queryInterface, logger) + if (serverSettings.authActiveAuthMethods?.includes('openid')) { + logger.info('[2.17.3 migration] OIDC is enabled, adding authOpenIDSubfolderForRedirectURLs to server settings') + serverSettings.authOpenIDSubfolderForRedirectURLs = '' + await updateServerSettings(queryInterface, logger, serverSettings) + } else { + logger.info('[2.17.3 migration] OIDC is not enabled, no action required') + } + + logger.info('[2.17.3 migration] UPGRADE END: 2.17.3-use-subfolder-for-oidc-redirect-uris') +} + +/** + * This downward migration script removes the subfolder setting for OIDC redirect URIs. + * + * @param {MigrationOptions} options - an object containing the migration context. + * @returns {Promise} - A promise that resolves when the migration is complete. + */ +async function down({ context: { queryInterface, logger } }) { + // Downward migration script + logger.info('[2.17.3 migration] DOWNGRADE BEGIN: 2.17.3-use-subfolder-for-oidc-redirect-uris ') + + // Remove the OIDC subfolder option from the server settings + const serverSettings = await getServerSettings(queryInterface, logger) + if (serverSettings.authOpenIDSubfolderForRedirectURLs !== undefined) { + logger.info('[2.17.3 migration] Removing authOpenIDSubfolderForRedirectURLs from server settings') + delete serverSettings.authOpenIDSubfolderForRedirectURLs + await updateServerSettings(queryInterface, logger, serverSettings) + } else { + logger.info('[2.17.3 migration] authOpenIDSubfolderForRedirectURLs not found in server settings, no action required') + } + + logger.info('[2.17.3 migration] DOWNGRADE END: 2.17.3-use-subfolder-for-oidc-redirect-uris ') +} + +async function getServerSettings(queryInterface, logger) { + const result = await queryInterface.sequelize.query('SELECT value FROM settings WHERE key = "server-settings";') + if (!result[0].length) { + logger.error('[2.17.3 migration] Server settings not found') + throw new Error('Server settings not found') + } + + let serverSettings = null + try { + serverSettings = JSON.parse(result[0][0].value) + } catch (error) { + logger.error('[2.17.3 migration] Error parsing server settings:', error) + throw error + } + + return serverSettings +} + +async function updateServerSettings(queryInterface, logger, serverSettings) { + await queryInterface.sequelize.query('UPDATE settings SET value = :value WHERE key = "server-settings";', { + replacements: { + value: JSON.stringify(serverSettings) + } + }) +} + +module.exports = { up, down } diff --git a/server/objects/settings/ServerSettings.js b/server/objects/settings/ServerSettings.js index 8ecb8ff0..ff28027f 100644 --- a/server/objects/settings/ServerSettings.js +++ b/server/objects/settings/ServerSettings.js @@ -78,6 +78,7 @@ class ServerSettings { this.authOpenIDMobileRedirectURIs = ['audiobookshelf://oauth'] this.authOpenIDGroupClaim = '' this.authOpenIDAdvancedPermsClaim = '' + this.authOpenIDSubfolderForRedirectURLs = undefined if (settings) { this.construct(settings) @@ -139,6 +140,7 @@ class ServerSettings { this.authOpenIDMobileRedirectURIs = settings.authOpenIDMobileRedirectURIs || ['audiobookshelf://oauth'] this.authOpenIDGroupClaim = settings.authOpenIDGroupClaim || '' this.authOpenIDAdvancedPermsClaim = settings.authOpenIDAdvancedPermsClaim || '' + this.authOpenIDSubfolderForRedirectURLs = settings.authOpenIDSubfolderForRedirectURLs if (!Array.isArray(this.authActiveAuthMethods)) { this.authActiveAuthMethods = ['local'] @@ -240,7 +242,8 @@ class ServerSettings { authOpenIDMatchExistingBy: this.authOpenIDMatchExistingBy, authOpenIDMobileRedirectURIs: this.authOpenIDMobileRedirectURIs, // Do not return to client authOpenIDGroupClaim: this.authOpenIDGroupClaim, // Do not return to client - authOpenIDAdvancedPermsClaim: this.authOpenIDAdvancedPermsClaim // Do not return to client + authOpenIDAdvancedPermsClaim: this.authOpenIDAdvancedPermsClaim, // Do not return to client + authOpenIDSubfolderForRedirectURLs: this.authOpenIDSubfolderForRedirectURLs } } @@ -286,6 +289,7 @@ class ServerSettings { authOpenIDMobileRedirectURIs: this.authOpenIDMobileRedirectURIs, // Do not return to client authOpenIDGroupClaim: this.authOpenIDGroupClaim, // Do not return to client authOpenIDAdvancedPermsClaim: this.authOpenIDAdvancedPermsClaim, // Do not return to client + authOpenIDSubfolderForRedirectURLs: this.authOpenIDSubfolderForRedirectURLs, authOpenIDSamplePermissions: User.getSampleAbsPermissions() } diff --git a/test/server/migrations/v2.17.3-use-subfolder-for-oidc-redirect-uris.test.js b/test/server/migrations/v2.17.3-use-subfolder-for-oidc-redirect-uris.test.js new file mode 100644 index 00000000..157b1ed4 --- /dev/null +++ b/test/server/migrations/v2.17.3-use-subfolder-for-oidc-redirect-uris.test.js @@ -0,0 +1,116 @@ +const { expect } = require('chai') +const sinon = require('sinon') +const { up, down } = require('../../../server/migrations/v2.17.3-use-subfolder-for-oidc-redirect-uris') +const { Sequelize } = require('sequelize') +const Logger = require('../../../server/Logger') + +describe('Migration v2.17.3-use-subfolder-for-oidc-redirect-uris', () => { + let queryInterface, logger, context + + beforeEach(() => { + queryInterface = { + sequelize: { + query: sinon.stub() + } + } + logger = { + info: sinon.stub(), + error: sinon.stub() + } + context = { queryInterface, logger } + }) + + describe('up', () => { + it('should add authOpenIDSubfolderForRedirectURLs if OIDC is enabled', async () => { + queryInterface.sequelize.query.onFirstCall().resolves([[{ value: JSON.stringify({ authActiveAuthMethods: ['openid'] }) }]]) + queryInterface.sequelize.query.onSecondCall().resolves() + + await up({ context }) + + expect(logger.info.calledWith('[2.17.3 migration] UPGRADE BEGIN: 2.17.3-use-subfolder-for-oidc-redirect-uris')).to.be.true + expect(logger.info.calledWith('[2.17.3 migration] OIDC is enabled, adding authOpenIDSubfolderForRedirectURLs to server settings')).to.be.true + expect(queryInterface.sequelize.query.calledTwice).to.be.true + expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true + expect( + queryInterface.sequelize.query.calledWith('UPDATE settings SET value = :value WHERE key = "server-settings";', { + replacements: { + value: JSON.stringify({ authActiveAuthMethods: ['openid'], authOpenIDSubfolderForRedirectURLs: '' }) + } + }) + ).to.be.true + expect(logger.info.calledWith('[2.17.3 migration] UPGRADE END: 2.17.3-use-subfolder-for-oidc-redirect-uris')).to.be.true + }) + + it('should not add authOpenIDSubfolderForRedirectURLs if OIDC is not enabled', async () => { + queryInterface.sequelize.query.onFirstCall().resolves([[{ value: JSON.stringify({ authActiveAuthMethods: [] }) }]]) + + await up({ context }) + + expect(logger.info.calledWith('[2.17.3 migration] UPGRADE BEGIN: 2.17.3-use-subfolder-for-oidc-redirect-uris')).to.be.true + expect(logger.info.calledWith('[2.17.3 migration] OIDC is not enabled, no action required')).to.be.true + expect(queryInterface.sequelize.query.calledOnce).to.be.true + expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true + expect(logger.info.calledWith('[2.17.3 migration] UPGRADE END: 2.17.3-use-subfolder-for-oidc-redirect-uris')).to.be.true + }) + + it('should throw an error if server settings cannot be parsed', async () => { + queryInterface.sequelize.query.onFirstCall().resolves([[{ value: 'invalid json' }]]) + + try { + await up({ context }) + } catch (error) { + expect(queryInterface.sequelize.query.calledOnce).to.be.true + expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true + expect(logger.error.calledWith('[2.17.3 migration] Error parsing server settings:')).to.be.true + expect(error).to.be.instanceOf(Error) + } + }) + + it('should throw an error if server settings are not found', async () => { + queryInterface.sequelize.query.onFirstCall().resolves([[]]) + + try { + await up({ context }) + } catch (error) { + expect(queryInterface.sequelize.query.calledOnce).to.be.true + expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true + expect(logger.error.calledWith('[2.17.3 migration] Server settings not found')).to.be.true + expect(error).to.be.instanceOf(Error) + } + }) + }) + + describe('down', () => { + it('should remove authOpenIDSubfolderForRedirectURLs if it exists', async () => { + queryInterface.sequelize.query.onFirstCall().resolves([[{ value: JSON.stringify({ authOpenIDSubfolderForRedirectURLs: '' }) }]]) + queryInterface.sequelize.query.onSecondCall().resolves() + + await down({ context }) + + expect(logger.info.calledWith('[2.17.3 migration] DOWNGRADE BEGIN: 2.17.3-use-subfolder-for-oidc-redirect-uris ')).to.be.true + expect(logger.info.calledWith('[2.17.3 migration] Removing authOpenIDSubfolderForRedirectURLs from server settings')).to.be.true + expect(queryInterface.sequelize.query.calledTwice).to.be.true + expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true + expect( + queryInterface.sequelize.query.calledWith('UPDATE settings SET value = :value WHERE key = "server-settings";', { + replacements: { + value: JSON.stringify({}) + } + }) + ).to.be.true + expect(logger.info.calledWith('[2.17.3 migration] DOWNGRADE END: 2.17.3-use-subfolder-for-oidc-redirect-uris ')).to.be.true + }) + + it('should not remove authOpenIDSubfolderForRedirectURLs if it does not exist', async () => { + queryInterface.sequelize.query.onFirstCall().resolves([[{ value: JSON.stringify({}) }]]) + + await down({ context }) + + expect(logger.info.calledWith('[2.17.3 migration] DOWNGRADE BEGIN: 2.17.3-use-subfolder-for-oidc-redirect-uris ')).to.be.true + expect(logger.info.calledWith('[2.17.3 migration] authOpenIDSubfolderForRedirectURLs not found in server settings, no action required')).to.be.true + expect(queryInterface.sequelize.query.calledOnce).to.be.true + expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true + expect(logger.info.calledWith('[2.17.3 migration] DOWNGRADE END: 2.17.3-use-subfolder-for-oidc-redirect-uris ')).to.be.true + }) + }) +})