Skip to content

Commit

Permalink
[FABC-711] Registration with LDAP throws better error
Browse files Browse the repository at this point in the history
Registration of identities when using LDAP is not a supported
action, the error message was improved to more accurately
state the reason for failure.

Change-Id: Ib37d35a64537c03de3e60c265d065804d8f9bd79
Signed-off-by: Saad Karim <[email protected]>
  • Loading branch information
Saad Karim committed Sep 10, 2018
1 parent b0e037c commit 785ebd6
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 12 deletions.
2 changes: 2 additions & 0 deletions lib/caerrors/servererror.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ const (
ErrRevokedID = 70
// Authorization failure
ErrAuthorizationFailure = 71
// Action is not allowed when using LDAP
ErrInvalidLDAPAction = 72
)

// CreateHTTPErr constructs a new HTTP error.
Expand Down
56 changes: 56 additions & 0 deletions lib/mocks/ServerRequestContext.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 18 additions & 12 deletions lib/serverregister.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ func newRegisterEndpoint(s *Server) *serverEndpoint {

// Handle a register request
func registerHandler(ctx *serverRequestContextImpl) (interface{}, error) {
ca, err := ctx.GetCA()
if err != nil {
return nil, err
}
return register(ctx, ca)
}

func register(ctx ServerRequestContext, ca *CA) (interface{}, error) {
// Read request body
var req api.RegistrationRequestNet
err := ctx.ReadBody(&req)
Expand All @@ -40,14 +48,12 @@ func registerHandler(ctx *serverRequestContextImpl) (interface{}, error) {
}
// Authenticate
callerID, err := ctx.TokenAuthentication()
log.Debugf("Received registration request from %s: %v", callerID, &req)
if err != nil {
return nil, err
}
// Get the target CA
ca, err := ctx.GetCA()
if err != nil {
return nil, err
log.Debugf("Received registration request from %s: %v", callerID, &req)
if ctx.IsLDAPEnabled() {
return nil, caerrors.NewHTTPErr(403, caerrors.ErrInvalidLDAPAction, "Registration is not supported when using LDAP")
}
// Register User
secret, err := registerUser(&req.RegistrationRequest, callerID, ca, ctx)
Expand All @@ -62,7 +68,7 @@ func registerHandler(ctx *serverRequestContextImpl) (interface{}, error) {
}

// RegisterUser will register a user and return the secret
func registerUser(req *api.RegistrationRequest, registrar string, ca *CA, ctx *serverRequestContextImpl) (string, error) {
func registerUser(req *api.RegistrationRequest, registrar string, ca *CA, ctx ServerRequestContext) (string, error) {
var err error
var registrarUser spi.User

Expand All @@ -74,7 +80,7 @@ func registerUser(req *api.RegistrationRequest, registrar string, ca *CA, ctx *s
normalizeRegistrationRequest(req, registrarUser)

// Check the permissions of member named 'registrar' to perform this registration
err = canRegister(registrarUser, req, ctx)
err = canRegister(registrarUser, req, ca, ctx)
if err != nil {
log.Debugf("Registration of '%s' failed: %s", req.Name, err)
return "", err
Expand All @@ -86,7 +92,7 @@ func registerUser(req *api.RegistrationRequest, registrar string, ca *CA, ctx *s
return "", errors.WithMessage(err, fmt.Sprintf("Registration of '%s' failed", req.Name))
}
// Set the location header to the URI of the identity that was created by the registration request
ctx.resp.Header().Set("Location", fmt.Sprintf("%sidentities/%s", apiPathPrefix, url.PathEscape(req.Name)))
ctx.GetResp().Header().Set("Location", fmt.Sprintf("%sidentities/%s", apiPathPrefix, url.PathEscape(req.Name)))
return secret, nil
}

Expand All @@ -105,7 +111,7 @@ func normalizeRegistrationRequest(req *api.RegistrationRequest, registrar spi.Us
}
}

func validateAffiliation(req *api.RegistrationRequest, ctx *serverRequestContextImpl) error {
func validateAffiliation(req *api.RegistrationRequest, ca *CA, ctx ServerRequestContext) error {
affiliation := req.Affiliation
log.Debugf("Validating affiliation: %s", affiliation)
err := ctx.ContainsAffiliation(affiliation)
Expand All @@ -118,7 +124,7 @@ func validateAffiliation(req *api.RegistrationRequest, ctx *serverRequestContext
return nil
}

_, err = ctx.ca.registry.GetAffiliation(affiliation)
_, err = ca.registry.GetAffiliation(affiliation)
if err != nil {
return errors.WithMessage(err, fmt.Sprintf("Failed getting affiliation '%s'", affiliation))
}
Expand Down Expand Up @@ -171,15 +177,15 @@ func registerUserID(req *api.RegistrationRequest, ca *CA) (string, error) {
return req.Secret, nil
}

func canRegister(registrar spi.User, req *api.RegistrationRequest, ctx *serverRequestContextImpl) error {
func canRegister(registrar spi.User, req *api.RegistrationRequest, ca *CA, ctx ServerRequestContext) error {
log.Debugf("canRegister - Check to see if user '%s' can register", registrar.GetName())

err := ctx.CanActOnType(req.Type)
if err != nil {
return err
}
// Check that the affiliation requested is of the appropriate level
err = validateAffiliation(req, ctx)
err = validateAffiliation(req, ca, ctx)
if err != nil {
return fmt.Errorf("Registration of '%s' failed in affiliation validation: %s", req.Name, err)
}
Expand Down
11 changes: 11 additions & 0 deletions lib/serverregister_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/hyperledger/fabric-ca/api"
"github.com/hyperledger/fabric-ca/lib/attr"
"github.com/hyperledger/fabric-ca/lib/caerrors"
"github.com/hyperledger/fabric-ca/lib/mocks"
"github.com/hyperledger/fabric-ca/util"
"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -525,3 +526,13 @@ func TestAffiliationAndTypeCheck(t *testing.T) {
err = srv.Stop()
assert.NoError(t, err, "Failed to start server")
}

func TestRegisterWithLDAP(t *testing.T) {
ctxMock := new(mocks.ServerRequestContext)
ctxMock.On("ReadBody", &api.RegistrationRequestNet{}).Return(nil)
ctxMock.On("TokenAuthentication").Return("", nil)
ctxMock.On("IsLDAPEnabled").Return(true)

_, err := register(ctxMock, &CA{})
util.ErrorContains(t, err, "72", "Failed to get back write error for registering identities with LDAP")
}
8 changes: 8 additions & 0 deletions lib/serverrequestcontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ type ServerRequestContext interface {
GetBoolQueryParm(name string) (bool, error)
GetResp() http.ResponseWriter
GetCertificates(server.CertificateRequest, string) (*sqlx.Rows, error)
IsLDAPEnabled() bool
ReadBody(interface{}) error
ContainsAffiliation(string) error
CanActOnType(string) error
}

// serverRequestContextImpl represents an HTTP request/response context in the server
Expand Down Expand Up @@ -703,6 +707,10 @@ func (ctx *serverRequestContextImpl) GetCAConfig() *CAConfig {
return ctx.ca.Config
}

func (ctx *serverRequestContextImpl) IsLDAPEnabled() bool {
return ctx.ca.Config.LDAP.Enabled
}

func convertAttrReqs(attrReqs []*api.AttributeRequest) []attrmgr.AttributeRequest {
rtn := make([]attrmgr.AttributeRequest, len(attrReqs))
for i := range attrReqs {
Expand Down
4 changes: 4 additions & 0 deletions scripts/fvt/ldap_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ for u in ${users1[*]}; do
checkUserCert $u
done

$FABRIC_CA_CLIENTEXEC register -d -u "$PROTO${CA_HOST_ADDRESS}:$PROXY_PORT" $TLSOPT \
--id.name "testldapuser" \
-c /tmp/ldap/users/testUser8/fabric-ca-client-config.yaml 2>&1 | egrep "Registration is not supported when using LDAP"
test $? -ne 0 && ErrorExit "Registration while using LDAP should have failed"
# Sleep for more than the idle connection timeout limit of 1 second
sleep 3

Expand Down

0 comments on commit 785ebd6

Please sign in to comment.