Skip to content

Commit

Permalink
Merge pull request #3053 from owncloud/fix-code-smells
Browse files Browse the repository at this point in the history 
Fix code smells
  • Loading branch information
@wkloucek
wkloucek authored Jan 28, 2022
2 parents 84f63c3 + 28496e5 commit 4d39225
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 52 deletions.
8 changes: 4 additions & 4 deletions graph/pkg/identity/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ type Backend interface {
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
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)
UpdateUser(ctx context.Context, nameOrID string, user libregraph.User) (*libregraph.User, error)

GetUser(ctx context.Context, nameOrId string) (*libregraph.User, error)
GetUser(ctx context.Context, nameOrID string) (*libregraph.User, error)
GetUsers(ctx context.Context, queryParam url.Values) ([]*libregraph.User, error)

GetGroup(ctx context.Context, nameOrId string) (*libregraph.Group, error)
GetGroup(ctx context.Context, nameOrID string) (*libregraph.Group, error)
GetGroups(ctx context.Context, queryParam url.Values) ([]*libregraph.Group, error)
}

Expand Down
10 changes: 7 additions & 3 deletions graph/pkg/identity/cs3.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,28 @@ import (
"github.com/owncloud/ocis/ocis-pkg/log"
)

var (
errNotImplemented = errorcode.New(errorcode.NotSupported, "not implemented")
)

type CS3 struct {
Config *config.Reva
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")
return nil, errNotImplemented
}

// 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")
return errNotImplemented
}

// 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")
return nil, errNotImplemented
}

func (i *CS3) GetUser(ctx context.Context, userID string) (*libregraph.User, error) {
Expand Down
28 changes: 17 additions & 11 deletions graph/pkg/identity/ldap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package identity

import (
"context"
"errors"
"fmt"
"net/url"

Expand All @@ -14,6 +15,11 @@ import (
"github.com/owncloud/ocis/ocis-pkg/log"
)

var (
errReadOnly = errorcode.New(errorcode.NotAllowed, "server is configured read-only")
errNotFound = errorcode.New(errorcode.ItemNotFound, "not found")
)

type LDAP struct {
useServerUUID bool
writeEnabled bool
Expand Down Expand Up @@ -47,7 +53,7 @@ type groupAttributeMap struct {
func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LDAP, error) {
if config.UserDisplayNameAttribute == "" || config.UserIDAttribute == "" ||
config.UserEmailAttribute == "" || config.UserNameAttribute == "" {
return nil, fmt.Errorf("Invalid user attribute mappings")
return nil, errors.New("invalid user attribute mappings")
}
uam := userAttributeMap{
displayName: config.UserDisplayNameAttribute,
Expand All @@ -57,7 +63,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD
}

if config.GroupNameAttribute == "" || config.GroupIDAttribute == "" {
return nil, fmt.Errorf("Invalid group attribute mappings")
return nil, errors.New("invalid group attribute mappings")
}
gam := groupAttributeMap{
name: config.GroupNameAttribute,
Expand All @@ -67,11 +73,11 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD
var userScope, groupScope int
var err error
if userScope, err = stringToScope(config.UserSearchScope); err != nil {
return nil, fmt.Errorf("Error configuring user scope: %w", err)
return nil, fmt.Errorf("error configuring user scope: %w", err)
}

if groupScope, err = stringToScope(config.GroupSearchScope); err != nil {
return nil, fmt.Errorf("Error configuring group scope: %w", err)
return nil, fmt.Errorf("error configuring group scope: %w", err)
}

return &LDAP{
Expand All @@ -95,7 +101,7 @@ func NewLDAPBackend(lc ldap.Client, config config.LDAP, logger *log.Logger) (*LD
// 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")
return nil, errReadOnly
}
ar := ldap.AddRequest{
DN: fmt.Sprintf("uid=%s,%s", *user.OnPremisesSamAccountName, i.userBaseDN),
Expand Down Expand Up @@ -161,7 +167,7 @@ func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregrap
// 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")
return errReadOnly
}
e, err := i.getLDAPUserByNameOrID(nameOrID)
if err != nil {
Expand All @@ -177,7 +183,7 @@ 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")
return nil, errReadOnly
}
e, err := i.getLDAPUserByNameOrID(nameOrID)
if err != nil {
Expand Down Expand Up @@ -249,7 +255,7 @@ func (i *LDAP) getUserByDN(dn string) (*ldap.Entry, error) {
return nil, errorcode.New(errorcode.ItemNotFound, err.Error())
}
if len(res.Entries) == 0 {
return nil, errorcode.New(errorcode.ItemNotFound, "not found")
return nil, errNotFound
}

return res.Entries[0], nil
Expand Down Expand Up @@ -283,7 +289,7 @@ func (i *LDAP) getLDAPUserByNameOrID(nameOrID string) (*ldap.Entry, error) {
return nil, errorcode.New(errorcode.ItemNotFound, errmsg)
}
if len(res.Entries) == 0 {
return nil, errorcode.New(errorcode.ItemNotFound, "not found")
return nil, errNotFound
}

return res.Entries[0], nil
Expand Down Expand Up @@ -367,7 +373,7 @@ func (i *LDAP) GetGroup(ctx context.Context, groupID string) (*libregraph.Group,
return nil, errorcode.New(errorcode.ItemNotFound, errmsg)
}
if len(res.Entries) == 0 {
return nil, errorcode.New(errorcode.ItemNotFound, "not found")
return nil, errNotFound
}

return i.createGroupModelFromLDAP(res.Entries[0]), nil
Expand Down Expand Up @@ -449,7 +455,7 @@ func stringToScope(scope string) (int, error) {
case "base":
s = ldap.ScopeBaseObject
default:
return 0, fmt.Errorf("Invalid Scope '%s'", scope)
return 0, fmt.Errorf("invalid Scope '%s'", scope)
}
return s, nil
}
14 changes: 9 additions & 5 deletions graph/pkg/identity/ldap/reconnect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import (
"github.com/owncloud/ocis/ocis-pkg/log"
)

var (
errMaxRetries = errors.New("max retries")
)

type ldapConnection struct {
Conn *ldap.Conn
Error error
Expand Down Expand Up @@ -61,7 +65,7 @@ func (c ConnWithReconnect) Search(sr *ldap.SearchRequest) (*ldap.SearchResult, e
c.logger.Debug().Msg("retrying LDAP Search")
}
// if we get here we reached the maximum retries. So return an error
return nil, ldap.NewError(ldap.ErrorNetwork, errors.New("max retries"))
return nil, ldap.NewError(ldap.ErrorNetwork, errMaxRetries)
}

func (c ConnWithReconnect) Add(a *ldap.AddRequest) error {
Expand All @@ -84,7 +88,7 @@ func (c ConnWithReconnect) Add(a *ldap.AddRequest) error {
c.logger.Debug().Msg("retrying LDAP Add")
}
// if we get here we reached the maximum retries. So return an error
return ldap.NewError(ldap.ErrorNetwork, errors.New("max retries"))
return ldap.NewError(ldap.ErrorNetwork, errMaxRetries)
}

func (c ConnWithReconnect) Del(d *ldap.DelRequest) error {
Expand All @@ -108,7 +112,7 @@ func (c ConnWithReconnect) Del(d *ldap.DelRequest) error {
c.logger.Debug().Msg("retrying LDAP Del")
}
// if we get here we reached the maximum retries. So return an error
return ldap.NewError(ldap.ErrorNetwork, errors.New("max retries"))
return ldap.NewError(ldap.ErrorNetwork, errMaxRetries)
}

func (c ConnWithReconnect) Modify(m *ldap.ModifyRequest) error {
Expand All @@ -132,7 +136,7 @@ func (c ConnWithReconnect) Modify(m *ldap.ModifyRequest) error {
c.logger.Debug().Msg("retrying LDAP Modify")
}
// if we get here we reached the maximum retries. So return an error
return ldap.NewError(ldap.ErrorNetwork, errors.New("max retries"))
return ldap.NewError(ldap.ErrorNetwork, errMaxRetries)
}

func (c ConnWithReconnect) ModifyDN(m *ldap.ModifyDNRequest) error {
Expand All @@ -156,7 +160,7 @@ func (c ConnWithReconnect) ModifyDN(m *ldap.ModifyDNRequest) error {
c.logger.Debug().Msg("retrying LDAP ModifyDN")
}
// if we get here we reached the maximum retries. So return an error
return ldap.NewError(ldap.ErrorNetwork, errors.New("max retries"))
return ldap.NewError(ldap.ErrorNetwork, errMaxRetries)
}

func (c ConnWithReconnect) GetConnection() (*ldap.Conn, error) {
Expand Down
2 changes: 1 addition & 1 deletion sonar-project.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ sonar.go.coverage.reportPaths=cache/coverage/*
sonar.go.golangci-lint.reportPaths=cache/checkstyle/accounts_checkstyle.xml,cache/checkstyle/glauth_checkstyle.xml,cache/checkstyle/graph_checkstyle.xml,cache/checkstyle/graph-explorer_checkstyle.xml,cache/checkstyle/idp_checkstyle.xml,cache/checkstyle/ocis_checkstyle.xml,cache/checkstyle/ocis-pkg_checkstyle.xml,cache/checkstyle/ocs_checkstyle.xml,cache/checkstyle/proxy_checkstyle.xml,cache/checkstyle/settings_checkstyle.xml,cache/checkstyle/storage_checkstyle.xml,cache/checkstyle/store_checkstyle.xml,cache/checkstyle/thumbnails_checkstyle.xml,cache/checkstyle/web_checkstyle.xml,cache/checkstyle/webdav_checkstyle.xml

# Exclude files
sonar.exclusions=**/third_party,docs/**,changelog/**,*/pkg/assets/embed.go,idp/assets/identifier/**,**/package.json,**/rollup.config.js,CHANGELOG.md,**/pkg/proto/**/*.pb.*,deployments/**,tests/**,vendor-bin/**,README.md
sonar.exclusions=**/third_party,docs/**,changelog/**,*/pkg/assets/embed.go,idp/assets/identifier/**,**/package.json,**/rollup.config.js,CHANGELOG.md,**/pkg/proto/**/*.pb.*,deployments/**,tests/**,vendor-bin/**,README.md,**/mocks/
sonar.coverage.exclusions=**/*_test.go
Loading

0 comments on commit 4d39225

Please sign in to comment.