Allow & Deny: A Security Primer
Meteor security is always an interesting topic.
Not because Meteor has bad security (it doesn’t), but simply because Meteor apps require slightly different security patterns compared to traditional web apps.
This is especially true when it comes to Allow & Deny, one of Meteor’s two approaches security (the other being Meteor Methods).
Now we’ve written about Allow & Deny vs Methods before, and we came down firmly on the site of Methods. That being said, if you do want to use Allow & Deny, you might as well get it right. So today, we’ll explore a couple patterns that can help you minimize security risks while keeping your code lightweight.
Read Me First
If you need to get up to speed, I recommend first reading our previous articles on the topic of Meteor security as well as the official Meteor documentation:
- Meteor & Security: Setting the Record Straight
- Meteor Methods vs Client-Side Operations
- Allow & Deny
You can also check out our Allow & Deny Security Challenge posts, which were the inspiration for this one:
Building A Chat App
Let’s pick a concrete example: a simple chat app with two collections, Meteor.users
and Messages
. Each message
has three properties: the message body
, the createdAt
timestamp, and a userId
pointing back to the message’s author.
Using SimpleSchema, we would define our schema like this:
MessageSchema = new SimpleSchema({
body: {
type: String
},
createdAt: {
type: Date
},
userId: {
type: String
}
});
The Allow & Deny Security Challenge
At this point, maybe you want to see if you can come up with a solution yourself first?
If so, you can take our Allow & Deny Security Challenge. Using this MeteorPad as a starting point, can you enforce update security permissions purely with Allow & Deny?
Over 20 people took the challenge and submitted their own solutions. Once you’ve given it a shot yourself, you can check out the results here and learn which mistakes were most common.
Enforcing Ownership
So what kind of security requirements would we need to implement such an app? Note that for simplicity’s sake, we’ll only focus on the update
operation for now.
Our first requirement is that you should not be able to edit other user’s messages. In terms of allow
code, this requirement gives us the following rule:
Messages.allow({
update: function (userId, doc, fields, modifier) {
if (userId && doc.userId === userId) {
return true;
}
}
});
Now we could stop here, but if we did we’d be leaving open a pretty big security hole. Do you see it?
Restricting Properties
The problem is that we’re only checking if a user owns a message, not which property of the message they’re modifying. Which makes it possible for them to change a message’s timestamp, or even assign it to another user altogether!
Note that I recently looked through the code of a popular Meteor open-source chat app, and it contained this exact issue. And for that matter, so did Telescope at one point.
So let’s add a second requirement: you should not be able to edit the createdAt
and userId
properties. Since we’re blacklisting properties, we’ll add a deny
callback this time:
Messages.deny({
update: function (userId, doc, fields, modifier) {
if (_.contains(fields, "createdAt") || _.contains(fields, "userId")) {
return true;
}
}
});
We’re using Underscore’s _.contains()
to check of the array of fields being modified contains either createdAt
or userId
. If so, the deny
callback will return true
and the operation will fail.
Filtering Arbitrary Properties
Despite our best efforts, it turns out we still have a potential security issue! We’re denying the update if it contains createdAt
or userId
, but what if it contains foo
, bar
, or literally any other random field?
As you may have guessed, there’s nothing in our code that prevents users from adding any arbitrary property. It might not seem like a huge problem, but it means that any user can pollute your database with hidden bits of data. So our second requirement should instead become: you can only modify the body
property.
We can implement this by adopting a whitelist instead of a blacklist. So we’ll get rid of our deny
callback and put everything in our allow
callback once more:
var whitelist = ["body"];
Messages.allow({
update: function (userId, doc, fields, modifier) {
if (userId && doc.userId === userId && _.difference(fields, whitelist).length === 0) {
return true;
}
}
});
What we’re doing here is computing the difference between the fields
and whitelist
arrays to obtain an array of any properties present in fields
but not in whitelist
. And we only permit the update if that array is empty.
So any update containing createdAt
and userId
will fail, but so will one containing foo
or bar
, since none of those are in the whitelist.
While we’re at it, we can also validate that our fields contain the right kind of data by hooking up our schema to our collection using the Collection2 package. Just add the following line of code:
Messages.attachSchema(MessageSchema);
Schema-Based Whitelisting
We can tidy things up a bit by using our schema to generate our whitelist directly. We’ll extend SimpleSchema with a new editable
boolean schema property, then use it to mark which fields should be whitelisted:
SimpleSchema.extendOptions({
editable: Match.Optional(Boolean)
});
MessageSchema = new SimpleSchema({
body: {
type: String,
editable: true
},
createdAt: {
type: Date
},
userId: {
type: String
}
});
All that’s left to do is generate our whitelist from the schema:
var whitelist = _.filter(_.keys(MessageSchema), function (property) {
return MessageSchema[property].editable;
});
And then, just as before, use it inside our allow
callback:
Messages.allow({
update: function (userId, doc, fields, modifier) {
if (userId && doc.userId === userId && _.difference(fields, whitelist).length === 0) {
return true;
}
}
});
We’re extracting the names of our three properties using _.keys
, then filtering them to only keep those marked as editable (note that we can’t filter directly on MessagesSchema
because we would lose the field names in the process).
Other Approaches
The main weakness with this strategy is that Meteor only requires one allow
callback to return true to approve an operation.
This is generally not a problem, but it’s still important to be conscious of this. As Eric Dobbertin states in his meteor-security package’s readme:
Most developers handle security by simply defining a few allow functions. This may work in most cases, but many people don’t realize that only one allow function needs to return true and then the rest of them aren’t even called.
If you use a lot of community packages in your app, there is the possibility that one of them will add an allow function that returns true for a perfectly good reason, but if you are not aware of it, you may not even realize that your allow function is never being called, and your security logic is not being applied.
Speaking of meteor-security, it’s an excellent way to handle allow/deny in a simpler and more secure manner. For example, here’s how you would rewrite our code:
Security.defineMethod("ifIsOwner", {
fetch: [],
transform: null,
deny: function (type, arg, userId, doc) {
return userId !== doc.userId;
}
});
var whitelist = _.filter(_.keys(MessageSchema), function (property) {
return MessageSchema[property].editable;
});
Messages.permit('update').ifLoggedIn().isIsOwner().onlyProps(whitelist).apply();
First, we’re defining a custom ifIsOwner
security check that takes care of enforcing ownership. Then, we define our whitelist and finally chain together our various security requirements.
Combining Callbacks
That being said, you might sometimes need to go beyond simple whitelisting. For example, let’s think about adding a “+1” feature to our chat app to let users like each other’s messages.
We need to keep track of which users have liked a message, so we’ll add a likes
array to each message to keep track of these users’ _id
s.
So we now have the following added requirements:
- We need to add a new
allow
callback that lets users modify any message, but only thelikes
property. - You can only add an
_id
to thelikes
array once. - You can only add your own
_id
.
Here’s what that new callback looks like (alternatively, we could also have added an if
statement to our first callback):
Messages.allow({
update: function(userId, doc, fields, modifier) {
return
userId
&& _.keys(modifier).isEqualTo(["$addToSet"])
&& _.keys(modifier.$addToSet).isEqualTo(["likes"])
&& modifier.$addToSet.likes === userId;
}
});
Comparing Arrays
We’re using a hypothetical isEqualTo
function that makes sure two arrays contain the same elements, independent of their order.
A possible implementation using Underscore could be:
Array.prototype.isEqualTo = function (array2) {
var array1 = this;
var intersection = _.intersection(array1, array2);
return array1.length === array2.length && array1.length === intersection.length;
}
First, we’re checking that the operation is using the $addToSet
modifier – and only the $addToSet
modifier! We’re then making sure it’s only modifying the likes
property. Finally, we’re ensuring that the user is adding their own _id
(i.e. not liking the post in somebody else’s name).
We now have two allow
callbacks. But remember that a security failure in one callback lets a malicious user bypass all callbacks. So every additional allow
callback we add increases our exposure to security issues.
A Note About fields
It might’ve seemed easier to check the fields
argument provided by Meteor rather than get the keys from modifier.$addToSet
.
But fields
has its limits: for example, it only contains top-level fields. Meaning that for an update on user.profile.name
and user.profile.email
, the fields
argument would only contain profile
. Although it doesn’t matter in this case, it’s good to keep this in mind.
We should also mention that nested fields come with their own set of issues, since you can either write {$set: {profile: {name: "foo"}}}
or {$set: {"profile.name": "foo"}}
.
The Limits of Allow & Deny
Let’s go one step further: what if you want to store the total likes count in a likesCount
property?
This means amending our previous allow
callback to also support using the $inc
operator on the likesCount
field:
Messages.allow({
update: function(userId, doc, fields, modifier) {
return
userId
&& !_.contains(doc.likes, userId)
&& _.keys(modifier).isEqualTo(["$addToSet", "$inc"])
&& _.keys(modifier.$addToSet).isEqualTo(["likes"])
&& _.keys(modifier.$inc).isEqualTo(["likesCount"])
&& modifier.$addToSet.likes === userId
&& modifier.$inc.likesCount === 1;
}
});
So to recap, we’re checking that:
- The user hasn’t previously liked the document.
- The operation is only using the
$addToSet
andlikes
modifiers. - The
$addToSet
modifier is only affecting thelikes
field. - The
$inc
modifier is only affecting thelikesCount
field. - The content of the
$addToSet
modifier is equal to the user’s_id
. - The content of the
$inc
modifier is equal to1
.
Compared to this mess, using a Meteor Method for the whole thing might turn out to be a lot simpler:
Meteor.methods({
likeMessage: function (messageId) {
if (this.userId) {
var message = Messages.findOne(messageId);
Messages.update(messageId, {
$addToSet: { likes: this.userId },
$set: {likesCount: message.likes.length+1}
});
}
}
});
Using Collection Hooks
Alternatively, another option would be keeping the allow
callback from the previous step and using a collection hook to calculate the value of likesCount
. You can learn more about collection hooks here.
The Pattern-Matching Approach
Our rule-by-rule solution works, but a better (and cleaner) approach was offered by Pete Corey: use Meteor’s check
method to match the modifier to a predefined pattern.
His solution might require a little more code, but it’s certainly easier to reason about, which matters a lot when it comes to plugging security holes.
Complete Code on MeteorPad
You can check out the complete code for this example on MeteorPad.
To keep things simple, we’ve used a method for the insert
operation and haven’t actually implemented message editing in the UI, but you should be able to play around with the console to see if you can hack the app.
Make sure you also look at Pete Corey’s solution. And if you want even more approaches, I examined quite a few in The Allow & Deny Security Challenge: Results.
Security Recap
So let’s review. Just for a relatively simple set of requirements like these, we had to check that:
- The user is logged in.
- The user owns the document being modified.
- The user hasn’t already performed the operation on that document.
- The operation only affects modifiable fields.
- The operation is using the proper modifiers.
- The operation is using the proper modifier values.
If you use Allow/Deny in your own code, now would be a good time to go through it and make sure it passes this checklist!
Conclusion
The reason why we recommend using Methods is not because they’re inherently superior to Allow/Deny, it’s because Allow/Deny is very tricky to get right, even for experienced Meteor developers.
But for reasonably simple use cases (especially if users are only ever modifying their own documents), Allow/Deny can still be a useful tool.
So by following stricter patterns or using packages such as meteor-security, you’ll hopefully be able to avoid some of the more common mistakes and make your app more secure!
Comments
comments powered by Disqus