Todo cleanup (#396)

* we are referring to a user not a uid * todo cleanu regarding claims
cs3org · Dec 4, 2019 · cbf238c · cbf238c
1 parent 4a6f9ea
commit cbf238c
Show file tree

Hide file tree

Showing 3 changed files with 4 additions and 10 deletions.
diff --git a/internal/grpc/services/authprovider/authprovider.go b/internal/grpc/services/authprovider/authprovider.go
@@ -94,7 +94,7 @@ func (s *service) Authenticate(ctx context.Context, req *provider.AuthenticateRe
 	username := req.ClientId
 	password := req.ClientSecret
 
-	uid, err := s.authmgr.Authenticate(ctx, username, password)
+	u, err := s.authmgr.Authenticate(ctx, username, password)
 	if err != nil {
 		err = errors.Wrap(err, "authsvc: error in Authenticate")
 		res := &provider.AuthenticateResponse{
@@ -103,10 +103,10 @@ func (s *service) Authenticate(ctx context.Context, req *provider.AuthenticateRe
 		return res, nil
 	}
 
-	log.Info().Msgf("user %s authenticated", uid.String())
+	log.Info().Msgf("user %s authenticated", u.String())
 	res := &provider.AuthenticateResponse{
 		Status: status.NewOK(ctx),
-		User:   uid,
+		User:   u,
 	}
 	return res, nil
 }
diff --git a/pkg/auth/manager/ldap/ldap.go b/pkg/auth/manager/ldap/ldap.go
@@ -136,13 +136,10 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, clientSecret string)
 	}
 
 	u := &user.User{
-		// TODO(jfd) clean up idp = iss, sub = opaque ... is redundant
 		Id: &user.UserId{
 			Idp:      am.c.Idp,
 			OpaqueId: sr.Entries[0].GetAttributeValue(am.c.Schema.UID),
 		},
-		// Subject:     claims.Sub, // TODO(labkode) remove from CS3, is in Id
-		// Issuer:      claims.Iss, // TODO(labkode) remove from CS3, is in Id
 		// TODO add more claims from the StandardClaims, eg EmailVerified
 		Username: sr.Entries[0].GetAttributeValue(am.c.Schema.UserName),
 		// TODO groups

diff --git a/pkg/auth/manager/oidc/oidc.go b/pkg/auth/manager/oidc/oidc.go
@@ -220,15 +220,12 @@ func (am *mgr) Authenticate(ctx context.Context, clientID, token string) (*user.
 	}
 
 	u := &user.User{
-		// TODO(jfd) clean up idp = iss, sub = opaque ... is redundant
 		Id: &user.UserId{
 			OpaqueId: claims.Sub, // a stable non reassignable id
 			Idp:      claims.Iss, // in the scope of this issuer
 		},
-		// Subject:     claims.Sub, // TODO(labkode) remove from CS3, is in Id
-		// Issuer:      claims.Iss, // TODO(labkode) remove from CS3, is in Id
 		Username: claims.PreferredUsername,
-		// TODO groups
+		// TODO groups are not covered by oidc but might be sent as a custom claim.
 		// TODO ... use all claims from oidc?
 		Groups:       []string{},
 		Mail:         claims.Email,