Python Code Review Flask Web Security Tutorial + Virtualenvs, requirements.txt

Key Takeaways

hey everyone so it's time for another code review unplugged episode and today I'm going to look at some code that Carl has sent me so this is actually a really cool project because it's um yeah it is a security a tutorial for fixing and I well identifying and fixing security vulnerabilities in a tiny flask web app and I like this it's not a lot of code so you know should be relatively easy for for people to go through this but I kind of like you know the short and sweet description here and I like the fact that that it's trying to teach people something with code and actually making them do something so I think this would be great for a little workshop or I don't really know where it is where this comes from if this if Carl you know if you were doing this at at a company or some kind of public workshop but I really like the idea and it should definitely keep going like I think it's a really really cool way to teach people so yeah so all I did so far was pull this up in github and then also just clone the repo locally let's also open that up in sublime text maybe crank down the font size a bit and yeah cool so so what I really like here is that the readme is pretty extensive and you know just kind of contains all you need I am not a hundred percent clear what version of Python we're dealing with like just looking at the requirements txt file it's great that we have a requirements txt file by the way this is using the flask module and a bunch of other stuff and it looks like we're supposed to just install this globally so with this like I typically I wouldn't recommend installing these things globally unless it's like a tool like a lint or something you need all the time so usually I would not install new Python libraries modules with pseudo pip install like what I would do I'm just going to assume this is Python 3 and we'll see what happens like I would create a new virtual environment with the PI vnf command and then I'm going to activate that this other command and I actually have a shortcut here in my shell setup so I can just do de to deactivate a vnf in AE to activate one maybe should crank up the font size a bit too so I was just saying like I can do de then AE to activate and deactivate these VMs and like a virtual end is essentially a local copy of your Python environment so that when you install new modules they don't gum up your global Python install but they're localized to just this folder so like we can look into this with LS and you know you can see that here we actually have a tiny well almost like a tiny Python install here right like with a bin folder and a library folder and all of that so look into Python virtual ends and then if because this is a tutorial for for newbies like I would potentially consider you know teaching them about virtual ends because in the long run they're probably not going to be happy installing these requirements into their global Python install so I'm going to do here um just install the requirements great that you have in a requirements file here always always great because it just makes setting up this stuff a lot easier and you've pinned your requirements to to fix versions that's great too ok so now I guess I'm yeah probably not going to play through the whole tutorial but I want to make sure I can run the tests and kind of also maybe spin up the app so it can make sure it's running okay so let's go Python app not pi okay cool looks like it's running nice I like the green sweet soups work Strahm OG works - excellent no unicode errors no I'm cool so it looks like this is running let's try let's try and make sure we get the test running as well so that's usually what I try to do with in with a new project just to make sure I can run the tests as well so okay so it looks like there's no instructions on how to run the tests so maybe with the way we can figure that out is we'll just look at the tests and then yeah we can see it's a script file or it it does dislike imports unit tests and just cause unit tests main so it looks like we can just run this like a regular Python script and in tests pass cool so it looks like we've got this running so why don't we take a look at the code now and I'm going to do this in sublime texts I think it's going to be a better environment to do that in okay like I said the requirements look good let's look at app dot pi okay so this is like a tiny flask app yeah we have got this global posts here I mean I think for an example like that that's totally fine that we're not doing any kind of persistence so I'm not going to focus on that okay so maybe the first thing that actually catches my eye is that these comments here they could be docstrings so just to show you what a doc string is it's like when you do these three like typically the convention would be it could be any old string as long as like the first thing that happens in your in your function but like typically you would format them like this and then this would be the doc string and the cool thing about dark strings is that they're treated a little bit special by the Python Python parser or the Python runtime because these doc strings are actually accessible you know through from from within the program and that means there are a lot of tools around that allow you to generate documentation based on these stock strings and so it's just like good practice to you know not have these kind of inline comments for dark strings just kind of use proper doc strings just because I'm seeing that here everywhere like you're really just describing the function I think that's that's great and that's the right way to go about it but you probably want to put that information into a dark string and not just into a an inline comment so that's probably what I would change I'm here with the output I mean I think I think that's fine too because you just you know you want to do this like minimum minimal example here obviously like for a larger app or whatever like this would be loaded from from some kind of template file and probably be handled through a proper templating system also to avoid stuff like this where we're just concatenate and I know you know this is part of the the challenge here's I don't want to spoil app for people but like you know you could really easily like inject all kinds of stuff like inject script tack or do something else like that and there's no like validation for that but that's part of the the game like it's part of the whole tutorial right so that's I really like that well done yeah in terms of you know how you're formatting this small app like I think it looks good like it's it's nice to read it's like formative consistently honestly the only thing I really saw was the the dark strings here um so yeah I can't really complain about that and then for the unit tests I think this works well too you know again like in terms of the the formatting like maybe one tiny thing like I'm seeing that you've switched your quote types you know you're using double quotes down here and in single quotes there but that's something that that personally like I I would I would change that for consistency but I mean in the grand scheme of things it's not a big deal and what like one way you can you can deal with that is just by having syntax highlighting that actually highlights them differently I mean if you wanna if you want to go for consistency right then then you would need that if you don't want to then it doesn't really matter but anyway okay so probably change that in terms of how the tests or what the tests look like I think that makes sense to based on what you're doing I don't think it makes sense to pull in like you know something like web test which is like a bigger framework for testing these kinds of web apps which you know it really helps when you're working on a larger project but like for what you're doing with this tutorial I think it is great that you're keeping the dependencies simple and just kind of focused on well basically flask right so actually with this stuff what I would probably do here in terms of the dependencies I would probably just put in flask as a dependency and then it's not really necessary to like spec out all these other ones because then it becomes really clear what the real dependencies are because all of this other stuff is presumably getting pulled in by flask like Jinja that's the templating thing and all that stuff and works like it's like the underlying libraries that flask is built on top off but on the other hand if you do you you know if you do like a pip freeze then then that's like the classical way to freeze your dependency so you know because this is a tutorial I I might think about like just having flasks there so that people who are looking at it they can realize like oh you know all we're really using here is flask which I might be like a valuable well I want to say lesson but like a valuable piece of information for people but like honestly like you know this is super nitpicky what I'm what I'm saying there yeah I got some CSS there yeah it's pretty self-contained I mean arguably we could also serve the CSS in line here but yeah that doesn't that doesn't really matter matter we've got a good ignore which i think is great it's gonna you know it's going to make sure that people who submit pull requests and stuff for for your your app here are not gonna hopefully not going to commit like all kinds of crazy files that you didn't have to sort out yeah I think this is pretty cool like honestly like there's not a lot of stuff I could I could find here and I really liked the tutorial so anyone who's watching this like try and play through the door tutorial I think it'll probably teach you some some valuable things about avoiding some of these vulnerabilities like I'm hoping there's some kind of solution sheep for that I think would be great if that was there because I really liked the idea behind this tutorial and yeah like I think we're we're ready to wrap this up please write a longer blog post about the tutorial I think it's a really good idea cool thanks Carl and good luck