From cdde45290a439edc468f1b5807944630825dce31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 29 Jul 2020 11:19:29 +0200 Subject: [PATCH 1/4] implement delete, better return codes, correct result payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/service/v0/data/meta.go | 8 +++- pkg/service/v0/data/user.go | 2 +- pkg/service/v0/users.go | 80 +++++++++++++++++++++++++++++++------ 3 files changed, 76 insertions(+), 14 deletions(-) diff --git a/pkg/service/v0/data/meta.go b/pkg/service/v0/data/meta.go index 66acd70e5..54d37d522 100644 --- a/pkg/service/v0/data/meta.go +++ b/pkg/service/v0/data/meta.go @@ -9,9 +9,15 @@ type Meta struct { ItemsPerPage string `json:"itemsperpage,omitempty" xml:"itemsperpage,omitempty"` } -// MetaOK is the default ok response +// MetaOK is the default ok response with code 100 var MetaOK = Meta{Status: "ok", StatusCode: 100, Message: "OK"} +// MetaFailure is a failure response with code 101 +var MetaFailure = Meta{Status: "", StatusCode: 101, Message: "Failure"} + +// MetaInvalidInput is an error response with code 102 +var MetaInvalidInput = Meta{Status: "", StatusCode: 102, Message: "Invalid Input"} + // MetaBadRequest is used for unknown errors var MetaBadRequest = Meta{Status: "error", StatusCode: 400, Message: "Bad Request"} diff --git a/pkg/service/v0/data/user.go b/pkg/service/v0/data/user.go index f713d18f5..c72f94690 100644 --- a/pkg/service/v0/data/user.go +++ b/pkg/service/v0/data/user.go @@ -3,7 +3,7 @@ package data // User holds the payload for a GetUser response type User struct { // TODO needs better naming, clarify if we need a userid, a username or both - UserID string `json:"userid" xml:"userid"` + UserID string `json:"id" xml:"id"` Username string `json:"username" xml:"username"` DisplayName string `json:"displayname" xml:"displayname"` Email string `json:"email" xml:"email"` diff --git a/pkg/service/v0/users.go b/pkg/service/v0/users.go index e7252e542..29bccdb58 100644 --- a/pkg/service/v0/users.go +++ b/pkg/service/v0/users.go @@ -33,15 +33,27 @@ func (o Ocs) GetUser(w http.ResponseWriter, r *http.Request) { userid = u.Id.OpaqueId } - accSvc := accounts.NewAccountsService("com.owncloud.api.accounts", grpc.NewClient()) + accSvc := o.getAccountService() account, err := accSvc.GetAccount(r.Context(), &accounts.GetAccountRequest{ Id: userid, }) if err != nil { - render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, err.Error())) + merr := merrors.FromError(err) + if merr.Code == http.StatusNotFound { + render.Render(w, r, response.ErrRender(data.MetaNotFound.StatusCode, "The requested user could not be found")) + } else { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, err.Error())) + } + o.logger.Error().Err(err).Str("userid", userid).Msg("could not get user") return } + // remove password from log if it is set + if account.PasswordProfile != nil { + account.PasswordProfile.Password = "" + } + o.logger.Debug().Interface("account", account).Msg("got user") + render.Render(w, r, response.DataRender(&data.User{ UserID: account.Id, // TODO userid vs username! implications for clients if we return the userid here? -> implement graph ASAP? Username: account.PreferredName, @@ -60,7 +72,7 @@ func (o Ocs) AddUser(w http.ResponseWriter, r *http.Request) { displayname := r.PostFormValue("displayname") email := r.PostFormValue("email") - accSvc := accounts.NewAccountsService("com.owncloud.api.accounts", grpc.NewClient()) + accSvc := o.getAccountService() account, err := accSvc.CreateAccount(r.Context(), &accounts.CreateAccountRequest{ Account: &accounts.Account{ DisplayName: displayname, @@ -75,11 +87,22 @@ func (o Ocs) AddUser(w http.ResponseWriter, r *http.Request) { }, }) if err != nil { - render.Render(w, r, response.DataRender(data.MetaServerError)) + merr := merrors.FromError(err) + if merr.Code == http.StatusBadRequest { + render.Render(w, r, response.ErrRender(data.MetaBadRequest.StatusCode, merr.Detail)) + } else { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, err.Error())) + } + o.logger.Error().Err(err).Str("userid", userid).Msg("could not add user") + // TODO check error if account already existed return } - o.logger.Debug().Interface("account", account).Msg("add user: account info") + // remove password from log if it is set + if account.PasswordProfile != nil { + account.PasswordProfile.Password = "" + } + o.logger.Debug().Interface("account", account).Msg("added user") render.Render(w, r, response.DataRender(&data.User{ UserID: account.Id, @@ -118,26 +141,55 @@ func (o Ocs) EditUser(w http.ResponseWriter, r *http.Request) { req.Account.DisplayName = value req.UpdateMask = &fieldmaskpb.FieldMask{Paths: []string{"DisplayName"}} default: - render.Render(w, r, response.DataRender(data.MetaServerError)) + // https://github.com/owncloud/core/blob/24b7fa1d2604a208582055309a5638dbd9bda1d1/apps/provisioning_api/lib/Users.php#L321 + render.Render(w, r, response.ErrRender(103, "unknown key '"+key+"'")) return } - accSvc := accounts.NewAccountsService("com.owncloud.api.accounts", grpc.NewClient()) + accSvc := o.getAccountService() account, err := accSvc.UpdateAccount(r.Context(), &req) if err != nil { - // TODO to be compliant with the spec, check whether the user exists or not - // https://doc.owncloud.com/server/admin_manual/configuration/user/user_provisioning_api.html#edit-user - render.Render(w, r, response.DataRender(data.MetaServerError)) + merr := merrors.FromError(err) + switch merr.Code { + case http.StatusNotFound: + render.Render(w, r, response.ErrRender(data.MetaNotFound.StatusCode, "The requested user could not be found")) + case http.StatusBadRequest: + render.Render(w, r, response.ErrRender(data.MetaBadRequest.StatusCode, merr.Detail)) + default: + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, err.Error())) + } + o.logger.Error().Err(err).Str("userid", req.Account.Id).Msg("could not edit user") return } - o.logger.Debug().Interface("account", account).Msg("update user: account info") + // remove password from log if it is set + if account.PasswordProfile != nil { + account.PasswordProfile.Password = "" + } + o.logger.Debug().Interface("account", account).Msg("updated user") render.Render(w, r, response.DataRender(struct{}{})) } // DeleteUser deletes a user func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { - + req := accounts.DeleteAccountRequest{ + Id: chi.URLParam(r, "userid"), + } + accSvc := o.getAccountService() + _, err := accSvc.DeleteAccount(r.Context(), &req) + if err != nil { + merr := merrors.FromError(err) + if merr.Code == http.StatusNotFound { + render.Render(w, r, response.ErrRender(data.MetaNotFound.StatusCode, "The requested user could not be found")) + } else { + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, err.Error())) + } + o.logger.Error().Err(err).Str("userid", req.Id).Msg("could not delete user") + return + } + o.logger.Debug().Str("userid", req.Id).Msg("deleted user") + render.Render(w, r, response.DataRender(struct{}{})) + return } // GetSigningKey returns the signing key for the current user. It will create it on the fly if it does not exist @@ -215,3 +267,7 @@ func (o Ocs) ListUsers(w http.ResponseWriter, r *http.Request) { render.Render(w, r, response.ErrRender(data.MetaUnknownError.StatusCode, "please check the syntax. API specifications are here: http://www.freedesktop.org/wiki/Specifications/open-collaboration-services")) } + +func (o Ocs) getAccountService() accounts.AccountsService { + return accounts.NewAccountsService("com.owncloud.api.accounts", grpc.NewClient()) +} From ee8195cd5f7301a85e47b3ae1d9ba3791a81e82a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 29 Jul 2020 12:14:44 +0200 Subject: [PATCH 2/4] initial user and group listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/service/v0/data/group.go | 6 ++++ pkg/service/v0/data/user.go | 5 +++ pkg/service/v0/groups.go | 66 ++++++++++++++++++++++++++++++++++++ pkg/service/v0/service.go | 24 ++++++++++++- pkg/service/v0/users.go | 27 +++++++++++++-- 5 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 pkg/service/v0/data/group.go create mode 100644 pkg/service/v0/groups.go diff --git a/pkg/service/v0/data/group.go b/pkg/service/v0/data/group.go new file mode 100644 index 000000000..1eab5d8bd --- /dev/null +++ b/pkg/service/v0/data/group.go @@ -0,0 +1,6 @@ +package data + +// Groups holds group ids for the groups listing +type Groups struct { + Groups []string `json:"groups" xml:"groups>element"` +} diff --git a/pkg/service/v0/data/user.go b/pkg/service/v0/data/user.go index c72f94690..b882d779e 100644 --- a/pkg/service/v0/data/user.go +++ b/pkg/service/v0/data/user.go @@ -10,6 +10,11 @@ type User struct { Enabled bool `json:"enabled" xml:"enabled"` } +// Users holds user ids for the user listing +type Users struct { + Users []string `json:"users" xml:"users>element"` +} + // SigningKey holds the Payload for a GetSigningKey response type SigningKey struct { User string `json:"user" xml:"user"` diff --git a/pkg/service/v0/groups.go b/pkg/service/v0/groups.go new file mode 100644 index 000000000..65df8d2c3 --- /dev/null +++ b/pkg/service/v0/groups.go @@ -0,0 +1,66 @@ +package svc + +import ( + "fmt" + "net/http" + + "github.com/go-chi/render" + + accounts "github.com/owncloud/ocis-accounts/pkg/proto/v0" + "github.com/owncloud/ocis-ocs/pkg/service/v0/data" + "github.com/owncloud/ocis-ocs/pkg/service/v0/response" +) + +// ListUserGroups lists a users groups +func (o Ocs) ListUserGroups(w http.ResponseWriter, r *http.Request) { + render.Render(w, r, response.ErrRender(data.MetaUnknownError.StatusCode, "not implemented")) +} + +// AddToGroup adds a user to a group +func (o Ocs) AddToGroup(w http.ResponseWriter, r *http.Request) { + render.Render(w, r, response.ErrRender(data.MetaUnknownError.StatusCode, "not implemented")) +} + +// RemoveFromGroup removes a user from a group +func (o Ocs) RemoveFromGroup(w http.ResponseWriter, r *http.Request) { + render.Render(w, r, response.ErrRender(data.MetaUnknownError.StatusCode, "not implemented")) +} + +// ListGroups lists all groups +func (o Ocs) ListGroups(w http.ResponseWriter, r *http.Request) { + search := r.URL.Query().Get("search") + query := "" + if search != "" { + query = fmt.Sprintf("id eq '%s' or on_premises_sam_account_name eq '%s'", escapeValue(search), escapeValue(search)) + } + accSvc := o.getGroupsService() + res, err := accSvc.ListGroups(r.Context(), &accounts.ListGroupsRequest{ + Query: query, + }) + if err != nil { + o.logger.Err(err).Msg("could not list users") + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, "could not list users")) + return + } + groups := []string{} + for i := range res.Groups { + groups = append(groups, res.Groups[i].Id) + } + + render.Render(w, r, response.DataRender(&data.Groups{Groups: groups})) +} + +// AddGroup adds a group +func (o Ocs) AddGroup(w http.ResponseWriter, r *http.Request) { + render.Render(w, r, response.ErrRender(data.MetaUnknownError.StatusCode, "not implemented")) +} + +// DeleteGroup deletes a group +func (o Ocs) DeleteGroup(w http.ResponseWriter, r *http.Request) { + render.Render(w, r, response.ErrRender(data.MetaUnknownError.StatusCode, "not implemented")) +} + +// GetGroupMembers lists all members of a group +func (o Ocs) GetGroupMembers(w http.ResponseWriter, r *http.Request) { + render.Render(w, r, response.ErrRender(data.MetaUnknownError.StatusCode, "not implemented")) +} diff --git a/pkg/service/v0/service.go b/pkg/service/v0/service.go index 0c92f8691..8bc38b038 100644 --- a/pkg/service/v0/service.go +++ b/pkg/service/v0/service.go @@ -6,7 +6,9 @@ import ( "github.com/go-chi/chi" "github.com/go-chi/chi/middleware" "github.com/go-chi/render" + "github.com/micro/go-micro/v2/client/grpc" + accounts "github.com/owncloud/ocis-accounts/pkg/proto/v0" "github.com/owncloud/ocis-ocs/pkg/config" ocsm "github.com/owncloud/ocis-ocs/pkg/middleware" "github.com/owncloud/ocis-ocs/pkg/service/v0/data" @@ -57,6 +59,18 @@ func NewService(opts ...Option) Service { r.Get("/{userid}", svc.GetUser) r.Put("/{userid}", svc.EditUser) r.Delete("/{userid}", svc.DeleteUser) + + r.Route("/{userid}/groups", func(r chi.Router) { + r.Get("/", svc.ListUserGroups) + r.Post("/", svc.AddToGroup) + r.Delete("/", svc.RemoveFromGroup) + }) + }) + r.Route("/groups", func(r chi.Router) { + r.Get("/", svc.ListGroups) + r.Post("/", svc.AddGroup) + r.Delete("/{groupid}", svc.DeleteGroup) + r.Get("/{groupid}", svc.GetGroupMembers) }) }) r.Route("/config", func(r chi.Router) { @@ -82,5 +96,13 @@ func (o Ocs) ServeHTTP(w http.ResponseWriter, r *http.Request) { // NotFound uses ErrRender to always return a proper OCS payload func (o Ocs) NotFound(w http.ResponseWriter, r *http.Request) { - render.Render(w, r, response.ErrRender(data.MetaUnknownError.StatusCode, "please check the syntax. API specifications are here: http://www.freedesktop.org/wiki/Specifications/open-collaboration-services")) + render.Render(w, r, response.ErrRender(data.MetaNotFound.StatusCode, "not found")) +} + +func (o Ocs) getAccountService() accounts.AccountsService { + return accounts.NewAccountsService("com.owncloud.api.accounts", grpc.NewClient()) +} + +func (o Ocs) getGroupsService() accounts.GroupsService { + return accounts.NewGroupsService("com.owncloud.api.accounts", grpc.NewClient()) } diff --git a/pkg/service/v0/users.go b/pkg/service/v0/users.go index 29bccdb58..208673d35 100644 --- a/pkg/service/v0/users.go +++ b/pkg/service/v0/users.go @@ -3,7 +3,9 @@ package svc import ( "crypto/rand" "encoding/hex" + "fmt" "net/http" + "strings" "github.com/cs3org/reva/pkg/user" "github.com/go-chi/chi" @@ -264,10 +266,29 @@ func (o Ocs) GetSigningKey(w http.ResponseWriter, r *http.Request) { // ListUsers lists the users func (o Ocs) ListUsers(w http.ResponseWriter, r *http.Request) { + search := r.URL.Query().Get("search") + query := "" + if search != "" { + query = fmt.Sprintf("id eq '%s' or on_premises_sam_account_name eq '%s'", escapeValue(search), escapeValue(search)) + } + accSvc := o.getAccountService() + res, err := accSvc.ListAccounts(r.Context(), &accounts.ListAccountsRequest{ + Query: query, + }) + if err != nil { + o.logger.Err(err).Msg("could not list users") + render.Render(w, r, response.ErrRender(data.MetaServerError.StatusCode, "could not list users")) + return + } + users := []string{} + for i := range res.Accounts { + users = append(users, res.Accounts[i].Id) + } - render.Render(w, r, response.ErrRender(data.MetaUnknownError.StatusCode, "please check the syntax. API specifications are here: http://www.freedesktop.org/wiki/Specifications/open-collaboration-services")) + render.Render(w, r, response.DataRender(&data.Users{Users: users})) } -func (o Ocs) getAccountService() accounts.AccountsService { - return accounts.NewAccountsService("com.owncloud.api.accounts", grpc.NewClient()) +// escapeValue escapes all special characters in the value +func escapeValue(value string) string { + return strings.ReplaceAll(value, "'", "''") } From 4c0adfd68d2762a10310c68e439fb16be86496d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 29 Jul 2020 14:42:10 +0200 Subject: [PATCH 3/4] fake empty groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/service/v0/groups.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/service/v0/groups.go b/pkg/service/v0/groups.go index 65df8d2c3..4f89a8154 100644 --- a/pkg/service/v0/groups.go +++ b/pkg/service/v0/groups.go @@ -13,7 +13,7 @@ import ( // ListUserGroups lists a users groups func (o Ocs) ListUserGroups(w http.ResponseWriter, r *http.Request) { - render.Render(w, r, response.ErrRender(data.MetaUnknownError.StatusCode, "not implemented")) + render.Render(w, r, response.DataRender(&data.Groups{Groups: []string{}})) } // AddToGroup adds a user to a group From 34c6b854a9e78122933a896916ef3e8069f21f68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 29 Jul 2020 14:47:27 +0200 Subject: [PATCH 4/4] fix staticcheck MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/service/v0/users.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/service/v0/users.go b/pkg/service/v0/users.go index 208673d35..4348474e4 100644 --- a/pkg/service/v0/users.go +++ b/pkg/service/v0/users.go @@ -191,7 +191,6 @@ func (o Ocs) DeleteUser(w http.ResponseWriter, r *http.Request) { } o.logger.Debug().Str("userid", req.Id).Msg("deleted user") render.Render(w, r, response.DataRender(struct{}{})) - return } // GetSigningKey returns the signing key for the current user. It will create it on the fly if it does not exist