Add authentication page in config, add /auth-settings GET endpoint, remove authOpenIDCallbackURL server setting

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)
     )

server/Server.js

@@ -163,8 +163,6 @@ class Server {
 
     this.server = http.createServer(app)
 
-
-
     router.use(fileUpload({
       defCharset: 'utf8',
       defParamCharset: 'utf8',

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()

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]) {

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) {