From 19d7545318f8a42168261d5c09b8978d2ae63b90 Mon Sep 17 00:00:00 2001 From: cfitz Date: Fri, 4 Jan 2019 04:46:18 +0100 Subject: [PATCH] FEATURE: Make auth_redirect param options on user_api_keys This is a possible solution for https://meta.discourse.org/t/user-api-keys-specification/48536/19 This allows for user-api-key requests to not require a redirect url. Instead, the encypted payload will just be displayed after creation ( which can be copied pasted into an env for a CLI, for example ) Also: Show instructions when creating user-api-key w/out redirect This adds a view to show instructions when requesting a user-api-key without a redirect. It adds a erb template and json format. Also adds a i18n user_api_key.instructions for server.en.yml --- app/controllers/user_api_keys_controller.rb | 24 +++++++++---- app/views/user_api_keys/new.html.erb | 2 +- app/views/user_api_keys/show.html.erb | 2 ++ config/locales/server.en.yml | 1 + .../requests/user_api_keys_controller_spec.rb | 35 +++++++++++++++++++ 5 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 app/views/user_api_keys/show.html.erb diff --git a/app/controllers/user_api_keys_controller.rb b/app/controllers/user_api_keys_controller.rb index 44c3efe1327..a9db864e907 100644 --- a/app/controllers/user_api_keys_controller.rb +++ b/app/controllers/user_api_keys_controller.rb @@ -51,9 +51,9 @@ class UserApiKeysController < ApplicationController require_params - unless SiteSetting.allowed_user_api_auth_redirects + if params.key?(:auth_redirect) && SiteSetting.allowed_user_api_auth_redirects .split('|') - .any? { |u| params[:auth_redirect] == u } + .none? { |u| params[:auth_redirect] == u } raise Discourse::InvalidAccess end @@ -61,12 +61,13 @@ class UserApiKeysController < ApplicationController raise Discourse::InvalidAccess unless meets_tl? validate_params + @application_name = params[:application_name] # destroy any old keys we had UserApiKey.where(user_id: current_user.id, client_id: params[:client_id]).destroy_all key = UserApiKey.create!( - application_name: params[:application_name], + application_name: @application_name, client_id: params[:client_id], user_id: current_user.id, push_url: params[:push_url], @@ -76,7 +77,7 @@ class UserApiKeysController < ApplicationController # we keep the payload short so it encrypts easily with public key # it is often restricted to 128 chars - payload = { + @payload = { key: key.key, nonce: params[:nonce], push: key.has_push?, @@ -84,9 +85,19 @@ class UserApiKeysController < ApplicationController }.to_json public_key = OpenSSL::PKey::RSA.new(params[:public_key]) - payload = Base64.encode64(public_key.public_encrypt(payload)) + @payload = Base64.encode64(public_key.public_encrypt(@payload)) - redirect_to "#{params[:auth_redirect]}?payload=#{CGI.escape(payload)}" + if params[:auth_redirect] + redirect_to("#{params[:auth_redirect]}?payload=#{CGI.escape(@payload)}") + else + respond_to do |format| + format.html { render :show } + format.json do + instructions = I18n.t("user_api_key.instructions", application_name: @application_name) + render json: { payload: @payload, instructions: instructions } + end + end + end end def revoke @@ -124,7 +135,6 @@ class UserApiKeysController < ApplicationController :nonce, :scopes, :client_id, - :auth_redirect, :application_name ].each { |p| params.require(p) } end diff --git a/app/views/user_api_keys/new.html.erb b/app/views/user_api_keys/new.html.erb index 28531e07db6..7657919d621 100644 --- a/app/views/user_api_keys/new.html.erb +++ b/app/views/user_api_keys/new.html.erb @@ -24,7 +24,7 @@ <%= hidden_field_tag 'access', @access %> <%= hidden_field_tag 'nonce', @nonce %> <%= hidden_field_tag 'client_id', @client_id %> - <%= hidden_field_tag 'auth_redirect', @auth_redirect %> + <%= hidden_field_tag('auth_redirect', @auth_redirect) if @auth_redirect %> <%= hidden_field_tag 'push_url', @push_url %> <%= hidden_field_tag 'public_key', @public_key%> <%= hidden_field_tag 'scopes', @scopes%> diff --git a/app/views/user_api_keys/show.html.erb b/app/views/user_api_keys/show.html.erb new file mode 100644 index 00000000000..7d657efe09e --- /dev/null +++ b/app/views/user_api_keys/show.html.erb @@ -0,0 +1,2 @@ +

<%= t("user_api_key.instructions", application_name: @application_name) %>

+<%= @payload %> diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 848fdc4a3fd..0cfb9cdd120 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -874,6 +874,7 @@ en: read: "read" read_write: "read/write" description: "\"%{application_name}\" is requesting the following access to your account:" + instructions: "We just generated a new user API key for you to use with \"%{application_name}\", please paste the following key into your application:" 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" scopes: diff --git a/spec/requests/user_api_keys_controller_spec.rb b/spec/requests/user_api_keys_controller_spec.rb index ddaedd25896..ee5ffb4d7ff 100644 --- a/spec/requests/user_api_keys_controller_spec.rb +++ b/spec/requests/user_api_keys_controller_spec.rb @@ -205,5 +205,40 @@ describe UserApiKeysController do expect(response.status).to eq(302) end + + it "will just show the payload if no redirect" do + user = Fabricate(:user, trust_level: 0) + sign_in(user) + + args.delete(:auth_redirect) + + SiteSetting.min_trust_level_for_user_api_key = 0 + post "/user-api-key", params: args + expect(response.status).not_to eq(302) + payload = Nokogiri::HTML(response.body).at('code').content + encrypted = Base64.decode64(payload) + key = OpenSSL::PKey::RSA.new(private_key) + parsed = JSON.parse(key.private_decrypt(encrypted)) + api_key = UserApiKey.find_by(key: parsed["key"]) + expect(api_key.user_id).to eq(user.id) + end + + it "will just show the JSON payload if no redirect" do + user = Fabricate(:user, trust_level: 0) + sign_in(user) + + args.delete(:auth_redirect) + + SiteSetting.min_trust_level_for_user_api_key = 0 + post "/user-api-key.json", params: args + expect(response.status).not_to eq(302) + payload = JSON.parse(response.body)["payload"] + encrypted = Base64.decode64(payload) + key = OpenSSL::PKey::RSA.new(private_key) + parsed = JSON.parse(key.private_decrypt(encrypted)) + api_key = UserApiKey.find_by(key: parsed["key"]) + expect(api_key.user_id).to eq(user.id) + + end end end