diff --git a/CHANGELOG.md b/CHANGELOG.md index a503e7bdc..d00c262e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,17 @@ +## Unreleased + +### API Changes + +* You can now have multiple API keys. +* API keys can be restricted to only work with certain IPs or certain API + endpoints. +* If you're an app or script developer, and you have an app that requests API + keys from users, you're highly encouraged to only request the minimum API + permissions necessary for your app to work. +* If you have a privileged account, and you run scripts under your account, + you're highly encouraged to restrict your API keys to limit damage in case + they get leaked or stolen. + ## 2021-02-05 ### Changes diff --git a/app/controllers/api_keys_controller.rb b/app/controllers/api_keys_controller.rb index 94ddfb448..a9ca5a382 100644 --- a/app/controllers/api_keys_controller.rb +++ b/app/controllers/api_keys_controller.rb @@ -1,23 +1,34 @@ class ApiKeysController < ApplicationController respond_to :html, :json, :xml + def new + @api_key = authorize ApiKey.new(user: CurrentUser.user, **permitted_attributes(ApiKey)) + respond_with(@api_key) + end + def create - @api_key = authorize ApiKey.new(user: CurrentUser.user) + @api_key = authorize ApiKey.new(user: CurrentUser.user, **permitted_attributes(ApiKey)) @api_key.save respond_with(@api_key, location: user_api_keys_path(CurrentUser.user.id)) end + def edit + @api_key = authorize ApiKey.find(params[:id]) + respond_with(@api_key) + end + + def update + @api_key = authorize ApiKey.find(params[:id]) + @api_key.update(permitted_attributes(@api_key)) + respond_with(@api_key, location: user_api_keys_path(CurrentUser.user.id)) + end + def index params[:search][:user_id] = params[:user_id] if params[:user_id].present? @api_keys = authorize ApiKey.visible(CurrentUser.user).paginated_search(params, count_pages: true) respond_with(@api_keys) end - def show - @api_key = authorize ApiKey.find(params[:id]) - respond_with(@api_key) - end - def destroy @api_key = authorize ApiKey.find(params[:id]) @api_key.destroy diff --git a/app/javascript/src/styles/common/simple_form.scss b/app/javascript/src/styles/common/simple_form.scss index b2489cee3..338f316f4 100644 --- a/app/javascript/src/styles/common/simple_form.scss +++ b/app/javascript/src/styles/common/simple_form.scss @@ -69,6 +69,13 @@ form.simple_form { } } } + + &.stacked-hints { + span.hint { + display: block; + padding-left: 0; + } + } } form.inline-form { diff --git a/app/logical/session_loader.rb b/app/logical/session_loader.rb index b3b8ce6f9..9ae708fc4 100644 --- a/app/logical/session_loader.rb +++ b/app/logical/session_loader.rb @@ -60,7 +60,7 @@ class SessionLoader end def has_api_authentication? - request.authorization.present? || params[:login].present? || params[:api_key].present? + request.authorization.present? || params[:login].present? || (params[:api_key].present? && params[:api_key].is_a?(String)) end private @@ -87,9 +87,10 @@ class SessionLoader authenticate_api_key(login, api_key) end - def authenticate_api_key(name, api_key) - user = User.find_by_name(name)&.authenticate_api_key(api_key) + def authenticate_api_key(name, key) + user, api_key = User.find_by_name(name)&.authenticate_api_key(key) raise AuthenticationFailure if user.blank? + raise User::PrivilegeError if !api_key.has_permission?(request.remote_ip, request.params[:controller], request.params[:action]) CurrentUser.user = user end diff --git a/app/models/api_key.rb b/app/models/api_key.rb index 61621ac5b..35b507733 100644 --- a/app/models/api_key.rb +++ b/app/models/api_key.rb @@ -1,6 +1,13 @@ class ApiKey < ApplicationRecord + array_attribute :permissions + array_attribute :permitted_ip_addresses + + normalize :permissions, :normalize_permissions + normalize :name, :normalize_text + belongs_to :user - validates_uniqueness_of :key + validate :validate_permissions, if: :permissions_changed? + validates :key, uniqueness: true, if: :key_changed? has_secure_token :key def self.visible(user) @@ -16,4 +23,45 @@ class ApiKey < ApplicationRecord q = q.apply_default_order(params) q end + + concerning :PermissionMethods do + def has_permission?(ip, controller, action) + ip_permitted?(ip) && action_permitted?(controller, action) + end + + def ip_permitted?(ip) + return true if permitted_ip_addresses.empty? + permitted_ip_addresses.any? { |permitted_ip| ip.in?(permitted_ip) } + end + + def action_permitted?(controller, action) + return true if permissions.empty? + + permissions.any? do |permission| + permission == "#{controller}:#{action}" + end + end + + def validate_permissions + permissions.each do |permission| + if !permission.in?(ApiKey.permissions_list) + errors.add(:permissions, "can't allow invalid permission '#{permission}'") + end + end + end + + class_methods do + def normalize_permissions(permissions) + permissions.compact_blank + end + + def permissions_list + Rails.application.routes.routes.select do |route| + route.defaults[:controller].present? && !route.internal + end.map do |route| + "#{route.defaults[:controller]}:#{route.defaults[:action]}" + end.uniq.sort + end + end + end end diff --git a/app/models/user.rb b/app/models/user.rb index a9fcb7707..6a50b10ba 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -209,7 +209,7 @@ class User < ApplicationRecord def authenticate_api_key(key) api_key = api_keys.find_by(key: key) - api_key.present? && ActiveSupport::SecurityUtils.secure_compare(api_key.key, key) && self + api_key.present? && ActiveSupport::SecurityUtils.secure_compare(api_key.key, key) && [self, api_key] end def authenticate_password(password) @@ -560,6 +560,7 @@ class User < ApplicationRecord ] end + # XXX def api_token api_keys.first.try(:key) end diff --git a/app/policies/api_key_policy.rb b/app/policies/api_key_policy.rb index d43083d81..a1cdd690a 100644 --- a/app/policies/api_key_policy.rb +++ b/app/policies/api_key_policy.rb @@ -1,4 +1,8 @@ class ApiKeyPolicy < ApplicationPolicy + def new? + !user.is_anonymous? + end + def create? !user.is_anonymous? end @@ -7,10 +11,22 @@ class ApiKeyPolicy < ApplicationPolicy !user.is_anonymous? end + def edit? + record.user == user + end + + def update? + record.user == user + end + def destroy? record.user == user end + def permitted_attributes + [:name, :permitted_ip_addresses, permissions: []] + end + def api_attributes super - [:key] end diff --git a/app/views/api_keys/_form.html.erb b/app/views/api_keys/_form.html.erb new file mode 100644 index 000000000..ec83e54c4 --- /dev/null +++ b/app/views/api_keys/_form.html.erb @@ -0,0 +1,6 @@ +<%= edit_form_for(api_key, html: { class: "stacked-hints" }) do |f| %> + <%= f.input :name, as: :string, hint: "An optional name to help you remember what this key is for." %> + <%= f.input :permitted_ip_addresses, label: "IP Addresses", as: :string, hint: "An optional list of IPs allowed to use this key. Leave blank to allow all IPs." %> + <%= f.input :permissions, as: :select, collection: ApiKey.permissions_list, hint: "An optional list of API endpoints this key can use. Ctrl+click to select multiple endpoints. Leave blank to allow all API endpoints.", input_html: { multiple: true, size: 10 } %> + <%= f.submit "Create" %> +<% end %> diff --git a/app/views/api_keys/_secondary_links.html.erb b/app/views/api_keys/_secondary_links.html.erb index d98882116..2900a3b73 100644 --- a/app/views/api_keys/_secondary_links.html.erb +++ b/app/views/api_keys/_secondary_links.html.erb @@ -1,5 +1,5 @@ <% content_for(:secondary_links) do %> <%= subnav_link_to "Listing", user_api_keys_path(CurrentUser.user.id) %> - <%= subnav_link_to "New", user_api_keys_path(CurrentUser.user.id), method: :post %> + <%= subnav_link_to "New", new_user_api_key_path(CurrentUser.user.id) %> <%= subnav_link_to "Help", wiki_page_path("help:api") %> <% end %> diff --git a/app/views/api_keys/edit.html.erb b/app/views/api_keys/edit.html.erb new file mode 100644 index 000000000..1bf651027 --- /dev/null +++ b/app/views/api_keys/edit.html.erb @@ -0,0 +1,8 @@ +<%= render "secondary_links" %> + +
If you're a developer, you can use an API key to access the <%= link_to_wiki "#{Danbooru.config.canonical_app_name} API", "help:api" %>. If you're not a - developer, you probably don't need an API key.
+ developer, and you're not using any third-party apps, then you probably don't need an API key.Your API key is like your password. Anyone who has it has full access to
your account. Don't give your API key to apps or people you don't trust, and don't post your
@@ -37,11 +37,20 @@
<% end %>
<% if params[:user_id].present? && !@api_keys.present? %>
- <%= link_to "Create API key", user_api_keys_path(CurrentUser.user.id), method: :post %>
+ <%= link_to "Create API key", new_user_api_key_path(CurrentUser.user.id) %>
<% else %>
<%= table_for @api_keys, width: "100%", class: "striped autofit" do |t| %>
+ <% t.column :name %>
<% t.column :key, td: { class: "col-expand" } %>
+ <% t.column :permissions do |api_key| %>
+ <%= safe_join(api_key.permissions, "
".html_safe).presence || "All" %>
+ <% end %>
+
+ <% t.column "IPs" do |api_key| %>
+ <%= safe_join(api_key.permitted_ip_addresses, "
".html_safe).presence || "All" %>
+ <% end %>
+
<% if !params[:user_id].present? %>
<% t.column "User" do |api_key| %>
<%= link_to_user api_key.user %>
@@ -53,7 +62,8 @@
<% end %>
<% t.column column: "control" do |api_key| %>
- <%= link_to "Delete", api_key, method: :delete %>
+ <%= link_to "Edit", edit_api_key_path(api_key) %>
+ | <%= link_to "Delete", api_key, method: :delete %>
<% end %>
<% end %>
diff --git a/app/views/api_keys/new.html.erb b/app/views/api_keys/new.html.erb
new file mode 100644
index 000000000..0ed5593d7
--- /dev/null
+++ b/app/views/api_keys/new.html.erb
@@ -0,0 +1,8 @@
+<%= render "secondary_links" %>
+
+