Autoprovsioning fixes

changelog/unreleased/config-autoprovision-claims.md

@@ -0,0 +1,15 @@
+Enhancement: Configurable claims for auto-provisioning user accounts
+
+We introduce the new environment variables
+"PROXY_AUTOPROVISION_CLAIM_USERNAME", "PROXY_AUTOPROVISION_CLAIM_EMAIL", and
+"PROXY_AUTOPROVISION_CLAIM_DISPLAYNAME" which can be used to configure the
+OIDC claims that should be used for auto-provisioning user accounts.
+
+The automatic fallback to use the 'email' claim value as the username when
+the 'preferred_username' claim is not set, has been removed.
+
+Also it is now possible to autoprovision users without an email address.
+
+https://github.com/owncloud/ocis/pull/8952
+https://github.com/owncloud/ocis/issues/8635
+https://github.com/owncloud/ocis/issues/6909

services/proxy/pkg/command/server.go

@@ -8,8 +8,8 @@ import (
 	"os"
 	"time"
 
-	"github.com/owncloud/ocis/v2/services/proxy/pkg/staticroutes"
-
+	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
+	"github.com/cs3org/reva/v2/pkg/store"
 	chimiddleware "github.com/go-chi/chi/v5/middleware"
 	"github.com/justinas/alice"
 	"github.com/oklog/run"
@@ -18,8 +18,6 @@ import (
 	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 	"go.opentelemetry.io/otel/trace"
 
-	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
-	"github.com/cs3org/reva/v2/pkg/store"
 	"github.com/owncloud/ocis/v2/ocis-pkg/config/configlog"
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
 	pkgmiddleware "github.com/owncloud/ocis/v2/ocis-pkg/middleware"
@@ -39,6 +37,7 @@ import (
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/router"
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/server/debug"
 	proxyHTTP "github.com/owncloud/ocis/v2/services/proxy/pkg/server/http"
+	"github.com/owncloud/ocis/v2/services/proxy/pkg/staticroutes"
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/user/backend"
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/userroles"
 	ocisstore "github.com/owncloud/ocis/v2/services/store/pkg/store"
@@ -227,6 +226,7 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config,
 			backend.WithMachineAuthAPIKey(cfg.MachineAuthAPIKey),
 			backend.WithOIDCissuer(cfg.OIDC.Issuer),
 			backend.WithServiceAccount(cfg.ServiceAccount),
+			backend.WithAutoProvisionClaims(cfg.AutoProvisionClaims),
 		)
 	default:
 		logger.Fatal().Msgf("Invalid accounts backend type '%s'", cfg.AccountBackend)

services/proxy/pkg/config/config.go

@@ -24,25 +24,26 @@ type Config struct {
 	GRPCClientTLS *shared.GRPCClientTLS `yaml:"grpc_client_tls"`
 	GrpcClient    client.Client         `yaml:"-"`
 
-	RoleQuotas            map[string]uint64  `yaml:"role_quotas"`
-	Policies              []Policy           `yaml:"policies"`
-	AdditionalPolicies    []Policy           `yaml:"additional_policies"`
-	OIDC                  OIDC               `yaml:"oidc"`
-	ServiceAccount        ServiceAccount     `yaml:"service_account"`
-	RoleAssignment        RoleAssignment     `yaml:"role_assignment"`
-	PolicySelector        *PolicySelector    `yaml:"policy_selector"`
-	PreSignedURL          PreSignedURL       `yaml:"pre_signed_url"`
-	AccountBackend        string             `yaml:"account_backend" env:"PROXY_ACCOUNT_BACKEND_TYPE" desc:"Account backend the PROXY service should use. Currently only 'cs3' is possible here." introductionVersion:"pre5.0"`
-	UserOIDCClaim         string             `yaml:"user_oidc_claim" env:"PROXY_USER_OIDC_CLAIM" desc:"The name of an OpenID Connect claim that is used for resolving users with the account backend. The value of the claim must hold a per user unique, stable and non re-assignable identifier. The availability of claims depends on your Identity Provider. There are common claims available for most Identity providers like 'email' or 'preferred_username' but you can also add your own claim." introductionVersion:"pre5.0"`
-	UserCS3Claim          string             `yaml:"user_cs3_claim" env:"PROXY_USER_CS3_CLAIM" desc:"The name of a CS3 user attribute (claim) that should be mapped to the 'user_oidc_claim'. Supported values are 'username', 'mail' and 'userid'." introductionVersion:"pre5.0"`
-	MachineAuthAPIKey     string             `mask:"password" yaml:"machine_auth_api_key" env:"OCIS_MACHINE_AUTH_API_KEY;PROXY_MACHINE_AUTH_API_KEY" desc:"Machine auth API key used to validate internal requests necessary to access resources from other services." introductionVersion:"pre5.0"`
-	AutoprovisionAccounts bool               `yaml:"auto_provision_accounts" env:"PROXY_AUTOPROVISION_ACCOUNTS" desc:"Set this to 'true' to automatically provision users that do not yet exist in the users service on-demand upon first sign-in. To use this a write-enabled libregraph user backend needs to be setup an running." introductionVersion:"pre5.0"`
-	EnableBasicAuth       bool               `yaml:"enable_basic_auth" env:"PROXY_ENABLE_BASIC_AUTH" desc:"Set this to true to enable 'basic authentication' (username/password)." introductionVersion:"pre5.0"`
-	InsecureBackends      bool               `yaml:"insecure_backends" env:"PROXY_INSECURE_BACKENDS" desc:"Disable TLS certificate validation for all HTTP backend connections." introductionVersion:"pre5.0"`
-	BackendHTTPSCACert    string             `yaml:"backend_https_cacert" env:"PROXY_HTTPS_CACERT" desc:"Path/File for the root CA certificate used to validate the server’s TLS certificate for https enabled backend services." introductionVersion:"pre5.0"`
-	AuthMiddleware        AuthMiddleware     `yaml:"auth_middleware"`
-	PoliciesMiddleware    PoliciesMiddleware `yaml:"policies_middleware"`
-	CSPConfigFileLocation string             `yaml:"csp_config_file_location" env:"PROXY_CSP_CONFIG_FILE_LOCATION" desc:"The location of the CSP configuration file." introductionVersion:"6.0"`
+	RoleQuotas            map[string]uint64   `yaml:"role_quotas"`
+	Policies              []Policy            `yaml:"policies"`
+	AdditionalPolicies    []Policy            `yaml:"additional_policies"`
+	OIDC                  OIDC                `yaml:"oidc"`
+	ServiceAccount        ServiceAccount      `yaml:"service_account"`
+	RoleAssignment        RoleAssignment      `yaml:"role_assignment"`
+	PolicySelector        *PolicySelector     `yaml:"policy_selector"`
+	PreSignedURL          PreSignedURL        `yaml:"pre_signed_url"`
+	AccountBackend        string              `yaml:"account_backend" env:"PROXY_ACCOUNT_BACKEND_TYPE" desc:"Account backend the PROXY service should use. Currently only 'cs3' is possible here." introductionVersion:"pre5.0"`
+	UserOIDCClaim         string              `yaml:"user_oidc_claim" env:"PROXY_USER_OIDC_CLAIM" desc:"The name of an OpenID Connect claim that is used for resolving users with the account backend. The value of the claim must hold a per user unique, stable and non re-assignable identifier. The availability of claims depends on your Identity Provider. There are common claims available for most Identity providers like 'email' or 'preferred_username' but you can also add your own claim." introductionVersion:"pre5.0"`
+	UserCS3Claim          string              `yaml:"user_cs3_claim" env:"PROXY_USER_CS3_CLAIM" desc:"The name of a CS3 user attribute (claim) that should be mapped to the 'user_oidc_claim'. Supported values are 'username', 'mail' and 'userid'." introductionVersion:"pre5.0"`
+	MachineAuthAPIKey     string              `mask:"password" yaml:"machine_auth_api_key" env:"OCIS_MACHINE_AUTH_API_KEY;PROXY_MACHINE_AUTH_API_KEY" desc:"Machine auth API key used to validate internal requests necessary to access resources from other services." introductionVersion:"pre5.0"`
+	AutoprovisionAccounts bool                `yaml:"auto_provision_accounts" env:"PROXY_AUTOPROVISION_ACCOUNTS" desc:"Set this to 'true' to automatically provision users that do not yet exist in the users service on-demand upon first sign-in. To use this a write-enabled libregraph user backend needs to be setup an running." introductionVersion:"pre5.0"`
+	AutoProvisionClaims   AutoProvisionClaims `yaml:"auto_provision_claims"`
+	EnableBasicAuth       bool                `yaml:"enable_basic_auth" env:"PROXY_ENABLE_BASIC_AUTH" desc:"Set this to true to enable 'basic authentication' (username/password)." introductionVersion:"pre5.0"`
+	InsecureBackends      bool                `yaml:"insecure_backends" env:"PROXY_INSECURE_BACKENDS" desc:"Disable TLS certificate validation for all HTTP backend connections." introductionVersion:"pre5.0"`
+	BackendHTTPSCACert    string              `yaml:"backend_https_cacert" env:"PROXY_HTTPS_CACERT" desc:"Path/File for the root CA certificate used to validate the server’s TLS certificate for https enabled backend services." introductionVersion:"pre5.0"`
+	AuthMiddleware        AuthMiddleware      `yaml:"auth_middleware"`
+	PoliciesMiddleware    PoliciesMiddleware  `yaml:"policies_middleware"`
+	CSPConfigFileLocation string              `yaml:"csp_config_file_location" env:"PROXY_CSP_CONFIG_FILE_LOCATION" desc:"The location of the CSP configuration file." introductionVersion:"6.0"`
 
 	Context context.Context `yaml:"-" json:"-"`
 }
@@ -153,6 +154,13 @@ type RoleMapping struct {
 	ClaimValue string `yaml:"claim_value" desc:"The value of the 'PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM' that matches the role defined in 'role_name'."`
 }
 
+// AutoProvisionClaims defines which claims from the OIDC userinfo response should be used for auto-provisioning user accounts
+type AutoProvisionClaims struct {
+	Username    string `yaml:"username" env:"PROXY_AUTOPROVISION_CLAIM_USERNAME" desc:"The name of the OIDC claim that holds the username." introductionVersion:"5.1"`
+	Email       string `yaml:"email" env:"PROXY_AUTOPROVISION_CLAIM_EMAIL" desc:"The name of the OIDC claim that holds the email." introductionVersion:"5.1"`
+	DisplayName string `yaml:"display_name" env:"PROXY_AUTOPROVISION_CLAIM_DISPLAYNAME" desc:"The name of the OIDC claim that holds the display name." introductionVersion:"5.1"`
+}
+
 // PolicySelector is the toplevel-configuration for different selectors
 type PolicySelector struct {
 	Static *StaticSelectorConf `yaml:"static"`

services/proxy/pkg/config/defaults/defaultconfig.go

@@ -84,6 +84,11 @@ func DefaultConfig() *config.Config {
 		UserOIDCClaim:         "preferred_username",
 		UserCS3Claim:          "username",
 		AutoprovisionAccounts: false,
+		AutoProvisionClaims: config.AutoProvisionClaims{
+			Username:    "preferred_username",
+			Email:       "email",
+			DisplayName: "name",
+		},
 		EnableBasicAuth:       false,
 		InsecureBackends:      false,
 		CSPConfigFileLocation: "",

services/proxy/pkg/user/backend/cs3.go

@@ -16,7 +16,6 @@ import (
 	"go-micro.dev/v4/selector"
 
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
-	"github.com/owncloud/ocis/v2/ocis-pkg/oidc"
 	"github.com/owncloud/ocis/v2/ocis-pkg/registry"
 	"github.com/owncloud/ocis/v2/services/graph/pkg/errorcode"
 	"github.com/owncloud/ocis/v2/services/proxy/pkg/config"
@@ -32,11 +31,12 @@ type Option func(o *Options)
 
 // Options defines the available options for this package.
 type Options struct {
-	logger            log.Logger
-	gatewaySelector   pool.Selectable[gateway.GatewayAPIClient]
-	machineAuthAPIKey string
-	oidcISS           string
-	serviceAccount    config.ServiceAccount
+	logger              log.Logger
+	gatewaySelector     pool.Selectable[gateway.GatewayAPIClient]
+	machineAuthAPIKey   string
+	oidcISS             string
+	serviceAccount      config.ServiceAccount
+	autoProvisionClaims config.AutoProvisionClaims
 }
 
 // WithLogger sets the logger option
@@ -74,6 +74,12 @@ func WithServiceAccount(c config.ServiceAccount) Option {
 	}
 }
 
+func WithAutoProvisionClaims(claims config.AutoProvisionClaims) Option {
+	return func(o *Options) {
+		o.autoProvisionClaims = claims
+	}
+}
+
 // NewCS3UserBackend creates a user-provider which fetches users from a CS3 UserBackend
 func NewCS3UserBackend(opts ...Option) UserBackend {
 	opt := Options{}
@@ -111,12 +117,12 @@ func (c *cs3backend) GetUserByClaims(ctx context.Context, claim, value string) (
 		if res.Status.Code == rpcv1beta1.Code_CODE_NOT_FOUND {
 			return nil, "", ErrAccountNotFound
 		}
-		return nil, "", fmt.Errorf("could not get user by claim %v with value %v : %s ", claim, value, res.Status.Message)
+		return nil, "", fmt.Errorf("could not get user by claim %v with value %v : %s ", claim, value, res.GetStatus().GetMessage())
 	}
 
 	user := res.User
 
-	return user, res.Token, nil
+	return user, res.GetToken(), nil
 }
 
 func (c *cs3backend) Authenticate(ctx context.Context, username string, password string) (*cs3.User, string, error) {
@@ -135,7 +141,7 @@ func (c *cs3backend) Authenticate(ctx context.Context, username string, password
 	case err != nil:
 		return nil, "", fmt.Errorf("could not authenticate with username and password user: %s, %w", username, err)
 	case res.Status.Code != rpcv1beta1.Code_CODE_OK:
-		return nil, "", fmt.Errorf("could not authenticate with username and password user: %s, got code: %d", username, res.Status.Code)
+		return nil, "", fmt.Errorf("could not authenticate with username and password user: %s, got code: %d", username, res.GetStatus().GetCode())
 	}
 
 	return res.User, res.Token, nil
@@ -161,10 +167,10 @@ func (c *cs3backend) CreateUserFromClaims(ctx context.Context, claims map[string
 		return nil, err
 	}
 	if authRes.GetStatus().GetCode() != rpcv1beta1.Code_CODE_OK {
-		return nil, fmt.Errorf("error authenticating service user: %s", authRes.Status.Message)
+		return nil, fmt.Errorf("error authenticating service user: %s", authRes.GetStatus().GetMessage())
 	}
 
-	lgClient, err := c.setupLibregraphClient(newctx, authRes.Token)
+	lgClient, err := c.setupLibregraphClient(newctx, authRes.GetToken())
 	if err != nil {
 		c.logger.Error().Err(err).Msg("Error setting up libregraph client.")
 		return nil, err
@@ -262,22 +268,21 @@ func (c cs3backend) isAlreadyExists(resp *http.Response) (bool, error) {
 }
 
 func (c cs3backend) libregraphUserFromClaims(ctx context.Context, claims map[string]interface{}) (libregraph.User, error) {
-	var ok bool
-	var dn, mail, username string
 	user := libregraph.User{}
-	if dn, ok = claims[oidc.Name].(string); !ok {
-		return user, fmt.Errorf("Missing claim '%s'", oidc.Name)
+	if dn, ok := claims[c.autoProvisionClaims.DisplayName].(string); ok {
+		user.SetDisplayName(dn)
+	} else {
+		return user, fmt.Errorf("Missing claim '%s' (displayName)", c.autoProvisionClaims.DisplayName)
 	}
-	if mail, ok = claims[oidc.Email].(string); !ok {
-		return user, fmt.Errorf("Missing claim '%s'", oidc.Email)
+	if username, ok := claims[c.autoProvisionClaims.Username].(string); ok {
+		user.SetOnPremisesSamAccountName(username)
+	} else {
+		return user, fmt.Errorf("Missing claim '%s' (username)", c.autoProvisionClaims.Username)
 	}
-	if username, ok = claims[oidc.PreferredUsername].(string); !ok {
-		c.logger.Warn().Str("claim", oidc.PreferredUsername).Msg("Missing claim for username, falling back to email address")
-		username = mail
+	// Email is optional so we don't need an 'else' here
+	if mail, ok := claims[c.autoProvisionClaims.Email].(string); ok {
+		user.SetMail(mail)
 	}
-	user.DisplayName = &dn
-	user.OnPremisesSamAccountName = &username
-	user.Mail = &mail
 	return user, nil
 }