<p><b>@gravitystorm</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1904#discussion_r222237958">app/models/capability.rb</a>:</p>
<pre style='color:#555'>> +  include CanCan::Ability
+
+  def initialize(user, token)
+    if user
+      can [:read, :read_one], UserPreference if capability?(token, :allow_read_prefs)
+      can [:update, :update_one, :delete_one], UserPreference if capability?(token, :allow_write_prefs)
+
+    end
+  end
+
+  private
+
+  # If a user provides no tokens, they've authenticated via a non-oauth method
+  # and permission to access to all capabilities is assumed.
+  def capability?(token, cap)
+    token.nil? || token.read_attribute(cap)
</pre>
<p>The original <code>token.nil?</code> check was here when you were combining both the Ability and Capability stuff, so for normal web stuff it was ok if the token was nil. But it's been refactored since, so that it's only used for token-based capability checks. I worry that this opens up a foot-gun situation, since as far as I can tell there's no reason for calling <code>capability?</code> without a token -  what do you think?</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1904#pullrequestreview-161073475">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABWnLV4v5oEIxg1WwYS9pL4W-y1EUooqks5uhH_agaJpZM4Uq7L9">mute the thread</a>.<img src="https://github.com/notifications/beacon/ABWnLSB-bpAUmt935eK7agMbBjOuw6Gyks5uhH_agaJpZM4Uq7L9.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/openstreetmap/openstreetmap-website","title":"openstreetmap/openstreetmap-website","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/openstreetmap/openstreetmap-website"}},"updates":{"snippets":[{"icon":"PERSON","message":"@gravitystorm commented on #1904"}],"action":{"name":"View Pull Request","url":"https://github.com/openstreetmap/openstreetmap-website/pull/1904#pullrequestreview-161073475"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/1904#pullrequestreview-161073475",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/1904#pullrequestreview-161073475",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
},
{
"@type": "MessageCard",
"@context": "http://schema.org/extensions",
"hideOriginalBody": "false",
"originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB",
"title": "@gravitystorm commented on 1904",
"sections": [
{
"text": "",
"activityTitle": "**Andy Allan**",
"activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png",
"activitySubtitle": "@gravitystorm",
"facts": [

]
}
],
"potentialAction": [
{
"targets": [
{
"os": "default",
"uri": "https://github.com/openstreetmap/openstreetmap-website/pull/1904#pullrequestreview-161073475"
}
],
"@type": "OpenUri",
"name": "View on GitHub"
},
{
"name": "Unsubscribe",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 346796797\n}"
}
],
"themeColor": "26292E"
}
]</script>