-
Notifications
You must be signed in to change notification settings - Fork 395
Improved gateway duplication check allowing same url with certain conditions #1424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
|
@kevalmahajan Our company is looking for this feature from long time and would like to access using helm deployment , would you be able to share image tag for helm chart to pull this fix ? |
|
Hi @omprak, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test Summary
All scenarios with different combinations of user, team, visibility, URL ,and credential, were tested.
🧩 Result:
All cases are working as expected — access behavior matches the configured visibility and credential rules.
| S.No | User | Team | Visibility | URL | Credential | Allowed |
|---|---|---|---|---|---|---|
| 1 | admin | dev | public | http://localhost:8001/sse | same | yes |
| 2 | admin | dev | public | http://localhost:8001/sse | same | no |
| 3 | admin | dev | team | http://localhost:8001/sse | same | yes |
| 4 | admin | dev | team | http://localhost:8001/sse | same | no |
| 5 | admin | dev | private | http://localhost:8001/sse | same | yes |
| 6 | admin | dev | private | http://localhost:8001/sse | same | no |
| 7 | rakhibiswas | dev | public | http://localhost:8001/sse | same | no |
| 8 | rakhibiswas | dev | team | http://localhost:8001/sse | same | no |
| 9 | rakhibiswas | dev | private | http://localhost:8001/sse | same | yes |
| 10 | admin | dev | team | http://localhost:8001/sse | different | yes |
| 11 | admin | dev | private | http://localhost:8001/sse | different | yes |
| 12 | admin | dev | public | http://localhost:8001/sse | different | yes |
…ditions (#1424) * imporved duplicated gateway check Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * error message changes Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * check gateway uniqueness while updating too Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * linting Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * code linting Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * added alembic migration script for removal of url uniquess constraint Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * lints Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * updated doctest Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * updated test cases Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * removed ununsed import Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> * updated docstring Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> --------- Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com>
|
@kevalmahajan We are using Helm/Argo to deploy context forge gateway , Would you be able to advise how to use "main" branch code as image id to pull latest changes for this Jira and other blocker Jira -#1412. We would like to replace with image id of Main branch code image: |
🐛 Bug-fix PR
📌 Summary
Closes #1392 (name uniqueness is already present, url uniqueness check is updated)
Implemented visibility-scoped uniqueness validation with credential-aware duplicate detection that handles encrypted authentication data and OAuth configurations, respecting user, team, and public boundaries. This ensures that each URL + credentials combination is unique within its visibility scope while allowing flexibility across different contexts.
Added corresponding alembic script for removing the existing url uniqueness constraint, as it handled in application layer now allowing duplicate url but with additional checks.
🔁 Reproduction Steps
https://internal-api.company.comand credentialsclient_id=devhttps://internal-api.company.combut different credentialsclient_id=testExpected: Different teams should access the same API with their own credentials
Actual: Rejected due to URL-only conflict check
🐞 Root Cause
Restrictive URL-Only Constraint
💡 Fix Description
High-Level Approach
Transformed the validation from "URL must be unique" to "URL + Credentials combination must be unique within scope":
OLD BEHAVIOR:
NEW BEHAVIOR:
✅ Yes - across different users
✅ Yes - across different teams
🧪 Verification
make lintmake testmake coverage📐 MCP Compliance (if relevant)
✅ Checklist
make black isort pre-commit)