Skip to content

Commit

Permalink
[FAB-6068] Update state after all checks done
Browse files Browse the repository at this point in the history
User state will now be updated only after the user has
successfully logged in and all the enrollment
checks have been done and resulted in no errors.

Change-Id: I5ab1730f869ba615d306c47599aad1cd622a905e
Signed-off-by: Saad Karim <[email protected]>
  • Loading branch information
Saad Karim committed Sep 18, 2017
1 parent c41f4f1 commit 2dd4f5b
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 10 deletions.
19 changes: 12 additions & 7 deletions lib/dbaccessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,6 @@ func (u *DBUser) GetName() string {

// Login the user with a password
func (u *DBUser) Login(pass string, caMaxEnrollments int) error {
var stateUpdateSQL string
var args []interface{}

log.Debugf("DB: Login user %s with max enrollments of %d and state of %d", u.Name, u.MaxEnrollments, u.State)

// Check the password by comparing to stored hash
Expand Down Expand Up @@ -388,7 +385,18 @@ func (u *DBUser) Login(pass string, caMaxEnrollments int) error {
return errors.Errorf("The identity %s has already enrolled %d times, it has reached its maximum enrollment allowance", u.Name, u.MaxEnrollments)
}

// Not exceeded, so attempt to increment the count
log.Debugf("DB: identity %s successfully logged in", u.Name)

return nil

}

// LoginComplete completes the login process by incrementing the state of the user
func (u *DBUser) LoginComplete() error {
var stateUpdateSQL string
var args []interface{}
var err error

state := u.State + 1
args = append(args, u.Name)
if u.MaxEnrollments == -1 {
Expand Down Expand Up @@ -418,9 +426,6 @@ func (u *DBUser) Login(pass string, caMaxEnrollments int) error {
}

log.Debugf("Successfully incremented state for identity %s to %d", u.Name, state)

log.Debugf("DB: identity %s successfully logged in", u.Name)

return nil

}
Expand Down
5 changes: 5 additions & 0 deletions lib/ldap/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@ func (u *User) Login(password string, caMaxEnrollment int) error {

}

// LoginComplete requires no action on LDAP
func (u *User) LoginComplete() error {
return nil
}

// GetAffiliationPath returns the affiliation path for this user
func (u *User) GetAffiliationPath() []string {
return reverse(strings.Split(u.dn, ","))
Expand Down
10 changes: 9 additions & 1 deletion lib/serverenroll.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,15 @@ func enrollHandler(ctx *serverRequestContext) (interface{}, error) {
if err != nil {
return nil, err
}
return handleEnroll(ctx, id)
resp, err := handleEnroll(ctx, id)
if err != nil {
return nil, err
}
err = ctx.ui.LoginComplete()
if err != nil {
return nil, err
}
return resp, nil
}

// Handle a reenroll request, guarded by token authentication
Expand Down
80 changes: 80 additions & 0 deletions lib/serverenroll_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
Copyright IBM Corp. 2017 All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package lib

import (
"os"
"testing"

"github.com/hyperledger/fabric-ca/api"
"github.com/hyperledger/fabric-ca/util"
"github.com/stretchr/testify/assert"
)

func TestStateUpdate(t *testing.T) {
os.RemoveAll(rootDir)
os.RemoveAll("../testdata/msp")
defer os.RemoveAll(rootDir)
defer os.RemoveAll("../testdata/msp")

var err error
srv := TestGetRootServer(t)

err = srv.Start()
assert.NoError(t, err, "Failed to start server")

client := getTestClient(rootPort)
_, err = client.Enroll(&api.EnrollmentRequest{
Name: "admin",
Secret: "adminpw",
})
assert.NoError(t, err, "Failed to enroll 'admin' user")

registry := srv.CA.DBAccessor()
userInfo, err := registry.GetUserInfo("admin")
assert.NoError(t, err, "Failed to get user 'admin' from database")
// User state should have gotten updated to 1 after a successful enrollment
if userInfo.State != 1 {
t.Error("Incorrect state set for user")
}

// Send bad CSR to cause the enroll to fail but the login to succeed
reqNet := &api.EnrollmentRequestNet{}
reqNet.SignRequest.Request = "badcsr"
body, err := util.Marshal(reqNet, "SignRequest")
assert.NoError(t, err, "Failed to marshal enroll request")

// Send the CSR to the fabric-ca server with basic auth header
post, err := client.newPost("enroll", body)
assert.NoError(t, err, "Failed to create post request")
post.SetBasicAuth("admin", "adminpw")
err = client.SendReq(post, nil)
if assert.Error(t, err, "Should have failed due to bad csr") {
assert.Contains(t, err.Error(), "CSR Decode failed")
}

// State should not have gotten updated because the enrollment failed
userInfo, err = registry.GetUserInfo("admin")
assert.NoError(t, err, "Failed to get user 'admin' from database")
if userInfo.State != 1 {
t.Error("Incorrect state set for user")
}

err = srv.Stop()
assert.NoError(t, err, "Failed to stop server")

}
6 changes: 4 additions & 2 deletions lib/serverrequestcontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/cloudflare/cfssl/signer"
"github.com/hyperledger/fabric-ca/api"
"github.com/hyperledger/fabric-ca/attrmgr"
"github.com/hyperledger/fabric-ca/lib/spi"
"github.com/hyperledger/fabric-ca/lib/tcert"
"github.com/hyperledger/fabric-ca/util"
"github.com/pkg/errors"
Expand All @@ -43,6 +44,7 @@ type serverRequestContext struct {
ca *CA
enrollmentID string
enrollmentCert *x509.Certificate
ui spi.User
body struct {
read bool // true after body is read
buf []byte // the body itself
Expand Down Expand Up @@ -85,12 +87,12 @@ func (ctx *serverRequestContext) BasicAuthentication() (string, error) {
return "", newAuthErr(ErrEnrollDisabled, "Enroll is disabled")
}
// Get the user info object for this user
ui, err := ca.registry.GetUser(username, nil)
ctx.ui, err = ca.registry.GetUser(username, nil)
if err != nil {
return "", newAuthErr(ErrInvalidUser, "Failed to get user: %s", err)
}
// Check the user's password and max enrollments if supported by registry
err = ui.Login(password, caMaxEnrollments)
err = ctx.ui.Login(password, caMaxEnrollments)
if err != nil {
return "", newAuthErr(ErrInvalidPass, "Login failure: %s", err)
}
Expand Down
2 changes: 2 additions & 0 deletions lib/spi/userregistry.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ type User interface {
GetAffiliationPath() []string
// GetAttribute returns the value for an attribute name
GetAttribute(name string) string
// LoginComplete completes the login process by incrementing the state of the user
LoginComplete() error
}

// UserRegistry is the API for retreiving users and groups
Expand Down

0 comments on commit 2dd4f5b

Please sign in to comment.