search

Azure / data-api-builder

Data API builder provides modern REST and GraphQL endpoints to your Azure Databases and on-prem stores.
https://aka.ms/dab/docs
MIT License
787 stars 142 forks source link

[Bug]: Data Api Builder should be consuming JwtBearerOptions #2254

Open tgermain-impirica opened 3 weeks ago

tgermain-impirica commented 3 weeks ago

What happened?

I have created a complete docker-compose environment for my development team. MSSQL, DAB, KeyCloak, Traefik, Next.js front end.

The entire system is up and running for dev and is working quite well.

I am now able to log in and retrieve a valid access token with KeyCloak, and send it downstream to my DAB.

The dev environment in docker is all running http (not https), so the keycloak issued token has an http issuer.

DAB rejects the token because the JwtBearer middleware rejects http issuers UNLESS RequireHttpsMetadata is set to false for development.

If DAB was properly consuming JwtBearerOptions, then I could simply add an environment variable, or an appsettings.json file to override certain auth settings for dev. However, DAB has essentially hard coded much of the configuration (Except the issuer and audience settings).

I would have to migrate a bunch of my dev container environment to use https in order to make the E2E scenario work, but would rather be able to run http in dev mode.

Version

0.12.0-rc

What database are you using?

Azure SQL

What hosting model are you using?

Custom Docker host

Which API approach are you accessing DAB through?

GraphQL

Relevant log output

2024-06-07 15:41:02 fail: Microsoft.AspNetCore.Server.Kestrel[13]
2024-06-07 15:41:02       Connection id "0HN477A3CGGK2", Request id "0HN477A3CGGK2:00000002": An unhandled exception was thrown by the application.
2024-06-07 15:41:02       System.InvalidOperationException: The MetadataAddress or Authority must use HTTPS unless disabled for development by setting RequireHttpsMetadata=false.
2024-06-07 15:41:02          at Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerPostConfigureOptions.PostConfigure(String name, JwtBearerOptions options)
2024-06-07 15:41:02          at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
2024-06-07 15:41:02          at Microsoft.Extensions.Options.OptionsMonitor`1.<>c__DisplayClass10_0.<Get>b__0()
2024-06-07 15:41:02          at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
2024-06-07 15:41:02          at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
2024-06-07 15:41:02          at System.Lazy`1.CreateValue()
2024-06-07 15:41:02          at Microsoft.AspNetCore.Authentication.AuthenticationHandler`1.InitializeAsync(AuthenticationScheme scheme, HttpContext context)
2024-06-07 15:41:02          at Microsoft.AspNetCore.Authentication.AuthenticationHandlerProvider.GetHandlerAsync(HttpContext context, String authenticationScheme)
2024-06-07 15:41:02          at Microsoft.AspNetCore.Authentication.AuthenticationService.AuthenticateAsync(HttpContext context, String scheme)
2024-06-07 15:41:02          at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
2024-06-07 15:41:02          at Azure.DataApiBuilder.Service.Startup.<>c__DisplayClass15_0.<<Configure>b__3>d.MoveNext() in /_/src/Service/Startup.cs:line 345
2024-06-07 15:41:02       --- End of stack trace from previous location ---
2024-06-07 15:41:02          at Azure.DataApiBuilder.Core.Services.PathRewriteMiddleware.InvokeAsync(HttpContext httpContext) in /_/src/Core/Services/PathRewriteMiddleware.cs:line 89
2024-06-07 15:41:02          at Azure.DataApiBuilder.Core.Services.CorrelationIdMiddleware.Invoke(HttpContext httpContext) in /_/src/Core/Services/CorrelationIdMiddleware.cs:line 53
2024-06-07 15:41:02          at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)

Code of Conduct

seantleonard commented 3 weeks ago

Thank you for reaching out and I understand the ask for wanting to change requirehttpsmetadata to false.

  1. My understanding is that your KeyCloak instance is on-premises, hence the http dev instance?
  2. What other JWT bearer options fields would you like the ability to change?

If DAB was properly consuming JwtBearerOptions, then I could simply add an environment variable, or an appsettings.json file to override certain auth settings for dev. However, DAB has essentially hard coded much of the configuration (Except the issuer and audience settings).

In this case, DAB doesn't hard code explicitly, it just doesn't modify the out of box .NET defaults:

Gets or sets if HTTPS is required for the metadata address or authority. The default is true. This should be disabled only in development environments. https://learn.microsoft.com/dotnet/api/microsoft.aspnetcore.authentication.jwtbearer.jwtbeareroptions.requirehttpsmetadata?view=aspnetcore-8.0

tgermain-impirica commented 2 weeks ago

Good morning,

In my scenario, I am only using Http while running locally on Docker Desktop using docker-compose. I have a Traefik gateway running in front of my front end services, and a KeyCloak identity provider running adjacent to Dab. KeyCloak is currently issuing an authority as http://... Which fails the Dab smell check.

I will be working on moving those services to https even within local dev environment, but thought it would be a useful change to allow further customization of Dab by simply reading configuration from appsettings.json the way the .Net Core Jwt auth is naturally designed!

In this example, I can add a “Bearer” section to my appsettings.json and set RequireHttpsMetadata (or any other setting I want):

{ "Logging": { "LogLevel": { "Default": "Information", "Microsoft.AspNetCore": "Warning" } }, "Bearer": { "RequireHttpsMetadata": false }, "AllowedHosts": "*" }

And in the Startup, just read that section prior to configuring JwtBearer auth.

builder.Services.Configure(JwtBearerDefaults.AuthenticationScheme, builder.Configuration);

builder.Services.AddAuthentication() .AddJwtBearer(config => { config.Authority ??= customHostBuilderAuthorityFromDabConfig; config.Audience ??= customHostBuilderAudienceFromDabConfig; });

Curious why appsettings.json isn’t the default config mechanism, rather than a custom json file?

Thank you for responding. I see this as a nice to have for now, as I can move my services to https in dev mode. I was just a little frustrated to see the code in the Dab Git repo and no configuration is read from appsettings.json. That would open up so many more options for consumers!

Sincerely, Trevor

From: Sean Leonard @.> Date: Friday, June 7, 2024 at 5:18 PM To: Azure/data-api-builder @.> Cc: Trevor Germain @.>, Author @.> Subject: Re: [Azure/data-api-builder] [Bug]: Data Api Builder should be consuming JwtBearerOptions (Issue #2254)

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

Thank you for reaching out and I understand the ask for wanting to change requirehttpsmetadata to false.

  1. My understanding is that your KeyCloak instance is on-premises, hence the http dev instance?
  2. What other JWT bearer options fields would you like the ability to change?

If DAB was properly consuming JwtBearerOptions, then I could simply add an environment variable, or an appsettings.json file to override certain auth settings for dev. However, DAB has essentially hard coded much of the configuration (Except the issuer and audience settings).

In this case, DAB doesn't hard code explicitly, it just doesn't modify the out of box .NET defaults:

Gets or sets if HTTPS is required for the metadata address or authority. The default is true. This should be disabled only in development environments. https://learn.microsoft.com/dotnet/api/microsoft.aspnetcore.authentication.jwtbearer.jwtbeareroptions.requirehttpsmetadata?view=aspnetcore-8.0

— Reply to this email directly, view it on GitHubhttps://github.com/Azure/data-api-builder/issues/2254#issuecomment-2155687040, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AV6LLC5ZBE4FJ4UHOQBY2VTZGI5U5AVCNFSM6AAAAABI7LJLQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJVGY4DOMBUGA. You are receiving this because you authored the thread.Message ID: @.***>

tgermain-impirica commented 2 weeks ago

After digging deeper into this, we are going to have a serious problem getting DAB to work in a containerized environment!

  1. Identity service (keycloak) inside Azure Container Apps (ACA)

  2. Data Api Builder (DAB) inside ACA

  3. Web application inside ACA

  4. Web Application Gateway with Firewall (AWG)

  5. User visits web app at https://mysite.comhttps://mysite.com/

  6. AWG forwards them to Identity service at https://identity-magicstring.azurecontainerapps.comhttps://identity-magicstring.azurecontainerapps.com/ (internal ingress)

  7. User acquires an access token. Issuer is https://mysite.com/identity/realms/myrealm

  8. Web app sends the token to DAB service at https://dab-magicstring.azurecontainerapps.comhttps://dab-magicstring.azurecontainerapps.com/ (internal ingress)

  9. DAB receives token and sees that the issuer is https://mysite.com/identity/realms/myrealm and tries to access to verify tokens. DAB cannot reach out to that domain. DAB cannot resolve that domain from within the container environment. DAB only knows how to reach other services at https://*-magicstring.azurecontainerapps.com (internal ingress)

  10. So, there is no way for DAB to receive the list of valid issuers, etc.

DAB properly secured in a container environment with internal identity services cannot work without exposing the JwtBearerOptions. This is a critical blocker for us to move foreward!

Any way to prioritize this?

Thanks, Trevor

From: Trevor Germain @.> Date: Monday, June 10, 2024 at 9:09 AM To: Azure/data-api-builder @.>, Azure/data-api-builder @.> Cc: Author @.> Subject: Re: [Azure/data-api-builder] [Bug]: Data Api Builder should be consuming JwtBearerOptions (Issue #2254) Good morning,

In my scenario, I am only using Http while running locally on Docker Desktop using docker-compose. I have a Traefik gateway running in front of my front end services, and a KeyCloak identity provider running adjacent to Dab. KeyCloak is currently issuing an authority as http://... Which fails the Dab smell check.

I will be working on moving those services to https even within local dev environment, but thought it would be a useful change to allow further customization of Dab by simply reading configuration from appsettings.json the way the .Net Core Jwt auth is naturally designed!

In this example, I can add a “Bearer” section to my appsettings.json and set RequireHttpsMetadata (or any other setting I want):

{ "Logging": { "LogLevel": { "Default": "Information", "Microsoft.AspNetCore": "Warning" } }, "Bearer": { "RequireHttpsMetadata": false }, "AllowedHosts": "*" }

And in the Startup, just read that section prior to configuring JwtBearer auth. builder.Services.Configure(JwtBearerDefaults.AuthenticationScheme, builder.Configuration);

builder.Services.AddAuthentication() .AddJwtBearer(config => { config.Authority ??= customHostBuilderAuthorityFromDabConfig; config.Audience ??= customHostBuilderAudienceFromDabConfig; });

Curious why appsettings.json isn’t the default config mechanism, rather than a custom json file?

Thank you for responding. I see this as a nice to have for now, as I can move my services to https in dev mode. I was just a little frustrated to see the code in the Dab Git repo and no configuration is read from appsettings.json. That would open up so many more options for consumers!

Sincerely, Trevor

From: Sean Leonard @.> Date: Friday, June 7, 2024 at 5:18 PM To: Azure/data-api-builder @.> Cc: Trevor Germain @.>, Author @.> Subject: Re: [Azure/data-api-builder] [Bug]: Data Api Builder should be consuming JwtBearerOptions (Issue #2254)

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

Thank you for reaching out and I understand the ask for wanting to change requirehttpsmetadata to false.

  1. My understanding is that your KeyCloak instance is on-premises, hence the http dev instance?
  2. What other JWT bearer options fields would you like the ability to change?

If DAB was properly consuming JwtBearerOptions, then I could simply add an environment variable, or an appsettings.json file to override certain auth settings for dev. However, DAB has essentially hard coded much of the configuration (Except the issuer and audience settings).

In this case, DAB doesn't hard code explicitly, it just doesn't modify the out of box .NET defaults:

Gets or sets if HTTPS is required for the metadata address or authority. The default is true. This should be disabled only in development environments. https://learn.microsoft.com/dotnet/api/microsoft.aspnetcore.authentication.jwtbearer.jwtbeareroptions.requirehttpsmetadata?view=aspnetcore-8.0

— Reply to this email directly, view it on GitHubhttps://github.com/Azure/data-api-builder/issues/2254#issuecomment-2155687040, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AV6LLC5ZBE4FJ4UHOQBY2VTZGI5U5AVCNFSM6AAAAABI7LJLQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJVGY4DOMBUGA. You are receiving this because you authored the thread.Message ID: @.***>