Refactoring authorisation and related code
Posted by Denny at 20:13 on Sun, 15 Dec 2019
Refactor capabilities and Pundit auth has got a few different pieces of work in it. The first chunk is intended to make it a bit lower-lift to set up authorisatino when adding new features (there's some inheritance stuff going on in the Pundit policies now which is either clever or 'clever', I haven't quite decided).
There are also some changes which almost certainly make the authorisation code more secure. I'm no longer using translation strings as keys for auth values in the database. I initially thought it would be nice to have the database content be localised for non-English speakers - but it added an extra layer of potentially easily attackable code (just have to futz with the translation strings) in the authorisation subsystem, which is pretty clearly suboptimal. I'm now using symbols most places in the code instead, and only translating those to local strings at the point of displaying on a view, which feels like a smarter choice overall.
This PR fixed a few tests that I had to comment out when I first introduced Pundit, and re-enabling those tests means that I'm back up to 100% coverage :D There are a couple of initializers with :nocov: cheating in them, as well as pretty much all the code in the rake tasks, but everything else is genuinely covered for at least its golden path - mostly by request specs, so there's scope for getting into the details a bit more with the model specs at some point, but I'm still pretty pleased by the current test suite.
One thing I've already noticed on this project is that having such high test coverage has made refactoring a much less fraught process. I know all the books tell us this, but I'd never worked on anything with anywhere near this level of coverage before - maybe 80% at best - and it's been a genuine shock to me how much difference it's made in how often I refactor things, and particularly to the size of the things I'll 'casually' refactor - in both directions. Things that would have been scarily big without the coverage, but also things that would have seemed too petty to be worth the risk, both are totally on the menu now. I'm enjoying it greatly.
Tags: pundit feature flags auth refactoring security i18n tests coverage