From 5d6e361cffcf13d767f5a50cdd85b992c0676491 Mon Sep 17 00:00:00 2001 From: Alex Unger Date: Tue, 4 Jan 2022 23:12:49 +0100 Subject: [PATCH 1/6] Implement CreateUser support for the Graph LDAP backend This adds basic support for creating users via the GraphAPI LDAP backend. This currently just maintains the bare minimum Attributes for the inetOrgPerson objectclass. Co-authored-by: Ralf Haferkamp --- graph/pkg/config/defaultconfig.go | 4 +- graph/pkg/identity/backend.go | 3 + graph/pkg/identity/cs3.go | 5 ++ graph/pkg/identity/ldap.go | 92 +++++++++++++++++++++++++++- graph/pkg/identity/ldap/reconnect.go | 9 ++- graph/pkg/service/v0/service.go | 1 + graph/pkg/service/v0/users.go | 30 ++++++++- 7 files changed, 137 insertions(+), 7 deletions(-) diff --git a/graph/pkg/config/defaultconfig.go b/graph/pkg/config/defaultconfig.go index 3bb022786e2..acd24e83816 100644 --- a/graph/pkg/config/defaultconfig.go +++ b/graph/pkg/config/defaultconfig.go @@ -33,13 +33,13 @@ func DefaultConfig() *Config { BindPassword: "", UserBaseDN: "ou=users,dc=ocis,dc=test", UserSearchScope: "sub", - UserFilter: "(objectClass=posixaccount)", + UserFilter: "(objectClass=inetOrgPerson)", UserEmailAttribute: "mail", UserDisplayNameAttribute: "displayName", UserNameAttribute: "uid", // FIXME: switch this to some more widely available attribute by default // ideally this needs to be constant for the lifetime of a users - UserIDAttribute: "ownclouduuid", + UserIDAttribute: "entryUUID", GroupBaseDN: "ou=groups,dc=ocis,dc=test", GroupSearchScope: "sub", GroupFilter: "(objectclass=groupOfNames)", diff --git a/graph/pkg/identity/backend.go b/graph/pkg/identity/backend.go index f7b497251cb..d716009af8a 100644 --- a/graph/pkg/identity/backend.go +++ b/graph/pkg/identity/backend.go @@ -9,6 +9,9 @@ import ( ) type Backend interface { + // CreateUser creates a given user in the identity backend. + CreateUser(ctx context.Context, user libregraph.User) (*libregraph.User, error) + GetUser(ctx context.Context, nameOrId string) (*libregraph.User, error) GetUsers(ctx context.Context, queryParam url.Values) ([]*libregraph.User, error) diff --git a/graph/pkg/identity/cs3.go b/graph/pkg/identity/cs3.go index f76da144f0b..272bccfe67e 100644 --- a/graph/pkg/identity/cs3.go +++ b/graph/pkg/identity/cs3.go @@ -20,6 +20,11 @@ type CS3 struct { Logger *log.Logger } +// CreateUser implements the Backend Interface. It's currently not supported for the CS3 backend +func (i *CS3) CreateUser(ctx context.Context, user libregraph.User) (*libregraph.User, error) { + return nil, errorcode.New(errorcode.NotSupported, "not implemented") +} + func (i *CS3) GetUser(ctx context.Context, userID string) (*libregraph.User, error) { client, err := pool.GetGatewayServiceClient(i.Config.Address) if err != nil { diff --git a/graph/pkg/identity/ldap.go b/graph/pkg/identity/ldap.go index 5942c6f9933..1becb2ca49d 100644 --- a/graph/pkg/identity/ldap.go +++ b/graph/pkg/identity/ldap.go @@ -84,6 +84,95 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD }, nil } +// CreateUser implements the Backend Interface. It converts the libregraph.User into an +// LDAP User Entry (using the inetOrgPerson LDAP Objectclass) add adds that to the +// configured LDAP server +func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregraph.User, error) { + + ar := ldap.AddRequest{ + DN: fmt.Sprintf("uid=%s,%s", *user.OnPremisesSamAccountName, i.userBaseDN), + Attributes: []ldap.Attribute{ + { + Type: "objectClass", + Vals: []string{"inetOrgPerson", "organizationalPerson", "person", "top"}, + }, + // inetOrgPerson requires "cn" + { + Type: "cn", + Vals: []string{*user.OnPremisesSamAccountName}, + }, + { + Type: i.userAttributeMap.mail, + Vals: []string{*user.Mail}, + }, + { + Type: i.userAttributeMap.userName, + Vals: []string{*user.OnPremisesSamAccountName}, + }, + { + Type: i.userAttributeMap.displayName, + Vals: []string{*user.DisplayName}, + }, + }, + } + + if user.PasswordProfile != nil && user.PasswordProfile.Password != nil { + // TODO? This relies to the LDAP server to properly hash the password. + // We might want to add support for the Password Modify LDAP Extended + // Operation for servers that implement it. (Or implement client-side + // hashing here. + ar.Attribute("userPassword", []string{*user.PasswordProfile.Password}) + } + + // inetOrgPerson requires "sn" to be set. Set it to the Username if + // Surname is not set in the Request + var sn string + if user.Surname != nil && *user.Surname != "" { + sn = *user.Surname + } else { + sn = *user.OnPremisesSamAccountName + } + ar.Attribute("sn", []string{sn}) + + if err := i.conn.Add(&ar); err != nil { + return nil, err + } + + // Read back user from LDAP to get the generated UUID + e, err := i.getUserByDN(ar.DN) + if err != nil { + return nil, err + } + return i.createUserModelFromLDAP(e), nil +} + +func (i *LDAP) getUserByDN(dn string) (*ldap.Entry, error) { + searchRequest := ldap.NewSearchRequest( + dn, ldap.ScopeBaseObject, ldap.NeverDerefAliases, 1, 0, false, + "(objectclass=*)", + []string{ + i.userAttributeMap.displayName, + i.userAttributeMap.id, + i.userAttributeMap.mail, + i.userAttributeMap.userName, + }, + nil, + ) + + i.logger.Debug().Str("backend", "ldap").Str("dn", dn).Msg("Search user by DN") + res, err := i.conn.Search(searchRequest) + + if err != nil { + i.logger.Error().Err(err).Str("backend", "ldap").Str("dn", dn).Msg("Search user by DN failed") + return nil, errorcode.New(errorcode.ItemNotFound, err.Error()) + } + if len(res.Entries) == 0 { + return nil, errorcode.New(errorcode.ItemNotFound, "not found") + } + + return res.Entries[0], nil +} + func (i *LDAP) GetUser(ctx context.Context, userID string) (*libregraph.User, error) { i.logger.Debug().Str("backend", "ldap").Msg("GetUser") userID = ldap.EscapeFilter(userID) @@ -106,7 +195,8 @@ func (i *LDAP) GetUser(ctx context.Context, userID string) (*libregraph.User, er if lerr, ok := err.(*ldap.Error); ok { if lerr.ResultCode == ldap.LDAPResultSizeLimitExceeded { errmsg = fmt.Sprintf("too many results searching for user '%s'", userID) - i.logger.Debug().Str("backend", "ldap").Err(lerr).Msg(errmsg) + i.logger.Debug().Str("backend", "ldap").Err(lerr). + Str("user", userID).Msg("too many results searching for user") } } return nil, errorcode.New(errorcode.ItemNotFound, errmsg) diff --git a/graph/pkg/identity/ldap/reconnect.go b/graph/pkg/identity/ldap/reconnect.go index f929e57a560..bf5c7a1bd3c 100644 --- a/graph/pkg/identity/ldap/reconnect.go +++ b/graph/pkg/identity/ldap/reconnect.go @@ -162,8 +162,13 @@ func (c ConnWithReconnect) ExternalBind() error { return ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) } -func (c ConnWithReconnect) Add(*ldap.AddRequest) error { - return ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) +func (c ConnWithReconnect) Add(a *ldap.AddRequest) error { + conn, err := c.GetConnection() + if err != nil { + return err + } + + return conn.Add(a) } func (c ConnWithReconnect) Del(*ldap.DelRequest) error { diff --git a/graph/pkg/service/v0/service.go b/graph/pkg/service/v0/service.go index 36d0cf451b8..15fa77e436d 100644 --- a/graph/pkg/service/v0/service.go +++ b/graph/pkg/service/v0/service.go @@ -67,6 +67,7 @@ func NewService(opts ...Option) Service { }) r.Route("/users", func(r chi.Router) { r.Get("/", svc.GetUsers) + r.Post("/", svc.PostUser) r.Route("/{userID}", func(r chi.Router) { r.Get("/", svc.GetUser) }) diff --git a/graph/pkg/service/v0/users.go b/graph/pkg/service/v0/users.go index c62337cbc1f..7cdd802e55d 100644 --- a/graph/pkg/service/v0/users.go +++ b/graph/pkg/service/v0/users.go @@ -1,6 +1,7 @@ package svc import ( + "encoding/json" "errors" "net/http" "net/url" @@ -8,9 +9,9 @@ import ( revactx "github.com/cs3org/reva/pkg/ctx" "github.com/go-chi/chi/v5" "github.com/go-chi/render" + libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/graph/pkg/identity" "github.com/owncloud/ocis/graph/pkg/service/v0/errorcode" - //msgraph "github.com/owncloud/open-graph-api-go" // FIXME needs OnPremisesSamAccountName, OnPremisesDomainName and AdditionalData ) // GetMe implements the Service interface. @@ -32,7 +33,6 @@ func (g Graph) GetMe(w http.ResponseWriter, r *http.Request) { } // GetUsers implements the Service interface. -// TODO use cs3 api to look up user func (g Graph) GetUsers(w http.ResponseWriter, r *http.Request) { users, err := g.identityBackend.GetUsers(r.Context(), r.URL.Query()) if err != nil { @@ -47,6 +47,28 @@ func (g Graph) GetUsers(w http.ResponseWriter, r *http.Request) { render.JSON(w, r, &listResponse{Value: users}) } +func (g Graph) PostUser(w http.ResponseWriter, r *http.Request) { + u := libregraph.NewUser() + err := json.NewDecoder(r.Body).Decode(u) + if err != nil { + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) + return + } + + if isNilOrEmpty(u.DisplayName) || isNilOrEmpty(u.OnPremisesSamAccountName) || isNilOrEmpty(u.Mail) { + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "Missing Required Attribute") + return + } + + if u, err = g.identityBackend.CreateUser(r.Context(), *u); err != nil { + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + return + } + + render.Status(r, http.StatusOK) + render.JSON(w, r, u) +} + // GetUser implements the Service interface. func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) { userID := chi.URLParam(r, "userID") @@ -74,3 +96,7 @@ func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) { render.Status(r, http.StatusOK) render.JSON(w, r, user) } + +func isNilOrEmpty(s *string) bool { + return s == nil || *s == "" +} From 4915195d9c250909fbe4836056b21d8cb4fed7e7 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 11 Jan 2022 17:22:48 +0100 Subject: [PATCH 2/6] Implement DeleteUser support for the Graph LDAP backend --- graph/pkg/identity/backend.go | 3 +++ graph/pkg/identity/cs3.go | 5 ++++ graph/pkg/identity/ldap.go | 36 ++++++++++++++++++++++------ graph/pkg/identity/ldap/reconnect.go | 9 +++++-- graph/pkg/service/v0/service.go | 1 + graph/pkg/service/v0/users.go | 26 ++++++++++++++++++++ 6 files changed, 71 insertions(+), 9 deletions(-) diff --git a/graph/pkg/identity/backend.go b/graph/pkg/identity/backend.go index d716009af8a..1a71a7b5d08 100644 --- a/graph/pkg/identity/backend.go +++ b/graph/pkg/identity/backend.go @@ -12,6 +12,9 @@ type Backend interface { // CreateUser creates a given user in the identity backend. CreateUser(ctx context.Context, user libregraph.User) (*libregraph.User, error) + // DeleteUser deletes a given user, identified by username or id, from the backend + DeleteUser(ctx context.Context, nameOrId string) error + GetUser(ctx context.Context, nameOrId string) (*libregraph.User, error) GetUsers(ctx context.Context, queryParam url.Values) ([]*libregraph.User, error) diff --git a/graph/pkg/identity/cs3.go b/graph/pkg/identity/cs3.go index 272bccfe67e..5fe0117c889 100644 --- a/graph/pkg/identity/cs3.go +++ b/graph/pkg/identity/cs3.go @@ -25,6 +25,11 @@ func (i *CS3) CreateUser(ctx context.Context, user libregraph.User) (*libregraph return nil, errorcode.New(errorcode.NotSupported, "not implemented") } +// DeleteUser implements the Backend Interface. It's currently not supported for the CS3 backend +func (i *CS3) DeleteUser(ctx context.Context, nameOrID string) error { + return errorcode.New(errorcode.NotSupported, "not implemented") +} + func (i *CS3) GetUser(ctx context.Context, userID string) (*libregraph.User, error) { client, err := pool.GetGatewayServiceClient(i.Config.Address) if err != nil { diff --git a/graph/pkg/identity/ldap.go b/graph/pkg/identity/ldap.go index 1becb2ca49d..b9f9c65cfdb 100644 --- a/graph/pkg/identity/ldap.go +++ b/graph/pkg/identity/ldap.go @@ -146,6 +146,20 @@ func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregrap return i.createUserModelFromLDAP(e), nil } +// DeleteUser implements the Backend Interface. It permanently deletes a User identified +// by name or id from the LDAP server +func (i *LDAP) DeleteUser(ctx context.Context, nameOrID string) error { + e, err := i.getLDAPUserByNameOrID(nameOrID) + if err != nil { + return err + } + dr := ldap.DelRequest{DN: e.DN} + if err = i.conn.Del(&dr); err != nil { + return err + } + return nil +} + func (i *LDAP) getUserByDN(dn string) (*ldap.Entry, error) { searchRequest := ldap.NewSearchRequest( dn, ldap.ScopeBaseObject, ldap.NeverDerefAliases, 1, 0, false, @@ -173,12 +187,11 @@ func (i *LDAP) getUserByDN(dn string) (*ldap.Entry, error) { return res.Entries[0], nil } -func (i *LDAP) GetUser(ctx context.Context, userID string) (*libregraph.User, error) { - i.logger.Debug().Str("backend", "ldap").Msg("GetUser") - userID = ldap.EscapeFilter(userID) +func (i *LDAP) getLDAPUserByNameOrID(nameOrID string) (*ldap.Entry, error) { + nameOrID = ldap.EscapeFilter(nameOrID) searchRequest := ldap.NewSearchRequest( i.userBaseDN, i.userScope, ldap.NeverDerefAliases, 1, 0, false, - fmt.Sprintf("(&%s(|(%s=%s)(%s=%s)))", i.userFilter, i.userAttributeMap.userName, userID, i.userAttributeMap.id, userID), + fmt.Sprintf("(&%s(|(%s=%s)(%s=%s)))", i.userFilter, i.userAttributeMap.userName, nameOrID, i.userAttributeMap.id, nameOrID), []string{ i.userAttributeMap.displayName, i.userAttributeMap.id, @@ -194,9 +207,9 @@ func (i *LDAP) GetUser(ctx context.Context, userID string) (*libregraph.User, er var errmsg string if lerr, ok := err.(*ldap.Error); ok { if lerr.ResultCode == ldap.LDAPResultSizeLimitExceeded { - errmsg = fmt.Sprintf("too many results searching for user '%s'", userID) + errmsg = fmt.Sprintf("too many results searching for user '%s'", nameOrID) i.logger.Debug().Str("backend", "ldap").Err(lerr). - Str("user", userID).Msg("too many results searching for user") + Str("user", nameOrID).Msg("too many results searching for user") } } return nil, errorcode.New(errorcode.ItemNotFound, errmsg) @@ -205,7 +218,16 @@ func (i *LDAP) GetUser(ctx context.Context, userID string) (*libregraph.User, er return nil, errorcode.New(errorcode.ItemNotFound, "not found") } - return i.createUserModelFromLDAP(res.Entries[0]), nil + return res.Entries[0], nil +} + +func (i *LDAP) GetUser(ctx context.Context, nameOrID string) (*libregraph.User, error) { + i.logger.Debug().Str("backend", "ldap").Msg("GetUser") + e, err := i.getLDAPUserByNameOrID(nameOrID) + if err != nil { + return nil, err + } + return i.createUserModelFromLDAP(e), nil } func (i *LDAP) GetUsers(ctx context.Context, queryParam url.Values) ([]*libregraph.User, error) { diff --git a/graph/pkg/identity/ldap/reconnect.go b/graph/pkg/identity/ldap/reconnect.go index bf5c7a1bd3c..272a0043e93 100644 --- a/graph/pkg/identity/ldap/reconnect.go +++ b/graph/pkg/identity/ldap/reconnect.go @@ -171,8 +171,13 @@ func (c ConnWithReconnect) Add(a *ldap.AddRequest) error { return conn.Add(a) } -func (c ConnWithReconnect) Del(*ldap.DelRequest) error { - return ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) +func (c ConnWithReconnect) Del(d *ldap.DelRequest) error { + conn, err := c.GetConnection() + if err != nil { + return err + } + + return conn.Del(d) } func (c ConnWithReconnect) Modify(*ldap.ModifyRequest) error { diff --git a/graph/pkg/service/v0/service.go b/graph/pkg/service/v0/service.go index 15fa77e436d..004d3689243 100644 --- a/graph/pkg/service/v0/service.go +++ b/graph/pkg/service/v0/service.go @@ -70,6 +70,7 @@ func NewService(opts ...Option) Service { r.Post("/", svc.PostUser) r.Route("/{userID}", func(r chi.Router) { r.Get("/", svc.GetUser) + r.Delete("/", svc.DeleteUser) }) }) r.Route("/groups", func(r chi.Router) { diff --git a/graph/pkg/service/v0/users.go b/graph/pkg/service/v0/users.go index 7cdd802e55d..0593c6bcb35 100644 --- a/graph/pkg/service/v0/users.go +++ b/graph/pkg/service/v0/users.go @@ -97,6 +97,32 @@ func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) { render.JSON(w, r, user) } +func (g Graph) DeleteUser(w http.ResponseWriter, r *http.Request) { + userID := chi.URLParam(r, "userID") + userID, err := url.PathUnescape(userID) + if err != nil { + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping user id failed") + } + + if userID == "" { + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing user id") + return + } + + err = g.identityBackend.DeleteUser(r.Context(), userID) + + if err != nil { + var errcode errorcode.Error + if errors.As(err, &errcode) { + errcode.Render(w, r) + } else { + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + } + } + render.Status(r, http.StatusNoContent) + render.NoContent(w, r) +} + func isNilOrEmpty(s *string) bool { return s == nil || *s == "" } From 775a556c23eb9f356dcec8c2541fbfccf7ac90b3 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 12 Jan 2022 13:15:16 +0100 Subject: [PATCH 3/6] graph: Add missing methods to Service Interface --- graph/pkg/service/v0/instrument.go | 12 +++++++++++- graph/pkg/service/v0/logging.go | 10 ++++++++++ graph/pkg/service/v0/service.go | 2 ++ graph/pkg/service/v0/tracing.go | 10 ++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/graph/pkg/service/v0/instrument.go b/graph/pkg/service/v0/instrument.go index 878945f786f..aeb9106f56e 100644 --- a/graph/pkg/service/v0/instrument.go +++ b/graph/pkg/service/v0/instrument.go @@ -34,7 +34,17 @@ func (i instrument) GetUsers(w http.ResponseWriter, r *http.Request) { i.next.GetUsers(w, r) } -// GetUsers implements the Service interface. +// GetUser implements the Service interface. func (i instrument) GetUser(w http.ResponseWriter, r *http.Request) { i.next.GetUser(w, r) } + +// PostUser implements the Service interface. +func (i instrument) PostUser(w http.ResponseWriter, r *http.Request) { + i.next.PostUser(w, r) +} + +// DeleteUser implements the Service interface. +func (i instrument) DeleteUser(w http.ResponseWriter, r *http.Request) { + i.next.DeleteUser(w, r) +} diff --git a/graph/pkg/service/v0/logging.go b/graph/pkg/service/v0/logging.go index 124a54cf3b5..2c1a10e17a2 100644 --- a/graph/pkg/service/v0/logging.go +++ b/graph/pkg/service/v0/logging.go @@ -38,3 +38,13 @@ func (l logging) GetUsers(w http.ResponseWriter, r *http.Request) { func (l logging) GetUser(w http.ResponseWriter, r *http.Request) { l.next.GetUser(w, r) } + +// PostUser implements the Service interface. +func (l logging) PostUser(w http.ResponseWriter, r *http.Request) { + l.next.PostUser(w, r) +} + +// DeleteUser implements the Service interface. +func (l logging) DeleteUser(w http.ResponseWriter, r *http.Request) { + l.next.DeleteUser(w, r) +} diff --git a/graph/pkg/service/v0/service.go b/graph/pkg/service/v0/service.go index 004d3689243..59a2c2f30c6 100644 --- a/graph/pkg/service/v0/service.go +++ b/graph/pkg/service/v0/service.go @@ -18,6 +18,8 @@ type Service interface { GetMe(http.ResponseWriter, *http.Request) GetUsers(http.ResponseWriter, *http.Request) GetUser(http.ResponseWriter, *http.Request) + PostUser(http.ResponseWriter, *http.Request) + DeleteUser(http.ResponseWriter, *http.Request) } // NewService returns a service implementation for Service. diff --git a/graph/pkg/service/v0/tracing.go b/graph/pkg/service/v0/tracing.go index 64cbc9708f3..27bbeb817bc 100644 --- a/graph/pkg/service/v0/tracing.go +++ b/graph/pkg/service/v0/tracing.go @@ -34,3 +34,13 @@ func (t tracing) GetUsers(w http.ResponseWriter, r *http.Request) { func (t tracing) GetUser(w http.ResponseWriter, r *http.Request) { t.next.GetUser(w, r) } + +// PostUser implements the Service interface. +func (t tracing) PostUser(w http.ResponseWriter, r *http.Request) { + t.next.PostUser(w, r) +} + +// DeleteUser implements the Service interface. +func (t tracing) DeleteUser(w http.ResponseWriter, r *http.Request) { + t.next.DeleteUser(w, r) +} From 840c9a7dddc9ed1868c84a4ada280f8a4dcc36a8 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 12 Jan 2022 16:28:37 +0100 Subject: [PATCH 4/6] Implement UpdateUser support for the GraphAPI backend --- graph/pkg/identity/backend.go | 3 ++ graph/pkg/identity/cs3.go | 5 +++ graph/pkg/identity/ldap.go | 51 ++++++++++++++++++++++++++++ graph/pkg/identity/ldap/reconnect.go | 27 +++++++++++---- graph/pkg/service/v0/instrument.go | 5 +++ graph/pkg/service/v0/logging.go | 5 +++ graph/pkg/service/v0/service.go | 2 ++ graph/pkg/service/v0/tracing.go | 5 +++ graph/pkg/service/v0/users.go | 35 +++++++++++++++++++ 9 files changed, 132 insertions(+), 6 deletions(-) diff --git a/graph/pkg/identity/backend.go b/graph/pkg/identity/backend.go index 1a71a7b5d08..d230a6b1333 100644 --- a/graph/pkg/identity/backend.go +++ b/graph/pkg/identity/backend.go @@ -15,6 +15,9 @@ type Backend interface { // DeleteUser deletes a given user, identified by username or id, from the backend DeleteUser(ctx context.Context, nameOrId string) error + // UpdateUser applies changes to given user, identified by username or id + UpdateUser(ctx context.Context, nameOrId string, user libregraph.User) (*libregraph.User, error) + GetUser(ctx context.Context, nameOrId string) (*libregraph.User, error) GetUsers(ctx context.Context, queryParam url.Values) ([]*libregraph.User, error) diff --git a/graph/pkg/identity/cs3.go b/graph/pkg/identity/cs3.go index 5fe0117c889..231107d11dc 100644 --- a/graph/pkg/identity/cs3.go +++ b/graph/pkg/identity/cs3.go @@ -30,6 +30,11 @@ func (i *CS3) DeleteUser(ctx context.Context, nameOrID string) error { return errorcode.New(errorcode.NotSupported, "not implemented") } +// UpdateUser implements the Backend Interface. It's currently not suported for the CS3 backend +func (i *CS3) UpdateUser(ctx context.Context, nameOrID string, user libregraph.User) (*libregraph.User, error) { + return nil, errorcode.New(errorcode.NotSupported, "not implemented") +} + func (i *CS3) GetUser(ctx context.Context, userID string) (*libregraph.User, error) { client, err := pool.GetGatewayServiceClient(i.Config.Address) if err != nil { diff --git a/graph/pkg/identity/ldap.go b/graph/pkg/identity/ldap.go index b9f9c65cfdb..47433c7e617 100644 --- a/graph/pkg/identity/ldap.go +++ b/graph/pkg/identity/ldap.go @@ -160,6 +160,57 @@ func (i *LDAP) DeleteUser(ctx context.Context, nameOrID string) error { return nil } +// UpdateUser implements the Backend Interface. It's currently not suported for the CS3 backedn +func (i *LDAP) UpdateUser(ctx context.Context, nameOrID string, user libregraph.User) (*libregraph.User, error) { + e, err := i.getLDAPUserByNameOrID(nameOrID) + if err != nil { + return nil, err + } + + // Don't allow updates of the ID + if user.Id != nil && *user.Id != "" { + if e.GetEqualFoldAttributeValue(i.userAttributeMap.id) != *user.Id { + return nil, errorcode.New(errorcode.NotAllowed, "changing the UserId is not allowed") + } + } + // TODO: In order to allow updating the user name we'd need to issue a ModRDN operation + // As we currently using uid as the naming Attribute for the user entries. (Do we even + // want to allow changing the user name?). For now just disallow it. + if user.OnPremisesSamAccountName != nil && *user.OnPremisesSamAccountName != "" { + if e.GetEqualFoldAttributeValue(i.userAttributeMap.userName) != *user.OnPremisesSamAccountName { + return nil, errorcode.New(errorcode.NotSupported, "changing the user name is currently not supported") + } + } + + mr := ldap.ModifyRequest{DN: e.DN} + if user.DisplayName != nil && *user.DisplayName != "" { + if e.GetEqualFoldAttributeValue(i.userAttributeMap.displayName) != *user.DisplayName { + mr.Replace(i.userAttributeMap.displayName, []string{*user.DisplayName}) + } + } + if user.Mail != nil && *user.Mail != "" { + if e.GetEqualFoldAttributeValue(i.userAttributeMap.mail) != *user.Mail { + mr.Replace(i.userAttributeMap.mail, []string{*user.Mail}) + } + } + if user.PasswordProfile != nil && user.PasswordProfile.Password != nil && *user.PasswordProfile.Password != "" { + // password are hashed server side there is no need to check if the new password + // is actually different from the old one. + mr.Replace("userPassword", []string{*user.PasswordProfile.Password}) + } + + if err := i.conn.Modify(&mr); err != nil { + return nil, err + } + + // Read back user from LDAP to get the generated UUID + e, err = i.getUserByDN(e.DN) + if err != nil { + return nil, err + } + return i.createUserModelFromLDAP(e), nil +} + func (i *LDAP) getUserByDN(dn string) (*ldap.Entry, error) { searchRequest := ldap.NewSearchRequest( dn, ldap.ScopeBaseObject, ldap.NeverDerefAliases, 1, 0, false, diff --git a/graph/pkg/identity/ldap/reconnect.go b/graph/pkg/identity/ldap/reconnect.go index 272a0043e93..b4e868846c8 100644 --- a/graph/pkg/identity/ldap/reconnect.go +++ b/graph/pkg/identity/ldap/reconnect.go @@ -180,16 +180,31 @@ func (c ConnWithReconnect) Del(d *ldap.DelRequest) error { return conn.Del(d) } -func (c ConnWithReconnect) Modify(*ldap.ModifyRequest) error { - return ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) +func (c ConnWithReconnect) Modify(m *ldap.ModifyRequest) error { + conn, err := c.GetConnection() + if err != nil { + return err + } + + return conn.Modify(m) } -func (c ConnWithReconnect) ModifyDN(*ldap.ModifyDNRequest) error { - return ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) +func (c ConnWithReconnect) ModifyDN(m *ldap.ModifyDNRequest) error { + conn, err := c.GetConnection() + if err != nil { + return err + } + + return conn.ModifyDN(m) } -func (c ConnWithReconnect) ModifyWithResult(*ldap.ModifyRequest) (*ldap.ModifyResult, error) { - return nil, ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) +func (c ConnWithReconnect) ModifyWithResult(m *ldap.ModifyRequest) (*ldap.ModifyResult, error) { + conn, err := c.GetConnection() + if err != nil { + return nil, err + } + + return conn.ModifyWithResult(m) } func (c ConnWithReconnect) Compare(dn, attribute, value string) (bool, error) { diff --git a/graph/pkg/service/v0/instrument.go b/graph/pkg/service/v0/instrument.go index aeb9106f56e..a0fb82542db 100644 --- a/graph/pkg/service/v0/instrument.go +++ b/graph/pkg/service/v0/instrument.go @@ -48,3 +48,8 @@ func (i instrument) PostUser(w http.ResponseWriter, r *http.Request) { func (i instrument) DeleteUser(w http.ResponseWriter, r *http.Request) { i.next.DeleteUser(w, r) } + +// PatchUser implements the Service interface. +func (i instrument) PatchUser(w http.ResponseWriter, r *http.Request) { + i.next.PatchUser(w, r) +} diff --git a/graph/pkg/service/v0/logging.go b/graph/pkg/service/v0/logging.go index 2c1a10e17a2..d5ce3373aaf 100644 --- a/graph/pkg/service/v0/logging.go +++ b/graph/pkg/service/v0/logging.go @@ -48,3 +48,8 @@ func (l logging) PostUser(w http.ResponseWriter, r *http.Request) { func (l logging) DeleteUser(w http.ResponseWriter, r *http.Request) { l.next.DeleteUser(w, r) } + +// PatchUser implements the Service interface. +func (l logging) PatchUser(w http.ResponseWriter, r *http.Request) { + l.next.PatchUser(w, r) +} diff --git a/graph/pkg/service/v0/service.go b/graph/pkg/service/v0/service.go index 59a2c2f30c6..451cdd872a4 100644 --- a/graph/pkg/service/v0/service.go +++ b/graph/pkg/service/v0/service.go @@ -20,6 +20,7 @@ type Service interface { GetUser(http.ResponseWriter, *http.Request) PostUser(http.ResponseWriter, *http.Request) DeleteUser(http.ResponseWriter, *http.Request) + PatchUser(http.ResponseWriter, *http.Request) } // NewService returns a service implementation for Service. @@ -73,6 +74,7 @@ func NewService(opts ...Option) Service { r.Route("/{userID}", func(r chi.Router) { r.Get("/", svc.GetUser) r.Delete("/", svc.DeleteUser) + r.Patch("/", svc.PatchUser) }) }) r.Route("/groups", func(r chi.Router) { diff --git a/graph/pkg/service/v0/tracing.go b/graph/pkg/service/v0/tracing.go index 27bbeb817bc..c1e54421451 100644 --- a/graph/pkg/service/v0/tracing.go +++ b/graph/pkg/service/v0/tracing.go @@ -44,3 +44,8 @@ func (t tracing) PostUser(w http.ResponseWriter, r *http.Request) { func (t tracing) DeleteUser(w http.ResponseWriter, r *http.Request) { t.next.DeleteUser(w, r) } + +// PatchUser implements the Service interface. +func (t tracing) PatchUser(w http.ResponseWriter, r *http.Request) { + t.next.PatchUser(w, r) +} diff --git a/graph/pkg/service/v0/users.go b/graph/pkg/service/v0/users.go index 0593c6bcb35..4a0ae17b87d 100644 --- a/graph/pkg/service/v0/users.go +++ b/graph/pkg/service/v0/users.go @@ -123,6 +123,41 @@ func (g Graph) DeleteUser(w http.ResponseWriter, r *http.Request) { render.NoContent(w, r) } +// PatchUser implements the Service Interface. Updates the specified attributes of an +// ExistingUser +func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) { + nameOrID := chi.URLParam(r, "userID") + nameOrID, err := url.PathUnescape(nameOrID) + if err != nil { + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "unescaping user id failed") + } + + if nameOrID == "" { + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing user id") + return + } + changes := libregraph.NewUser() + err = json.NewDecoder(r.Body).Decode(changes) + if err != nil { + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) + return + } + + u, err := g.identityBackend.UpdateUser(r.Context(), nameOrID, *changes) + if err != nil { + var errcode errorcode.Error + if errors.As(err, &errcode) { + errcode.Render(w, r) + } else { + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + } + } + + render.Status(r, http.StatusOK) + render.JSON(w, r, u) + +} + func isNilOrEmpty(s *string) bool { return s == nil || *s == "" } From 53efa9ca145c2e0e0aa8c29c685b15fa1d3f611f Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 13 Jan 2022 15:14:51 +0100 Subject: [PATCH 5/6] Switch to generating our own UUIDs for users By default the GraphAPI will generate the UUID itself now instead of relying on the LDAP server to generate a valid entryUUID attribute. This can been be switched off via the new `use_server_uuid` toggle in the LDAP config. --- graph/pkg/config/config.go | 7 ++++--- graph/pkg/config/defaultconfig.go | 5 +++-- graph/pkg/identity/ldap.go | 16 +++++++++++----- graph/pkg/service/v0/users.go | 8 ++++++++ 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/graph/pkg/config/config.go b/graph/pkg/config/config.go index d094cf54f92..6196bcdfc4e 100644 --- a/graph/pkg/config/config.go +++ b/graph/pkg/config/config.go @@ -34,9 +34,10 @@ type Spaces struct { } type LDAP struct { - URI string `ocisConfig:"uri" env:"GRAPH_LDAP_URI"` - BindDN string `ocisConfig:"bind_dn" env:"GRAPH_LDAP_BIND_DN"` - BindPassword string `ocisConfig:"bind_password" env:"GRAPH_LDAP_BIND_PASSWORD"` + URI string `ocisConfig:"uri" env:"GRAPH_LDAP_URI"` + BindDN string `ocisConfig:"bind_dn" env:"GRAPH_LDAP_BIND_DN"` + BindPassword string `ocisConfig:"bind_password" env:"GRAPH_LDAP_BIND_PASSWORD"` + UseServerUUID bool `ocisConfig:"use_server_uuid" env:"GRAPH_LDAP_SERVER_UUID"` UserBaseDN string `ocisConfig:"user_base_dn" env:"GRAPH_LDAP_USER_BASE_DN"` UserSearchScope string `ocisConfig:"user_search_scope" env:"GRAPH_LDAP_USER_SCOPE"` diff --git a/graph/pkg/config/defaultconfig.go b/graph/pkg/config/defaultconfig.go index acd24e83816..ae04f752ad7 100644 --- a/graph/pkg/config/defaultconfig.go +++ b/graph/pkg/config/defaultconfig.go @@ -31,6 +31,7 @@ func DefaultConfig() *Config { URI: "ldap://localhost:9125", BindDN: "", BindPassword: "", + UseServerUUID: false, UserBaseDN: "ou=users,dc=ocis,dc=test", UserSearchScope: "sub", UserFilter: "(objectClass=inetOrgPerson)", @@ -39,12 +40,12 @@ func DefaultConfig() *Config { UserNameAttribute: "uid", // FIXME: switch this to some more widely available attribute by default // ideally this needs to be constant for the lifetime of a users - UserIDAttribute: "entryUUID", + UserIDAttribute: "owncloudUUID", GroupBaseDN: "ou=groups,dc=ocis,dc=test", GroupSearchScope: "sub", GroupFilter: "(objectclass=groupOfNames)", GroupNameAttribute: "cn", - GroupIDAttribute: "cn", + GroupIDAttribute: "owncloudUUID", }, }, } diff --git a/graph/pkg/identity/ldap.go b/graph/pkg/identity/ldap.go index 47433c7e617..b9d1cae2b0d 100644 --- a/graph/pkg/identity/ldap.go +++ b/graph/pkg/identity/ldap.go @@ -6,6 +6,7 @@ import ( "net/url" "github.com/go-ldap/ldap/v3" + "github.com/gofrs/uuid" libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/graph/pkg/config" @@ -14,6 +15,8 @@ import ( ) type LDAP struct { + useServerUUID bool + userBaseDN string userFilter string userScope int @@ -71,6 +74,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD } return &LDAP{ + useServerUUID: config.UseServerUUID, userBaseDN: config.UserBaseDN, userFilter: config.UserFilter, userScope: userScope, @@ -88,14 +92,9 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD // LDAP User Entry (using the inetOrgPerson LDAP Objectclass) add adds that to the // configured LDAP server func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregraph.User, error) { - ar := ldap.AddRequest{ DN: fmt.Sprintf("uid=%s,%s", *user.OnPremisesSamAccountName, i.userBaseDN), Attributes: []ldap.Attribute{ - { - Type: "objectClass", - Vals: []string{"inetOrgPerson", "organizationalPerson", "person", "top"}, - }, // inetOrgPerson requires "cn" { Type: "cn", @@ -116,6 +115,8 @@ func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregrap }, } + objectClasses := []string{"inetOrgPerson", "organizationalPerson", "person", "top"} + if user.PasswordProfile != nil && user.PasswordProfile.Password != nil { // TODO? This relies to the LDAP server to properly hash the password. // We might want to add support for the Password Modify LDAP Extended @@ -123,6 +124,11 @@ func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregrap // hashing here. ar.Attribute("userPassword", []string{*user.PasswordProfile.Password}) } + if !i.useServerUUID { + ar.Attribute("owncloudUUID", []string{uuid.Must(uuid.NewV4()).String()}) + objectClasses = append(objectClasses, "owncloud") + } + ar.Attribute("objectClass", objectClasses) // inetOrgPerson requires "sn" to be set. Set it to the Username if // Surname is not set in the Request diff --git a/graph/pkg/service/v0/users.go b/graph/pkg/service/v0/users.go index 4a0ae17b87d..88d87a420eb 100644 --- a/graph/pkg/service/v0/users.go +++ b/graph/pkg/service/v0/users.go @@ -60,6 +60,14 @@ func (g Graph) PostUser(w http.ResponseWriter, r *http.Request) { return } + // Disallow user-supplied IDs. It's supposed to be readonly. We're either + // generating them in the backend ourselves or rely on the Backend's + // storage (e.g. LDAP) to provide a unique ID. + if !isNilOrEmpty(u.Id) { + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "user id is a read-only attribute") + return + } + if u, err = g.identityBackend.CreateUser(r.Context(), *u); err != nil { errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) return From cb7f9f792200327e4ed1d5fd4a1cbf778a8faa36 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 13 Jan 2022 16:25:27 +0100 Subject: [PATCH 6/6] Introduce write_enabled flag for graph user backend Defaults to `false` (for now). So the /graph/users endpoints are read-only by default, which should be the default configured against and existing external LDAP server. --- graph/pkg/config/config.go | 1 + graph/pkg/config/defaultconfig.go | 1 + graph/pkg/identity/ldap.go | 11 +++++++++++ 3 files changed, 13 insertions(+) diff --git a/graph/pkg/config/config.go b/graph/pkg/config/config.go index 6196bcdfc4e..953a5ad55dc 100644 --- a/graph/pkg/config/config.go +++ b/graph/pkg/config/config.go @@ -38,6 +38,7 @@ type LDAP struct { BindDN string `ocisConfig:"bind_dn" env:"GRAPH_LDAP_BIND_DN"` BindPassword string `ocisConfig:"bind_password" env:"GRAPH_LDAP_BIND_PASSWORD"` UseServerUUID bool `ocisConfig:"use_server_uuid" env:"GRAPH_LDAP_SERVER_UUID"` + WriteEnabled bool `ocisConfig:"write_enabled" env:"GRAPH_LDAP_SERVER_WRITE_ENABLED"` UserBaseDN string `ocisConfig:"user_base_dn" env:"GRAPH_LDAP_USER_BASE_DN"` UserSearchScope string `ocisConfig:"user_search_scope" env:"GRAPH_LDAP_USER_SCOPE"` diff --git a/graph/pkg/config/defaultconfig.go b/graph/pkg/config/defaultconfig.go index ae04f752ad7..3c98b0b277c 100644 --- a/graph/pkg/config/defaultconfig.go +++ b/graph/pkg/config/defaultconfig.go @@ -32,6 +32,7 @@ func DefaultConfig() *Config { BindDN: "", BindPassword: "", UseServerUUID: false, + WriteEnabled: false, UserBaseDN: "ou=users,dc=ocis,dc=test", UserSearchScope: "sub", UserFilter: "(objectClass=inetOrgPerson)", diff --git a/graph/pkg/identity/ldap.go b/graph/pkg/identity/ldap.go index b9d1cae2b0d..bf16afe93ca 100644 --- a/graph/pkg/identity/ldap.go +++ b/graph/pkg/identity/ldap.go @@ -16,6 +16,7 @@ import ( type LDAP struct { useServerUUID bool + writeEnabled bool userBaseDN string userFilter string @@ -85,6 +86,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD groupAttributeMap: gam, logger: logger, conn: lc, + writeEnabled: config.WriteEnabled, }, nil } @@ -92,6 +94,9 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD // LDAP User Entry (using the inetOrgPerson LDAP Objectclass) add adds that to the // configured LDAP server func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregraph.User, error) { + if !i.writeEnabled { + return nil, errorcode.New(errorcode.NotAllowed, "server is configured read-only") + } ar := ldap.AddRequest{ DN: fmt.Sprintf("uid=%s,%s", *user.OnPremisesSamAccountName, i.userBaseDN), Attributes: []ldap.Attribute{ @@ -155,6 +160,9 @@ func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregrap // DeleteUser implements the Backend Interface. It permanently deletes a User identified // by name or id from the LDAP server func (i *LDAP) DeleteUser(ctx context.Context, nameOrID string) error { + if !i.writeEnabled { + return errorcode.New(errorcode.NotAllowed, "server is configured read-only") + } e, err := i.getLDAPUserByNameOrID(nameOrID) if err != nil { return err @@ -168,6 +176,9 @@ func (i *LDAP) DeleteUser(ctx context.Context, nameOrID string) error { // UpdateUser implements the Backend Interface. It's currently not suported for the CS3 backedn func (i *LDAP) UpdateUser(ctx context.Context, nameOrID string, user libregraph.User) (*libregraph.User, error) { + if !i.writeEnabled { + return nil, errorcode.New(errorcode.NotAllowed, "server is configured read-only") + } e, err := i.getLDAPUserByNameOrID(nameOrID) if err != nil { return nil, err