Account: Allow OIDC and LDAP users with password to use 2FA #782 #808

Signed-off-by: Michael Mayer <michael@photoprism.app>

frontend/src/common/session.js

@@ -43,6 +43,7 @@ export default class Session {
     this.storage_key = "sessionStorage";
     this.auth = false;
     this.config = config;
+    this.provider = "";
     this.user = new User(false);
     this.data = null;
 
@@ -64,6 +65,11 @@ export default class Session {
       if (userJson !== "undefined") {
         this.user = new User(JSON.parse(userJson));
       }
+
+      const provider = this.storage.getItem("provider");
+      if (provider !== null) {
+        this.provider = provider;
+      }
     }
 
     // Authenticated?
@@ -200,9 +206,23 @@ export default class Session {
   }
 
   getProvider() {
+    if (!this.provider) {
+      return "";
+    }
+
     return this.provider;
   }
 
+  hasPassword() {
+    switch (this.getProvider()) {
+      case "local":
+      case "ldap":
+        return true;
+      default:
+        return false;
+    }
+  }
+
   hasProvider() {
     return !!this.provider;
   }

frontend/src/dialog/account/passcode.vue

@@ -261,6 +261,7 @@ export default {
       recoveryCodeCopied: false,
       password: "",
       showPassword: false,
+      session: this.$session,
       minLength: this.$config.get("passwordLength"),
       maxLength: 72,
       rtl: this.$rtl,
@@ -269,7 +270,7 @@ export default {
   },
   computed: {
     page() {
-      if (this.model?.AuthProvider !== "default" && this.model?.AuthProvider !== "local" && this.model?.AuthProvider !== "ldap") {
+      if (!this.session.hasPassword()) {
         return "not_available";
       } else if (this.model?.AuthMethod === "2fa") {
         return "deactivate";

frontend/src/model/user.js

@@ -289,7 +289,7 @@ export class User extends RestModel {
     }).then((response) => Promise.resolve(response.data));
   }
 
-  disablePasscodeSetup() {
+  disablePasscodeSetup(hasPassword) {
     if (!this.Name || !this.CanLogin || this.ID < 1) {
       return true;
     }
@@ -297,6 +297,8 @@ export class User extends RestModel {
     switch (this.AuthProvider) {
       case "":
       case "default":
+      case "oidc":
+        return !hasPassword;
       case "local":
       case "ldap":
         return false;

frontend/src/page/settings/account.vue

@@ -183,16 +183,16 @@
         <v-card-actions>
           <v-layout wrap align-top>
             <v-flex xs12 sm6 class="pa-2">
-              <v-btn block depressed color="secondary-light" class="action-change-password compact" :disabled="isPublic || isDemo || user.Name === '' || provider !== 'local'" @click.stop="showDialog('password')">
+              <v-btn block depressed color="secondary-light" class="action-change-password compact" :disabled="isPublic || isDemo || user.Name === '' || getProvider() !== 'local'" @click.stop="showDialog('password')">
                 <translate>Change Password</translate>
                 <v-icon :right="!rtl" :left="rtl" dark>lock</v-icon>
               </v-btn>
             </v-flex>
             <v-flex xs12 sm6 class="pa-2">
-              <v-btn block depressed color="secondary-light" class="action-passcode-dialog compact" :disabled="isPublic || isDemo || user.disablePasscodeSetup()" @click.stop="showDialog('passcode')">
+              <v-btn block depressed color="secondary-light" class="action-passcode-dialog compact" :disabled="isPublic || isDemo || user.disablePasscodeSetup(session.hasPassword())" @click.stop="showDialog('passcode')">
                 <translate>2-Factor Authentication</translate>
                 <v-icon v-if="user.AuthMethod === '2fa'" :right="!rtl" :left="rtl" dark>gpp_good</v-icon>
-                <v-icon v-else-if="user.disablePasscodeSetup()" :right="!rtl" :left="rtl" dark>shield</v-icon>
+                <v-icon v-else-if="user.disablePasscodeSetup(session.hasPassword())" :right="!rtl" :left="rtl" dark>shield</v-icon>
                 <v-icon v-else :right="!rtl" :left="rtl" dark>gpp_maybe</v-icon>
               </v-btn>
             </v-flex>
@@ -371,7 +371,7 @@ export default {
       rtl: this.$rtl,
       user: user,
       countries: countries,
-      provider: this.$session.provider ? this.$session.provider : user.AuthProvider,
+      session: this.$session,
       dialog: {
         apps: false,
         passcode: false,
@@ -404,6 +404,9 @@ export default {
     }
   },
   methods: {
+    getProvider() {
+      return this.$session.provider ? this.$session.provider : this.user.AuthProvider;
+    },
     showDialog(name) {
       if (!name) {
         return;

internal/api/oauth_token.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/dustin/go-humanize/english"
 	"github.com/gin-gonic/gin"
@@ -145,14 +146,14 @@ func OAuthToken(router *gin.RouterGroup) {
 					event.AuditErr([]string{clientIp, "oauth2", actor, action, authn.Denied})
 					AbortInvalidCredentials(c)
 					return
-				} else if !authUser.Equal(s.User()) {
+				} else if authMethod.Is(authn.Method2FA) && errors.Is(authErr, authn.ErrPasscodeRequired) {
-					event.AuditErr([]string{clientIp, "oauth2", actor, action, authn.ErrInvalidUsername.Error()})
+					// Ok.
+				} else if authErr != nil {
+					event.AuditErr([]string{clientIp, "oauth2", actor, action, "%s"}, strings.ToLower(clean.Error(authErr)))
 					AbortInvalidCredentials(c)
 					return
-				} else if authMethod.Is(authn.Method2FA) && errors.Is(authErr, authn.ErrPasscodeRequired) {
+				} else if !authUser.Equal(s.User()) {
-					// Ignore.
+					event.AuditErr([]string{clientIp, "oauth2", actor, action, authn.ErrUserDoesNotMatch.Error()})
-				} else if authErr != nil {
-					event.AuditErr([]string{clientIp, "oauth2", actor, action, "%s"}, clean.Error(authErr))
 					AbortInvalidCredentials(c)
 					return
 				}

internal/api/users_passcode.go

@@ -1,7 +1,9 @@
 package api
 
 import (
+	"errors"
 	"net/http"
+	"strings"
 
 	"github.com/dustin/go-humanize/english"
 	"github.com/gin-gonic/gin"
@@ -31,21 +33,22 @@ func CreateUserPasscode(router *gin.RouterGroup) {
 			return
 		}
 
+		// Get client IP address for logs and rate limiting checks.
+		clientIp := ClientIP(c)
+
 		// Check request rate limit.
-		r := limiter.Login.Request(ClientIP(c))
+		r := limiter.Login.Request(clientIp)
 
 		if r.Reject() {
 			limiter.AbortJSON(c)
 			return
 		}
 
-		// Check password if user authenticates with a local account.
+		// Check user password and abort if invalid.
-		switch user.Provider() {
+		if code, msg, err := checkUserPasscodePassword(c, user, frm.Password); err != nil {
-		case authn.ProviderDefault, authn.ProviderLocal:
+			event.AuditErr([]string{clientIp, "session %s", authn.Users, user.UserName, authn.ErrPasscodeGenerateFailed.Error(), strings.ToLower(clean.Error(err))}, s.RefID)
-			if user.InvalidPassword(frm.Password) {
+			Abort(c, code, msg)
-				Abort(c, http.StatusForbidden, i18n.ErrInvalidPassword)
+			return
-				return
-			}
 		}
 
 		// Return the reserved request rate limit tokens after successful authentication.
@@ -169,22 +172,22 @@ func DeactivateUserPasscode(router *gin.RouterGroup) {
 			return
 		}
 
+		// Get client IP address for logs and rate limiting checks.
+		clientIp := ClientIP(c)
+
 		// Check request rate limit.
-		r := limiter.Login.Request(ClientIP(c))
+		r := limiter.Login.Request(clientIp)
 
 		if r.Reject() {
 			limiter.AbortJSON(c)
 			return
 		}
 
-		// Check password if user authenticates with a local account.
+		// Check user password and abort if invalid.
-		switch user.Provider() {
+		if code, msg, err := checkUserPasscodePassword(c, user, frm.Password); err != nil {
-		case authn.ProviderDefault, authn.ProviderLocal:
+			event.AuditErr([]string{clientIp, "session %s", authn.Users, user.UserName, authn.ErrPasscodeDeactivationFailed.Error(), strings.ToLower(clean.Error(err))}, s.RefID)
-			// Check password and abort if invalid.
+			Abort(c, code, msg)
-			if user.InvalidPassword(frm.Password) {
+			return
-				Abort(c, http.StatusForbidden, i18n.ErrInvalidPassword)
-				return
-			}
 		}
 
 		// Return the reserved request rate limit tokens after successful authentication.
@@ -239,7 +242,7 @@ func checkUserPasscodeAuth(c *gin.Context, action acl.Permission) (*entity.Sessi
 	user := s.User()
 
 	// Regular users can only set up a passcode for their own account.
-	if user.UserUID != uid {
+	if user.UserUID != uid || !user.CanLogIn() {
 		AbortForbidden(c)
 		return s, nil, nil, authn.ErrUnauthorized
 	}
@@ -263,3 +266,56 @@ func checkUserPasscodeAuth(c *gin.Context, action acl.Permission) (*entity.Sessi
 
 	return s, user, frm, nil
 }
+
+// checkUserPasscodePassword checks if the specified password is valid.
+func checkUserPasscodePassword(c *gin.Context, user *entity.User, password string) (code int, msg i18n.Message, err error) {
+	// Set result defaults.
+	code = http.StatusForbidden
+	msg = i18n.ErrInvalidPassword
+
+	if user == nil {
+		return code, msg, authn.ErrUserRequired
+	} else if c == nil {
+		return code, msg, authn.ErrContextRequired
+	}
+
+	username := user.Username()
+
+	if username == "" {
+		return code, msg, authn.ErrUsernameRequired
+	}
+
+	switch user.Provider() {
+	// Check local account password.
+	case authn.ProviderLocal:
+		if user.InvalidPassword(password) {
+			return code, msg, authn.ErrInvalidPassword
+		}
+	// Use generic authentication check.
+	default:
+		// Create user login form.
+		f := form.Login{
+			Username: username,
+			Password: password,
+		}
+
+		// Check if user login credentials are valid.
+		if authUser, provider, method, authErr := entity.Auth(f, nil, c); method == authn.Method2FA && errors.Is(authErr, authn.ErrPasscodeRequired) {
+			return http.StatusOK, i18n.MsgVerified, nil
+		} else if authErr != nil {
+			// Abort if authentication has failed otherwise.
+			return code, msg, authErr
+		} else if authUser == nil {
+			// Abort if account was not found.
+			return code, msg, authn.ErrAccountNotFound
+		} else if !authUser.Equal(user) {
+			// Abort if user accounts do not match.
+			return code, msg, authn.ErrUserDoesNotMatch
+		} else if !provider.SupportsPasscodeAuthentication() || method != authn.MethodDefault {
+			// Abort if e.g. an app password was provided.
+			return code, msg, authn.ErrInvalidCredentials
+		}
+	}
+
+	return http.StatusOK, i18n.MsgVerified, nil
+}

internal/entity/auth_session_login.go

@@ -109,7 +109,7 @@ func AuthLocal(user *User, f form.Login, s *Session, c *gin.Context) (provider a
 	// Authentication with personal access token if a valid secret has been provided as password.
 	if authSess, authUser, authErr := AuthSession(f, c); authSess != nil && authUser != nil && authErr == nil {
 		if !authUser.IsRegistered() || authUser.UserUID != user.UserUID {
-			message := authn.ErrInvalidUsername.Error()
+			message := authn.ErrInvalidUser.Error()
 
 			if s != nil {
 				event.AuditErr([]string{clientIp, "session %s", "login as %s", "app password", message}, s.RefID, clean.LogQuote(username))

pkg/authn/errors.go

@@ -24,6 +24,7 @@ var (
 	ErrInsufficientScope      = errors.New("insufficient scope")
 	ErrNameRequired           = errors.New("name required")
 	ErrScopeRequired          = errors.New("scope required")
+	ErrContextRequired        = errors.New("context required")
 	ErrDisabledInPublicMode   = errors.New("disabled in public mode")
 	ErrAuthenticationDisabled = errors.New("authentication disabled")
 	ErrRateLimitExceeded      = errors.New("rate limit exceeded")
@@ -47,9 +48,11 @@ var (
 
 // User-related error messages:
 var (
+	ErrUserRequired               = errors.New("user required")
 	ErrUsernameRequired           = errors.New("username required")
 	ErrUsernameRequiredToRegister = errors.New("username required to register")
-	ErrInvalidUsername            = errors.New("invalid username")
+	ErrInvalidUser                = errors.New("invalid user")
+	ErrUserDoesNotMatch           = errors.New("user does not match")
 	ErrUsernameDoesNotMatch       = errors.New("specified username does not match")
 )
 

pkg/authn/providers.go

@@ -49,6 +49,7 @@ var PasswordProviders = list.List{
 var PasscodeProviders = list.List{
 	string(ProviderDefault),
 	string(ProviderLocal),
+	string(ProviderOIDC),
 	string(ProviderLDAP),
 }
 

pkg/authn/providers_test.go

@@ -65,7 +65,7 @@ func TestProviderType_IsLocal(t *testing.T) {
 
 func TestProviderType_SupportsPasscode(t *testing.T) {
 	assert.True(t, ProviderLocal.SupportsPasscodeAuthentication())
-	assert.False(t, ProviderOIDC.SupportsPasscodeAuthentication())
+	assert.True(t, ProviderOIDC.SupportsPasscodeAuthentication())
 	assert.True(t, ProviderLDAP.SupportsPasscodeAuthentication())
 	assert.False(t, ProviderClient.SupportsPasscodeAuthentication())
 	assert.False(t, ProviderApplication.SupportsPasscodeAuthentication())