Skip to content

fix(ers): ldap multi-strategy ers#3117

Merged
c-r33d merged 8 commits into
mainfrom
fix-multi-strategy
Mar 12, 2026
Merged

fix(ers): ldap multi-strategy ers#3117
c-r33d merged 8 commits into
mainfrom
fix-multi-strategy

Conversation

@c-r33d

@c-r33d c-r33d commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

Proposed Changes

1.) Load goldap implementation for backend of ldap provider.
2.) Use interface method with stubs for unit testing.

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

@c-r33d
c-r33d requested review from a team as code owners March 9, 2026 15:03
@github-actions github-actions Bot added the size/m label Mar 9, 2026
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the LDAP multi-strategy entity resolution service by replacing its previous stubbed implementation with a concrete integration of the github.com/go-ldap/ldap/v3 library. The core change involves introducing Backend and Conn interfaces to abstract LDAP client interactions, which not only enables real LDAP functionality but also greatly improves the testability and maintainability of the LDAP provider. This refactoring ensures that the service can now perform actual LDAP lookups while maintaining a flexible architecture for future development and testing.

Highlights

  • LDAP Backend Implementation: The LDAP multi-strategy provider now uses the github.com/go-ldap/ldap/v3 library for its backend implementation, moving away from a stubbed version.
  • Interface-Based Design for Testability: Introduced Backend and Conn interfaces to abstract LDAP client operations, allowing for easier unit testing and the flexibility to swap out implementations.
  • Refactored LDAP Provider: The LDAPProvider has been updated to utilize the new Backend interface for all LDAP-related calls, improving modularity and maintainability.
  • Updated Test Error Checks: Comprehensive tests have been adjusted to reflect the new error messages from the goldap implementation, replacing checks for the old stub error strings.
  • Code Organization and Cleanup: LDAP constants and character validation logic were moved into dedicated files and helper functions, and the previous standalone stub implementation was removed.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • service/entityresolution/integration/multistrategy_comprehensive_test.go
    • Updated error message checks to match the new LDAP implementation, removing references to the old 'stub function' string.
  • service/entityresolution/multi-strategy/providers/ldap/ldap_backend.go
    • Added a new file defining Conn, SearchRequest, Backend, SearchResult, Entry, and Attribute interfaces and structs to abstract LDAP operations.
  • service/entityresolution/multi-strategy/providers/ldap/ldap_backend_go_ldap.go
    • Added a new file providing a concrete implementation of the Backend and Conn interfaces using the github.com/go-ldap/ldap/v3 library.
  • service/entityresolution/multi-strategy/providers/ldap/ldap_constants.go
    • Added a new file defining LDAP scope and dereference alias constants.
  • service/entityresolution/multi-strategy/providers/ldap/ldap_mapper.go
    • Refactored character validation logic by introducing new helper functions isASCIIAlpha and isASCIIAlphaNumeric.
  • service/entityresolution/multi-strategy/providers/ldap/ldap_provider.go
    • Modified the Provider struct to include a Backend interface for LDAP operations.
    • Updated NewProvider to use a new newProviderWithBackend function, injecting the NewGoLDAPBackend implementation.
    • Refactored getConnection, ResolveEntity, and buildSearchFilter methods to use the new Backend interface methods.
    • Removed a commented-out import for the go-ldap/ldap/v3 library, as it is now directly used via the Backend interface.
  • service/entityresolution/multi-strategy/providers/ldap/ldap_stubs.go
    • Refactored the file to provide a stubBackend implementation of the new Backend interface, removing the previous standalone stub types and functions.
  • service/go.mod
    • Added new module dependencies for github.com/go-ldap/ldap/v3 and its transitive dependencies (github.com/Azure/go-ntlmssp, github.com/go-asn1-ber/asn1-ber, github.com/alexbrainman/sspi, github.com/hashicorp/go-uuid, github.com/jcmturner/aescts/v2, github.com/jcmturner/dnsutils/v2, github.com/jcmturner/gofork, github.com/jcmturner/goidentity/v6, github.com/jcmturner/gokrb5/v8, github.com/jcmturner/rpc/v2).
  • service/go.sum
    • Updated module checksums to include the newly added dependencies.
Activity
  • No specific activity (comments, reviews, or progress updates) was provided in the context for this pull request.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


LDAP now connects, Stubs replaced, real search takes flight, Go code finds its path.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request replaces the stub LDAP implementation with a real one using go-ldap/ldap/v3 and introduces a well-executed Backend interface for modularity and testability. However, a critical logic flaw was identified in the LDAP filter construction where parameters are escaped twice, which will cause functional issues and potential denial of service for users with special characters in their JWT claims. It is recommended to ensure escaping is performed exactly once, preferably within the provider's filter building logic. Additionally, consider improving the API design for creating search requests to enhance type safety and maintainability.

Comment thread service/entityresolution/multi-strategy/providers/ldap/ldap_backend.go Outdated
@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

X-Test Failure Report

opentdfplatformBNULXK.dockerbuild

@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 190.123498ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 97.45462ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 381.030538ms
Throughput 262.45 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.212792784s
Average Latency 390.309534ms
Throughput 127.51 requests/second

@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 180.941959ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 96.859252ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 374.043688ms
Throughput 267.35 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.376947899s
Average Latency 391.749803ms
Throughput 126.98 requests/second

@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 184.914099ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 94.784285ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 379.358284ms
Throughput 263.60 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 37.955809389s
Average Latency 377.333109ms
Throughput 131.73 requests/second

@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

X-Test Failure Report

opentdfplatformLIQHES.dockerbuild

@c-r33d c-r33d changed the title FIx ldap multi-strategy ers fix(service): ldap multi-strategy ers Mar 9, 2026
@c-r33d c-r33d changed the title fix(service): ldap multi-strategy ers fix(ers): ldap multi-strategy ers Mar 9, 2026
@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 167.468627ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 86.455785ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 391.074532ms
Throughput 255.71 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 39.852093894s
Average Latency 396.522176ms
Throughput 125.46 requests/second

@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 196.817575ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 123.332144ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 375.429222ms
Throughput 266.36 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 40.741403619s
Average Latency 405.74006ms
Throughput 122.73 requests/second

@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Contributor

jrschumacher
jrschumacher previously approved these changes Mar 12, 2026
@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 194.106976ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 99.976103ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 386.906438ms
Throughput 258.46 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 40.846015261s
Average Latency 406.845874ms
Throughput 122.41 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

@c-r33d
c-r33d requested a review from jrschumacher March 12, 2026 14:14
@c-r33d
c-r33d added this pull request to the merge queue Mar 12, 2026
Merged via the queue into main with commit d3aaf1a Mar 12, 2026
37 checks passed
@c-r33d
c-r33d deleted the fix-multi-strategy branch March 12, 2026 16:13
JBCongdon pushed a commit to JBCongdon/platform that referenced this pull request May 24, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.14.0](opentdf/platform@service/v0.13.0...service/v0.14.0)
(2026-04-21)


### ⚠ BREAKING CHANGES

* **sdk:** reclassify KAS 400 errors — distinguish tamper from
misconfiguration
([opentdf#3166](opentdf#3166))
* **policy:** optional namespace for RRs
([opentdf#3165](opentdf#3165))
* **policy:** Namespace subject mappings and subject condition sets.
([opentdf#3143](opentdf#3143))
* **policy:** Optional namespace on actions protos, NamespacedPolicy
feature flag ([opentdf#3155](opentdf#3155))
* **policy:** add namespaced actions schema and namespace-aware action
queries ([opentdf#3154](opentdf#3154))
* **policy:** only require namespace on GetAction if no id provided
([opentdf#3144](opentdf#3144))
* **policy:** add namespace field to Actions proto
([opentdf#3130](opentdf#3130))
* **policy:** namespace Registered Resources
([opentdf#3111](opentdf#3111))
* **policy:** add namespace field to RegisteredResource proto
([opentdf#3110](opentdf#3110))

### Features

* **authz:** Namespaced policy in decisioning
([opentdf#3226](opentdf#3226))
([0355934](opentdf@0355934))
* **cli:** migrate otdfctl into platform monorepo
([opentdf#3205](opentdf#3205))
([5177bec](opentdf@5177bec))
* fix tracing ([opentdf#3242](opentdf#3242))
([57e5680](opentdf@57e5680))
* **policy:** add GetObligationTrigger RPC
([opentdf#3318](opentdf#3318))
([d68e39d](opentdf@d68e39d))
* **policy:** add namespace field to Actions proto
([opentdf#3130](opentdf#3130))
([bedc9b3](opentdf@bedc9b3))
* **policy:** add namespace field to RegisteredResource proto
([opentdf#3110](opentdf#3110))
([04fd85d](opentdf@04fd85d))
* **policy:** add namespaced actions schema and namespace-aware action
queries ([opentdf#3154](opentdf#3154))
([c0443f1](opentdf@c0443f1))
* **policy:** add sort ListSubjectMappings API
([opentdf#3255](opentdf#3255))
([9d5d757](opentdf@9d5d757))
* **policy:** Add sort support listregisteredresources api
([opentdf#3312](opentdf#3312))
([91a3ff3](opentdf@91a3ff3))
* **policy:** add sort support to ListAttributes API
([opentdf#3223](opentdf#3223))
([ec3312f](opentdf@ec3312f))
* **policy:** add sort support to ListKeyAccessServer
([opentdf#3287](opentdf#3287))
([7fae2d7](opentdf@7fae2d7))
* **policy:** Add sort support to ListNamespaces API
([opentdf#3192](opentdf#3192))
([aac86cd](opentdf@aac86cd))
* **policy:** add sort support to listobligations api
([opentdf#3300](opentdf#3300))
([9221cac](opentdf@9221cac))
* **policy:** add sort support to ListSubjectConditionSets API
([opentdf#3272](opentdf#3272))
([9010f12](opentdf@9010f12))
* **policy:** add SortField proto and update PageRequest for sort
support ([opentdf#3187](opentdf#3187))
([6cf1862](opentdf@6cf1862))
* **policy:** Enforce same namespace when actions referenced downstream
([opentdf#3206](opentdf#3206))
([4b5463a](opentdf@4b5463a))
* **policy:** namespace Registered Resources
([opentdf#3111](opentdf#3111))
([6db1883](opentdf@6db1883))
* **policy:** Namespace subject mappings and condition sets
([opentdf#3172](opentdf#3172))
([6deed50](opentdf@6deed50))
* **policy:** Namespace subject mappings and subject condition sets.
([opentdf#3143](opentdf#3143))
([3006780](opentdf@3006780))
* **policy:** optional namespace for RRs
([opentdf#3165](opentdf#3165))
([8948018](opentdf@8948018))
* **policy:** rollback migration strategy for namespaced actions
([opentdf#3235](opentdf#3235))
([f7e5e01](opentdf@f7e5e01))
* **policy:** Seed existing namespaces with standard actions
([opentdf#3228](opentdf#3228))
([12136b0](opentdf@12136b0))
* **policy:** Seed namespaces with standard actions on creation +
namespaced actions for obligation triggers
([opentdf#3161](opentdf#3161))
([984d76b](opentdf@984d76b))


### Bug Fixes

* **ci:** Upgrade toolchain version to 1.25.8
([opentdf#3116](opentdf#3116))
([e1b7882](opentdf@e1b7882))
* **core:** do not concat slashes directly in url/file paths
([opentdf#3290](opentdf#3290))
([114c2a7](opentdf@114c2a7))
* **deps:** bump github.com/jackc/pgx/v5 from 5.7.5 to 5.9.0 in /service
([opentdf#3316](opentdf#3316))
([017362e](opentdf@017362e))
* **deps:** bump github.com/opentdf/platform/lib/identifier from 0.2.0
to 0.3.0 in /service
([opentdf#3162](opentdf#3162))
([8bc5dcd](opentdf@8bc5dcd))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.16.0 to
0.17.0 in /service
([opentdf#3125](opentdf#3125))
([29fec61](opentdf@29fec61))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.17.0 to
0.21.0 in /service
([opentdf#3220](opentdf#3220))
([e63add2](opentdf@e63add2))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.21.0 to
0.22.0 in /service
([opentdf#3248](opentdf#3248))
([1ebce73](opentdf@1ebce73))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.22.0 to
0.23.0 in /service
([opentdf#3271](opentdf#3271))
([3338b8e](opentdf@3338b8e))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.23.0 to
0.24.0 in /service
([opentdf#3321](opentdf#3321))
([78e6022](opentdf@78e6022))
* **deps:** bump github.com/opentdf/platform/protocol/go from 0.24.0 to
0.25.0 in /service
([opentdf#3333](opentdf#3333))
([3940bf8](opentdf@3940bf8))
* **deps:** bump github.com/opentdf/platform/sdk from 0.13.0 to 0.16.0
in /service ([opentdf#3356](opentdf#3356))
([5617077](opentdf@5617077))
* **deps:** bump
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp from
1.42.0 to 1.43.0 in /service
([opentdf#3282](opentdf#3282))
([046374a](opentdf@046374a))
* **deps:** bump go.opentelemetry.io/otel/sdk from 1.42.0 to 1.43.0 in
/service ([opentdf#3281](opentdf#3281))
([56b33f2](opentdf@56b33f2))
* **deps:** bump google.golang.org/grpc from 1.77.0 to 1.79.3 in
/service ([opentdf#3176](opentdf#3176))
([3289502](opentdf@3289502))
* **deps:** remove direct github.com/docker/docker dependency
([opentdf#3229](opentdf#3229))
([2becb27](opentdf@2becb27))
* **deps:** upgrade testcontainers-go to resolve vulns
([opentdf#3299](opentdf#3299))
([72c6f9b](opentdf@72c6f9b))
* **ers:** include standard JWT claims in claims mode entity resolution
([opentdf#3196](opentdf#3196))
([6d50da1](opentdf@6d50da1))
* **ers:** ldap multi-strategy ers
([opentdf#3117](opentdf#3117))
([d3aaf1a](opentdf@d3aaf1a))
* **policy:** deprecate ListAttributeValues in favor of existing
GetAttribute ([opentdf#3108](opentdf#3108))
([7e17c2d](opentdf@7e17c2d))
* **policy:** make obligation trigger uniqueness client-aware
([opentdf#3114](opentdf#3114))
([9265bc3](opentdf@9265bc3))
* **policy:** omit empty attribute values from create responses
([opentdf#3193](opentdf#3193))
([d298378](opentdf@d298378))
* **policy:** only require namespace on GetAction if no id provided
([opentdf#3144](opentdf#3144))
([10d0c0f](opentdf@10d0c0f))
* **policy:** Optional namespace on actions protos, NamespacedPolicy
feature flag ([opentdf#3155](opentdf#3155))
([c20f039](opentdf@c20f039))
* **policy:** order List* results by created_at
([opentdf#3088](opentdf#3088))
([ea90ac2](opentdf@ea90ac2))
* **sdk:** normalize issuer URL before OIDC discovery
([opentdf#3261](opentdf#3261))
([61f98c9](opentdf@61f98c9))
* **sdk:** reclassify KAS 400 errors — distinguish tamper from
misconfiguration
([opentdf#3166](opentdf#3166))
([f04a385](opentdf@f04a385))
* **sdk:** remove testcontainers from consumer dependency graph
([opentdf#3129](opentdf#3129))
([f17dcdd](opentdf@f17dcdd))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants