OIDC: resolve users with no uid/gid by username (#3481)

changelog/unreleased/oidc-resolve-users.md

@@ -0,0 +1,8 @@
+Bugfix: OIDC: resolve users with no uid/gid by username
+
+Previously we resolved such users (so called "lightweight" or "external" accounts in the CERN realm)
+by email, but it turns out that the same email may have multiple accounts associated to it.
+
+Therefore we now resolve them by username, that is the upn, which is unique.
+
+https://github.com/cs3org/reva/pull/3481

docs/content/en/docs/config/grpc/services/appprovider/_index.md

@@ -8,27 +8,19 @@ description: >
 
 # _struct: config_
 
-{{% dir name="iopsecret" type="string" default="" %}}
-The iopsecret used to connect to the wopiserver. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L60)
+{{% dir name="mime_types" type="[]string" default=nil %}}
+A list of mime types supported by this app. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L63)
 {{< highlight toml >}}
 [grpc.services.appprovider]
-iopsecret = ""
+mime_types = nil
 {{< /highlight >}}
 {{% /dir %}}
 
-{{% dir name="wopiurl" type="string" default="" %}}
-The wopiserver's URL. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L61)
+{{% dir name="custom_mime_types_json" type="string" default="nil" %}}
+An optional mapping file with the list of supported custom file extensions and corresponding mime types. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L64)
 {{< highlight toml >}}
 [grpc.services.appprovider]
-wopiurl = ""
-{{< /highlight >}}
-{{% /dir %}}
-
-{{% dir name="wopibridgeurl" type="string" default="" %}}
-The wopibridge's URL. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/grpc/services/appprovider/appprovider.go#L62)
-{{< highlight toml >}}
-[grpc.services.appprovider]
-wopibridgeurl = ""
+custom_mime_types_json = "nil"
 {{< /highlight >}}
 {{% /dir %}}
 

docs/content/en/docs/config/http/services/mailer/_index.md

@@ -0,0 +1,42 @@
+---
+title: "mailer"
+linkTitle: "mailer"
+weight: 10
+description: >
+  Configuration for the mailer service
+---
+
+# _struct: config_
+
+{{% dir name="smtp_server" type="string" default="" %}}
+The hostname and port of the SMTP server. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/mailer/mailer.go#L54)
+{{< highlight toml >}}
+[http.services.mailer]
+smtp_server = ""
+{{< /highlight >}}
+{{% /dir %}}
+
+{{% dir name="sender_login" type="string" default="" %}}
+The email to be used to send mails. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/mailer/mailer.go#L55)
+{{< highlight toml >}}
+[http.services.mailer]
+sender_login = ""
+{{< /highlight >}}
+{{% /dir %}}
+
+{{% dir name="sender_password" type="string" default="" %}}
+The sender's password. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/mailer/mailer.go#L56)
+{{< highlight toml >}}
+[http.services.mailer]
+sender_password = ""
+{{< /highlight >}}
+{{% /dir %}}
+
+{{% dir name="disable_auth" type="bool" default=false %}}
+Whether to disable SMTP auth. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/mailer/mailer.go#L57)
+{{< highlight toml >}}
+[http.services.mailer]
+disable_auth = false
+{{< /highlight >}}
+{{% /dir %}}
+

docs/content/en/docs/config/http/services/owncloud/ocdav/_index.md

@@ -9,7 +9,7 @@ description: >
 # _struct: Config_
 
 {{% dir name="insecure" type="bool" default=false %}}
-Whether to skip certificate checks when sending requests. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/owncloud/ocdav/ocdav.go#L102)
+Whether to skip certificate checks when sending requests. [[Ref]](https://github.com/cs3org/reva/tree/master/internal/http/services/owncloud/ocdav/ocdav.go#L101)
 {{< highlight toml >}}
 [http.services.owncloud.ocdav]
 insecure = false

docs/content/en/docs/config/packages/app/provider/wopi/_index.md

@@ -8,78 +8,78 @@ description: >
 
 # _struct: config_
 
+{{% dir name="mime_types" type="[]string" default= %}}
+Inherited from the appprovider. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L59)
+{{< highlight toml >}}
+[app.provider.wopi]
+mime_types = 
+{{< /highlight >}}
+{{% /dir %}}
+
 {{% dir name="iop_secret" type="string" default="" %}}
-The IOP secret used to connect to the wopiserver. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L59)
+The IOP secret used to connect to the wopiserver. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L60)
 {{< highlight toml >}}
 [app.provider.wopi]
 iop_secret = ""
 {{< /highlight >}}
 {{% /dir %}}
 
 {{% dir name="wopi_url" type="string" default="" %}}
-The wopiserver's URL. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L60)
+The wopiserver's URL. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L61)
 {{< highlight toml >}}
 [app.provider.wopi]
 wopi_url = ""
 {{< /highlight >}}
 {{% /dir %}}
 
 {{% dir name="app_name" type="string" default="" %}}
-The App user-friendly name. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L61)
+The App user-friendly name. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L62)
 {{< highlight toml >}}
 [app.provider.wopi]
 app_name = ""
 {{< /highlight >}}
 {{% /dir %}}
 
 {{% dir name="app_icon_uri" type="string" default="" %}}
-A URI to a static asset which represents the app icon. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L62)
+A URI to a static asset which represents the app icon. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L63)
 {{< highlight toml >}}
 [app.provider.wopi]
 app_icon_uri = ""
 {{< /highlight >}}
 {{% /dir %}}
 
 {{% dir name="app_url" type="string" default="" %}}
-The App URL. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L63)
+The App URL. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L64)
 {{< highlight toml >}}
 [app.provider.wopi]
 app_url = ""
 {{< /highlight >}}
 {{% /dir %}}
 
 {{% dir name="app_int_url" type="string" default="" %}}
-The internal app URL in case of dockerized deployments. Defaults to AppURL [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L64)
+The internal app URL in case of dockerized deployments. Defaults to AppURL [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L65)
 {{< highlight toml >}}
 [app.provider.wopi]
 app_int_url = ""
 {{< /highlight >}}
 {{% /dir %}}
 
 {{% dir name="app_api_key" type="string" default="" %}}
-The API key used by the app, if applicable. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L65)
+The API key used by the app, if applicable. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L66)
 {{< highlight toml >}}
 [app.provider.wopi]
 app_api_key = ""
 {{< /highlight >}}
 {{% /dir %}}
 
 {{% dir name="jwt_secret" type="string" default="" %}}
-The JWT secret to be used to retrieve the token TTL. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L66)
+The JWT secret to be used to retrieve the token TTL. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L67)
 {{< highlight toml >}}
 [app.provider.wopi]
 jwt_secret = ""
 {{< /highlight >}}
 {{% /dir %}}
 
-{{% dir name="custom_mime_types_json" type="string" default="nil" %}}
-An optional mapping file with the list of supported custom file extensions and corresponding mime types. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L67)
-{{< highlight toml >}}
-[app.provider.wopi]
-custom_mime_types_json = "nil"
-{{< /highlight >}}
-{{% /dir %}}
-
 {{% dir name="app_desktop_only" type="bool" default=false %}}
 Specifies if the app can be opened only on desktop. [[Ref]](https://github.com/cs3org/reva/tree/master/pkg/app/provider/wopi/wopi.go#L68)
 {{< highlight toml >}}

internal/grpc/services/appprovider/appprovider_test.go

@@ -47,6 +47,7 @@ func Test_parseConfig(t *testing.T) {
 		},
 		{
 			name: "all configurations set for wopi driver",
+			// Note that the wopi driver is not loaded by this unit test, therefore those properties are just a non-validated example
 			m: map[string]interface{}{
 				"Driver":  "wopi",
 				"Drivers": map[string]map[string]interface{}{"wopi": {"iop_secret": "very-secret", "wopi_url": "https://my.wopi:9871"}},

internal/grpc/services/authprovider/authprovider.go

@@ -150,7 +150,7 @@ func (s *service) Authenticate(ctx context.Context, req *provider.AuthenticateRe
 	u, scope, err := s.authmgr.Authenticate(ctx, username, password)
 	switch v := err.(type) {
 	case nil:
-		log.Info().Msgf("user %s authenticated", u.Id)
+		log.Info().Interface("userId", u.Id).Msg("user authenticated")
 		return &provider.AuthenticateResponse{
 			Status:     status.NewOK(ctx),
 			User:       u,

pkg/auth/manager/oidc/oidc.go

@@ -199,7 +199,7 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
 		return nil, nil, fmt.Errorf("no \"email\" attribute found in userinfo: maybe the client did not request the oidc \"email\"-scope")
 	}
 
-	err = am.resolveUser(ctx, claims)
+	err = am.resolveUser(ctx, claims, clientID)
 	if err != nil {
 		return nil, nil, errors.Wrapf(err, "oidc: error resolving username for external user '%v'", claims["email"])
 	}
@@ -302,9 +302,8 @@ func (am *mgr) getOIDCProvider(ctx context.Context) (*oidc.Provider, error) {
 	return am.provider, nil
 }
 
-func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) error {
+func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}, clientID string) error {
 	var (
-		claim   string
 		value   string
 		resolve bool
 	)
@@ -316,7 +315,6 @@ func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) e
 	}
 
 	if len(am.oidcUsersMapping) > 0 {
-		claim = "username"
 		// map and discover the user's username when a mapping is defined
 		if claims[am.c.GroupClaim] == nil {
 			// we are required to perform a user mapping but the group claim is not available
@@ -342,8 +340,7 @@ func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) e
 		}
 		resolve = true
 	} else if uid == 0 || gid == 0 {
-		claim = "mail"
-		value = claims["email"].(string)
+		value = clientID
 		resolve = true
 	}
 
@@ -356,11 +353,11 @@ func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) e
 		return errors.Wrap(err, "error getting user provider grpc client")
 	}
 	getUserByClaimResp, err := upsc.GetUserByClaim(ctx, &user.GetUserByClaimRequest{
-		Claim: claim,
+		Claim: "username",
 		Value: value,
 	})
 	if err != nil {
-		return errors.Wrapf(err, "error getting user by %s '%v'", claim, value)
+		return errors.Wrapf(err, "error getting user by username '%v'", value)
 	}
 	if getUserByClaimResp.Status.Code != rpc.Code_CODE_OK {
 		return status.NewErrorFromCode(getUserByClaimResp.Status.Code, "oidc")
@@ -372,7 +369,12 @@ func (am *mgr) resolveUser(ctx context.Context, claims map[string]interface{}) e
 	claims["iss"] = getUserByClaimResp.GetUser().GetId().Idp
 	claims[am.c.UIDClaim] = getUserByClaimResp.GetUser().UidNumber
 	claims[am.c.GIDClaim] = getUserByClaimResp.GetUser().GidNumber
-	appctx.GetLogger(ctx).Debug().Str("username", value).Interface("claims", claims).Msg("resolveUser: claims overridden from mapped user")
+	log := appctx.GetLogger(ctx).Debug().Str("username", value).Interface("claims", claims)
+	if uid == 0 || gid == 0 {
+		log.Msgf("resolveUser: claims overridden from '%s'", clientID)
+	} else {
+		log.Msg("resolveUser: claims overridden from mapped user")
+	}
 	return nil
 }