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

Conversation

girijaasoni
Copy link
Contributor

@girijaasoni girijaasoni commented Dec 4, 2024

Link to UI PR: #10357

@girijaasoni girijaasoni changed the title Fixes #37936 - As a user, I want to invalidate jwt for specific user(… Fixes #37936 - Invalidate jwt for any user or users(API) Dec 4, 2024
end
end

api :DELETE, '/users/:id/registration_tokens', N_("Invalidate all JSON Web Tokens (JWTs) for a specific user.")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
api :DELETE, '/users/:id/registration_tokens', N_("Invalidate all JSON Web Tokens (JWTs) for a specific user.")
api :DELETE, '/users/:id/registration_tokens', N_("Invalidate all registration tokens for a specific user.")


class Api::V2::RegistrationTokensControllerTest < ActionController::TestCase
test 'user shall invalidate tokens for self' do
user = User.create :login => "foo", :mail => "[email protected]", :auth_source => auth_sources(:one)
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 use FactoryBot.create(:user) here.

class Api::V2::RegistrationTokensControllerTest < ActionController::TestCase
test 'user shall invalidate tokens for self' do
user = User.create :login => "foo", :mail => "[email protected]", :auth_source => auth_sources(:one)
FactoryBot.build(:jwt_secret, token: 'test_jwt_secret', user: user)
Copy link
Member

Choose a reason for hiding this comment

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

You probably want to create the token as part of the user creation, and use the create method - to make sure the token is saved before the action.

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?

@nofaralfasi
Copy link
Contributor

@girijaasoni In the UI PR, we used the term JWT token for the phrasing. Why are we referring to it as a registration token here? Wouldn't that create confusion?

@girijaasoni
Copy link
Contributor Author

girijaasoni commented Dec 19, 2024

@girijaasoni In the UI PR, we used the term JWT token for the phrasing. Why are we referring to it as a registration token here? Wouldn't that create confusion?

@nofaralfasi , we can handle that in the documentation. Registration tokens make more sense from user POV(specially from API POV), as we are creating a new controller. For UI, we are trying to use the existing controller where jwt is used as phrasing.

@girijaasoni girijaasoni force-pushed the jwt-api branch 7 times, most recently from 8858b6c to 6d841de Compare December 24, 2024 13:45
@girijaasoni girijaasoni marked this pull request as draft December 24, 2024 14:14
@girijaasoni girijaasoni marked this pull request as ready for review December 24, 2024 14:24
raise ::Foreman::Exception.new(N_("No record found for %s"), params[:id])
end
@user.jwt_secret&.destroy
process_success _('Successfully invalidated JWTs for %s.' % @user.login)

Choose a reason for hiding this comment

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

@girijaasoni Can we add a newline here as well?

@shweta83
Copy link

@girijaasoni invalidate token for other users doesn't work for invalidating self token. Can we fix that too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants