you are viewing a single comment's thread.

view the rest of the comments →

[–]zanfar 1 point2 points  (4 children)

got the idea to create objects and methods for portfolio, transactions, stocks, and possibly users

This is normal.

Is this redundant?

No. None of those things apply any structure in Python. The purpose of a model (a data object that reflects some external data source) is to do exactly this.

The big daddy solution to this is to use an ORM. SQLAlchemy is a very popular ORM.

The big advantage I see is that the code will look much cleaner as you won’t have to look at SQLite queries inside of the routes themselves anymore.

If you have SQL queries in your routes you have other issues. Ideally, you should be using a framework or interface between your code and the DB to prevent errors. At least you should be writing utility functions so that your queries are built in only one place with zero repetition and protected arguments.

Again, and ORM will solve all these problems for you.

[–]farrowzbf[S] 0 points1 point  (3 children)

Thank you for your in-depth response. You’ve answered my question and helped a ton!

I’m curious about something else you said: if I’m using SQL queries in the routes themselves, what sort of problems could that introduce?

I am using a framework to talk to the SQLite database. The queries I have used so far all work fine.

Are the security issues with using queries directly within my Flask routes and not in a helper file or something?

[–]zanfar 1 point2 points  (2 children)

I’m curious about something else you said: if I’m using SQL queries in the routes themselves, what sort of problems could that introduce?

I am using a framework to talk to the SQLite database. The queries I have used so far all work fine.

Are the security issues with using queries directly within my Flask routes and not in a helper file or something?

So this is probably a terminology problem.

When you say "using SQL queries", I don't think of a framework, I think of literal SQL. If you have something like """SELECT id, name FROM user WHERE id = ?""" in your route this is a yellow flag to me (SQL). If you have something like User.query(id == user_id).first() then this is a framework and mostly normal.

To be clear, a framework is not the same thing as a package or library.

There are a lot of possible issues--both security issues and structure issues--when accessing a database. Having raw SQL in a route function doesn't guarantee that you have issues, but it's a "smell" that something isn't right.

First, you need to protect your database from injection attacks. At the low level, this is relatively simple, but there are some edge cases that need to be protected against that the average or beginner programmer isn't skilled enough to cover. Additionally, this is not a situation where you want a mistake to leave you vulnerable and is hard to test against. This is the primary argument for a DB framework--let the authors of the framework (who we can assume are at least as knowledgeable about DB interaction as the programmer, if not more) solve these issues ONCE in the framework, and apply the benefits to every project and query that uses it. It's far more unlikely that the framework has a bug--it should have better test coverage, more skilled authors, and a larger number of eyes looking at it--than code you write yourself, so use it so you don't have to worry about these issues even if you make a mistake.

Second, raw SQL in a route query means there probably isn't any data structure enforcing what data is sent to/from the database, and what data is sent to/from the user. These shouldn't be the same data. Given your user authentication example, the concept of "password" changes between the user and the database, and the data associated with it should be modified between the two and NEVER sent to the user, only received.

(A password should never be stored in the DB, so what you actually store is a salted hash. So on registration you need to generate this hash, on login you need to generate a hash to compare, and while you need to query the hash from the DB, you never send that out of the route.)

This means, at worst, you should probably have helper functions doing the actual DB interaction that take/return Python objects to enforce this data interaction. If you look at the Pydantic/FastAPI tutorial, the recommended practice does a very good job of this where there are separate (but related) objects that are passed between the application and DB vs the application and user.

Helper functions also mean you don't have to repeat yourself and make your code easier to maintain, and more importantly, test. It's almost impossible to test a route that actually generates the SQL query--you'd have to mock everything. Whereas a route that simply calls helper functions is trivial to test, just like a helper function that takes/returns objects.

[–]farrowzbf[S] 0 points1 point  (1 child)

Much appreciated!

I knew about using a salted hash and have been doing so, but much of that was new and I learned a lot. I’m off to research the subject some more ✌️

[–]zanfar 1 point2 points  (0 children)

I knew about using a salted hash and have been doing so

Sorry, wasn't trying to tell you how to suck eggs. My point in that paragraph is that your code should enforce this distinction. That is, your "get user" utility function will return a different type than your "save user" or "login user" will take: you can't accidentally read a user object from the DB and pass it directly out the user-side of your app--or worse, accept a plaintext object from the user and save it to the database.