fix(services-bff): Fix bff proxy download service aws cognito #16719
Conversation
WalkthroughThe changes introduce a new annotation in the ingress configuration for both Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/services/bff/infra/admin-portal.infra.ts (1)
Line range hint
58-72: Consider documenting the security implications.While disabling global auth is necessary for the download service, it's important to document:
- Why this configuration is needed
- What security measures are in place to protect the endpoints
- Why it's safe to disable in dev/staging but not in prod
Would you like me to help create documentation explaining these security considerations?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
apps/services/bff/infra/admin-portal.infra.ts(1 hunks)charts/islandis/values.dev.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/services/bff/infra/admin-portal.infra.ts (2)
Pattern apps/services/**/*: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (3)
apps/services/bff/infra/admin-portal.infra.ts (1)
Line range hint 58-72: LGTM! The changes align with the PR objective.
The addition of nginx.ingress.kubernetes.io/enable-global-auth: 'false' in dev and staging environments should resolve the AWS Cognito blocking issue for the download service.
Let's verify the ingress configuration in other services to ensure this change doesn't have unintended side effects:
✅ Verification successful
The ingress configuration changes follow established patterns
The changes to disable global auth in dev and staging environments while maintaining proxy buffering settings align with existing patterns in the codebase:
- Auth-related services (auth-admin-api, ids-api, auth-public-api) consistently use
enable-global-auth: falsewith similar proxy buffering configurations - Search indexer service also disables global auth in dev/staging while keeping prod secure
- The proxy buffer settings (8k size, buffering on) match the configurations used in other services
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for similar auth configurations in other services
# Expected: This should be a unique case specific to the download service
# Search for global auth configurations
echo "Checking for global auth configurations in other services:"
rg -l "enable-global-auth" apps/services
# Search for download-related ingress configurations
echo "Checking for download service ingress configurations:"
rg -l "download.*ingress" apps/services
Length of output: 593
Script:
#!/bin/bash
# Let's check the actual auth configurations in these files to understand the pattern
echo "Checking auth configurations in related services:"
rg -B 2 -A 2 "enable-global-auth" apps/services/search-indexer/infra/search-indexer-service.ts apps/services/auth/public-api/infra/auth-public-api.ts apps/services/auth/ids-api/infra/identity-server.ts apps/services/auth/admin-api/infra/auth-admin-api.ts
# Let's also check if there are any proxy buffer configurations in these files
echo -e "\nChecking proxy buffer configurations:"
rg "proxy-buffer" apps/services/search-indexer/infra/search-indexer-service.ts apps/services/auth/public-api/infra/auth-public-api.ts apps/services/auth/ids-api/infra/identity-server.ts apps/services/auth/admin-api/infra/auth-admin-api.ts
Length of output: 6472
charts/islandis/values.dev.yaml (2)
Line range hint 1453-1557: LGTM: Download service has proper authentication.
The download-service configuration includes:
- Identity Server integration for authentication
- Proper health checks
- Resource limits and requests
- Appropriate security context settings
2332-2332:
Verify security implications of disabling global auth.
The addition of nginx.ingress.kubernetes.io/enable-global-auth: 'false' will disable AWS Cognito authentication for the BFF proxy service. While this fixes the download functionality, it could potentially expose endpoints that should be protected.
Let's check if other download-related services have similar configurations:
Please ensure that:
- Only specific download endpoints are exposed without authentication
- Consider using path-based auth exclusion instead of disabling auth for the entire service
- Document the security implications and mitigation strategies
✅ Verification successful
Disabling global auth is a common pattern for specific services and endpoints
Based on the codebase analysis, disabling global auth is an established pattern used across multiple services where authentication needs to be handled differently:
- Identity Server and Auth APIs have it disabled by design since they handle authentication themselves
- Public endpoints like
/api/swaggerand/api/publicpaths have it disabled intentionally - The BFF service has this configuration in dev, staging and prod environments
- Similar services like search-indexer and github-actions-cache also follow this pattern
The change aligns with existing security practices in the codebase where certain endpoints need to bypass the global auth mechanism. The PR's change to disable global auth for the BFF proxy download service follows this established pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for download-related ingress configurations and their auth settings
rg -A 5 -B 5 "download.*ingress:|nginx\.ingress\.kubernetes\.io/enable-global-auth"
Length of output: 26042
Fix bff proxy download service aws cognito
What
Since the introduction of our bff server we need to be able to download from the download service through the bff proxy. Currently we are blocked by aws cognito
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Chores