-
Notifications
You must be signed in to change notification settings - Fork 55
fix: enforce user auth check third party service credentials #730
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
fix: enforce user auth check third party service credentials #730
Conversation
Implemented a permission check in the resolve_credentials method of ProviderCredentialsType to ensure that only authorized users can access integration credentials based on their organization.
…tations Implemented a check in multiple sync mutation classes to ensure that the provided credentials belong to the correct organization, raising an error if they do not. This enhances security by preventing unauthorized access to resources.
…multiple resolvers Enhanced security by implementing permission checks in the resolvers for Cloudflare, AWS, GitHub, and GitLab credentials. Users must now have the appropriate permissions to access integration credentials based on their organization, raising an error if they do not.
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.
Pull request overview
This PR addresses a security vulnerability (Finding V2) by adding authorization checks before allowing access to third-party integration credentials (AWS keys, GitHub tokens, etc.) through GraphQL resolvers. The fix ensures that users have proper permissions before retrieving or using sensitive credential data.
Key Changes:
- Added
user_has_permissionchecks to credential field resolvers and query resolvers - Added organization validation to sync creation mutations
- Enforces "read" permission for "IntegrationCredentials" resource before exposing credential data
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| backend/backend/graphene/types.py | Added permission check to resolve_credentials field resolver, returning None when unauthorized |
| backend/backend/graphene/queries/syncing.py | Added permission checks to query resolvers that retrieve credentials for Cloudflare Pages, AWS Secrets Manager, GitHub repos, and GitLab projects |
| backend/backend/graphene/mutations/syncing.py | Added organization membership validation to sync creation mutations for various providers (AWS, GitLab, GitHub, Cloudflare, etc.) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🔍 Overview
Enforce authorization checks for Integration Credential retrieval in GraphQL.
💡 Proposed Changes
Updated the
resolve_credentialsmethod within theProviderCredentialsTypeGraphQL resolver to validate that the requesting user has the necessary permissions before returning decrypted third-party credentials (e.g., AWS keys, GitHub tokens).Previously, the resolver returned credentials for any
ProviderCredentialsobject found by ID without verifying the caller's organization-level permissions for theIntegrationCredentialsresource. The fix now explicitly callsuser_has_permission(info.context.user, "read", "IntegrationCredentials", self.organisation).This addresses security finding V2 from the Kolega.dev report.
📝 Release Notes
➕ Additional Context
Severity Dispute
The original report flagged this finding as Critical. However, the practical severity is assessed as Medium for the following reasons:
ProviderCredentialsuse Version 4 UUIDs. These are non-enumerable and have high entropy, making it practically impossible for an attacker to guess the ID of credentials belonging to another organization.get_credentialsremains restricted to backend use, and this change specifically patches the client-facing GraphQL entry point to ensure proper access control.Finding reported by kolega.dev.
🧪 Testing
credentialsfield for aProviderCredentialsobject belonging to Organization B (using a known ID).null(or raises an error depending on schema configuration) instead of the decrypted credential object.💚 Did You...