Stop adding connection string into code when scaffolding

[KallDrexx]

When I run the command:

dotnet ef dbcontext scaffold "<connection-string>" Microsoft.EntityFrameworkCore.SqlServer -o "Data" -c "ApiDbContext" -f

It generates a ApiDbContext.cs file with the following:

       protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            #warning To protect potentially sensitive information in your connection string, you should move it out of source code. See http://go.microsoft.com/fwlink/?LinkId=723263 for guidance on storing connection strings.
            optionsBuilder.UseSqlServer(@"<connection string>");
        }

I have literally just spent 2 hours trying to figure out why my our staging environments were hitting our development databases instead of our production databases. We run this command every time we have any database schema changes (because we do database first instead of code first) and this time I forgot to remove this.

It got lost in a code review because of other changes in that file as well as lots of changes in other areas of the code.

It got lost in the warnings message because the current build has a lot of warnings because of [Obsolete] tags that we cannot remedy immediately.

To me this whole situation is not good, if for no other reason then you are encouraging users to put their sensitive connection strings in source control while at the same time saying "this is a bad practice but we will still shove this in your code every time you run this command". All it takes is one person to miss that message and it's in source control forever (or can be a royal pain to remove if it's an active code base).

While running dotnet ef dcontext scaffold --help I do not see any option to help remedy this either.

[ErikEJ]

I will investigate adding the option to have the connectionstring removed to my "EF Core Power Tools" ErikEJ/SqlCeToolbox#582 - is the best approach to simply remove the 2 lines of code in OnConfiguring?

[ErikEJ]

@KallDrexx Implemented in the latest daily build: https://github.com/ErikEJ/SqlCeToolbox/wiki/EF-Core-Power-Tools

[KallDrexx]

That's neat and I 100% appreciate it, but unfortunately most of our developers do not use Visual Studio proper anymore, we are mixed between Rider and VS Code (mostly for cross-platform support) :(.

[ajcvickers]

@KallDrexx This was actually discussed at great length a few years ago. The problem is that we want the result of reverse engineering to be runnable. If it isn't then people tend to give up immediately. We also couldn't come up with a secure way of doing this that would work across platforms/environments/frameworks. So after much debate we went with putting the connection string in, but with a warning comment. This certainly isn't ideal, but as of now it seems the best compromise between the competing requirements.

Note that the work planned around update-model-from-database #831 should make this much less of a problem for your scenario because it will not be necessary to run a complete , new scaffolding every time.

[josh-sachs]

For what it is worth, it seems to me the best compromise would have been to include an additional, optional flag that would result in the current behavior - and to have the default behavior be something that is suitable for use against a production code base.

The inclusion of the connection string is bad practice, and the tooling should work in a way that encourages people to wean themselves off of functionality that was included for the sake of mitigating any learning curve that may exist.

People following a guide can easily include a --hold-my-hand flag... they are probably copying and pasting from the web anyway. When present, the included warning comment should then advise them to remove that line of code, and omit the flag when they are ready to scaffold for use in a production environment.

It is discouraging to me that this isn't obvious, and that the current behavior is actually the product of intentional deliberation, rather than just an oversight.

Can we at least consider adding a flag to the CLI tool to prevent this behavior? The warning comment could be updated to inform developers of the option.

[KallDrexx]

The likelihood of someone coming to EF Core to just try it out and just stumbling on how to scaffold C# entities from an existing database without looking at any documentation seems pretty slim to me. If they are already following documentation to perform the scaffold I don't see why it's that difficult to instruct them how to pass a connection string to a DbContext. That's only one extra step they have to use to execute and gives them insight how to properly extract their connection string into a more secure area.

[smitpatel]

After looking at this more closely,
the very first code snippet is actually not what we are generating with 2.0 packages.
#6686 explicitly converted code to following style.

       protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            if (!optionsBuilder.IsConfigured)
            {
            #warning To protect potentially sensitive information in your connection string, you should move it out of source code. See http://go.microsoft.com/fwlink/?LinkId=723263 for guidance on storing connection strings.
                optionsBuilder.UseSqlServer(@"<connection string>");
            }
        }

Which makes sure that if the DbContext is configured already, it won't be reconfigured. So scaffolding actually does not change connection string and does not cause any issue of hitting wrong database. The problem does not exist actually.

Further, re-running scaffolding erases all custom modifications #831 fixes it. This is just one of the points of that issue.

It is quite ironic thread though.
Beginners should read whole documentation line by line but reading code reviews in detail is not necessary.
Beginners should learn to pass in extra flag as an extra step when removing 2 lines of code is huge step.
It is unacceptable to put connectionstring in code when it is acceptable to ignore warnings during the build time.

[smitpatel]

Given that after removing OnConfiguring first time, "Update model from database" will preserve that modification as part of #831 and #831 being slated for 2.1.0 release, adding a flag in 2.1.0 release provides very little value.

[josh-sachs]

What is ironic is you missed the part where nobody claimed beginners read documentation line-by-line, the assumption was made they are copy pasting. The warning should educate them on how to alleviate the concern instead of requiring them to remember to revisit to the same constructor every time they update their database outside of code-first.

What is ironic is that the content of the message also describes the security risks associated with having the connection string in the code, adding an if statement doesn't alleviate this concern whatsoever.

What is Ironic is you are referencing an issue that may resolve it which has been open now for what - 3 years? Also, if I'm not mistaken the 2.1 .net core sdk isn't even released yet?

Tooling should do predictable things, and hard-wiring the codebase to the reference database during scaffold is unpredictable. The current behavior causes issues for teams who don't use code first to manage their database (e.g. that have an actual DBA) and seems trivial to address (a PR has already been submitted). The sarcasm at the end of your post @smitpatel is unnecessary and IMO violates your own repository guidelines around decorum.

[ralmsdeveloper]

@josh-sachs , I do not see sarcasm in the text @smitpatel , if you look, quotes being programmed to 2.1.

You have your opinion and your point of view, but I do not find your text productive.
@smitpatel just wanted to show something, and I believe that when he asked the beginners to see the documentation, surely the intention was good and did not disqualify anyone!

to complement: @smitpatel is one of the people who help most here, I say that because the same, like everyone on the team, always point me in the right direction.

[josh-sachs]

It's fine - I might just be having a bad day and took it the wrong way, but reading it still seems condescending. Perhaps sarcastic was the wrong word. I recognize that I have a tendency to get combative, so apologies for that.

Regardless there is, and apparently may continue to be an issue here for teams not using code-first to drive the database schema. My original text attempts to highlight the issue that appears to just be, at this point, symptomatic of the direction the framework appears to be moving in.

The PR submitted by Kalldrex proposes to alleviate it, so at this point there isn't much more to discuss. It is either determined to be a worthwhile inclusion or it isn't. I don't quite see how intentionally keeping the tooling ham stringed for this particular scenario benefits anyone, but that is just me and my opinion.

[ralmsdeveloper]

@josh-sachs , I'm sure the team will look at it with affection. I realized that these days they are working hard to improve many things and also make more implementations.

Your feedback is certainly valuable to the team!

[Jrubzjeknf]

For anyone wondering how to use this functionality: you must specify your connectionstring to be named, which looks like name=MyConnectionString. The name of your connection string corresponds with the one defined in your appsettings.json.

[woodmanz]

I agree this this behavior is scary and dangerous. I like the model only updating, but when you add a huge module with 20+ tables you just want a clean change. Our solution has been to create our own class that inherits from the DbContext class and ensures that the optionsbuilder is configured, then we have a step in our checking that will only allow the generated dbcontext class to be referenced in itself and the extended class. An absurd solution to have to go through, I don't understand why they don't just do this for us and only generate the class that sets the cnx string if it doesn't exist. Would love to see something like that.

[bricelam]

Note, you can do this in recent versions:

dotnet user-secrets set ConnectionStrings.MyConnection "Data Source=MyServer"
dotnet ef dbcontext scaffold Name=MyConnection Microsoft.EntityFrameworkCore.SqlServer

[ErikEJ]

@bricelam Would be a nice addition to the docs?

[bricelam]

dotnet/EntityFramework.Docs#508 (comment)

[kasperkronborg]

@bricelam Using your solution got me the following error:

A named connection string was used, but the name 'MyConnection' was not found in the application's configuration. Note that named connection strings are only supported when using 'IConfiguration' and a service provider, such as in a typical ASP.NET Core application. See https://go.microsoft.com/fwlink/?linkid=850912 for more information.

[bricelam]

How is your DbContext created at design time? Using the Program.CreateWebHostBuilder() approach works best for this scenario.

[kasperkronborg]

I'm not sure I completely understand your example. My setup is as the following:

I have a Console Application MyConsoleApplication.App.
Then I have all my db related stuff (context and models) in a project of it's own MyConsoleApplication.Db, which contains no DbContext created at design time. Only scaffold files. Does that mean I can't use the named connection to create a scaffold if I don't have a DbContext created at design time?

[bricelam]

Sorry, that was a silly question. 😄 You're trying to scaffold a DbContext...

It really only works with ASP.NET Core projects at the moment. The best solution is to update the code after scaffolding to load the connection string from config.

[kasperkronborg]

No worries, but okay, cause right now my order of work is scaffold the DbContext and then go into the file and remove the #warning pragma (Because I don't want to commit my connection string). Which can be a little cumbersome. Cause my Console Application makes sure to setup the DbContext and load the connection string from the config.

The "best" solution I have made to use a connection string from e.g. a sh script is the following:

CONNECTION=$(dotnet user-secrets list | grep "MyConnectionString" | cut -d "=" -f 2- | xargs)
dotnet ef dbcontext scaffold "$CONNECTION" Microsoft.EntityFrameworkCore.SqlServer

And it's not pretty!

[DXSdata]

FYI, for those who need a quick solution, I created a simple tool which, among others, removes the OnConfiguring method from the context file after creation:
https://github.com/DXSdata/DotnetEfDbcontextConverter

You can then e.g. set your custom connection string via implementing a partial class file which will not be overwritten, again via OnConfiguring method.

[dnknitro]

This can be corrected (core 2.2) using IDesignTimeServices by implementing custom CSharpDbContextGenerator:

    public class DesignTimeServices : IDesignTimeServices
    {
        public void ConfigureDesignTimeServices(IServiceCollection serviceCollection)
        {
            serviceCollection.AddSingleton<ICSharpDbContextGenerator, DbContextGenerator>();
        }
    }

    public class DbContextGenerator : CSharpDbContextGenerator
    {
        public DbContextGenerator(IEnumerable<IScaffoldingProviderCodeGenerator> legacyProviderCodeGenerators, IEnumerable<IProviderConfigurationCodeGenerator> providerCodeGenerators, IAnnotationCodeGenerator annotationCodeGenerator, ICSharpHelper cSharpHelper) : base(legacyProviderCodeGenerators, providerCodeGenerators, annotationCodeGenerator, cSharpHelper)
        {
        }

        public override string WriteCode(IModel model, string @namespace, string contextName, string connectionString, bool useDataAnnotations, bool suppressConnectionStringWarning)
        {
            var code = base.WriteCode(model, @namespace, contextName, connectionString, useDataAnnotations, suppressConnectionStringWarning);

            return code.Replace(
                    @"#warning To protect potentially sensitive information in your connection string, you should move it out of source code. See http://go.microsoft.com/fwlink/?LinkId=723263 for guidance on storing connection strings.", "                // removed by DbContextGenerator"
                );
        }
    }

Also don't forget to specify -StartupProject <Project With IDesignTimeServices class> for your Scaffold-DbContext command.

[DipenParekh01]

Ideally it should be an option in the command but I just extended on what @bleachlei had suggested and created a .bat file that will generate the scaffold and used powershell to remove the hard coded strings
I have added the bat file file to External Tools of Visual Studio so it is easier to execute it from there

@echo off
SETLOCAL EnableDelayedExpansion
:: Set the below array for list of DBs to be updated. 
:: The DB models will be generated in respective sub-folders under the "Models" folder
set DB_LIST=(Database1,Database2)
:: Server (\instance) name. Change it based on environment. 
set server=localhost\NewInstance
:: Loop each db and update the context. Also remove the hard coded connection string. Connection string should be handled in appsettings and DBContext should be passed through as Dependancy Injection
for %%d in %DB_LIST% do (
	set "status=success"
	echo Updating Db Context for %%d
	dotnet ef dbcontext scaffold "Server=%server%;Database=%%d;Trusted_Connection=True;" Microsoft.EntityFrameworkCore.SqlServer --output-dir=Models\%%d --force
	CALL :CHECK_FAIL %%d,status
	if NOT !status!==fail (
		echo Removing hard coded connection string for %%d
		powershell -Command "(gc Models\%%d\%%dContext.cs) -replace '\s{0,}optionsBuilder.UseSqlServer.+', '' -replace '\s{0,}#warning To protect.+', '' | Out-File -encoding ASCII Models\%%d\%%dContext.cs"
	)
	CALL :CHECK_FAIL %%d,status
	if NOT !status!==fail (
		echo Update successful for %%d
	)
)

exit /b

:CHECK_FAIL
if NOT ["%errorlevel%"]==["0"] (
    echo Update failed for %~1
	SET "%~2=fail"
)
exit /B 0

[auserx]

This can be corrected (core 2.2) using IDesignTimeServices by implementing custom CSharpDbContextGenerator:

@dnknitro IScaffoldingProviderCodeGenerator seems to be obsolete - any workaround for this?

[dnknitro]

@auserx I am not aware of that.

[jeremycook]

I've got to say this little feature turned out to be a significant time sink when I tried to deploy (not to mention the highly questionable security implications that are completely ignored here). Who would have guessed that the connection string from IConfiguration was never used even though I utilized the classic .UseSqlServer(connectionString) call in Startup.cs. It worked on my machine, of course, since the connection string I used to scaffold matched that in IConfiguration. But on the server was a whole other ball game I didn't even know I was playing. The principal of least surprise need not show its face here. As a very experienced .NET Core and EF Core code-first dev I was pretty surprised. Not to mention the hoops I jumped through to get things to work as desired on the server. I can't imagine someone new to this rodeo would have a chance.

This issue should have died a long time ago and yet people keep commenting. I wish someone with authority would take note. Security implications and a crappy user experience (for me and a lot of other commenters at least) shouldn't be ignored. If #831 happened it would be one thing, but uh yeah, when is that going to happen.

I hope this is reconsidered. But like many others I fear I'm wasting keystrokes.

[Pearseak]

Really?
Can't we just have a simple flag for this?
I just want to generate Context and POCO that won't work unless you inject them with connection string.

[darkguy2008]

I can't believe this was reported on 2017 and yet in 2020 we're still facing this issue. Is it too hard to add a flag? Tons of enhancements come to VSCode yet it is really hard to add just a commandline flag and a boolean value to decide whether to do this or not? I keep adding/removing that bit of code between commits everytime I regenerate my dbcontext. This is just ridiculous.

+1 to everyone else's voice here. We shouldn't resort to use custom tools or batch files for something so simple, yet our voice remains unheard.

[markl66]

I run my scaffold from git bash shell. At the moment I have to manually replace that string everytim, but surely it's easy enough to just run another shell script to replace that line. sed, python... etc

[lajones]

Just so it's clear for anyone who missed it, the fix to add a flag to suppress generation of the OnConfiguring() method was checked in on May 27, 2020 - see #20811. We still recommend using the Name= syntax though - see these docs for an example.

[darkguy2008]

Just so it's clear for anyone who missed it, the fix to add a flag to suppress generation of the OnConfiguring() method was checked in on May 27, 2020 - see #20811. We still recommend using the Name= syntax though - see these docs for an example.

Thanks @lajones ! Although I'm a bit confused... in "dotnet ef" (using 3.1.5) I'm using this to generate my context, and even though I see the argument in the commit, it doesn't work, see:

dotnet ef dbcontext scaffold -d "Data Source=c:\users\blah\desktop\file.db" Microsoft.EntityFrameworkCore.Sqlite --no-onconfiguring
Build started...
Build succeeded.
Specify --help for a list of available options and commands.
Unrecognized option '--no-onconfiguring'

So, what is the commandline argument then? What if I don't want to use a named connection string, but just exclude the onconfiguring change altogether? I had assumed that --no-onconfiguring would work. What am I doing wrong?

[ErikEJ]

@darkguy2008 This feature is in EFCore 5.

[leanbardelli]

I'm following the Mysql Official documentation, wasting hours to solve this, so don't give us the "rtfm" because you are lazy to help lazy people. This is the law of the minor effort and you blame people that don't read but you neither write doc....

[asilverstein]

Please tell us what is the name of the command line option?

[jeremycook]

@asilverstein The option is --no-onconfiguring if you are scaffolding with the EF Core CLI, and -NoOnConfiguring if you are scaffolding with PowerShell from the Package Manager Console. Watch out for comments like "Added in EF Core 5.0" in the docs. I've seen some folks confused because they were trying out a new option (like this one) with an older CLI (i.e. 3.1 and earlier).

[Byron2016]

My project is structured in this way:

• A solution
  • A main asp.net core mvc project car
  • A LibraryClass named car.entities

car project has a reference to car.entites
car project file "appsettings.json" has a "ConnectionStrings" named carDB

Located into "car.entites" directory I had generated entities inside car.entities library using this command:
dotnet ef dbcontext scaffold "Server=DESKTOP-xxxxx\MSSQLSERVER_2019;Database=xxxx;User Id=xxx;Password=xxx;MultipleActiveResultSets=true" Microsoft.EntityFrameworkCore.SqlServer -o Models

It works perfect and I am able to display data from Database.

but I need to use the connectionString name instead to have the connectionString inside of my carDBContext file.
If I run this command form NuGet Administrator Console, inside my "car.entities" directory; it works perfect
Scaffold-DbContext -Connection name=CarDB -OutputDir Models Microsoft.EntityFrameworkCore.SqlServer -force

But If I work with powerShell following the the same steps:
From powerShell inside my "car.entities" directory run this command:
dotnet ef dbcontext scaffold Name=CarDB Microsoft.EntityFrameworkCore.SqlServer -o Models -f but the system cannot find the connectionString, the error message is:
A named connection string was used, but the name 'CarDB' was not found in the application's configuration. Note that named connection strings are only supported when using 'IConfiguration' and a service provider, such as in a typical ASP.NET Core application. See https://go.microsoft.com/fwlink/?linkid=850912 for more information.

How can I tell donet ef where to find my connection string name?

[jeremycook]

@Byron2016 I'm shooting from the hip but here are some thoughts.

You need to be in the main asp.net core mvc project car folder when you run dotnet ef using the Name=CarDB style of connection string. That project needs to be able to compile and must provide a connection string named CarDB via an appsettings file, user secrets, environment variable, etc.

Let's call main mvc project "car.webapp" for the sake of clarity. It sounds like you have a second project named "car.entities" that is beside the car.webapp. So your solution folder might look like this:

/path/to/solution/car/
  car.entities/
    Models/ <-- this is where the classes will be scaffolded
    car.entities.csproj
  car.webapp/
    appsettings.json <-- this file contains { "ConnectionStrings": { "CarDB": "Server=..." } ... }
    car.webapp.csproj
  car.sln

With that structure you should be able to run the following commands.

cd /path/to/solution/car/car.webapp/
dotnet ef dbcontext scaffold Name=CarDB Microsoft.EntityFrameworkCore.SqlServer -o ../car.entities/Models/ -f

You will probably want to tweak the namespace. With the EF Core .NET 5 tooling you can provide a namespace argument like so:

dotnet ef dbcontext scaffold Name=CarDB Microsoft.EntityFrameworkCore.SqlServer -o ../car.entities/Models/ -n car.entities.Models -f

Also there are times when I've scaffolded changes to the entities that make it so my main web application cannot compile. You may want to re-scaffold to undo or tweak your entities, but if the project that provides the named connection string cannot compile then the scaffolding can't run. One way to work around that is to have a project that is dedicated to providing the named connection string (and other design time stuff if you want) which does not depend on the project that contains the entities.

[RaulGarciaCarrillo]

I'm ussing --no-onconfiguring flag with version EF Core 5.0.0-rc.2.20475.6, but it's still creating method OnConfiguring

[ajcvickers]

@RaulGarciaCarrillo Make sure you have the 5.0-rc2 dotnet-ef tool and have the 5.0-rc2 EF Core packages referenced in your projects.

[RaulGarciaCarrillo]

@ajcvickers Thanks! That was the problem

[bobwah]

TLDR;

There is now a flag:
--no-onconfiguring

This comes back as a top search on google for things like: "dotnet ef scaffold avoid connection string" and I ended up reading a lot of a discussion which is now irrelevant, I hope this helps someone.

[KittyChen913]

TLDR;

There is now a flag: --no-onconfiguring

This comes back as a top search on google for things like: "dotnet ef scaffold avoid connection string" and I ended up reading a lot of a discussion which is now irrelevant, I hope this helps someone.

It works for me, thanks.

[ci-vamp]

@bobwah when i run with --no-onconfiguring it removes the warning and all but it also removes the default constructor which causes the build to fail. any idea why?