-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Api key implementation #466
base: master
Are you sure you want to change the base?
Changes from all commits
636367a
d390724
55ab055
3b2c707
9cfb6ee
cf14ca4
98e5f38
d46f1d1
a24f1f9
99385e2
62ef0c7
a814f92
896d616
0a84054
0e60fad
5be2472
be2b48a
3b22322
c94ec41
5823b20
7007ebc
543e5ad
23f2551
b912261
a6b181d
2f19aa3
bc914f9
fd171e1
2431af6
8e7d33f
c6258a9
0a5fde7
2917a3c
3587ade
862cab8
f403dda
5737cc0
a89d1cd
d1c5a4c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
# frozen_string_literal: true | ||
|
||
class ApiKeyAbility | ||
include CanCan::Ability | ||
def initialize(api_key) | ||
return if api_key.blank? | ||
|
||
can :read, :all | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
# frozen_string_literal: true | ||
|
||
class Ability | ||
class UserAbility | ||
include CanCan::Ability | ||
|
||
def initialize(user) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
# frozen_string_literal: true | ||
|
||
class ApiKeysController < ApplicationController | ||
include ApiKeyAuthenticatable | ||
include SessionsHelper | ||
|
||
# Require token authentication for index | ||
|
||
def index | ||
@api_keys = ApiKey.accessible_by(current_ability) | ||
authorize! :index, @api_keys | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: differs from how we manage index in user_controller |
||
end | ||
|
||
def new | ||
@api_key = ApiKey.new | ||
authorize! :new, @api_key | ||
end | ||
|
||
def create | ||
@api_key = ApiKey.new(api_key_params) | ||
authorize! :create, @api_key | ||
respond_to do |format| | ||
if @api_key.save | ||
format.html do | ||
flash[:success] = "ApiKey added! It is #{@api_key.key}" | ||
redirect_to api_keys_url | ||
end | ||
else | ||
format.html { render 'new', status: :unprocessable_entity } | ||
end | ||
end | ||
end | ||
|
||
def destroy | ||
@api_key = ApiKey.find(params[:id]) | ||
authorize! :destroy, @api_key | ||
@api_key.destroy | ||
flash[:success] = 'ApiKey deleted!' | ||
redirect_to api_keys_url | ||
end | ||
|
||
private | ||
|
||
def api_key_params | ||
params.require(:api_key).permit(:bearer_name) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -2,12 +2,32 @@ | |||
|
||||
class ApplicationController < ActionController::Base | ||||
include SessionsHelper | ||||
include ApiKeyAuthenticatable | ||||
|
||||
before_action :still_authenticated? | ||||
before_action :api_auth | ||||
|
||||
def current_ability | ||||
@current_ability ||= if !session[:api_key_id].nil? | ||||
|
||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
ApiKeyAbility.new(current_bearer) | ||||
elsif !session[:user_id].nil? | ||||
UserAbility.new(current_user) | ||||
else | ||||
UserAbility.new(nil) | ||||
end | ||||
end | ||||
|
||||
private | ||||
|
||||
def still_authenticated? | ||||
log_out if should_log_out? | ||||
end | ||||
|
||||
def api_auth | ||||
return unless request.path[0, 5] == '/api/' | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Surely useless comment, but there is hard-coded text right here |
||||
|
||||
current_bearer = authenticate_or_request_with_http_token { |token, _options| authenticator(token) } | ||||
log_in_api current_bearer | ||||
end | ||||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
# frozen_string_literal: true | ||
|
||
module ApiKeyAuthenticatable | ||
extend ActiveSupport::Concern | ||
|
||
include ActionController::HttpAuthentication::Basic::ControllerMethods | ||
include ActionController::HttpAuthentication::Token::ControllerMethods | ||
|
||
attr_reader :current_api_key | ||
|
||
private | ||
|
||
attr_writer :current_api_key | ||
|
||
def authenticator(http_token) | ||
@current_api_key = ApiKey.authenticate_by_token! http_token | ||
|
||
current_api_key | ||
end | ||
end |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -1,6 +1,7 @@ | ||||
# frozen_string_literal: true | ||||
|
||||
class SessionsController < ApplicationController | ||||
include ApiKeyAuthenticatable | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I'm not sure this is still useful |
||||
def create | ||||
user = User.upsert_from_auth_hash(request.env['omniauth.auth']) | ||||
log_in user | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
# frozen_string_literal: true | ||
|
||
module ApiKeysHelper | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,8 +9,15 @@ def current_user | |
@current_user | ||
end | ||
|
||
def current_bearer | ||
return nil if session[:api_key_id].nil? | ||
|
||
@current_bearer = ApiKey.find(session[:api_key_id]) | ||
@current_bearer | ||
end | ||
|
||
def logged_in? | ||
!current_user.nil? | ||
!current_user.nil? || !current_bearer.nil? | ||
end | ||
|
||
def log_in(user) | ||
|
@@ -20,6 +27,12 @@ def log_in(user) | |
session[:groups] = user.groups | ||
end | ||
|
||
def log_in_api(bearer) | ||
reset_session # For security reasons, we clear the session data before login | ||
session[:api_key_id] = bearer.id | ||
session[:expires_at] = Time.current | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't understand why it's done this way |
||
end | ||
|
||
# TODO: also logout of sso | ||
def log_out | ||
reset_session | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
# frozen_string_literal: true | ||
|
||
class ApiKey < ApplicationRecord | ||
HMAC_SECRET_KEY = Rails.application.credentials.api_key_hmac_secret_key! | ||
|
||
validates :bearer_name, presence: true, allow_blank: false | ||
|
||
before_create :generate_token_hmac_digest | ||
|
||
attr_accessor :key | ||
|
||
def self.authenticate_by_token!(key) | ||
digest = OpenSSL::HMAC.hexdigest 'SHA256', HMAC_SECRET_KEY, key | ||
|
||
find_by! api_key: digest | ||
end | ||
|
||
private | ||
|
||
def generate_token_hmac_digest | ||
@key = SecureRandom.hex(32) | ||
|
||
digest = OpenSSL::HMAC.hexdigest 'SHA256', HMAC_SECRET_KEY, @key | ||
self.api_key = digest | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
<%# locals: (api_key:) -%> | ||
|
||
<div class="api_key"> | ||
<span><%= api_key.bearer_name %></span> | ||
<span><%=api_key.created_at %></span> | ||
<% if can?(:destroy, api_key) %> | ||
<%= button_to(api_key,method: :delete, data: { turbo_confirm: "Are you sure ?" }, 'aria-label': 'Delete this api key') do %> | ||
<%= svg_icon_tag 'icon_delete' %> | ||
<% end %> | ||
<% end %> | ||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
<%# locals: () -%> | ||
|
||
<%= form_with(model: @api_key, class: 'form') do |f| %> | ||
<div> | ||
<%= f.label :bearer_name %> | ||
<%= f.text_field :bearer_name, required: true %> | ||
</div> | ||
<div> | ||
<%= f.submit yield(:button_text) %> | ||
</div> | ||
<% end %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
<%# locals: () -%> | ||
|
||
<main> | ||
<div class="container"> | ||
<div class="card-details-container card-api-keys card-details-api-keys"> | ||
<div class="card card-details"> | ||
|
||
<div class="card-title"> | ||
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 448 512"><!--! Font Awesome Pro 6.2.0 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license (Commercial License) Copyright 2022 Fonticons, Inc. --> | ||
<path xmlns="http://www.w3.org/2000/svg" d="M224 256c70.7 0 128-57.3 128-128S294.7 0 224 0S96 57.3 96 128s57.3 128 128 128zm-45.7 48C79.8 304 0 383.8 0 482.3C0 498.7 13.3 512 29.7 512H418.3c16.4 0 29.7-13.3 29.7-29.7C448 383.8 368.2 304 269.7 304H178.3z"/> | ||
</svg> | ||
<h2>Api Keys</h2> | ||
</div> | ||
|
||
<%# Need to pass an instance for an ability with block https://github.com/CanCanCommunity/cancancan/blob/develop/docs/define_abilities_with_blocks.md %> | ||
<% if can?(:create, ApiKey) %> | ||
<%= render "components/buttons/button_primary_create_api", text: "new api key", path: new_api_key_path, aria_label: "Add a new api key" %> | ||
<% end %> | ||
|
||
</div> | ||
|
||
<div class="divider"></div> | ||
</div> | ||
<div class="card card-api-key card-content"> | ||
<div class="card-content-machines"> | ||
<%= render(@api_keys) || "No bearers of an api key" %> | ||
</div> | ||
</div> | ||
</div> | ||
</main> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# frozen_string_literal: true | ||
|
||
# locals: () | ||
|
||
json.array! @api_keys, :id, :bearer_name, :created_at, :updated_at |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<%# locals: () -%> | ||
|
||
<% provide :button_text, "Create" %> | ||
|
||
<h1>ApiKey#new</h1> | ||
|
||
<%= render 'form' %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
<%# locals: (aria_label:, path:, text:) -%> | ||
|
||
<%= button_to(path, method: :get, class: "button-primary", 'aria-label': aria_label) do %> | ||
<%= text %> | ||
<%= svg_icon_tag 'icon_plus' %> | ||
<% end %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
VfC7hNP55EjMyi4vt8jK8lAaRmOkql+wsyAFxSVsQdVT8tbZk1l0dgyELsvoVfHwU9kqzI+wMIFyuH6MoXBhcghZfa6m5g683FM06BDkYPwwDflEVeox0DvWmgGji4qzk3oFe57T9qQT736mz3dWfeQTzvjHnaUpq27gYQTpQHOuELjYwKsMXbFRFiNNMmG5phiG2k0Asc7dqZ8CRPwmhJYPm5aJBc9Bzrz3ebBnhxmZ+JzZ8AsZnnvnAFnylm2jRwgN91kJE9l3fkWVTlF6rm0EoCJ9r3neibTeDu2PtNnkYISp3O7chrBJKo6FS9GFAcOxgkxB8QPiX2TV2s3jPiNyxffxLqUe+jziPP4dDiHdZvKrONIltblTJV2WTzur7/82fCEhQUf2oM7uwavq/yvQNOlay4nUD9TyBGZrs1fyFuulT4Ik2/w4T7usUC6am1xIzB2ITF8IuAgolkT/5EPsIs45gJEpJy9Fqlg1PAv01NN9hhViEl+A--tARvrsUmwyVJ9/XI--ytpULZnEcYeOSqfUYhHAnA== | ||
Ctjn6rzrT8lN8apnt2Pd02BtoVstxzsZTgu7DACFn3A6pXt5rbEb1Z79Tcx5R6t2lB9MbLauB/DwUyS9JdzjzKeQQnGvGDHo0j9/vR1I1N3m3+sEXOuPNihRzzCLgePkh4jYtqNZ8NK1MdjmeG6kfPvB4fYNgLz+LWHudMLDhW98Ee3U6yso/edEMlXolmXyp3C4LeyqW/4qVPqaonUU4LJpYeG+UZndxjifEU8vpIhY4dW10mVpdO5JSnteNbtRBc/ZojuVla69hLEgYyr1uiyoy23ArtztfpyXhZEc6cmuJoXi7gpJd9sIDOZf2/xwasRW1EkI3NgpREj4Gy1t7lSwWLf8vp4j3qpC3/ifnGUUNAoJfBjax18WVOfzf2z051Q1TRN0tMhTknihNcUjAer54eT+JYHv7PLbxACcHs4evI/mnuglBOvI9DggODsUIe/7BqM0oQ0j1lj0/m7FHnexdc8FFHrrZ5IXN3VvZ6E0dAEMXNY8gS66mNjNEf2jfXKRicG/eu0FHYRiR00QgtVKsrggUy5IQnLc5Nj3lAq74Bi53UFDQI/AnaIA6uGQtf2lG3XTyfaadwMA6hOFKSQtVd1/OVJAptFIyJkkMiBiJAcU--rNRzrZK7K/fPFPxc--fNlBrjqZoGt/U748IRtjng== |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
QF2+HIDkGNTPjx81hqlGjQAZyIzZVAp9c7J+dWPqlggPt9u/LWhxWWi6vY7KXGYe3opGafwsWlOgrO0BXtzbOg+RK+Bq9a8rUgsKchqHi44B487Y9/rTvS5bcCTJj5c78AJFdZCsEdOXqUv6khBGHB2i8fGQ5LZyh/SFJhmyt3QqGuL6Ig4MsuD/275o2M94CjrlXT2nNGe6BPl94GurtQv7s67bbgrSy3N2f0Kc+VR4VfrIhTwgkpXxEBv7vk/ok/1WvYpqRbFNuyPhiqkZ6Rj+0oK7JCxtFDL15tBtrIRbdpNuBeWO8BQwf/CfvcgBsJnOGg//LU/AJe+ndMCdUzdfVpuB1QKG4A62lmrLNOEwlw/K3JdPsWqPEh6rSVIr5nAzl4oaxMo0NZs7VK7ZhNpGxkvdNVQEZDxchZkaanQOIFU02240w3nMHYx5aed1MEj2ZpuR26Jg+W85m3K0BvsCDcXAouLTvMCB8kGZ8H6SOWExIdbIhmM3--x6H3t+K9w244qMLp--x+vXvS3cObYjuhqsAb4HYA== | ||
Zmnkjz2oYau2HVmSP0GPQ/fzdTezrNhjFyIiP7okk4UZoOATR8X1mbTwulJF0/1vmtmoCr8jTAPWLfRY+0SwZAidd7yh16vhWCy6mpd5/OMeupQencSQCl3T0wgkkMtIa/J64DbfRav8BPhPBHi4St+1x62FM+WRw0udfj+xA2AUPxtIy/FZXIm+WUHcj6wQKXvIY2xJ1YcpA2WAsmow+1d1Aps1UWzMm0pwysm4SsUCDVS3nloaNTQ25g5+0HZ49biY7nbU6E1IM6dsgwK2/yRrrPxgDpv22L/r3BNp1wv0u4XP5LG+/yfaRfcdaO34WegffGsGEbuBf+hJ/fmxEJ3kq3Uk7DVlbFft94kIfzhHPOU/fi6EX4pWwrS4hExJ43X6/nEMtUY2b6ZXQDLazHv6rI2FcVzsSP+CRQwFc3VU2sjiJbHoe9hh1Jz27d/8rc+BjAwiwwpzZyTsAUlFy92qLZIENJskOhiowZewErJIUmV31L1ImRw/NCCC/Qt+hE8VvTkYmlWFU++9v4wddd5u2GUHzmHh7h7QFUCgKYFeqZiZbi58ihfIn/j7mpdJVk6lc73C+zAMI52rN65bZ6pGySU1/IJMGkhPc181hVC8tdiryw==--ss5nXaT6eOm/wEgh--oHW8aumKNzcCbzU7XFQyXg== |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
151fe6f76326223cfb5f7cef5369c504 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
mrjWeVMW0ZzGnTS1Yw+YRq/mJUTEyb7vTGp781GRYBv7s4XMK4eRkNrcG3jlb6StbugQFUa3yKSSUac1E/LHrtRgDS8FO7lkN3m3gnD/apzZ6QFQ+ub/Gzhgg9htCB2OSTwKwHINeCuog/wlxeGd3VGon0nNltGlYKv8Fet2W8KyMaOBNSFTwf6zv0jPrke1LRGiqAlNuaA=--w7kZ8Fb41WHlHnQB--mk0Xq4zeXvm7j1VYpZ39jQ== |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,5 +1,8 @@ | ||||||
# frozen_string_literal: true | ||||||
|
||||||
# Path to login via api key | ||||||
AUTH_API_PATH = '/auth/api' | ||||||
|
||||||
Comment on lines
+3
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think it's an artifact of the previous version that uses full connection and not Authorization header |
||||||
# Path to login via SSO authentication | ||||||
AUTH_PATH = '/auth/keycloak' | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,14 @@ | |
resources :free_accesses, shallow: true, except: [:index, :show] | ||
end | ||
|
||
resources :api_keys | ||
|
||
scope 'api' do | ||
resources :users, controller: :users, as: 'api_users' | ||
resources :api_keys, controller: :api_keys, as: 'api_api_keys' | ||
resources :machines, controller: :machines, as: 'api_machines' | ||
Comment on lines
+19
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Surely things to do about these endpoints but that's not the goal of this PR |
||
end | ||
|
||
get '/search', as: 'search', to: 'search#search' | ||
|
||
# Reveal health status on /up that returns 200 if the app boots with no exceptions, otherwise 500. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# frozen_string_literal: true | ||
|
||
class ApiKeys < ActiveRecord::Migration[7.0] | ||
def change | ||
create_table :api_keys do |t| | ||
t.integer :bearer_id, null: false, index: { unique: true } | ||
t.string :bearer_name, null: false | ||
t.string :api_key, null: false, index: { unique: true } | ||
t.datetime :api_key_start_at, index: { unique: true } | ||
|
||
t.timestamps | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
# frozen_string_literal: true | ||
|
||
class FixApiKey < ActiveRecord::Migration[7.0] | ||
def change | ||
change_table :api_keys do |t| | ||
t.remove :api_key_start_at, | ||
type: :datetime | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
# frozen_string_literal: true | ||
|
||
class FixApiKeyBearerId < ActiveRecord::Migration[7.0] | ||
def change | ||
change_table :api_keys do |t| | ||
t.remove :bearer_id, | ||
type: :integer | ||
end | ||
end | ||
end |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need in a near future to refine the permissions of each key, maybe by listing all the endpoints when creating one and let the user select which he wants the bearer to access (read, create, update, destroy), and change the permissions in the edit view.