diff --git a/internal/api/oidc_redirect.go b/internal/api/oidc_redirect.go index ef47a4005..c2f2830ca 100644 --- a/internal/api/oidc_redirect.go +++ b/internal/api/oidc_redirect.go @@ -136,8 +136,13 @@ func OIDCRedirect(router *gin.RouterGroup) { userName = user.Username() event.AuditInfo([]string{clientIp, "create session", "oidc", "found user", userName}) - // Check if OIDC subject identifier matches. - if authn.ProviderOIDC.NotEqual(user.AuthProvider) { + // Check if the account is enabled and the OIDC Subject ID matches. + if !user.CanLogIn() { + event.AuditErr([]string{clientIp, "create session", "oidc", userName, authn.ErrAccountDisabled.Error()}) + event.LoginError(clientIp, "oidc", userName, userAgent, authn.ErrAccountDisabled.Error()) + c.HTML(http.StatusUnauthorized, "auth.gohtml", CreateSessionError(http.StatusUnauthorized, i18n.Error(i18n.ErrInvalidCredentials))) + return + } else if authn.ProviderOIDC.NotEqual(user.AuthProvider) { event.AuditErr([]string{clientIp, "create session", "oidc", userName, authn.ErrAuthProviderIsNotOIDC.Error()}) event.LoginError(clientIp, "oidc", userName, userAgent, authn.ErrAuthProviderIsNotOIDC.Error()) c.HTML(http.StatusUnauthorized, "auth.gohtml", CreateSessionError(http.StatusUnauthorized, i18n.Error(i18n.ErrInvalidCredentials))) @@ -297,7 +302,7 @@ func OIDCRedirect(router *gin.RouterGroup) { return } - // Login allowed? + // Check if login is allowed. if !user.CanLogIn() { event.AuditErr([]string{clientIp, "create session", "oidc", userName, authn.ErrAccountDisabled.Error()}) event.LoginError(clientIp, "oidc", userName, userAgent, authn.ErrAccountDisabled.Error()) @@ -305,7 +310,7 @@ func OIDCRedirect(router *gin.RouterGroup) { return } - // Update subject identifier (auth id). + // Update Subject ID (auth_id). user.SetAuthID(userInfo.Subject) // Step 2: Create user session. diff --git a/internal/auth/oidc/client.go b/internal/auth/oidc/client.go index c2fa4276b..f376e58a1 100644 --- a/internal/auth/oidc/client.go +++ b/internal/auth/oidc/client.go @@ -16,6 +16,7 @@ import ( "github.com/zitadel/oidc/v3/pkg/oidc" "github.com/photoprism/photoprism/internal/event" + "github.com/photoprism/photoprism/pkg/authn" "github.com/photoprism/photoprism/pkg/clean" "github.com/photoprism/photoprism/pkg/rnd" ) @@ -96,7 +97,7 @@ func NewClient(issuerUri *url.URL, oidcClient, oidcSecret, oidcScopes, siteUrl s // Set default scopes if no scopes were specified. if oidcScopes == "" { - oidcScopes = "openid email profile" + oidcScopes = authn.OidcRequiredScopes } event.AuditDebug([]string{"oidc", "provider", "scopes", oidcScopes}) diff --git a/internal/auth/oidc/client_test.go b/internal/auth/oidc/client_test.go index 4fcfd7e9b..1dbb032b1 100644 --- a/internal/auth/oidc/client_test.go +++ b/internal/auth/oidc/client_test.go @@ -4,6 +4,8 @@ import ( "net/url" "testing" + "github.com/photoprism/photoprism/pkg/authn" + "github.com/stretchr/testify/assert" ) @@ -17,7 +19,7 @@ func TestNewClient(t *testing.T) { uri, "csg6yqvykh0780f9", "nd09wkee0ElsMvzLGkgWS9wJAttHwF2h", - "openid email profile", + authn.OidcDefaultScopes, "https://app.localssl.dev/", false, ) @@ -34,7 +36,7 @@ func TestNewClient(t *testing.T) { uri, "csg6yqvykh0780f9", "nd09wkee0ElsMvzLGkgWS9wJAttHwF2h", - "openid email profile", + authn.OidcDefaultScopes, "https://app.localssl.dev/", true, ) diff --git a/internal/auth/oidc/username.go b/internal/auth/oidc/username.go index 86e032360..132bd0c67 100644 --- a/internal/auth/oidc/username.go +++ b/internal/auth/oidc/username.go @@ -10,7 +10,7 @@ import ( // Username returns the preferred username based on the userinfo and the preferred username OIDC claim. func Username(userInfo *oidc.UserInfo, preferredClaim string) (userName string) { switch preferredClaim { - case authn.ClaimName: + case authn.OidcClaimName: if name := clean.Handle(userInfo.Name); len(name) > 0 { userName = name } else if name = clean.Handle(userInfo.PreferredUsername); len(name) > 0 { @@ -20,7 +20,7 @@ func Username(userInfo *oidc.UserInfo, preferredClaim string) (userName string) } else if name = clean.Email(userInfo.Email); userInfo.EmailVerified && len(name) > 4 { userName = name } - case authn.ClaimNickname: + case authn.OidcClaimNickname: if name := clean.Handle(userInfo.Nickname); len(name) > 0 { userName = name } else if name = clean.Handle(userInfo.PreferredUsername); len(name) > 0 { @@ -30,7 +30,7 @@ func Username(userInfo *oidc.UserInfo, preferredClaim string) (userName string) } else if name = clean.Email(userInfo.Email); userInfo.EmailVerified && len(name) > 4 { userName = name } - case authn.ClaimEmail: + case authn.OidcClaimEmail: if name := clean.Email(userInfo.Email); userInfo.EmailVerified && len(name) > 4 { userName = name } else if name = clean.Handle(userInfo.PreferredUsername); len(name) > 0 { diff --git a/internal/auth/oidc/username_test.go b/internal/auth/oidc/username_test.go index e3f86fe32..330ae14a0 100644 --- a/internal/auth/oidc/username_test.go +++ b/internal/auth/oidc/username_test.go @@ -10,7 +10,7 @@ import ( ) func TestUsername(t *testing.T) { - t.Run("ClaimPreferredUsername", func(t *testing.T) { + t.Run("PreferredUsername", func(t *testing.T) { info := &oidc.UserInfo{} info.Name = "Jane Doe" info.GivenName = "Jane" @@ -19,19 +19,19 @@ func TestUsername(t *testing.T) { info.EmailVerified = true info.Subject = "e3a9f4a6-9d60-47cb-9bf5-02bd15b0c68d" info.PreferredUsername = "Jane Doe" - result := Username(info, authn.ClaimPreferredUsername) + result := Username(info, authn.OidcClaimPreferredUsername) assert.Equal(t, "jane.doe", result) }) - t.Run("ClaimPreferredUsernameMissing", func(t *testing.T) { + t.Run("PreferredUsernameMissing", func(t *testing.T) { info := &oidc.UserInfo{} info.Name = "Jane Doe" info.FamilyName = "Doe" info.Email = "jane@doe.com" info.EmailVerified = true - result := Username(info, authn.ClaimPreferredUsername) + result := Username(info, authn.OidcClaimPreferredUsername) assert.Equal(t, "jane.doe", result) }) - t.Run("ClaimName", func(t *testing.T) { + t.Run("Name", func(t *testing.T) { info := &oidc.UserInfo{} info.Name = "Jane Doe" info.GivenName = "Jane" @@ -40,10 +40,10 @@ func TestUsername(t *testing.T) { info.Email = "jane@doe.com" info.EmailVerified = true info.Subject = "abcd123" - result := Username(info, authn.ClaimName) + result := Username(info, authn.OidcClaimName) assert.Equal(t, "jane.doe", result) }) - t.Run("ClaimNickname", func(t *testing.T) { + t.Run("Nickname", func(t *testing.T) { info := &oidc.UserInfo{} info.Name = "Jane Doe" info.GivenName = "Jane" @@ -52,10 +52,10 @@ func TestUsername(t *testing.T) { info.Email = "jane@doe.com" info.EmailVerified = true info.Subject = "abcd123" - result := Username(info, authn.ClaimNickname) + result := Username(info, authn.OidcClaimNickname) assert.Equal(t, "jens.mander", result) }) - t.Run("ClaimEmail", func(t *testing.T) { + t.Run("Email", func(t *testing.T) { info := &oidc.UserInfo{} info.Name = "Jane Doe" info.GivenName = "Jane" @@ -63,7 +63,7 @@ func TestUsername(t *testing.T) { info.Email = "jane@doe.com" info.EmailVerified = true info.Subject = "abcd123" - result := Username(info, authn.ClaimEmail) + result := Username(info, authn.OidcClaimEmail) assert.Equal(t, "jane@doe.com", result) }) } diff --git a/internal/config/config_oidc.go b/internal/config/config_oidc.go index b6472b15d..66e739aa6 100644 --- a/internal/config/config_oidc.go +++ b/internal/config/config_oidc.go @@ -61,7 +61,7 @@ func (c *Config) OIDCSecret() string { // OIDCScopes returns the user information scopes for single sign-on via OIDC. func (c *Config) OIDCScopes() string { if c.options.OIDCScopes == "" { - return authn.OidcScopes + return authn.OidcDefaultScopes } return c.options.OIDCScopes @@ -98,14 +98,14 @@ func (c *Config) OIDCRegister() bool { // OIDCUsername returns the preferred username claim for new users signing up via OIDC. func (c *Config) OIDCUsername() string { switch c.options.OIDCUsername { - case authn.ClaimName: - return authn.ClaimName - case authn.ClaimNickname: - return authn.ClaimNickname - case authn.ClaimEmail: - return authn.ClaimEmail + case authn.OidcClaimName: + return authn.OidcClaimName + case authn.OidcClaimNickname: + return authn.OidcClaimNickname + case authn.OidcClaimEmail: + return authn.OidcClaimEmail default: - return authn.ClaimPreferredUsername + return authn.OidcClaimPreferredUsername } } diff --git a/internal/config/config_oidc_test.go b/internal/config/config_oidc_test.go index a2ad2d6c6..fafeadc1e 100644 --- a/internal/config/config_oidc_test.go +++ b/internal/config/config_oidc_test.go @@ -77,11 +77,11 @@ func TestConfig_OIDCSecret(t *testing.T) { func TestConfig_OIDCScopes(t *testing.T) { c := NewConfig(CliTestContext()) - assert.Equal(t, authn.OidcScopes, c.OIDCScopes()) + assert.Equal(t, authn.OidcDefaultScopes, c.OIDCScopes()) c.options.OIDCScopes = "" - assert.Equal(t, authn.OidcScopes, c.OIDCScopes()) + assert.Equal(t, authn.OidcDefaultScopes, c.OIDCScopes()) } func TestConfig_OIDCProvider(t *testing.T) { @@ -113,23 +113,23 @@ func TestConfig_OIDCRedirect(t *testing.T) { func TestConfig_OIDCUsername(t *testing.T) { c := NewConfig(CliTestContext()) - assert.Equal(t, authn.ClaimPreferredUsername, c.OIDCUsername()) + assert.Equal(t, authn.OidcClaimPreferredUsername, c.OIDCUsername()) c.options.OIDCUsername = "name" - assert.Equal(t, authn.ClaimName, c.OIDCUsername()) + assert.Equal(t, authn.OidcClaimName, c.OIDCUsername()) c.options.OIDCUsername = "nickname" - assert.Equal(t, authn.ClaimNickname, c.OIDCUsername()) + assert.Equal(t, authn.OidcClaimNickname, c.OIDCUsername()) c.options.OIDCUsername = "email" - assert.Equal(t, authn.ClaimEmail, c.OIDCUsername()) + assert.Equal(t, authn.OidcClaimEmail, c.OIDCUsername()) c.options.OIDCUsername = "" - assert.Equal(t, authn.ClaimPreferredUsername, c.OIDCUsername()) + assert.Equal(t, authn.OidcClaimPreferredUsername, c.OIDCUsername()) } func TestConfig_OIDCDomain(t *testing.T) { diff --git a/internal/config/flags.go b/internal/config/flags.go index b2885fd9b..7a97f0385 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -66,7 +66,7 @@ var Flags = CliFlags{ Name: "oidc-scopes", Hidden: true, Usage: "user information `SCOPES` for single sign-on via OpenID Connect", - Value: authn.OidcScopes, + Value: authn.OidcDefaultScopes, EnvVar: EnvVar("OIDC_SCOPES"), }}, { Flag: cli.StringFlag{ @@ -94,7 +94,7 @@ var Flags = CliFlags{ Flag: cli.StringFlag{ Name: "oidc-username", Usage: "preferred username `CLAIM` for new OpenID Connect users (preferred_username, name, nickname, email)", - Value: authn.ClaimPreferredUsername, + Value: authn.OidcClaimPreferredUsername, EnvVar: EnvVar("OIDC_USERNAME"), }}, { Flag: cli.BoolFlag{ diff --git a/internal/config/test.go b/internal/config/test.go index 1150ff29c..17ce566b9 100644 --- a/internal/config/test.go +++ b/internal/config/test.go @@ -267,7 +267,7 @@ func CliTestContext() *cli.Context { LogErr(c.Set("oidc-uri", config.OIDCUri)) LogErr(c.Set("oidc-client", config.OIDCClient)) LogErr(c.Set("oidc-secret", config.OIDCSecret)) - LogErr(c.Set("oidc-scopes", authn.OidcScopes)) + LogErr(c.Set("oidc-scopes", authn.OidcDefaultScopes)) LogErr(c.Set("storage-path", config.StoragePath)) LogErr(c.Set("sidecar-path", config.SidecarPath)) LogErr(c.Set("sidecar-yaml", fmt.Sprintf("%t", config.SidecarYaml))) diff --git a/pkg/authn/oidc.go b/pkg/authn/oidc.go index 480e19cf7..bd786e1d7 100644 --- a/pkg/authn/oidc.go +++ b/pkg/authn/oidc.go @@ -1,9 +1,12 @@ package authn +// OpenID Connect (OIDC) scope and claim identifiers: +// https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims const ( - ClaimPreferredUsername = "preferred_username" - ClaimEmail = "email" - ClaimName = "name" - ClaimNickname = "nickname" - OidcScopes = "openid email profile" + OidcClaimPreferredUsername = "preferred_username" + OidcClaimEmail = "email" + OidcClaimName = "name" + OidcClaimNickname = "nickname" + OidcRequiredScopes = "openid email profile" + OidcDefaultScopes = "openid email profile address" )