Fixes #38108 - As a user I want to invalidate tokens for self(UI) #10405
+111
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
girijaasoni
force-pushed
the
38108
branch
2 times, most recently
from
f3234ce to
95f093f
Compare
Lennonka
reviewed
Dec 17, 2024
| <h1>{__('JWT Tokens')}</h1> | ||
| <p> | ||
| {__( | ||
| 'By invalidating tokens,you will no longer be able to register hosts by using their JWTs.' |
There was a problem hiding this comment.
Suggested change
| 'By invalidating tokens,you will no longer be able to register hosts by using their JWTs.' | |
| 'By invalidating your JSON Web Tokens (JWTs), you will no longer be able to register hosts by using your existing JWTs.' |
stejskalleos
requested changes
Dec 19, 2024
app/controllers/users_controller.rb
Outdated
| process_success( | ||
| :success_msg => _('Successfully invalidated JWTs for %s.') % @user.login | ||
| ) | ||
| unless editing_self? |
There was a problem hiding this comment.
Why this change? Why not to have same response as before?
There was a problem hiding this comment.
@girijaasoni I think we should use respond_to here. It would be more consistent.
webpack/assets/javascripts/react_app/components/users/JwtTokens/JwtTokens.js
Show resolved
Hide resolved
webpack/assets/javascripts/react_app/components/users/JwtTokens/JwtTokens.js
Show resolved
Hide resolved
webpack/assets/javascripts/react_app/components/users/JwtTokens/JwtTokens.js
Show resolved
Hide resolved
Lennonka
approved these changes
Dec 20, 2024
ShimShtein
reviewed
Dec 23, 2024
| test "User should be able to invalidate jwt for self" do | ||
| User.current = users(:one) | ||
| user = User.current | ||
| FactoryBot.build(:jwt_secret, token: 'test_jwt_secret', user: user) |
There was a problem hiding this comment.
Suggested change
| FactoryBot.build(:jwt_secret, token: 'test_jwt_secret', user: user) | |
| FactoryBot.create(:jwt_secret, token: 'test_jwt_secret', user: user) |
With build the object is not saved in the database, making the assertion pass even if the action was not performing correctly
girijaasoni
force-pushed
the
38108
branch
3 times, most recently
from
d19a69c to
c0c8fdb
Compare
ShimShtein
reviewed
Dec 25, 2024
app/controllers/users_controller.rb
Outdated
| process_success( | ||
| :success_msg => _('Successfully invalidated JWTs for %s.') % @user.login | ||
| ) | ||
| unless editing_self? |
There was a problem hiding this comment.
@girijaasoni I think we should use respond_to here. It would be more consistent.
app/controllers/users_controller.rb
Outdated
| ) | ||
| unless editing_self? | ||
| process_success( | ||
| :success_msg => _('Successfully invalidated JWTs for %s.') % @user.login |
There was a problem hiding this comment.
Suggested change
| :success_msg => _('Successfully invalidated JWTs for %s.') % @user.login | |
| :success_msg => _('Successfully invalidated registration tokens for %s.') % @user.login |
app/views/users/_form.html.erb
Outdated
| @@ -18,6 +18,9 @@ | |||
| <% if @editing_self || (@user.persisted? && authorized_for(hash_for_api_user_personal_access_tokens_path(user_id: @user))) %> | |||
| <li><a href='#personal_access_tokens' data-toggle='tab'><%= _('Personal Access Tokens') %></a></li> | |||
| <% end %> | |||
| <% if @editing_self %> | |||
| <li><a href='#jwt_tokens' data-toggle='tab'><%= _('JWT Tokens') %></a></li> | |||
There was a problem hiding this comment.
Suggested change
| <li><a href='#jwt_tokens' data-toggle='tab'><%= _('JWT Tokens') %></a></li> | |
| <li><a href='#jwt_tokens' data-toggle='tab'><%= _('Registration Tokens') %></a></li> |
| FactoryBot.create(:jwt_secret, token: 'test_jwt_secret', user: user) | ||
| patch :invalidate_jwt, params: { :id => user.id }, session: set_session_user(User.current) | ||
| user.reload | ||
| assert_response :success |
There was a problem hiding this comment.
Please add assertion for the user object:
assert_nil user.jwt_token
ShimShtein
approved these changes
Jan 5, 2025
nofaralfasi
approved these changes
Jan 7, 2025
|
@stejskalleos Could you please approve this PR so that I can merge it? |
stejskalleos
approved these changes
Jan 8, 2025
|
Thanks everyone! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.