OIDC: Add "address" to the default userinfo request scopes #782

see https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims

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

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.
+			// Check if the account is enabled and the OIDC Subject ID matches.
-			if authn.ProviderOIDC.NotEqual(user.AuthProvider) {
+			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.

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

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

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 {

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

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:
+	case authn.OidcClaimName:
-		return authn.ClaimName
+		return authn.OidcClaimName
-	case authn.ClaimNickname:
+	case authn.OidcClaimNickname:
-		return authn.ClaimNickname
+		return authn.OidcClaimNickname
-	case authn.ClaimEmail:
+	case authn.OidcClaimEmail:
-		return authn.ClaimEmail
+		return authn.OidcClaimEmail
 	default:
-		return authn.ClaimPreferredUsername
+		return authn.OidcClaimPreferredUsername
 	}
 }
 

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

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{

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

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"
+	OidcClaimPreferredUsername = "preferred_username"
-	ClaimEmail             = "email"
+	OidcClaimEmail             = "email"
-	ClaimName              = "name"
+	OidcClaimName              = "name"
-	ClaimNickname          = "nickname"
+	OidcClaimNickname          = "nickname"
-	OidcScopes             = "openid email profile"
+	OidcRequiredScopes         = "openid email profile"
+	OidcDefaultScopes          = "openid email profile address"
 )