Skip to content

Commit

Permalink
more auth handlers for audit log (#373)
Browse files Browse the repository at this point in the history 
* more auth handlers for audit log

* minor optimization
  • Loading branch information
@irshadaj
irshadaj authored Jan 31, 2024
1 parent 3902523 commit d744c31
Show file tree
Hide file tree
Showing 6 changed files with 202 additions and 46 deletions.
10 changes: 5 additions & 5 deletions cmd/api/src/api/v2/auth/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ func (s ManagementResource) CreateUser(response http.ResponseWriter, request *ht
}
}

if newUser, err := s.db.CreateUser(userTemplate); err != nil {
if newUser, err := s.db.CreateUser(request.Context(), userTemplate); err != nil {
api.HandleDatabaseError(request, response, err)
} else {
api.WriteBasicResponse(request.Context(), newUser, http.StatusOK, response)
Expand All @@ -493,9 +493,9 @@ func (s ManagementResource) updateUser(response http.ResponseWriter, request *ht
}
}

func (s ManagementResource) ensureUserHasNoAuthSecret(context ctx.Context, user model.User) error {
func (s ManagementResource) ensureUserHasNoAuthSecret(ctx context.Context, user model.User) error {
if user.AuthSecret != nil {
if err := s.db.DeleteAuthSecret(*user.AuthSecret); err != nil {
if err := s.db.DeleteAuthSecret(ctx, *user.AuthSecret); err != nil {
return api.FormatDatabaseError(err)
} else {
return nil
Expand Down Expand Up @@ -549,7 +549,7 @@ func (s ManagementResource) UpdateUser(response http.ResponseWriter, request *ht
// We're setting a SAML provider. If the user has an associated secret the secret will be removed.
if samlProviderID, err := serde.ParseInt32(updateUserRequest.SAMLProviderID); err != nil {
api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, fmt.Sprintf("SAML Provider ID must be a number: %v", err.Error()), request), response)
} else if err := s.ensureUserHasNoAuthSecret(context, user); err != nil {
} else if err := s.ensureUserHasNoAuthSecret(request.Context(), user); err != nil {
api.HandleDatabaseError(request, response, err)
} else if provider, err := s.db.GetSAMLProvider(samlProviderID); err != nil {
api.HandleDatabaseError(request, response, err)
Expand Down Expand Up @@ -602,7 +602,7 @@ func (s ManagementResource) DeleteUser(response http.ResponseWriter, request *ht
api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, api.ErrorResponseDetailsIDMalformed, request), response)
} else if user, err = s.db.GetUser(userID); err != nil {
api.HandleDatabaseError(request, response, err)
} else if err := s.db.DeleteUser(user); err != nil {
} else if err := s.db.DeleteUser(request.Context(), user); err != nil {
api.HandleDatabaseError(request, response, err)
} else {
response.WriteHeader(http.StatusOK)
Expand Down
165 changes: 154 additions & 11 deletions cmd/api/src/api/v2/auth/auth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func TestManagementResource_EnableUserSAML(t *testing.T) {
mockDB.EXPECT().GetUser(goodUserID).Return(model.User{}, nil)
mockDB.EXPECT().GetSAMLProvider(samlProviderID).Return(model.SAMLProvider{}, nil).Times(2)
mockDB.EXPECT().UpdateUser(gomock.Any(), gomock.Any()).Return(nil).Times(2)
mockDB.EXPECT().DeleteAuthSecret(gomock.Any()).Return(nil)
mockDB.EXPECT().DeleteAuthSecret(gomock.Any(), gomock.Any()).Return(nil)

// Happy path
test.Request(t).
Expand Down Expand Up @@ -1057,7 +1057,7 @@ func TestCreateUser_Failure(t *testing.T) {
mockDB.EXPECT().GetRoles(badRole).Return(model.Roles{}, fmt.Errorf("db error"))
mockDB.EXPECT().GetRoles(gomock.Not(badRole)).Return(model.Roles{}, nil).AnyTimes()
mockDB.EXPECT().AppendAuditLog(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
mockDB.EXPECT().CreateUser(badUser).Return(model.User{}, fmt.Errorf("db error"))
mockDB.EXPECT().CreateUser(gomock.Any(), badUser).Return(model.User{}, fmt.Errorf("db error"))

type Input struct {
Body v2.CreateUserRequest
Expand Down Expand Up @@ -1171,7 +1171,7 @@ func TestCreateUser_Success(t *testing.T) {
}),
}, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{}, nil)
mockDB.EXPECT().CreateUser(gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().CreateUser(gomock.Any(), gomock.Any()).Return(goodUser, nil).AnyTimes()

ctx := context.WithValue(context.Background(), ctx.ValueKey, &ctx.Context{})
input := v2.CreateUserRequest{
Expand Down Expand Up @@ -1224,7 +1224,7 @@ func TestCreateUser_ResetPassword(t *testing.T) {
}),
}, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{}, nil)
mockDB.EXPECT().CreateUser(gomock.Any()).Return(goodUser, nil)
mockDB.EXPECT().CreateUser(gomock.Any(), gomock.Any()).Return(goodUser, nil)

input := struct {
Body v2.CreateUserRequest
Expand Down Expand Up @@ -1297,7 +1297,7 @@ func TestManagementResource_UpdateUser_IDMalformed(t *testing.T) {
}),
}, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{}, nil)
mockDB.EXPECT().CreateUser(gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().CreateUser(gomock.Any(), gomock.Any()).Return(goodUser, nil).AnyTimes()

ctx := context.WithValue(context.Background(), ctx.ValueKey, &ctx.Context{})
input := v2.CreateUserRequest{
Expand Down Expand Up @@ -1360,7 +1360,7 @@ func TestManagementResource_UpdateUser_GetUserError(t *testing.T) {
}),
}, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{}, nil)
mockDB.EXPECT().CreateUser(gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().CreateUser(gomock.Any(), gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().GetUser(gomock.Any()).Return(model.User{}, fmt.Errorf("foo"))

ctx := context.WithValue(context.Background(), ctx.ValueKey, &ctx.Context{})
Expand Down Expand Up @@ -1424,7 +1424,7 @@ func TestManagementResource_UpdateUser_GetRolesError(t *testing.T) {
}),
}, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{}, nil)
mockDB.EXPECT().CreateUser(gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().CreateUser(gomock.Any(), gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().GetUser(gomock.Any()).Return(goodUser, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{}, fmt.Errorf("foo"))

Expand Down Expand Up @@ -1482,7 +1482,7 @@ func TestManagementResource_UpdateUser_SelfDisable(t *testing.T) {
}),
}, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{}, nil)
mockDB.EXPECT().CreateUser(gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().CreateUser(gomock.Any(), gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().GetUser(gomock.Any()).Return(goodUser, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{model.Role{
Name: "admin",
Expand Down Expand Up @@ -1563,7 +1563,7 @@ func TestManagementResource_UpdateUser_LookupActiveSessionsError(t *testing.T) {
}),
}, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{}, nil)
mockDB.EXPECT().CreateUser(gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().CreateUser(gomock.Any(), gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().GetUser(gomock.Any()).Return(goodUser, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{model.Role{
Name: "admin",
Expand Down Expand Up @@ -1644,7 +1644,7 @@ func TestManagementResource_UpdateUser_DBError(t *testing.T) {
}),
}, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{}, nil)
mockDB.EXPECT().CreateUser(gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().CreateUser(gomock.Any(), gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().GetUser(gomock.Any()).Return(goodUser, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{model.Role{
Name: "admin",
Expand Down Expand Up @@ -1702,6 +1702,149 @@ func TestManagementResource_UpdateUser_DBError(t *testing.T) {
require.Equal(t, http.StatusInternalServerError, response.Code)
}

func TestManagementResource_DeleteUser_BadUserID(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

endpoint := "/api/v2/bloodhound-users"
userID := "badUserID"

resources, _ := apitest.NewAuthManagementResource(mockCtrl)

ctx := context.WithValue(context.Background(), ctx.ValueKey, &ctx.Context{})
req, err := http.NewRequestWithContext(ctx, "DELETE", endpoint, nil)
require.Nil(t, err)

req = mux.SetURLVars(req, map[string]string{api.URIPathVariableUserID: userID})
req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String())

rr := httptest.NewRecorder()
handler := http.HandlerFunc(resources.DeleteUser)
handler.ServeHTTP(rr, req)

require.Equal(t, rr.Code, http.StatusBadRequest)
}

func TestManagementResource_DeleteUser_UserNotFound(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

endpoint := "/api/v2/bloodhound-users"

userID, err := uuid.NewV4()
require.Nil(t, err)

resources, mockDB := apitest.NewAuthManagementResource(mockCtrl)
mockDB.EXPECT().GetUser(userID).Return(model.User{}, database.ErrNotFound)

ctx := context.WithValue(context.Background(), ctx.ValueKey, &ctx.Context{})
req, err := http.NewRequestWithContext(ctx, "DELETE", endpoint, nil)
require.Nil(t, err)

req = mux.SetURLVars(req, map[string]string{api.URIPathVariableUserID: userID.String()})
req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String())

rr := httptest.NewRecorder()
handler := http.HandlerFunc(resources.DeleteUser)
handler.ServeHTTP(rr, req)

require.Equal(t, rr.Code, http.StatusNotFound)
}

func TestManagementResource_DeleteUser_GetUserError(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

endpoint := "/api/v2/bloodhound-users"

userID, err := uuid.NewV4()
require.Nil(t, err)

resources, mockDB := apitest.NewAuthManagementResource(mockCtrl)
mockDB.EXPECT().GetUser(userID).Return(model.User{}, fmt.Errorf("foo"))

ctx := context.WithValue(context.Background(), ctx.ValueKey, &ctx.Context{})
req, err := http.NewRequestWithContext(ctx, "DELETE", endpoint, nil)
require.Nil(t, err)

req = mux.SetURLVars(req, map[string]string{api.URIPathVariableUserID: userID.String()})
req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String())

rr := httptest.NewRecorder()
handler := http.HandlerFunc(resources.DeleteUser)
handler.ServeHTTP(rr, req)

require.Equal(t, rr.Code, http.StatusInternalServerError)
}

func TestManagementResource_DeleteUser_DeleteUserError(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

endpoint := "/api/v2/bloodhound-users"

userID, err := uuid.NewV4()
require.Nil(t, err)

user := model.User{
PrincipalName: "good user",
Unique: model.Unique{
ID: userID,
},
}

resources, mockDB := apitest.NewAuthManagementResource(mockCtrl)
mockDB.EXPECT().GetUser(userID).Return(user, nil)
mockDB.EXPECT().DeleteUser(gomock.Any(), user).Return(fmt.Errorf("foo"))

ctx := context.WithValue(context.Background(), ctx.ValueKey, &ctx.Context{})
req, err := http.NewRequestWithContext(ctx, "DELETE", endpoint, nil)
require.Nil(t, err)

req = mux.SetURLVars(req, map[string]string{api.URIPathVariableUserID: userID.String()})
req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String())

rr := httptest.NewRecorder()
handler := http.HandlerFunc(resources.DeleteUser)
handler.ServeHTTP(rr, req)

require.Equal(t, rr.Code, http.StatusInternalServerError)
}

func TestManagementResource_DeleteUser_Success(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

endpoint := "/api/v2/bloodhound-users"

userID, err := uuid.NewV4()
require.Nil(t, err)

user := model.User{
PrincipalName: "good user",
Unique: model.Unique{
ID: userID,
},
}

resources, mockDB := apitest.NewAuthManagementResource(mockCtrl)
mockDB.EXPECT().GetUser(userID).Return(user, nil)
mockDB.EXPECT().DeleteUser(gomock.Any(), user).Return(nil)

ctx := context.WithValue(context.Background(), ctx.ValueKey, &ctx.Context{})
req, err := http.NewRequestWithContext(ctx, "DELETE", endpoint, nil)
require.Nil(t, err)

req = mux.SetURLVars(req, map[string]string{api.URIPathVariableUserID: userID.String()})
req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String())

rr := httptest.NewRecorder()
handler := http.HandlerFunc(resources.DeleteUser)
handler.ServeHTTP(rr, req)

require.Equal(t, rr.Code, http.StatusOK)
}

func TestManagementResource_UpdateUser_Success(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
Expand All @@ -1726,7 +1869,7 @@ func TestManagementResource_UpdateUser_Success(t *testing.T) {
}),
}, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{}, nil)
mockDB.EXPECT().CreateUser(gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().CreateUser(gomock.Any(), gomock.Any()).Return(goodUser, nil).AnyTimes()
mockDB.EXPECT().GetUser(gomock.Any()).Return(goodUser, nil)
mockDB.EXPECT().GetRoles(gomock.Any()).Return(model.Roles{model.Role{
Name: "admin",
Expand Down
37 changes: 25 additions & 12 deletions cmd/api/src/database/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func (s *BloodhoundDB) HasInstallation() (bool, error) {

// CreateUser creates a new user
// INSERT INTO users (...) VALUES (...)
func (s *BloodhoundDB) CreateUser(user model.User) (model.User, error) {
func (s *BloodhoundDB) CreateUser(ctx context.Context, user model.User) (model.User, error) {
updatedUser := user

if newID, err := uuid.NewV4(); err != nil {
Expand All @@ -355,8 +355,13 @@ func (s *BloodhoundDB) CreateUser(user model.User) (model.User, error) {
updatedUser.ID = newID
}

result := s.db.Create(&updatedUser)
return updatedUser, CheckError(result)
auditEntry := model.AuditEntry{
Action: "CreateUser",
Model: &updatedUser,
}
return updatedUser, s.AuditableTransaction(ctx, auditEntry, func(tx *gorm.DB) error {
return CheckError(tx.Create(&updatedUser))
})
}

// UpdateUser updates the roles associated with the user according to the input struct
Expand Down Expand Up @@ -412,18 +417,20 @@ func (s *BloodhoundDB) GetUser(id uuid.UUID) (model.User, error) {

// DeleteUser removes all roles for a given user, thereby revoking all permissions
// UPDATE users SET roles = nil WHERE user_id = ....
func (s *BloodhoundDB) DeleteUser(user model.User) error {
err := s.db.Transaction(func(tx *gorm.DB) error {
func (s *BloodhoundDB) DeleteUser(ctx context.Context, user model.User) error {
auditEntry := model.AuditEntry{
Action: "DeleteUser",
Model: &user,
}

return s.AuditableTransaction(ctx, auditEntry, func(tx *gorm.DB) error {
// Clear associations first
if err := tx.Model(&user).Association("Roles").Clear(); err != nil {
return err
}

result := tx.Delete(&user)
return CheckError(result)
return CheckError(tx.Delete(&user))
})

return err
}

// LookupUser retrieves the User row associated with the provided name. The name is matched against both the
Expand Down Expand Up @@ -571,9 +578,15 @@ func (s *BloodhoundDB) UpdateAuthSecret(ctx context.Context, authSecret model.Au

// DeleteAuthSecret deletes the auth secret row corresponding to the struct specified
// DELETE FROM auth_secrets WHERE user_id = ...
func (s *BloodhoundDB) DeleteAuthSecret(authSecret model.AuthSecret) error {
result := s.db.Delete(&authSecret)
return CheckError(result)
func (s *BloodhoundDB) DeleteAuthSecret(ctx context.Context, authSecret model.AuthSecret) error {
auditEntry := model.AuditEntry{
Action: "DeleteAuthSecret",
Model: &authSecret,
}

return s.AuditableTransaction(ctx, auditEntry, func(tx *gorm.DB) error {
return CheckError(tx.Delete(&authSecret))
})
}

// CreateSAMLProvider creates a new saml_providers row using the data in the input struct
Expand Down
6 changes: 3 additions & 3 deletions cmd/api/src/database/auth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func initAndCreateUser(t *testing.T) (database.Database, model.User) {
}
)

if newUser, err := dbInst.CreateUser(user); err != nil {
if newUser, err := dbInst.CreateUser(context.Background(), user); err != nil {
t.Fatalf("Error creating user: %v", err)
} else {
return dbInst, newUser
Expand Down Expand Up @@ -198,7 +198,7 @@ func TestDatabase_CreateGetUser(t *testing.T) {
)

for _, user := range users {
if _, err := dbInst.CreateUser(user); err != nil {
if _, err := dbInst.CreateUser(context.Background(), user); err != nil {
t.Fatalf("Error creating user: %v", err)
} else if newUser, err := dbInst.LookupUser(user.PrincipalName); err != nil {
t.Fatalf("Failed looking up user by principal %s: %v", user.PrincipalName, err)
Expand Down Expand Up @@ -304,7 +304,7 @@ func TestDatabase_CreateGetDeleteAuthSecret(t *testing.T) {
t.Fatalf("Expected updated auth secret digest to be %s but saw %s", updatedDigest, updatedSecret.Digest)
}

if err := dbInst.DeleteAuthSecret(newSecret); err != nil {
if err := dbInst.DeleteAuthSecret(ctx, newSecret); err != nil {
t.Fatalf("Failed to delete auth token: %v", err)
}
}
Expand Down
Loading

0 comments on commit d744c31

Please sign in to comment.