Skip to content

Conversation

@Bencheng21
Copy link
Contributor

@Bencheng21 Bencheng21 commented Jun 18, 2025

Description.

Follow doc to add account deprovisioning.

As BB-906 described, account deprovisioning could be disable user login, so Delete() function is used to disable user.

Test.

  1. Before account deprovisioning
image
  1. Run the delete command
image 3. Check if the user is diabled. image

Comment on lines +39 to +40
query := fmt.Sprintf(`
ALTER LOGIN [%s] DISABLE;`, userName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to sanitize the username to prevent sql injection

if err != nil {
return nil, err
}
return nil, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, err
return nil, nil

"go.uber.org/zap"
)

var _ connectorbuilder.ResourceDeleter = (*userPrincipalSyncer)(nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one can check if userPrincipalSyncer implements all the methods in ResourceDeleter`. If not, an error is thrown in compile time.

It acts as a safety check, any changes in ResourceDeleter requires us to change baton-sql-server.
It's good for migration, IMO

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great

}

func (c *Client) DisableUserFromServer(ctx context.Context, userName string) error {
if strings.ContainsAny(userName, "[]\"';") {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sanitize the username here.

@Bencheng21 Bencheng21 merged commit 963fcbd into main Jun 20, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants