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.

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

Add support for environment variables for configuration #2480

Closed
markjritchie opened this issue Sep 26, 2023 · 4 comments · Fixed by #2483
Closed

Add support for environment variables for configuration #2480

markjritchie opened this issue Sep 26, 2023 · 4 comments · Fixed by #2483
Assignees
Labels
enhancement New feature or request feature request
Milestone

Comments

@markjritchie
Copy link
Contributor

As far as I can see there is no way to use environment variables to override the appsettings.json for the downstream API.

The file src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquirerFactory.cs has the following method:

private IConfiguration ReadConfiguration()
        {
            if (Configuration == null)
            {
                // Read the configuration from a file
                var builder = new ConfigurationBuilder();
                string basePath = DefineConfiguration(builder);
                builder.SetBasePath(basePath)
                       .AddJsonFile("appsettings.json", optional: true);
                Configuration = builder.Build();
            }
            return Configuration;
        }

Changing it to:

private IConfiguration ReadConfiguration()
        {
            if (Configuration == null)
            {
                // Read the configuration from a file
                var builder = new ConfigurationBuilder();
                string basePath = DefineConfiguration(builder);
                builder.SetBasePath(basePath)
                       .AddJsonFile("appsettings.json", optional: true)
                       .AddEnvironmentVariables();
                Configuration = builder.Build();
            }
            return Configuration;
        }

would provide better flexibility.

Alternatively a way to pass in the configuration to be used would also work.

@markjritchie markjritchie added enhancement New feature or request feature request labels Sep 26, 2023
@jmprieur
Copy link
Collaborator

Thanks @markjritchie
Would you like to propose a PR?

@markjritchie
Copy link
Contributor Author

Happy to, but thought I should check that I wasn't missing something

@markjritchie
Copy link
Contributor Author

Is there a guide on setting up the necessary PAT to push to a branch and create the PR?

I get authentication failed when I try to push the branch

@jmprieur
Copy link
Collaborator

No. there is no PAT. You would have to fork the repo.

@jennyf19 jennyf19 added this to the 2.14.1 milestone Sep 28, 2023
jmprieur added a commit that referenced this issue Sep 30, 2023
jmprieur added a commit that referenced this issue Sep 30, 2023
* Fix build on .NET FW after #2480

* Fix the "The referenced project is targeted to a different framework family" warnings
by changing the order of the frameworks (older to newer) as advised in https://www.primordialcode.com/blog/post/referenced-project-targeted-different-framework-family
jmprieur added a commit that referenced this issue Oct 1, 2023
* Fix build on .NET FW after #2480

* Fix the "The referenced project is targeted to a different framework family" warnings
by changing the order of the frameworks (older to newer) as advised in https://www.primordialcode.com/blog/post/referenced-project-targeted-different-framework-family

* Fixes #2456

* Addressing comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature request
Projects
None yet
3 participants