On my last project I had to integrate our code with an external REST provider. The provider was a banking service (I’ll call them “TheBank” to protect their identity) and we had to record financial transactions with them.
A good Java interview question would be – what have they done wrong in this REST API design? Have a read of the docs before reading on, see if you can come up with a list of what could be improved.
(Plug: our JavaEE with Wildfly and Spring Remoting courses both explore good REST API design.)
Ok, you’re back and hopefully you’re face-palming. There’s a quick answer – it’s all wrong. I can’t find a single good decision in that entire API. Good going, TheBank.
Designing APIs is hard, admittedly, but whoever put this together hasn’t even grasped the fundamentals of REST (or HTTP). But this isn’t an isolated case – I’ve lost count of how many times I’ve had to integrate with similarly broken APIs – in fact it would be much quicker to count the ones which *are* well implemented (the figure is not far north of “zero”). This is probably the worst I’ve seen.
I’m not a REST zealot by any means, in fact on our course I openly admit that HATEOAS is a bit of a lofty goal and it’s no disgrace if you don’t go that far. I don’t care about purity or satisfying some aesthetic goal. What I do care about is wasted time and development effort, and I care if I’m forced to write brittle and error prone code.
So let’s run through TheBank’s blunders and see why it matters:
No URIs or Representations
Leaving aside the dodgy looking “endpoint.shtml” (what does this even mean? SHTML was a server side include, some kind of Apache extension. Why do I as the client care about this?), they are routing every single API call in through a single URI. Thus they are immediately losing the expressiveness of URIs. The URIs *are* the API.
So rather than an API, we have a single method with a huge telescoping list of query parameters.
Even though they call their API “RESTful”, there’s no trace of any kind of representation. This means that all the data for every call has to be converted into a long series of query params, leading to the very ugly and unreadable construction.
[There’s nothing wrong with query params – we use them on our REST courses. But only for constraints or extra information that doesn’t belong in the representations. Example – if you only want the first 20 records in a query, then this would make a good query param.]
Why this matters: if done properly, I could have quickly coded up a “Transaction” class in my client and let my framework (I was using Spring) to convert to JSON. Instead I had to spend time string concatenating, always an error prone and tedious process.
Invalid use of GET. No use of HTTP verbs.
GET, by definition, is for “safe” and “idempotent” operations. Meaning, no changes to state and no side effects. The “record” method is of course recording new transactions, so this has violated the contract of GET.
They’re clearly unaware that other verbs exisit. POST should have been used for this non-idempotent operation, but update and delete would have also been needed to avoid the ugly use of “method=record”.
Why this matters: I had to be extremely careful to ensure that my get requests are issued once and once only. Every call made to this API looked more or less identical because the very important “method=” is buried in an unreadable list of query params.
Implementing their own authentication scheme
The very weird process of hashing your API Key and Secret is clearly an authentication mechanism that they’ve invented themselves. Why? HTTP has a specified and well understood form of authentication – Basic Authentication. Under the standard, all I would have to do is send a “Authorization” header like this:
Authorization: Basic QWxhZGRpbjpPcGVuU2VzYW1l
The odd looking string there is the username (API key) and password (secret) separated by a colon and then base64 encoded.
Instead, they want me to SHA-256 my key and secret before sending in a weird custom header.
I imagine their reasoning here is that SHA-256 is a secure one way hash that can’t be intercepted and reversed. (The Base64 encoding is definitely not secure). This is totally unnecessary – if they’d simply mandated HTTPs, then the traffic would be automatically encrypted, including the username and password. My guess is they’ve had a business directive asking them to not insist on HTTPs, and they’ve tried to fix that by rolling their own (almost certainly broken) encryption scheme.
A fundamental rule of security is that you should never roll your own security scheme, because it *will* be flawed.
Why this matters: I don’t care that the bank might get hacked, but I do care about the wasted day I spent trying to comply with their weird hashing rules. If they’d done it properly, my REST Client would have been able to handle the key and secret through a simple method call.
Bad return codes
This one wins them the jackpot. Every one of their API calls returns “Success!” (HTTP 200), until you check the body string and find out it actually failed. So I’m forced to write client code like this:
ResponseBody response = rest.get("big ugly uri");
if (response.getEntity().equals("Transaction Suceeded"))
{
// continue
}
YES – they have misspelled “Suceeded” (should be two c’s). So – when they fix this typo my code will instantly break. Thanks, Bank.
Why this matters: I had to spend a long time probing their API to find out the strings they’re returning. I now have brittle string checks which are very likely to break at any time they decide to change those strings. And they will.
In many REST textbooks, they get themselves excited about HATEOAS. Forget that, the basics of URIs, Representations, HTTP Verbs, HTTP Return Codes and Security are all fundamental. Not many get all of these right, and a huge number get them ALL wrong. Don’t be like the bank.
Certainly all valid points raised by Mr Chesterwood. Thanks for highlighting.