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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
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
before_action :authenticate, :only => [:invalidate_tokens]

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?

end

def action_permission
case params[:action]
when 'invalidate_tokens', 'destroy'
'edit'
else
super
end
end

api :DELETE, '/users/:id/registration_tokens', N_("Invalidate all registration Tokens for a specific user.")
girijaasoni marked this conversation as resolved.
Show resolved Hide resolved
description <<-DOC
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 destroy
@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 JWTs for %s.' % @user.login)
end

api :DELETE, "/registration_tokens", N_("Invalidate all JSON Web Tokens (JWTs) for multiple users.")
param :search, String, :required => true
Copy link
Contributor

Choose a reason for hiding this comment

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

@girijaasoni could you please share an example for this query? What does the search param should contain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

curl --user test-user1:changeme -X DELETE http://localhost:3000/api/registration_tokens/remove_multiple?search=id%20%5E%20%281%2C2%2C3%29 -H 'Content-Type: application/json'

this is the command I'm using for a scoped search query id ^ ( 5 , 2, 3) you will need to URL encode in the search params as curl does not accept brackets and operators.

description <<-DOC
The users you specify will no longer be able to register hosts by using their JWTs.
DOC

def invalidate_tokens
raise ::Foreman::Exception.new(N_("Please provide search parameters")) 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 %s"), params[:search])
end
JwtSecret.where(user_id: @users).destroy_all
process_success _('Successfully invalidated JWTs for %s.' % @users.pluck(:login).to_sentence)
end
end
end
end
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_tokens, :destroy]
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#destroy', :on => :member
end
end

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

Expand Down
38 changes: 38 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,38 @@
require 'test_helper'

class Api::V2::RegistrationTokensControllerTest < ActionController::TestCase
test 'user shall invalidate tokens for self' do
user = FactoryBot.create(:user)
FactoryBot.create(:jwt_secret, token: 'test_jwt_secret', user: user)
delete :destroy, params: { :id => user.id.to_s}
user.reload
assert_nil user.jwt_secret
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_tokens, params: { :search => "id ^ (#{user.id})"}
user.reload
assert_nil user.jwt_secret
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_tokens, params: { :search => "id ^ (#{user.id})"}
user.reload
assert_response :forbidden
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_tokens
user.reload
assert_response :error
end
Comment on lines +30 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

We expect an exception to be raised in this case, correct? If so, the test should be updated to reflect that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think using assert_response :error is the way to handle that, do you have any specific assertion on mind

Copy link
Contributor

Choose a reason for hiding this comment

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

You can check if an exception was raised.

Copy link
Member

Choose a reason for hiding this comment

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

You can inspect the response, that the error text is correct.

end
Loading