diff --git a/app/assets/javascripts/discourse/templates/user/preferences.hbs b/app/assets/javascripts/discourse/templates/user/preferences.hbs index 0f2597edbb0..171da9306b9 100644 --- a/app/assets/javascripts/discourse/templates/user/preferences.hbs +++ b/app/assets/javascripts/discourse/templates/user/preferences.hbs @@ -336,7 +336,13 @@ {{else}} {{d-button action="revokeApiKey" actionParam=key class="btn" label="user.revoke_access"}} {{/if}} -

{{i18n "user.api_permissions"}} {{#if key.write}}{{i18n "user.api_read_write"}}{{else}}{{i18n "user.api_read"}}{{/if}}

+

+

+

{{i18n "user.api_approved"}} {{bound-date key.created_at}}

{{/each}} diff --git a/app/controllers/user_api_keys_controller.rb b/app/controllers/user_api_keys_controller.rb index 20ba2d84aa5..8d11d46c1e7 100644 --- a/app/controllers/user_api_keys_controller.rb +++ b/app/controllers/user_api_keys_controller.rb @@ -6,7 +6,7 @@ class UserApiKeysController < ApplicationController skip_before_filter :check_xhr, :preload_json before_filter :ensure_logged_in, only: [:create, :revoke, :undo_revoke] - AUTH_API_VERSION ||= 1 + AUTH_API_VERSION ||= 2 def new @@ -34,14 +34,14 @@ class UserApiKeysController < ApplicationController 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] @nonce = params[:nonce] - @access = params[:access] @client_id = params[:client_id] @auth_redirect = params[:auth_redirect] @push_url = params[:push_url] + @localized_scopes = params[:scopes].split(",").map{|s| I18n.t("user_api_key.scopes.#{s}")} + @scopes = params[:scopes] rescue Discourse::InvalidAccess @generic_error = true @@ -60,10 +60,6 @@ class UserApiKeysController < ApplicationController raise Discourse::InvalidAccess unless meets_tl? - request_read = params[:access].include? 'r' - request_read ||= params[:access].include? 'p' - request_write = params[:access].include? 'w' - validate_params # destroy any old keys we had @@ -72,12 +68,10 @@ class UserApiKeysController < ApplicationController key = UserApiKey.create!( application_name: params[:application_name], client_id: params[:client_id], - read: request_read, - push: params[:push_url].present?, user_id: current_user.id, - write: request_write, + push_url: params[:push_url], key: SecureRandom.hex, - push_url: params[:push_url] + scopes: params[:scopes].split(",") ) # we keep the payload short so it encrypts easily with public key @@ -85,7 +79,8 @@ class UserApiKeysController < ApplicationController payload = { key: key.key, nonce: params[:nonce], - access: key.access + push: key.has_push?, + api: AUTH_API_VERSION }.to_json public_key = OpenSSL::PKey::RSA.new(params[:public_key]) @@ -100,7 +95,7 @@ class UserApiKeysController < ApplicationController if current_key = request.env['HTTP_USER_API_KEY'] request_key = UserApiKey.find_by(key: current_key) revoke_key ||= request_key - if request_key && request_key.id != revoke_key.id && !request_key.write + if request_key && request_key.id != revoke_key.id && !request_key.scopes.include?("write") raise Discourse::InvalidAccess end end @@ -127,7 +122,7 @@ class UserApiKeysController < ApplicationController [ :public_key, :nonce, - :access, + :scopes, :client_id, :auth_redirect, :application_name @@ -135,13 +130,9 @@ class UserApiKeysController < ApplicationController end def validate_params - request_read = params[:access].include? 'r' - request_read ||= params[:access].include? 'p' - request_write = params[:access].include? 'w' + requested_scopes = Set.new(params[:scopes].split(",")) - 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 + raise Discourse::InvalidAccess unless UserApiKey.allowed_scopes.superset?(requested_scopes) # our pk has got to parse OpenSSL::PKey::RSA.new(params[:public_key]) diff --git a/app/models/user_api_key.rb b/app/models/user_api_key.rb index b8709328f76..6335fa5bdc4 100644 --- a/app/models/user_api_key.rb +++ b/app/models/user_api_key.rb @@ -1,10 +1,63 @@ class UserApiKey < ActiveRecord::Base + + SCOPES = { + read: [:get], + write: [:get, :post, :patch], + message_bus: [[:post, 'message_bus']], + push: nil, + notifications: [[:post, 'message_bus'], [:get, 'notifications#index'], [:put, 'notifications#mark_read']], + session_info: [[:get, 'session#current'], [:get, 'users#topic_tracking_state']] + } + belongs_to :user - def access - has_push = push && push_url.present? && SiteSetting.allowed_user_api_push_urls.include?(push_url) - "#{read ? "r" : ""}#{write ? "w" : ""}#{has_push ? "p" : ""}" + def self.allowed_scopes + Set.new(SiteSetting.allow_user_api_key_scopes.split("|")) end + + def self.available_scopes + @available_scopes ||= Set.new(SCOPES.keys.map(&:to_s)) + end + + def self.allow_permission?(permission, env) + verb, action = permission + actual_verb = env["REQUEST_METHOD"] || "" + + # safe in Ruby 2.3 which is only one supported + return false unless actual_verb.downcase == verb.to_s + return true unless action + + # not a rails route, special handling + return true if action == "message_bus" && env["PATH_INFO"] =~ /^\/message-bus\/.*\/poll/ + + params = env['action_dispatch.request.path_parameters'] + + return false unless params + + actual_action = "#{params[:controller]}##{params[:action]}" + actual_action == action + end + + def self.allow_scope?(name, env) + if allowed = SCOPES[name.to_sym] + good = allowed.any? do |permission| + allow_permission?(permission, env) + end + + good || allow_permission?([:post, 'user_api_keys#revoke'], env) + end + end + + def has_push? + (scopes.include?("push") || scopes.include?("notifications")) && push_url.present? && SiteSetting.allowed_user_api_push_urls.include?(push_url) + end + + def allow?(env) + scopes.any? do |name| + UserApiKey.allow_scope?(name, env) + end + end + end # == Schema Information diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 40163f2450b..4017061a46a 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -148,8 +148,7 @@ class UserSerializer < BasicUserSerializer { id: k.id, application_name: k.application_name, - read: k.read, - write: k.write, + scopes: k.scopes.map{|s| I18n.t("user_api_key.scopes.#{s}")}, created_at: k.created_at } end diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 83a06aeaa12..3d9aeed20d8 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -396,9 +396,9 @@ class PostAlerter end def push_notification(user, payload) - if SiteSetting.allow_push_user_api_keys && SiteSetting.allowed_user_api_push_urls.present? + if SiteSetting.allow_user_api_key_scopes.split("|").include?("push") && SiteSetting.allowed_user_api_push_urls.present? clients = user.user_api_keys - .where('push AND push_url IS NOT NULL AND position(push_url in ?) > 0 AND revoked_at IS NULL', + .where("'push' = ANY(scopes) AND push_url IS NOT NULL AND position(push_url in ?) > 0 AND revoked_at IS NULL", SiteSetting.allowed_user_api_push_urls) .pluck(:client_id, :push_url) diff --git a/app/views/user_api_keys/new.html.erb b/app/views/user_api_keys/new.html.erb index 6e0a6892091..fb689adc864 100644 --- a/app/views/user_api_keys/new.html.erb +++ b/app/views/user_api_keys/new.html.erb @@ -1,5 +1,5 @@

<%= t "user_api_key.title" %>

-
+
<% if @no_trust_level %>

<%= t("user_api_key.no_trust_level") %> @@ -10,7 +10,14 @@

<% else %>

- <%= t("user_api_key.description", application_name: @application_name, access: @access_description) %> + <%= t("user_api_key.description", application_name: @application_name) %> +

+

+

<%= form_tag(user_api_key_path) do %> <%= hidden_field_tag 'application_name', @application_name %> @@ -20,6 +27,7 @@ <%= hidden_field_tag 'auth_redirect', @auth_redirect %> <%= hidden_field_tag 'push_url', @push_url %> <%= hidden_field_tag 'public_key', @public_key%> + <%= hidden_field_tag 'scopes', @scopes%> <%= submit_tag t('user_api_key.authorize'), class: 'btn btn-danger', id: 'submit' %> <% end %>