Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FEATURE: Add read-only scope to API keys (#14856) 

This commit adds a global read-only scope that can be used to create
new API keys.
This commit is contained in:
Bianca Nenciu 2021-11-10 17:48:00 +02:00 committed by GitHub
parent 6a749b95c9
commit 3791fbd919
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 132 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
app/assets/javascripts/admin/addon/controllers/admin-api-keys-new.js
View File

@ -10,7 +10,8 @@ import { ajax } from "discourse/lib/ajax";
export default Controller.extend({
userModes: null,
useGlobalKey: false,
scopeModes: null,
globalScopes: null,
scopes: null,
init() {
@ -20,6 +21,13 @@ export default Controller.extend({
{ id: "all", name: I18n.t("admin.api.all_users") },
{ id: "single", name: I18n.t("admin.api.single_user") },
]);
this.set("scopeModes", [
{ id: "granular", name: I18n.t("admin.api.scopes.granular") },
{ id: "read_only", name: I18n.t("admin.api.scopes.read_only") },
{ id: "global", name: I18n.t("admin.api.scopes.global") },
]);
this._loadScopes();
},
@ -49,14 +57,23 @@ export default Controller.extend({
this.set("userMode", userMode);
},
@action
changeScopeMode(scopeMode) {
this.set("scopeMode", scopeMode);
},
@action
save() {
if (!this.useGlobalKey) {
if (this.scopeMode === "granular") {
const selectedScopes = Object.values(this.scopes)
.flat()
.filterBy("selected");
this.model.set("scopes", selectedScopes);
} else if (this.scopeMode === "read_only") {
this.model.set("scopes", [this.globalScopes.findBy("key", "read")]);
} else if (this.scopeMode === "all") {
this.model.set("scopes", null);
}
return this.model.save().catch(popupAjaxError);
@ -78,6 +95,10 @@ export default Controller.extend({
_loadScopes() {
return ajax("/admin/api/keys/scopes.json")
.then((data) => {
// remove global scopes because there is a different dropdown
this.set("globalScopes", data.scopes.global);
delete data.scopes.global;
this.set("scopes", data.scopes);
})
.catch(popupAjaxError);

16
app/assets/javascripts/admin/addon/templates/api-keys-new.hbs
View File

@ -36,12 +36,18 @@
{{/admin-form-row}}
{{/if}}
{{#admin-form-row label="admin.api.use_global_key"}}
{{input type="checkbox" checked=useGlobalKey}}
{{#admin-form-row label="admin.api.scope_mode"}}
{{combo-box content=scopeModes value=scopeMode onChange=(action "changeScopeMode")}}
{{#if (eq scopeMode "read_only")}}
<p>{{i18n "admin.api.scopes.descriptions.global.read"}}</p>
{{else if (eq scopeMode "global")}}
<p>{{i18n "admin.api.scopes.global_description"}}</p>
{{/if}}
{{/admin-form-row}}
{{#unless useGlobalKey}}
<div class="scopes-title">{{i18n "admin.api.scopes.title"}}</div>
{{#if (eq scopeMode "granular")}}
<h2 class="scopes-title">{{i18n "admin.api.scopes.title"}}</h2>
<p>{{i18n "admin.api.scopes.description"}}</p>
<table class="scopes-table grid">
<thead>
@ -82,7 +88,7 @@
{{/each-in}}
</tbody>
</table>
{{/unless}}
{{/if}}
{{d-button icon="check" label="admin.api.save" action=(action "save") class="btn-primary" disabled=saveDisabled}}
{{/if}}

2
app/assets/javascripts/admin/addon/templates/api-keys-show.hbs
View File

@ -83,7 +83,7 @@
{{/admin-form-row}}
{{#if model.api_key_scopes.length}}
<div class="scopes-title">{{i18n "admin.api.scopes.title"}}</div>
<h2 class="scopes-title">{{i18n "admin.api.scopes.title"}}</h2>
<table class="scopes-table grid">
<thead>

3
app/assets/stylesheets/common/admin/api.scss
View File

@ -179,9 +179,6 @@ table.api-keys {
width: 50%;
}
.scopes-title {
font-size: $font-up-2;
font-weight: bold;
text-decoration: underline;
margin-top: 20px;
}
}

63
app/models/api_key_scope.rb
View File

@ -17,6 +17,9 @@ class ApiKeyScope < ActiveRecord::Base
return @default_mappings unless @default_mappings.nil?
mappings = {
global: {
read: { methods: %i[get] }
},
topics: {
write: { actions: %w[posts#create], params: %i[topic_id] },
read: {
@ -48,12 +51,7 @@ class ApiKeyScope < ActiveRecord::Base
}
}
mappings.each_value do |resource_actions|
resource_actions.each_value do |action_data|
action_data[:urls] = find_urls(action_data[:actions])
end
end
parse_resources!(mappings)
@default_mappings = mappings
end
@ -62,33 +60,48 @@ class ApiKeyScope < ActiveRecord::Base
return default_mappings if plugin_mappings.empty?
default_mappings.deep_dup.tap do |mappings|
plugin_mappings.each do |resource|
resource.each_value do |resource_actions|
resource_actions.each_value do |action_data|
action_data[:urls] = find_urls(action_data[:actions])
end
end
mappings.deep_merge!(resource)
plugin_mappings.each do |plugin_mapping|
parse_resources!(plugin_mapping)
mappings.deep_merge!(plugin_mapping)
end
end
end
def find_urls(actions)
Rails.application.routes.routes.reduce([]) do |memo, route|
defaults = route.defaults
action = "#{defaults[:controller].to_s}##{defaults[:action]}"
path = route.path.spec.to_s.gsub(/\(\.:format\)/, '')
api_supported_path = path.end_with?('.rss') || route.path.requirements[:format]&.match?('json')
excluded_paths = %w[/new-topic /new-message /exception]
def parse_resources!(mappings)
mappings.each_value do |resource_actions|
resource_actions.each_value do |action_data|
action_data[:urls] = find_urls(actions: action_data[:actions], methods: action_data[:methods])
end
end
end
memo.tap do |m|
if actions.include?(action) && api_supported_path && !excluded_paths.include?(path)
m << "#{path} (#{route.verb})"
def find_urls(actions:, methods:)
action_urls = []
method_urls = []
if actions.present?
Rails.application.routes.routes.reduce([]) do |memo, route|
defaults = route.defaults
action = "#{defaults[:controller].to_s}##{defaults[:action]}"
path = route.path.spec.to_s.gsub(/\(\.:format\)/, '')
api_supported_path = path.end_with?('.rss') || route.path.requirements[:format]&.match?('json')
excluded_paths = %w[/new-topic /new-message /exception]
memo.tap do |m|
if actions.include?(action) && api_supported_path && !excluded_paths.include?(path)
m << "#{path} (#{route.verb})"
end
end
end
end
if methods.present?
methods.each do |method|
method_urls << "* (#{method})"
end
end
action_urls + method_urls
end
end

8
config/locales/client.en.yml
View File

@ -4144,6 +4144,7 @@ en:
no_description: (no description)
all_api_keys: All API Keys
user_mode: User Level
scope_mode: Scope
impersonate_all_users: Impersonate any user
single_user: "Single User"
user_placeholder: Enter username
@ -4154,12 +4155,15 @@ en:
delete: Permanently Delete
not_shown_again: This key will not be displayed again. Make sure you take a copy before continuing.
continue: Continue
use_global_key: Global Key (allows all actions)
scopes:
description: |
When using scopes, you can restrict an API key to a specific set of endpoints.
You can also define which parameters will be allowed. Use commas to separate multiple values.
title: Scopes
granular: Granular
read_only: Read-only
global: Global
global_description: API key has no restriction and all endpoints are accessible.
resource: Resource
action: Action
allowed_parameters: Allowed Parameters
@ -4167,6 +4171,8 @@ en:
any_parameter: (any parameter)
allowed_urls: Allowed URLs
descriptions:
global:
read: Restrict API key to read-only endpoints.
topics:
read: Read a topic or a specific post in it. RSS is also supported.
write: Create a new topic or post to an existing one.

86
spec/models/api_key_spec.rb
View File

@ -107,47 +107,63 @@ describe ApiKey do
let(:env) { request.env }
let(:scope) do
ApiKeyScope.new(resource: 'topics', action: 'read', allowed_parameters: { topic_id: '3' })
end
let(:key) { ApiKey.new(api_key_scopes: [scope]) }
it 'allows the request if there are no allowed IPs' do
key.allowed_ips = nil
key.api_key_scopes = []
expect(key.request_allowed?(env)).to eq(true)
context 'with regular scopes' do
let(:scope) do
ApiKeyScope.new(resource: 'topics', action: 'read', allowed_parameters: { topic_id: '3' })
end
it 'allows the request if there are no allowed IPs' do
key.allowed_ips = nil
key.api_key_scopes = []
expect(key.request_allowed?(env)).to eq(true)
end
it 'rejects the request if the IP is not allowed' do
key.allowed_ips = %w[115.65.76.87]
expect(key.request_allowed?(env)).to eq(false)
end
it 'allow the request if there are not allowed params' do
scope.allowed_parameters = nil
expect(key.request_allowed?(env)).to eq(true)
end
it 'rejects the request when params are different' do
request.path_parameters = { controller: "topics", action: "show", topic_id: "4" }
expect(key.request_allowed?(env)).to eq(false)
end
it 'accepts the request if one of the parameters match' do
request.path_parameters = { controller: "topics", action: "show", topic_id: "4" }
scope.allowed_parameters = { topic_id: %w[3 4] }
expect(key.request_allowed?(env)).to eq(true)
end
it 'allow the request when the scope has an alias' do
request.path_parameters = { controller: "topics", action: "show", id: "3" }
expect(key.request_allowed?(env)).to eq(true)
end
it 'rejects the request when the main parameter and the alias are both used' do
request.path_parameters = { controller: "topics", action: "show", topic_id: "3", id: "3" }
expect(key.request_allowed?(env)).to eq(false)
end
end
it 'rejects the request if the IP is not allowed' do
key.allowed_ips = %w[115.65.76.87]
expect(key.request_allowed?(env)).to eq(false)
end
context 'with global:read scope' do
let(:scope) do
ApiKeyScope.new(resource: 'global', action: 'read')
end
it 'allow the request if there are not allowed params' do
scope.allowed_parameters = nil
expect(key.request_allowed?(env)).to eq(true)
end
it 'allows only GET requests for global:read' do
request.request_method = 'GET'
expect(key.request_allowed?(env)).to eq(true)
it 'rejects the request when params are different' do
request.path_parameters = { controller: "topics", action: "show", topic_id: "4" }
expect(key.request_allowed?(env)).to eq(false)
end
it 'accepts the request if one of the parameters match' do
request.path_parameters = { controller: "topics", action: "show", topic_id: "4" }
scope.allowed_parameters = { topic_id: %w[3 4] }
expect(key.request_allowed?(env)).to eq(true)
end
it 'allow the request when the scope has an alias' do
request.path_parameters = { controller: "topics", action: "show", id: "3" }
expect(key.request_allowed?(env)).to eq(true)
end
it 'rejects the request when the main parameter and the alias are both used' do
request.path_parameters = { controller: "topics", action: "show", topic_id: "3", id: "3" }
expect(key.request_allowed?(env)).to eq(false)
request.request_method = 'POST'
expect(key.request_allowed?(env)).to eq(false)
end
end
end
end

2
spec/requests/admin/api_controller_spec.rb
View File

@ -237,7 +237,7 @@ describe Admin::ApiController do
scopes = response.parsed_body['scopes']
expect(scopes.keys).to contain_exactly('topics', 'users', 'email', 'posts')
expect(scopes.keys).to contain_exactly('topics', 'users', 'email', 'posts', 'global')
end
end
end