Legacy Security Manager in Haskell by Mark Seemann
A translation of the kata, and my first attempt at it.
In early 2013 Richard Dalton published an article about legacy code katas. The idea is to present a piece of 'legacy code' that you have to somehow refactor or improve. Of course, in order to make the exercise manageable, it's necessary to reduce it to some essence of what we might regard as legacy code. It'll only be one aspect of true legacy code. For the legacy Security Manager exercise, the main problem is that the code is difficult to unit test.
The original kata presents the 'legacy code' in C#, which may exclude programmers who aren't familiar with that language and platform. Since I find the exercise useful, I've previous published a port to Python. In this article, I'll port the exercise to Haskell, as well as walk through one attempt at achieving the goals of the kata.
The legacy code #
The original C# code is a static procedure that uses the Console API to ask a user a few simple questions, do some basic input validation, and print a message to the standard output stream. That's easy enough to port to Haskell:
module SecurityManager (createUser) where import Text.Printf (printf) createUser :: IO () createUser = do putStrLn "Enter a username" username <- getLine putStrLn "Enter your full name" fullName <- getLine putStrLn "Enter your password" password <- getLine putStrLn "Re-enter your password" confirmPassword <- getLine if password /= confirmPassword then putStrLn "The passwords don't match" else if length password < 8 then putStrLn "Password must be at least 8 characters in length" else do -- Encrypt the password (just reverse it, should be secure) let array = reverse password putStrLn $ printf "Saving Details for User (%s, %s, %s)" username fullName array
Notice how the Haskell code seems to suffer slightly from the Arrow code smell, which is a problem that the C# code actually doesn't exhibit. The reason is that when using Haskell in an 'imperative style' (which you can, after a fashion, with do notation), you can't 'exit early' from a an if check. The problem is that you can't have if-then without else.
Haskell has other language features that enable you to get rid of Arrow code, but in the spirit of the exercise, this would take us too far away from the original C# code. Making the code prettier should be a task for the refactoring exercise, rather than the starting point.
I've published the code to GitHub, if you want a leg up.
Combined with Richard Dalton's original article, that's all you need to try your hand at the exercise. In the rest of this article, I'll go through my own attempt at the exercise. That said, while this was my first attempt at the Haskell version of it, I've done it multiple times in C#, and once in Python. In other words, this isn't my first rodeo.
Break the dependency on the Console #
As warned, the rest of the article is a walkthrough of the exercise, so if you'd like to try it yourself, stop reading now. On the other hand, if you want to read on, but follow along in the GitHub repository, I've pushed the rest of the code to a branch called first-pass.
The first part of the exercise is to break the dependency on the console. In a language like Haskell where functions are first-class citizens, this part is trivial. I removed the type declaration, moved putStrLn and getLine to parameters and renamed them. Finally, I asked the compiler what the new type is, and added the new type signature.
import Text.Printf (printf, IsChar) createUser :: (Monad m, Eq a, IsChar a) => (String -> m ()) -> m [a] -> m () createUser writeLine readLine = do writeLine "Enter a username" username <- readLine writeLine "Enter your full name" fullName <- readLine writeLine "Enter your password" password <- readLine writeLine "Re-enter your password" confirmPassword <- readLine if password /= confirmPassword then writeLine "The passwords don't match" else if length password < 8 then writeLine "Password must be at least 8 characters in length" else do -- Encrypt the password (just reverse it, should be secure) let array = reverse password writeLine $ printf "Saving Details for User (%s, %s, %s)" username fullName array
I also changed the main action of the program to pass putStrLn and getLine as arguments:
import SecurityManager (createUser) main :: IO () main = createUser putStrLn getLine
Manual testing indicates that I didn't break any functionality.
Get the password comparison feature under test #
The next task is to get the password comparison feature under test. Over a small series of Git commits, I added these inlined, parametrized HUnit tests:
"Matching passwords" ~: do pw <- ["password", "12345678", "abcdefgh"] let actual = comparePasswords pw pw return $ Right pw ~=? actual , "Non-matching passwords" ~: do (pw1, pw2) <- [ ("password", "PASSWORD"), ("12345678", "12345677"), ("abcdefgh", "bacdefgh"), ("aaa", "bbb") ] let actual = comparePasswords pw1 pw2 return $ Left "The passwords don't match" ~=? actual
The resulting implementation is this comparePasswords function:
comparePasswords :: String -> String -> Either String String comparePasswords pw1 pw2 = if pw1 == pw2 then Right pw1 else Left "The passwords don't match"
You'll notice that I chose to implement it as an Either-valued function. While I consider validation a solved problem, the usual solution involves some applicative validation container. In this exercise, validation is already short-circuiting, which means that we can use the standard monadic composition that Either affords.
At this point in the exercise, I just left the comparePasswords function there, without trying to use it within createUser. The reason for that is that Either-based composition is sufficiently different from if-then-else code that I wanted to get the entire system under test before I attempted that.
Get the password validation feature under test #
The third task of the exercise is to get the password validation feature under test. That's similar to the previous task. Once more, I'll show the tests first, and then the function driven by those tests, but I want to point out that both code artefacts came iteratively into existence through the usual red-green-refactor cycle.
"Validate short password" ~: do pw <- ["", "1", "12", "abc", "1234", "gtrex", "123456", "1234567"] let actual = validatePassword pw return $ Left "Password must be at least 8 characters in length" ~=? actual , "Validate long password" ~: do pw <- ["12345678", "123456789", "abcdefghij", "elevenchars"] let actual = validatePassword pw return $ Right pw ~=? actual
The resulting function is hardly surprising.
validatePassword :: String -> Either String String validatePassword pw = if length pw < 8 then Left "Password must be at least 8 characters in length" else Right pw
As in the previous step, I chose to postpone using this function from within createUser until I had a set of characterization tests. That may not be entirely in the spirit of the four subtasks of the exercise, but on the other hand, I intended to do more than just those four activities. The code here is actually simple enough that I could easily refactor without full test coverage, but recalling that this is a legacy code exercise, I find it warranted to pretend that it's complicated.
To be fair to the exercise, there'd also be a valuable exercise in attempting to extract each feature piecemeal, because it's not alway possible to add complete characterization test coverage to a piece of gnarly legacy code. Be that as it may, I've already done that kind of exercise in C# a few times, and I had a different agenda for the Haskell exercise. In short, I was curious about what sort of inferred type createUser would have, once I'd gone through all four subtasks. I'll return to that topic in a moment. First, I want to address the fourth subtask.
Allow different encryption algorithms to be used #
The final part of the exercise is to add a feature to allow different encryption algorithms to be used. Once again, when you're working in a language where functions are first-class citizens, and higher-order functions are idiomatic, one solution is easily at hand:
createUser :: (Monad m, Foldable t, Eq (t a), PrintfArg (t a), PrintfArg b) => (String -> m ()) -> m (t a) -> (t a -> b) -> m () createUser writeLine readLine encrypt = do writeLine "Enter a username" username <- readLine writeLine "Enter your full name" fullName <- readLine writeLine "Enter your password" password <- readLine writeLine "Re-enter your password" confirmPassword <- readLine if password /= confirmPassword then writeLine "The passwords don't match" else if length password < 8 then writeLine "Password must be at least 8 characters in length" else do let array = encrypt password writeLine $ printf "Saving Details for User (%s, %s, %s)" username fullName array
The only change I've made is to promote encrypt to a parameter. This, of course, ripples through the code that calls the action, but currently, that's only the main action, where I had to add reverse as a third argument:
main :: IO () main = createUser putStrLn getLine reverse
Before I made the change, I removed the type annotation from createUser, because adding a parameter causes the type to change. Keeping the type annotation would have caused a compilation error. Eschewing type annotations makes it easier to make changes. Once I'd made the change, I added the new annotation, inferred by the Haskell Visual Studio Code extension.
I was curious what kind of abstraction would arise. Would it be testable in some way?
Testability #
Consider the inferred type of createUser above. It's quite abstract, and I was curious if it was flexible enough to allow testability without adding test-induced damage. In short, in object-oriented programming, you often need to add Dependency Injection to make code testable, and the valid criticism is that this makes code more complicated than it would otherwise have been. I consider such reproval justified, although I disagree with the conclusion. It's not the desire for testability that causes the damage, but rather that object-oriented design is at odds with testability.
That's my conjecture, anyway, so I'm always curious when working with other paradigms like functional programming. Is idiomatic code already testable, or do you need to 'do damage to it' in order to make it testable?
As a Haskell action goes, I would consider its type fairly idiomatic. The code, too, is straightforward, although perhaps rather naive. It looks like beginner Haskell, and as we'll see later, we can rewrite it to be more elegant.
Before I started the exercise, I wondered whether it'd be necessary to use free monads to model pure command-line interactions. Since createUser returns m (), where m is any Monad instance, using a free monad would be possible, but turns out to be overkill. After having thought about it a bit, I recalled that in many languages and platforms, you can redirect standard in and standard out for testing purposes. The way you do that is typically by replacing each with some kind of text stream. Based on that knowledge, I thought I could use the State monad for characterization testing, with a list of strings for each text stream.
In other words, the code is already testable as it is. No test-induced damage here.
Characterization tests #
To use the State monad, I started by importing Control.Monad.Trans.State.Lazy into my test code. This enabled me to write the first characterization test:
"Happy path" ~: flip evalState (["just.inhale", "Justin Hale", "12345678", "12345678"], []) $ do let writeLine x = modify (second (++ [x])) let readLine = state (\(i, o) -> (head i, (tail i, o))) let encrypt = reverse createUser writeLine readLine encrypt actual <- gets snd let expected = [ "Enter a username", "Enter your full name", "Enter your password", "Re-enter your password", "Saving Details for User (just.inhale, Justin Hale, 87654321)"] return $ expected ~=? actual
I consulted my earlier code from An example of state-based testing in Haskell instead of reinventing the wheel, so if you want a more detailed walkthrough, you may want to consult that article as well as this one.
The type of the state that the test makes use of is ([String], [String]). As the lambda expression suggests by naming the elements i and o, the two string lists are used for respectively input and output. The test starts with an 'input stream' populated by 'user input' values, corresponding to each of the four answers a user might give to the questions asked.
The readLine function works by pulling the head off the input list i, while on the other hand not touching the output list o. Its type is State ([a], b) a, compatible with createUser, which requires its readLine parameter to have the type m (t a), where m is a Monad instance, and t a Foldable instance. The effective type turns out to be t a ~ [Char] = String, so that readLine effectively has the type State ([String], b) String. Since State ([String], b) is a Monad instance, it fits the m type argument of the requirement.
The same kind of reasoning applies to writeLine, which appends the input value to the 'output stream', which is the second list in the I/O tuple.
The test runs the createUser action and then checks that the output list contains the expected values.
A similar test verifies the behaviour when the passwords don't match:
"Mismatched passwords" ~: flip evalState (["i.lean.right", "Ilene Wright", "password", "Password"], []) $ do let writeLine x = modify (second (++ [x])) let readLine = state (\(i, o) -> (head i, (tail i, o))) let encrypt = reverse createUser writeLine readLine encrypt actual <- gets snd let expected = [ "Enter a username", "Enter your full name", "Enter your password", "Re-enter your password", "The passwords don't match"] return $ expected ~=? actual
You can see the third and final characterization test in the GitHub repository.
Refactored action #
With full test coverage I could proceed to refactor the createUser action, pulling in the two functions I'd test-driven into existence earlier:
createUser :: (Monad m, PrintfArg a) => (String -> m ()) -> m String -> (String -> a) -> m () createUser writeLine readLine encrypt = do writeLine "Enter a username" username <- readLine writeLine "Enter your full name" fullName <- readLine writeLine "Enter your password" password <- readLine writeLine "Re-enter your password" confirmPassword <- readLine writeLine $ either id (printf "Saving Details for User (%s, %s, %s)" username fullName . encrypt) (validatePassword =<< comparePasswords password confirmPassword)
Because createUser now calls comparePasswords and validatePassword, the type of the overall composition is also more concrete. That's really just an artefact of my (misguided?) decision to give each of the two helper functions types that are more concrete than necessary.
As you can see, I left the initial call-and-response sequence intact, since I didn't feel that it needed improvement.
Conclusion #
I ported the Legacy Security Manager kata to Haskell because I thought it'd be easy enough to port the code itself, and I also found the exercise compelling enough in its own right.
The most interesting point, I think, is that the createUser action remains testable without making any other concession to testability than turning it into a higher-order function. For pure functions, we would expect this to be the case, since pure functions are intrinsically testable, but for impure actions like createUser, this isn't a given. Interacting exclusively with the command-line API is, however, sufficiently simple that we can get by with the State monad. No free monad is needed, and so test-induced damage is kept at a minimum.
