From 691f739f116852db798517f58271f1a3f973815d Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Tue, 23 Aug 2016 16:48:00 +1000
Subject: [PATCH] better error handling push notifications imply read access,
 no need for a special permission

---
 app/controllers/user_api_keys_controller.rb | 25 ++++++++++-----------
 app/views/user_api_keys/new.html.erb        | 10 +++++++++
 config/locales/server.en.yml                |  2 ++
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/app/controllers/user_api_keys_controller.rb b/app/controllers/user_api_keys_controller.rb
index 16afd0ac0..5ab817980 100644
--- a/app/controllers/user_api_keys_controller.rb
+++ b/app/controllers/user_api_keys_controller.rb
@@ -16,7 +16,7 @@ class UserApiKeysController < ApplicationController
     end
 
     require_params
-    validate_params(_skip_push=true)
+    validate_params
 
     unless current_user
       cookies[:destination_url] = request.fullpath
@@ -24,6 +24,11 @@ class UserApiKeysController < ApplicationController
       return
     end
 
+    if current_user.trust_level < SiteSetting.min_trust_level_for_user_api_key
+      @no_trust_level = true
+      return
+    end
+
     @access_description = params[:access].include?("w") ? t("user_api_key.read_write") : t("user_api_key.read")
     @application_name = params[:application_name]
     @public_key = params[:public_key]
@@ -40,6 +45,8 @@ class UserApiKeysController < ApplicationController
         @push_url = nil
       end
     end
+  rescue Discourse::InvalidAccess
+    @generic_error = true
   end
 
   def create
@@ -56,7 +63,7 @@ class UserApiKeysController < ApplicationController
     raise Discourse::InvalidAccess if current_user.trust_level < SiteSetting.min_trust_level_for_user_api_key
 
     request_read = params[:access].include? 'r'
-    request_push = params[:access].include? 'p'
+    request_read ||= params[:access].include? 'p'
     request_write = params[:access].include? 'w'
 
     validate_params
@@ -68,11 +75,11 @@ class UserApiKeysController < ApplicationController
       application_name: params[:application_name],
       client_id: params[:client_id],
       read: request_read,
-      push: request_push,
+      push: params[:push_url].present?,
       user_id: current_user.id,
       write: request_write,
       key: SecureRandom.hex,
-      push_url: request_push ? params[:push_url] : nil
+      push_url: params[:push_url]
     )
 
     # we keep the payload short so it encrypts easily with public key
@@ -118,21 +125,13 @@ class UserApiKeysController < ApplicationController
 
   def validate_params(skip_push_check = false)
     request_read = params[:access].include? 'r'
-    request_push = params[:access].include? 'p'
+    request_read ||= params[:access].include? 'p'
     request_write = params[:access].include? 'w'
 
     raise Discourse::InvalidAccess unless request_read || request_push
     raise Discourse::InvalidAccess if request_read && !SiteSetting.allow_read_user_api_keys
     raise Discourse::InvalidAccess if request_write && !SiteSetting.allow_write_user_api_keys
 
-    unless skip_push_check
-      raise Discourse::InvalidAccess if request_push && !SiteSetting.allow_push_user_api_keys
-
-      if request_push && !SiteSetting.allowed_user_api_push_urls.split('|').any?{|u| params[:push_url] == u}
-        raise Discourse::InvalidAccess
-      end
-    end
-
     # our pk has got to parse
     OpenSSL::PKey::RSA.new(params[:public_key])
   end
diff --git a/app/views/user_api_keys/new.html.erb b/app/views/user_api_keys/new.html.erb
index 04044f36f..f9cd18f41 100644
--- a/app/views/user_api_keys/new.html.erb
+++ b/app/views/user_api_keys/new.html.erb
@@ -1,5 +1,14 @@
 <h1><%= t "user_api_key.title" %></h1>
 <div>
+<% if @no_trust_level %>
+  <h3>
+  <%= t("user_api_key.no_trust_level") %>
+  </h3>
+<% elsif @generic_error %>
+  <h3>
+  <%= t("user_api_key.generic_error") %>
+  </h3>
+<% else %>
 <p>
   <%= t("user_api_key.description", application_name: @application_name, access: @access_description) %>
 </p>
@@ -14,5 +23,6 @@
   <%= submit_tag t('user_api_key.authorize'), class: 'btn btn-danger' %>
 <% end %>
 </div>
+<% end %>
 
 
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 88b35191f..4208612d5 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -646,6 +646,8 @@ en:
     read: "read"
     read_write: "read/write"
     description: "Would you like to grant \"%{application_name}\" %{access} access to your account?"
+    no_trust_level: "Sorry, you do not have the required trust level to access the user API"
+    generic_error: "Sorry, we are unable to issue user API keys, this feature may be disabled by the site admin"
 
   reports:
     visits: