5 ms·
What is "IsAdmin" doing in the "create user" request DTO in the first place? The examples seem to indicate inappropriate re-use of data models. Would it not be
by octo888 1y ago
What is "IsAdmin" doing in the "create user" request DTO in the first place? The examples seem to indicate inappropriate re-use of data models.
Would it not be better to:
type CreateUserRequest struct {
Username string
Password string
}
type UserView struct {
Username string
IsAdmin boolean
}
etc?
No need to just have just 1 model that maps 1:1 to your DB row. This applies to all languages
- ljm 1y agoAside from security issues it presents, it’s just a really bad idea to tightly couple your data model to your API like that. Any change to your DB schema is liable to become a breaking change on your API. If you need separate types for your requests and responses so be it.
- matthew16550 1y agoIt's a concept from Fieldings REST thesis that is important to the current meaning of REST: Transfer Objects are not the Storage Objects.
- deleted 1y ago[deleted]
- db48x 1y agoYes!
- WhyNotHugo 1y ago> No need to just have just 1 model that maps 1:1 to your DB row. This applies to all languages I agree with your stance, but creating lots of DTOs is more work and we developers are lazy. It's incredibly common to see a single "User" type used everywhere. This also makes all User-related functions re-usable in all contexts (again: it's the fast and lazy approach).
- delusional 1y ago> No need to just have just 1 model that maps 1:1 to your DB row. This applies to all languages One reason is to avoid copying data constantly. I don't just mean this from an efficiency perspective, but also (and maybe more so) from a simplicity one. If you have a library for shoving data into a struct mechanistically, but you then take the data from that struct and shove it into an additional struct, what's the point of the library? You're writing the code move the data anyway. In my dayjob I see this tendency constantly to have a lot of different very narrow structs that somehow integrate into some library, and then a TON of supporting code to copy between those structs. Only to then do very little actually useful work with any of the data at the end. I generally think you'd be happier with fatter structs that integrated less with weird "struct-filling" libraries.
- masklinn 1y ago> In my dayjob I see this tendency constantly to have a lot of different very narrow structs that somehow integrate into some library, and then a TON of supporting code to copy between those structs. Maybe that's the problem to solve, rather than exposing the entire internal world to the outside? Because different views of the same entities is pretty critical otherwise it's way too easy to start e.g. returning PII to public endpoints because some internal process needed it.
- delusional 1y ago> exposing the entire internal world to the outside That's not at all what I said. You don't need a struct to avoid exposing internal data. If you're building a JSON object, you can just not write the code to format some fields out. You don't need a new data layout for that.
- CactusRocket 1y agoThat's the whole point of the first item in the article, and the original comment you were replying to. In Go (and some other languages) that formatting is implicit and automatic. So you need to write to code to NOT format the fields out. Which leads to the potential security issues where data is leaked, or you can inject data into "hidden" fields. So since it's implicit and automatic, it's safer to, as a rule, define separate structs for the data input and map them, so that there is absolutely no chance (implicit or explicit) to leak or inject data.
- masklinn 1y ago> If you're building a JSON object, you can just not write the code to format some fields out. Did you fail to read the article somehow? Few if any modern language requires you to write the code to format individual fields out. Even Go does not, though when it comes to json most fields need to be annotated because of the choices it made to tie casing and visibility, but then it doesn't even require you to opt the type itself into being serializable, every struct is serializable by default. Skipping fields on a serializable structure is what requires extra work.
- liampulles 1y agoYeah its a bad pattern. And we can expect to see it in a sizable number of production codebases.
- diggan 1y agoORMs love to put people and projects in the situation where it's much easier for data types in your program to match exactly with the tables in your database, but reality often requires them to be decoupled instead. That "create user" request contains "isAdmin" smells like someone mapped that exact request to a table, and just because the table has "isAdmin", the data type now also has it.
- ajross 1y agoExactly. This isn't a flaw in the runtime or the metaphor, the bug here is that the app exposes private data in a public API. That's a mistake that doesn't care about implementation choice. You can make the same blunder with a RESTful interface or a CGI script. At most, you can argue that simple serialization libraries (Go's is indeed one of the best) make it more tempting to "just send the data" in such a design, so if you squint really (really) hard, you can call this a "footgun" I guess. But the rest of the headline is 100% nonsense. This is not about "Go" or "parsers". At all.
- yencabulator 1y agoExcept in the case of encoding/xml garbage, that's really a limitation of the Go parser. It's a very non-strict parser and doesn't actually validate that the input conforms to XML. Think of it more as a query language for a stream of XML fragments.
- tln 1y agoWhat? It's about parsers in Go's standard library.
- ajross 1y agoA "parser" is the piece of software that translates unstructured input (usually text) into structured output that better reflects the runtime of a programming language. A "security bug" in a parser is normally construed to be the existence of an input that causes the software to do something incorrect/undocumented/unexpected. Nothing in the article discusses a parser or anything like a parser bug. The article doesn't like that the semantics of the user-facing API wrapped around the parser is, I guess, "easy to make mistakes with". That's an article about API design, at most. But that's boring and specious and doesn't grab clicks, so they want you to think that Go's parsers are insecure instead.
- sfvisser 1y agoExactly right. Better have a domain layer with data types representing the domain object 1:1 and add one or more API layers on top for interacting with those for some modality. Creation, deletion, verification, auth etc. The security failure is not the parsing library, but failing to model your application architecture properly.