Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix CORS setup #1782

Closed
wants to merge 1 commit into from
Closed

Fix CORS setup #1782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions app/backend/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,8 @@ def create_app():
logging.basicConfig(level=os.getenv("APP_LOG_LEVEL", default_level))

if allowed_origin := os.getenv("ALLOWED_ORIGIN"):
app.logger.info("CORS enabled for %s", allowed_origin)
cors(app, allow_origin=allowed_origin, allow_methods=["GET", "POST"])
allowed_origin = allowed_origin.split(";")
if len(allowed_origin) > 0:
app.logger.info("CORS enabled for %s", allowed_origin)
cors(app, allow_origin=allowed_origin, allow_methods=["GET", "POST"])
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We do need to allow credentials

return app
7 changes: 1 addition & 6 deletions infra/core/host/appservice.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,6 @@ param authenticationIssuerUri string = ''
param publicNetworkAccess string = 'Enabled'
param enableUnauthenticatedAccess bool = false

var msftAllowedOrigins = [ 'https://portal.azure.com', 'https://ms.portal.azure.com' ]
var loginEndpoint = environment().authentication.loginEndpoint
var loginEndpointFixed = lastIndexOf(loginEndpoint, '/') == length(loginEndpoint) - 1 ? substring(loginEndpoint, 0, length(loginEndpoint) - 1) : loginEndpoint
var allMsftAllowedOrigins = !(empty(clientAppId)) ? union(msftAllowedOrigins, [ loginEndpointFixed ]) : msftAllowedOrigins

// .default must be the 1st scope for On-Behalf-Of-Flow combined consent to work properly
// Please see https://learn.microsoft.com/entra/identity-platform/v2-oauth2-on-behalf-of-flow#default-and-combined-consent
var requiredScopes = [ 'api://${serverAppId}/.default', 'openid', 'profile', 'email', 'offline_access' ]
Expand All @@ -71,7 +66,7 @@ var coreConfig = {
functionAppScaleLimit: functionAppScaleLimit != -1 ? functionAppScaleLimit : null
healthCheckPath: healthCheckPath
cors: {
allowedOrigins: union(allMsftAllowedOrigins, allowedOrigins)
allowedOrigins: allowedOrigins
}
}

Expand Down
15 changes: 13 additions & 2 deletions infra/main.bicep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,16 @@ var tags = { 'azd-env-name': environmentName }
var tenantIdForAuth = !empty(authTenantId) ? authTenantId : tenantId
var authenticationIssuerUri = '${environment().authentication.loginEndpoint}${tenantIdForAuth}/v2.0'

// Configure CORS for allowing different web apps to use the backend
// For more information please see https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS
var msftAllowedOrigins = [ 'https://portal.azure.com', 'https://ms.portal.azure.com' ]
var loginEndpoint = environment().authentication.loginEndpoint
var loginEndpointFixed = lastIndexOf(loginEndpoint, '/') == length(loginEndpoint) - 1 ? substring(loginEndpoint, 0, length(loginEndpoint) - 1) : loginEndpoint
var allMsftAllowedOrigins = !(empty(clientAppId)) ? union(msftAllowedOrigins, [ loginEndpointFixed ]) : msftAllowedOrigins
var allowedOrigins = union(split(allowedOrigin, ';'), allMsftAllowedOrigins)
// Filter out any empty origin strings and remove any duplicate origins
var allowedOriginsEnv = join(reduce(filter(allowedOrigins, o => length(trim(o)) > 0), [], (cur, next) => union(cur, [next])), ';')

@description('Whether the deployment is running on GitHub Actions')
param runningOnGh string = ''

Expand Down Expand Up @@ -283,7 +293,7 @@ module backend 'core/host/appservice.bicep' = {
managedIdentity: true
virtualNetworkSubnetId: isolation.outputs.appSubnetId
publicNetworkAccess: publicNetworkAccess
allowedOrigins: [ allowedOrigin ]
allowedOrigins: allowedOrigins
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this always end up allowing origins even if they don't enable user auth? I dont see a conditional that makes it empty in that case. But maybe my eyes have glazed over.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or maybe we always allowed them anyway?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

allowedOrigin should be empty if they don't enable user auth. I will double check this, good catch

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So this is an interesting case

  1. We were never enabling the portal origins before due to a bug
  2. So unless they explicitly specified an allowed origin, we never enabled cors

My thought is that we want to only add in the portal / login origins if auth is enabled, otherwise just pick the origins they added. I'll make this adjustment

clientAppId: clientAppId
serverAppId: serverAppId
enableUnauthenticatedAccess: enableUnauthenticatedAccess
Expand Down Expand Up @@ -336,7 +346,7 @@ module backend 'core/host/appservice.bicep' = {
AZURE_AUTH_TENANT_ID: tenantIdForAuth
AZURE_AUTHENTICATION_ISSUER_URI: authenticationIssuerUri
// CORS support, for frontends on other hosts
ALLOWED_ORIGIN: allowedOrigin
ALLOWED_ORIGIN: allowedOriginsEnv
USE_VECTORS: useVectors
USE_GPT4V: useGPT4V
USE_USER_UPLOAD: useUserUpload
Expand Down Expand Up @@ -868,4 +878,5 @@ output AZURE_USERSTORAGE_RESOURCE_GROUP string = storageResourceGroup.name

output AZURE_USE_AUTHENTICATION bool = useAuthentication

output ALLOWED_ORIGIN string = allowedOriginsEnv
output BACKEND_URI string = backend.outputs.uri
Loading