Skip to content
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

Fixes #37936 - Invalidate jwt for any user or users(API) #10397

Merged
merged 1 commit into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions app/controllers/api/v2/registration_tokens_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
module Api
module V2
class RegistrationTokensController < V2::BaseController
include Foreman::Controller::UsersMixin

include Foreman::Controller::Parameters::User
include Foreman::Controller::AutoCompleteSearch
before_action :authenticate, :only => [:invalidate_jwt_tokens, :invalidate_jwt]

def resource_class
User
end

def find_resource(permission = :view_users)
editing_self? ? User.find(User.current.id) : User.authorized(permission).except_hidden.find(params[:id])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekohl what do you think, should we block changes to hidden accounts here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also wondering about that.

end

def action_permission
case params[:action]
when 'invalidate_jwt_tokens', 'invalidate_jwt'
'edit'
else
super
end
end

api :DELETE, '/users/:id/registration_tokens', N_("Invalidate all registration tokens for a specific user.")
description <<-DOC
ShimShtein marked this conversation as resolved.
Show resolved Hide resolved
The user you specify will no longer be able to register hosts by using their JWTs.
DOC
param :id, String, :desc => N_("ID of the user"), :required => true

def invalidate_jwt
@user = find_resource(:edit_users)
unless @user
raise ::Foreman::Exception.new(N_("No record found for %s"), params[:id])
end
@user.jwt_secret&.destroy
girijaasoni marked this conversation as resolved.
Show resolved Hide resolved
process_success _("Successfully invalidated registration tokens for %s.\n" % @user.login)
end

api :DELETE, "/registration_tokens", N_("Invalidate all registration tokens for multiple users.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why didn't we use /users here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to REST principles, you should state what resource you are acting upon. If you wan to use nested resources (as in users/:id/registration_token) that would work only for a specific user. There is no good way to specify a sub-object of multiple parent objects. So since registration token is an object by itself, we are addressing multiple registration token objects by a search condition.

param :search, String, :desc => N_("URL-encoded search query that selects users for which registration tokens will be invalidated. Search query example: id ^ (2, 4, 6)"), :required => true
description <<-DOC
The users you specify will no longer be able to register hosts by using their JWTs.
DOC

def invalidate_jwt_tokens
raise ::Foreman::Exception.new(N_("Please provide search parameter")) if params[:search].blank?
@users = resource_scope_for_index(:permission => :edit_users).except_hidden.uniq
if @users.blank?
raise ::Foreman::Exception.new(N_("No record found for search '%s'"), params[:search]) end
JwtSecret.where(user_id: @users).destroy_all
process_success _("Successfully invalidated registration tokens for %s.\n" % @users.pluck(:login).to_sentence)
girijaasoni marked this conversation as resolved.
Show resolved Hide resolved
end
end
end
end
5 changes: 4 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,10 @@ def editing_self?(options = {})
options[:user_id].to_i == id ||
options[:controller].to_s == 'api/v2/personal_access_tokens' &&
options[:action] =~ /show|destroy|index|create/ &&
options[:user_id].to_i == id
options[:user_id].to_i == id ||
options[:controller].to_s == 'api/v2/registration_tokens' &&
options[:action] =~ /invalidate_jwt/ &&
options[:id].to_i == id
end

def taxonomy_foreign_conditions
Expand Down
3 changes: 2 additions & 1 deletion config/initializers/f_foreman_permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,8 @@
:"api/v2/users" => [:create]
map.permission :edit_users,
:users => [:edit, :update, :invalidate_jwt],
:"api/v2/users" => [:update]
:"api/v2/users" => [:update],
:"api/v2/registration_tokens" => [:invalidate_jwt_tokens, :invalidate_jwt]
map.permission :destroy_users,
:users => [:destroy],
:"api/v2/users" => [:destroy]
Expand Down
7 changes: 7 additions & 0 deletions config/routes/api/v2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,13 @@
resources :mail_notifications, :only => [:create, :destroy, :update]
get 'mail_notifications', :to => 'mail_notifications#user_mail_notifications', :on => :member
get 'extlogin', :to => 'users#extlogin', :on => :collection
delete 'registration_tokens', :to => 'registration_tokens#invalidate_jwt', :on => :member
end
end

resources :registration_tokens, :only => [:invalidate_jwt_tokens] do
collection do
delete '/', :action => :invalidate_jwt_tokens
end
end

Expand Down
46 changes: 46 additions & 0 deletions test/controllers/api/v2/registration_tokens_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
require 'test_helper'

class Api::V2::RegistrationTokensControllerTest < ActionController::TestCase
test 'user shall invalidate tokens for self' do
user = users(:one)
FactoryBot.create(:jwt_secret, token: 'test_jwt_secret', user: user)
delete :invalidate_jwt, params: { :id => user.id.to_s}, session: set_session_user(user)
user.reload
assert_response :success
assert_nil user.jwt_secret
girijaasoni marked this conversation as resolved.
Show resolved Hide resolved
end

test 'user with edit permission should be able to invalidate jwt for another user' do
setup_user 'edit', 'users'
user = users(:one)
FactoryBot.create(:jwt_secret, token: 'test_jwt_secret', user: user)
delete :invalidate_jwt_tokens, params: { :search => "id ^ (#{user.id})"}, session: set_session_user(User.current)
user.reload
assert_response :success
assert_nil user.jwt_secret
girijaasoni marked this conversation as resolved.
Show resolved Hide resolved
end

test 'user without edit permission should not be able to invalidate jwt for another user' do
User.current = users(:one)
user = users(:two)
FactoryBot.create(:jwt_secret, token: 'test_jwt_secret', user: user)
delete :invalidate_jwt_tokens, params: { :search => "id ^ (#{user.id})"}, session: set_session_user(User.current)
user.reload
assert_response :forbidden
girijaasoni marked this conversation as resolved.
Show resolved Hide resolved
assert_not_nil user.jwt_secret
response = JSON.parse(@response.body)
assert_equal "Missing one of the required permissions: edit_users", response['error']['details']
end

test 'invalidating jwt should fail without search params' do
setup_user 'edit', 'users'
user = users(:two)
FactoryBot.create(:jwt_secret, token: 'test_jwt_secret', user: user)
delete :invalidate_jwt_tokens, session: set_session_user(User.current)
user.reload
assert_response :error
girijaasoni marked this conversation as resolved.
Show resolved Hide resolved
assert_not_nil user.jwt_secret
response = JSON.parse(@response.body)
assert_equal "ERF42-7534 [Foreman::Exception]: Please provide search parameter", response['error']['message']
end
end
Loading