Add configurable Search DNs for Users, Groups and Roles #72
Conversation
Also update to more modern baton patterns like a configuration pkg.
WalkthroughThe pull request introduces significant updates to the Go module and various components of the LDAP connector. The Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (8)
pkg/connector/group_test.go (2)
12-77: Enhance test robustness and readabilityThe
TestGroupGrantRevokefunction is well-structured and covers various scenarios. However, consider the following improvements:
Add more descriptive error messages to assertions. For example:
require.Len(t, groups, 2, "Expected 2 groups to be returned")Replace magic numbers with named constants or variables. For instance:
const expectedGroupCount = 2 require.Len(t, groups, expectedGroupCount, "Unexpected number of groups")Consider parameterizing the test to make it more flexible. This could involve creating a test table with different LDIF files and expected outcomes.
These changes will improve the test's maintainability and make it easier to understand and debug if it fails.
79-87: Enhancepluckfunction with documentation and improved return valueThe
pluckfunction is well-implemented, but consider the following improvements:
Add a brief documentation comment explaining the function's purpose and behavior:
// pluck returns the first element in the slice that satisfies the given predicate function. // If no element satisfies the predicate, it returns the zero value of type T.Consider modifying the function to return a boolean along with the value, indicating whether a match was found:
func pluck[T any](slice []T, fn func(v T) bool) (T, bool) { for _, v := range slice { if fn(v) { return v, true } } var emptyT T return emptyT, false }This change would allow callers to distinguish between a successful match and a zero value returned due to no match.
These changes will improve the function's usability and make it clearer to callers when no match is found.
pkg/connector/connector.go (1)
83-86: Ensure sensitive information is not loggedWhile logging the creation of a new LDAP connector, verify that no sensitive information, such as passwords, is included in the logs. Currently, the logged fields are:
l.Debug("creating new LDAP connector", zap.Stringer("server_url", cf.ServerURL), zap.Stringer("bind_dn", cf.BindDN), zap.Bool("disable_operational_attrs", cf.DisableOperationalAttrs))Double-check that all logged fields are safe to include in logs.
pkg/config/config.go (1)
29-29: Consider reformatting code instead of disabling linter ruleInstead of disabling the
line-length-limitlinting rule, consider reformatting the code to comply with line length guidelines. This helps maintain consistency and readability.Apply this diff to reformat the code:
-disableOperationalAttrsField = field.BoolField("disable-operational-attrs", field.WithDescription("Disable fetching operational attributes. Some LDAP servers don't support these. If disabled, created_at and last login info will not be fetched")) +disableOperationalAttrsField = field.BoolField( + "disable-operational-attrs", + field.WithDescription("Disable fetching operational attributes. Some LDAP servers don't support these. If disabled, created_at and last login info will not be fetched"), +)pkg/ldap/client.go (1)
161-161: Logging improvement for clarity whensearchDNis nilConsider adding a conditional log message to handle cases when
searchDNisnilto improve log clarity.Suggested change:
l.Debug("searching for ldap entries", - zap.String("search_dn", baseDN), + zap.String("search_dn", baseDN != "" ? baseDN : "root DSE"), zap.String("filter", filter), zap.Strings("attrNames", attrNames))pkg/connector/connector_test.go (2)
18-18: Remove redundant import of "embed".The package
"embed"is imported twice—once at line 8 and again at line 18 with a blank import (_ "embed"). The second import is unnecessary and can be removed to clean up the code.Apply this diff to remove the redundant import:
- _ "embed"
79-79: Use a more descriptive variable name instead ofsux.The variable name
suxis not descriptive of its purpose and could be confusing. Consider renaming it toparsedURLorserverURLParsedfor better code readability and maintainability.Apply this diff to rename the variable:
-sux, err := url.Parse(serverURL) +parsedURL, err := url.Parse(serverURL)Update the reference in the configuration:
cf := &config.Config{ - ServerURL: sux, + ServerURL: parsedURL,pkg/connector/role.go (1)
31-31: Consider adding documentation forroleSearchDN.Adding a comment for the new
roleSearchDNfield in theroleResourceTypestruct will enhance code readability and help other developers understand its purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (290)
cmd/baton-ldap/config.gois excluded by none and included by nonecmd/baton-ldap/main.gois excluded by none and included by nonecompose.yamlis excluded by none and included by nonego.sumis excluded by!**/*.sumand included by nonevendor/dario.cat/mergo/.deepsource.tomlis excluded by!vendor/**and included by nonevendor/dario.cat/mergo/.gitignoreis excluded by!vendor/**and included by nonevendor/dario.cat/mergo/.travis.ymlis excluded by!vendor/**and included by nonevendor/dario.cat/mergo/CODE_OF_CONDUCT.mdis excluded by!vendor/**and included by nonevendor/dario.cat/mergo/CONTRIBUTING.mdis excluded by!vendor/**and included by nonevendor/dario.cat/mergo/LICENSEis excluded by!vendor/**and included by nonevendor/dario.cat/mergo/README.mdis excluded by!vendor/**and included by nonevendor/dario.cat/mergo/SECURITY.mdis excluded by!vendor/**and included by nonevendor/dario.cat/mergo/doc.gois excluded by!vendor/**and included by nonevendor/dario.cat/mergo/map.gois excluded by!vendor/**and included by nonevendor/dario.cat/mergo/merge.gois excluded by!vendor/**and included by nonevendor/dario.cat/mergo/mergo.gois excluded by!vendor/**and included by nonevendor/filippo.io/age/README.mdis excluded by!vendor/**and included by nonevendor/filippo.io/age/age.gois excluded by!vendor/**and included by nonevendor/filippo.io/age/scrypt.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/LICENSEis excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/README.mdis excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/constants.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/context.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/csi_entry_state.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/csi_param_state.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/escape_intermediate_state.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/escape_state.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/event_handler.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/ground_state.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/osc_string_state.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/parser.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/parser_action_helpers.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/parser_actions.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/states.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/utilities.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/winterm/ansi.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/winterm/api.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/winterm/attr_translation.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/winterm/cursor_helpers.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/winterm/erase_helpers.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/winterm/scroll_helper.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/winterm/utilities.gois excluded by!vendor/**and included by nonevendor/github.com/Azure/go-ansiterm/winterm/win_event_handler.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/.gitattributesis excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/.gitignoreis excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/.golangci.ymlis excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/CODEOWNERSis excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/LICENSEis excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/README.mdis excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/SECURITY.mdis excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/backup.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/doc.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/ea.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/file.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/fileinfo.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/hvsock.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/internal/fs/doc.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/internal/fs/fs.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/internal/fs/security.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/internal/fs/zsyscall_windows.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/internal/socket/rawaddr.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/internal/socket/socket.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/internal/socket/zsyscall_windows.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/internal/stringbuffer/wstring.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/pipe.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/pkg/guid/guid.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/pkg/guid/guid_nonwindows.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/pkg/guid/guid_windows.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/pkg/guid/variant_string.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/privilege.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/reparse.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/sd.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/syscall.gois excluded by!vendor/**and included by nonevendor/github.com/Microsoft/go-winio/zsyscall_windows.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/accountid_endpoint_mode.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/config.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/credentials.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/endpoints.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/middleware/private/metrics/metrics.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id_retriever.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/middleware/user_agent.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/retry/attempt_metrics.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/retry/middleware.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/retry/retryable_error.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/headers.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/middleware.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/v4.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/client.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/config/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/config/config.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/config/env_config.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/config/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/config/load_options.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/config/provider.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/config/resolve.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/config/resolve_credentials.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/config/shared_config.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/credentials/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/client.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/provider.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/credentials/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/provider.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_cached_token.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_credentials_provider.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/assume_role_provider.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/web_identity_provider.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/bucket_region.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/download.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/feature/s3/manager/upload.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/v4signer_adapter.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/awsutil/path_value.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/configsources/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/configsources/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/context/context.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partition.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.jsonis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/ini/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/ini/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/middleware/middleware.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/v4a/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/v4a/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/internal/v4a/smithy.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/internal/checksum/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/internal/presigned-url/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/internal/s3shared/metadata_retriever.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_client.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_AbortMultipartUpload.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CompleteMultipartUpload.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CopyObject.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateBucket.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateMultipartUpload.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_CreateSession.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucket.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketAnalyticsConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketCors.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketEncryption.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketIntelligentTieringConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketInventoryConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketLifecycle.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketMetricsConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketOwnershipControls.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketPolicy.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketReplication.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketTagging.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteBucketWebsite.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteObject.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteObjectTagging.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeleteObjects.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_DeletePublicAccessBlock.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketAccelerateConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketAcl.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketAnalyticsConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketCors.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketEncryption.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketIntelligentTieringConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketInventoryConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketLifecycleConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketLocation.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketLogging.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketMetricsConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketNotificationConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketOwnershipControls.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketPolicy.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketPolicyStatus.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketReplication.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketRequestPayment.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketTagging.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketVersioning.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetBucketWebsite.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetObject.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetObjectAcl.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetObjectAttributes.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetObjectLegalHold.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetObjectLockConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetObjectRetention.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetObjectTagging.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetObjectTorrent.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_GetPublicAccessBlock.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_HeadBucket.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_HeadObject.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListBucketAnalyticsConfigurations.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListBucketIntelligentTieringConfigurations.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListBucketInventoryConfigurations.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListBucketMetricsConfigurations.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListBuckets.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListDirectoryBuckets.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListMultipartUploads.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListObjectVersions.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListObjects.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListObjectsV2.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_ListParts.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketAccelerateConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketAcl.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketAnalyticsConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketCors.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketEncryption.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketIntelligentTieringConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketInventoryConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketLifecycleConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketLogging.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketMetricsConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketNotificationConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketOwnershipControls.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketPolicy.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketReplication.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketRequestPayment.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketTagging.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketVersioning.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutBucketWebsite.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutObject.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutObjectAcl.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutObjectLegalHold.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutObjectLockConfiguration.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutObjectRetention.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutObjectTagging.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_PutPublicAccessBlock.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_RestoreObject.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_SelectObjectContent.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_UploadPart.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_UploadPartCopy.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/api_op_WriteGetObjectResponse.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/auth.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/deserializers.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/endpoints.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/express_user_agent.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/internal/endpoints/endpoints.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/options.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/presign_post.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/serializers.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/types/enums.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/types/errors.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/types/types.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/s3/validators.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/api_client.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/api_op_GetRoleCredentials.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/api_op_ListAccountRoles.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/api_op_ListAccounts.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/api_op_Logout.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/auth.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/deserializers.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/doc.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/endpoints.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/internal/endpoints/endpoints.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/options.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/serializers.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sso/types/types.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/api_client.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/api_op_CreateToken.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/api_op_CreateTokenWithIAM.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/api_op_RegisterClient.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/api_op_StartDeviceAuthorization.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/auth.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/deserializers.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/doc.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/endpoints.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/go_module_metadata.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/internal/endpoints/endpoints.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/options.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/serializers.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/ssooidc/types/errors.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sts/CHANGELOG.mdis excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sts/api_client.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sts/api_op_AssumeRole.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sts/api_op_AssumeRoleWithSAML.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sts/api_op_AssumeRoleWithWebIdentity.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sts/api_op_DecodeAuthorizationMessage.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sts/api_op_GetAccessKeyInfo.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sts/api_op_GetCallerIdentity.gois excluded by!vendor/**and included by nonevendor/github.com/aws/aws-sdk-go-v2/service/sts/api_op_GetFederationToken.gois excluded by!vendor/**and included by none
📒 Files selected for processing (10)
- go.mod (2 hunks)
- pkg/config/config.go (1 hunks)
- pkg/connector/connector.go (3 hunks)
- pkg/connector/connector_test.go (1 hunks)
- pkg/connector/group.go (9 hunks)
- pkg/connector/group_test.go (1 hunks)
- pkg/connector/role.go (6 hunks)
- pkg/connector/testfixtures/simple..ldif (1 hunks)
- pkg/connector/user.go (4 hunks)
- pkg/ldap/client.go (3 hunks)
🧰 Additional context used
🪛 GitHub Check: go-lint
pkg/connector/connector_test.go
[failure] 51-51:
unhandled-error: Unhandled error in call to function os.File.Close (revive)
[failure] 53-53:
unhandled-error: Unhandled error in call to function os.Remove (revive)
🔇 Additional comments (33)
pkg/connector/testfixtures/simple..ldif (7)
1-2: LGTM: File header and version are correctly defined.The comment clearly describes the purpose of the file, and the version declaration is correct for the LDIF format.
4-7: LGTM: Groups organizational unit is correctly defined.The entry for the 'groups' organizational unit is well-structured with the correct DN, objectClasses, and 'ou' attribute.
9-13: LGTM: Staff group is correctly defined.The 'staff' group entry is well-structured with the correct DN, objectClasses, and required attributes ('cn' and 'gidnumber').
15-20: LGTM: Test group is correctly defined with a member reference.The 'test' group entry is well-structured with the correct DN, objectClasses, and required attributes. The 'memberUid' attribute correctly references the user 'roger', which is defined later in the file.
22-26: Clarify the purpose of the commented-out users OU entry.The users organizational unit entry is correctly formatted but commented out. This might cause issues with the user entry that follows, as it references this OU in its DN. Could you please clarify why this entry is commented out and if it should be uncommented?
28-41: LGTM: User entry is well-defined, but verify OU existence.The user entry for Roger Rabbit is correctly structured with appropriate objectClasses and attributes. The password is properly hashed. However, please ensure that the 'ou=users,dc=example,dc=org' organizational unit exists, as it's referenced in the user's DN but is commented out earlier in the file.
1-41: Overall, the LDIF file is well-structured but requires clarification on the users OU.The LDIF file is well-formatted and contains appropriate entries for testing LDAP operations. It defines organizational units, groups, and a user entry with correct attributes and object classes. However, the commented-out users organizational unit (lines 22-26) needs attention, as it's referenced in the user entry's DN. Please clarify if this OU should be uncommented or if there's a specific reason for its current state.
To ensure the LDIF file's validity, you can use the
ldapmodifycommand with the-c(continuous operation mode) and-n(show what would be done) options:This will perform a dry run and highlight any potential issues in the LDIF file structure.
go.mod (5)
Line range hint
1-136: Summary of go.mod changes: Comprehensive update with potential impactsThis update to
go.modintroduces several significant changes:
- Go version upgrade to 1.22.0
- Major update to baton-sdk (v0.2.41)
- Introduction of new test container dependencies
- Comprehensive update of indirect dependencies, including AWS SDK consolidation
While these updates are generally positive for security, performance, and feature enhancements, they also introduce potential risks:
- The Go version upgrade may introduce breaking changes.
- The baton-sdk update may require code adjustments.
- New test dependencies may require infrastructure changes.
- AWS SDK consolidation may affect existing AWS-related functionality.
To ensure a smooth transition with these changes:
- Review the Go 1.22 release notes and test thoroughly.
- Check the baton-sdk changelog and update usage if necessary.
- Set up and verify the new test container environment.
- Test all AWS-related functionality with the consolidated SDK.
- Run comprehensive tests across the entire codebase.
Consider creating a checklist or issue to track the verification of each major change to ensure nothing is overlooked during the integration of these updates.
8-8: Verify compatibility with updated baton-sdkThe
github.com/conductorone/baton-sdkhas been updated to v0.2.41. This update may introduce new features, changes, or bug fixes that could affect your project.
- Review the changelog or release notes for
baton-sdkto understand the changes introduced in this version.- Ensure that your code is compatible with any API changes or new features in the updated SDK.
Other dependency updates appear to be minor version bumps or patch updates, which are generally safe but should still be tested:
github.com/jackc/puddle/v2to v2.2.2golang.org/x/expto a newer commitgoogle.golang.org/protobufto v1.35.1To verify the impact of these updates, especially the baton-sdk, you can run:
#!/bin/bash # Check for any compilation issues go build ./... # Run all tests to ensure compatibility go test ./... # If there are integration tests specifically for baton-sdk functionality # go test -tags=integration ./...Also applies to: 11-11, 15-15, 16-16
12-13: New test dependencies added: Verify test setupTwo new test-related dependencies have been added:
github.com/testcontainers/testcontainers-go v0.33.0github.com/testcontainers/testcontainers-go/modules/openldap v0.33.0These additions suggest that containerized tests, particularly for LDAP functionality, are being introduced or enhanced in the project.
- Ensure that the necessary infrastructure (Docker) is available in your development and CI environments to support these test containers.
- Review any new test files that utilize these dependencies to understand the new testing approach.
- Verify that the OpenLDAP module is correctly configured for your LDAP testing needs.
To verify the setup and usage of these new test dependencies:
#!/bin/bash # Check for test files using the new dependencies grep -R "testcontainers" . # If found, run the relevant tests # go test -v ./path/to/test/package
Line range hint
20-136: Comprehensive indirect dependency update: Note AWS SDK changesA significant update has been made to the AWS SDK and numerous other indirect dependencies:
AWS SDK consolidation:
- The AWS SDK has been consolidated into a single entry
github.com/aws/aws-sdk-go-v2 v1.32.2(line 62).- Various AWS service-specific packages have been updated (lines 63-80).
Other notable updates include:
github.com/klauspost/compressto v1.17.11github.com/pelletier/go-toml/v2to v2.2.3github.com/shirou/gopsutil/v3to v3.24.5github.com/spf13/viperto v1.19.0golang.org/x/cryptoto v0.28.0golang.org/x/netto v0.30.0modernc.org/libcto v1.61.0modernc.org/sqliteto v1.33.1This comprehensive update of indirect dependencies suggests a thorough dependency refresh, which is generally good for security and performance.
To verify the impact of these updates, especially the AWS SDK changes:
#!/bin/bash # Check for any AWS SDK usage in the codebase grep -R "aws-sdk-go-v2" . # If found, run relevant tests # go test -v ./path/to/aws/related/package # Run all tests to ensure overall compatibility go test ./...Ensure that any AWS-related functionality in your codebase is still working as expected with the consolidated SDK.
3-3: Significant Go version upgrade: Verify compatibilityThe Go version has been updated from 1.21 to 1.22.0, which is a major version upgrade. This change may introduce breaking changes or new features that could affect the codebase.
- Ensure all code is compatible with Go 1.22.0.
- Review the Go 1.22 release notes for any changes that may impact your project.
- Consider running thorough tests to catch any potential issues introduced by this upgrade.
The toolchain version (1.22.3) is appropriately newer than the Go version, which is good for stability.
To verify the impact of this upgrade, you can run:
Also applies to: 5-5
pkg/connector/connector.go (5)
43-44: Good use of centralized configuration in LDAP structAdding the
config *config.Configfield to theLDAPstruct centralizes configuration management, enhancing maintainability and readability.
49-51: Verify the parameters passed toResourceSyncersIn the
ResourceSyncersmethod, please confirm that passingl.config.UserSearchDNas a parameter togroupBuilderis intentional. Specifically, in:groupBuilder(l.client, l.config.GroupSearchDN, l.config.UserSearchDN),Ensure that
l.config.UserSearchDNis required bygroupBuilder. If not, it might be an oversight and could lead to unexpected behavior.
67-68: Confirm the changes in LDAP search scope inValidatemethodThe LDAP search scope has been changed to
ldap3.ScopeBaseObjectwith attributes set tonil. Please verify that this modification still effectively validates the LDAP connection as intended.
75-75: Improved error message enhances clarityUpdating the error message to:
return nil, fmt.Errorf("ldap-connector: failed to validate connection: %w", err)provides a clearer indication of the failure during the connection validation process.
98-99: Proper initialization of the LDAP structThe returned
LDAPstruct correctly initializes both theclientandconfigfields:return &LDAP{ client: ldapClient, config: cf, }, nilThis ensures that all necessary components are available for the connector's operation.
pkg/ldap/client.go (2)
122-127: Enhancement ofLdapSearchfunction signatureThe addition of
searchScopeandsearchDNparameters to theLdapSearchmethod increases the flexibility of LDAP search operations, allowing for more granular control over the search scope and distinguished name.
156-159: Verify the handling of a nilsearchDNWhen
searchDNisnil,baseDNremains an empty string. Depending on your LDAP server configuration, an emptyBaseDNmay refer to the root DSE or might not be acceptable. Please verify that this behavior aligns with your intended use and that it will function correctly in all target environments.pkg/connector/group.go (12)
5-5: Addition of 'errors' package for enhanced error handling.Importing the
errorspackage is appropriate given the additional error handling implemented in the code.
39-42: Inclusion of 'groupSearchDN' and 'userSearchDN' in 'groupResourceType'.Adding
groupSearchDNanduserSearchDNfields improves the configurability and flexibility of LDAP searches within the group resource type.
88-89: Verify performance implications of setting search scope to 'ldap3.ScopeWholeSubtree'.Changing the search scope to
ldap3.ScopeWholeSubtreewhile usingg.groupSearchDNas the search base may impact performance if the subtree contains many entries. Ensure thatg.groupSearchDNis appropriately scoped to limit unnecessary searches.
96-96: Enhanced error message with search DN information.Including
g.groupSearchDNin the error message provides clearer context for debugging.
156-159: Improved error handling for invalid group DNs in 'Grants' method.Adding error handling for invalid group DNs increases the robustness of the
Grantsmethod by catching and reporting parsing errors promptly.
185-186: Appropriate use of 'ldap3.ScopeBaseObject' in group search.Using
ldap3.ScopeBaseObjectfor searching the specific group DN is suitable and efficient.
196-197: Effective application of group ID filter in group search.Applying a group ID filter enhances the precision of the search, ensuring that the correct group is retrieved.
236-237: Confirm performance impact of 'ldap3.ScopeWholeSubtree' in user search.Setting the search scope to
ldap3.ScopeWholeSubtreefor user searches may affect performance ifg.userSearchDNcovers a large subtree. Ensure thatg.userSearchDNis as specific as necessary to optimize search performance.
262-265: Improved error handling in 'getGroup' method for invalid DNs.Checking for invalid group DNs enhances error reporting and prevents potential runtime errors.
269-270: Use of 'ldap3.ScopeBaseObject' in group retrieval.Limiting the search scope to
ldap3.ScopeBaseObjectefficiently retrieves the specific group entry.
362-367: Graceful handling of 'LDAPResultNoSuchAttribute' error in 'Revoke' method.By specifically checking for
LDAPResultNoSuchAttribute, the method avoids returning errors when the attribute to delete does not exist, which is acceptable behavior for idempotent operations.
374-379: Modification of 'groupBuilder' to accept search DNs enhances configurability.Allowing
groupBuilderto receivegroupSearchDNanduserSearchDNas parameters increases flexibility in specifying search bases for groups and users.pkg/connector/role.go (2)
209-213: Ensure all calls toroleBuilderare updated with the new parameter.With the addition of
roleSearchDNto theroleBuilderfunction signature, verify that all invocations of this function provide the new parameter to prevent build errors.Run the following script to find and review all usages of
roleBuilder:✅ Verification successful
All
roleBuilderinvocations correctly include the newroleSearchDNparameter.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all calls to roleBuilder and check for correct parameters. # Test: Search for roleBuilder function calls. Expect: All calls include the new parameter. rg --type go 'roleBuilder\(' -A 1Length of output: 535
74-75: Verify the impact of changing the search scope and base DN.Changing the search scope to
ldap3.ScopeWholeSubtreeand usingr.roleSearchDNas the search base could affect performance and search results, especially in large directories. Ensure that this change is intended and that it doesn't introduce unintended side effects.Run the following script to check the number of entries returned with the new search parameters:
Replace
<<roleSearchDN>>with the actual DN value used in your configuration.
pkg/config/config.go
Outdated
| // depreciated: use userBindDNField | ||
| userDNField = field.StringField("user-dn", field.WithDescription("The user DN to bind to the LDAP server (depreciated, use user-bind-dn)"), field.WithHidden(true)) |
There was a problem hiding this comment.
Correct misspelling of "depreciated" to "deprecated"
The word "depreciated" in the comment and field description should be "deprecated".
Apply this diff to fix the typo:
-// depreciated: use userBindDNField
+// deprecated: use userBindDNField
-userDNField = field.StringField("user-dn", field.WithDescription("The user DN to bind to the LDAP server (depreciated, use user-bind-dn)"), field.WithHidden(true))
+userDNField = field.StringField("user-dn", field.WithDescription("The user DN to bind to the LDAP server (deprecated, use user-bind-dn)"), field.WithHidden(true))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // depreciated: use userBindDNField | |
| userDNField = field.StringField("user-dn", field.WithDescription("The user DN to bind to the LDAP server (depreciated, use user-bind-dn)"), field.WithHidden(true)) | |
| // deprecated: use userBindDNField | |
| userDNField = field.StringField("user-dn", field.WithDescription("The user DN to bind to the LDAP server (deprecated, use user-bind-dn)"), field.WithHidden(true)) |
pkg/config/config.go
Outdated
| // depreciated: use urlField | ||
| domainField = field.StringField("domain", field.WithDescription(`The fully-qualified LDAP domain to connect to. Example: "baton.example.com (depreciated, use url"`), field.WithHidden(true)) |
There was a problem hiding this comment.
Fix typo and string formatting in domainField definition
The word "depreciated" is misspelled; it should be "deprecated". Additionally, there is a mismatched quotation mark and missing closing parenthesis in the field description.
Apply this diff to correct the spelling and fix the string formatting:
-// depreciated: use urlField
+// deprecated: use urlField
-domainField = field.StringField("domain", field.WithDescription(`The fully-qualified LDAP domain to connect to. Example: "baton.example.com (depreciated, use url"`), field.WithHidden(true))
+domainField = field.StringField("domain", field.WithDescription(`The fully-qualified LDAP domain to connect to. Example: "baton.example.com" (deprecated, use url)`), field.WithHidden(true))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // depreciated: use urlField | |
| domainField = field.StringField("domain", field.WithDescription(`The fully-qualified LDAP domain to connect to. Example: "baton.example.com (depreciated, use url"`), field.WithHidden(true)) | |
| // deprecated: use urlField | |
| domainField = field.StringField("domain", field.WithDescription(`The fully-qualified LDAP domain to connect to. Example: "baton.example.com" (deprecated, use url)`), field.WithHidden(true)) |
| func groupBuilder(client *ldap.Client, groupSearchDN *ldap3.DN, | ||
| userSearchDN *ldap3.DN) *groupResourceType { |
There was a problem hiding this comment.
Ensure non-nil 'groupSearchDN' and 'userSearchDN' in 'groupBuilder'.
Passing nil values for groupSearchDN or userSearchDN could lead to runtime nil pointer dereferences during LDAP searches. Consider adding validation to ensure these parameters are not nil.
Apply this diff to add validation:
func groupBuilder(client *ldap.Client, groupSearchDN *ldap3.DN,
userSearchDN *ldap3.DN) *groupResourceType {
+ if groupSearchDN == nil || userSearchDN == nil {
+ panic("groupSearchDN and userSearchDN must not be nil")
+ }
return &groupResourceType{
groupSearchDN: groupSearchDN,
userSearchDN: userSearchDN,
resourceType: resourceTypeGroup,
client: client,
}
}Alternatively, handle the nil case gracefully or return an error to prevent potential nil pointer dereferences.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func groupBuilder(client *ldap.Client, groupSearchDN *ldap3.DN, | |
| userSearchDN *ldap3.DN) *groupResourceType { | |
| func groupBuilder(client *ldap.Client, groupSearchDN *ldap3.DN, | |
| userSearchDN *ldap3.DN) *groupResourceType { | |
| if groupSearchDN == nil || userSearchDN == nil { | |
| panic("groupSearchDN and userSearchDN must not be nil") | |
| } | |
| return &groupResourceType{ | |
| groupSearchDN: groupSearchDN, | |
| userSearchDN: userSearchDN, | |
| resourceType: resourceTypeGroup, | |
| client: client, | |
| } | |
| } |
| dnx, err := ldap3.ParseDN(dn) | ||
| if err != nil { | ||
| return nil, "", nil, fmt.Errorf("ldap-connector: failed to get user with dn %s: %w", dn, err) | ||
| return nil, "", nil, fmt.Errorf("ldap-connector: invalid DN in role_members: '%s': %w", dn, err) | ||
| } | ||
|
|
||
| if len(memberEntry) == 0 { | ||
| return nil, "", nil, fmt.Errorf("ldap-connector: failed to find user with dn %s", dn) | ||
| } | ||
|
|
||
| memberCopy := memberEntry | ||
| ur, err := userResource(ctx, memberCopy[0]) | ||
| urId, err := rs.NewResourceID(resourceTypeUser, dnx.String()) | ||
| if err != nil { | ||
| return nil, "", nil, err | ||
| return nil, "", nil, fmt.Errorf("ldap-connector: failed to find user with dn %s", dn) |
There was a problem hiding this comment.
Clarify error messages in the Grants method.
The error message at line 145 implies that the user was not found, but the error originates from creating a new resource ID. Consider updating the error message to accurately reflect the issue.
Apply this diff to improve the error handling:
urId, err := rs.NewResourceID(resourceTypeUser, dnx.String())
if err != nil {
- return nil, "", nil, fmt.Errorf("ldap-connector: failed to find user with dn %s", dn)
+ return nil, "", nil, fmt.Errorf("ldap-connector: failed to create resource ID for user DN '%s': %w", dn, err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dnx, err := ldap3.ParseDN(dn) | |
| if err != nil { | |
| return nil, "", nil, fmt.Errorf("ldap-connector: failed to get user with dn %s: %w", dn, err) | |
| return nil, "", nil, fmt.Errorf("ldap-connector: invalid DN in role_members: '%s': %w", dn, err) | |
| } | |
| if len(memberEntry) == 0 { | |
| return nil, "", nil, fmt.Errorf("ldap-connector: failed to find user with dn %s", dn) | |
| } | |
| memberCopy := memberEntry | |
| ur, err := userResource(ctx, memberCopy[0]) | |
| urId, err := rs.NewResourceID(resourceTypeUser, dnx.String()) | |
| if err != nil { | |
| return nil, "", nil, err | |
| return nil, "", nil, fmt.Errorf("ldap-connector: failed to find user with dn %s", dn) | |
| dnx, err := ldap3.ParseDN(dn) | |
| if err != nil { | |
| return nil, "", nil, fmt.Errorf("ldap-connector: invalid DN in role_members: '%s': %w", dn, err) | |
| } | |
| urId, err := rs.NewResourceID(resourceTypeUser, dnx.String()) | |
| if err != nil { | |
| return nil, "", nil, fmt.Errorf("ldap-connector: failed to create resource ID for user DN '%s': %w", dn, err) |
Summary by CodeRabbit
New Features
Bug Fixes
Chores