-
Notifications
You must be signed in to change notification settings - Fork 391
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 <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
Signed-off-by: Keval Mahajan <[email protected]>
|
@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 <[email protected]> * error message changes Signed-off-by: Keval Mahajan <[email protected]> * check gateway uniqueness while updating too Signed-off-by: Keval Mahajan <[email protected]> * linting Signed-off-by: Keval Mahajan <[email protected]> * code linting Signed-off-by: Keval Mahajan <[email protected]> * added alembic migration script for removal of url uniquess constraint Signed-off-by: Keval Mahajan <[email protected]> * lints Signed-off-by: Keval Mahajan <[email protected]> * updated doctest Signed-off-by: Keval Mahajan <[email protected]> * updated test cases Signed-off-by: Keval Mahajan <[email protected]> * removed ununsed import Signed-off-by: Keval Mahajan <[email protected]> * updated docstring Signed-off-by: Keval Mahajan <[email protected]> --------- Signed-off-by: Keval Mahajan <[email protected]>
|
@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)