:construction: 🦋 Updates Tokens page by josemigallas · Pull Request #4254 · 3scale/porta

josemigallas · 2026-03-19T10:33:00Z

😄 Improved tokens index page
😅 Slight change to edit token page
😓 Needed to change users page for consistency with table actions (see image below).

Before:

After

Empty states:

Create button transformed into Card action. Edit button replaced with name link. Delete button transformed into icon button:

Confirmation message improved:

"new token" page is improved by adding a proper alert message:

User index page

For consistency and since both tables use the same button icon component, this page has also been updated (edit and delete links):

Edit page action buttons

Added cancel button (goes back to index) and moved delete to the right for better UX:

josemigallas · 2026-03-19T10:34:19Z

+          if (id = flash[:token]) && (token = current_user.access_tokens.find_by(id:))
+            render 'access_token_created', locals: { token: }


Used to be handled in the template, I think it's the controller's responsibility and it renders now a completely different template without instantiating the presenter or fetching any tokens.

qltysh · 2026-03-19T10:35:34Z

❌ 61 blocking issues (61 total)

Tool	Category	Rule	Count
reek	Lint	AccessToken#generate_if_missing calls 'self.class' 2 times	49	❌
reek	Lint	ApiDocs::ProviderUserData#service_tokens refers to 'service' more than self (maybe move it to another class?)	2	❌
rubocop	Lint	Block has too many lines. [44/25]	2	❌
reek	Lint	Provider::Admin::User::AccessTokensControllerTest assumes too much for instance variable '@admin'	2	❌
rubocop	Lint	Avoid using `update\_columns` because it skips validations.	2	❌
rubocop	Lint	Class has too many lines. [354/200]	2	❌
reek	Lint	AccessToken#self.find_from_value has approx 8 statements	1	❌
rubocop	Lint	Use `ActionDispatch::IntegrationTest` instead.	1	❌

josemigallas · 2026-03-19T10:37:17Z

I changed this component to be an icon button (much better looking) and that's why users page needed to be updated.

josemigallas · 2026-03-19T10:38:31Z

 }
+
+.pf-c-form__actions {
+  .pf-c-button.pf-m-danger {


This will include action-list styles to every form and move Delete buttons to the far right (if the form implements action-list, which only CMS ones do and they already are in the right).

josemigallas · 2026-03-19T10:40:57Z

-      []
-    end
+  def accessible_services_with_token
+    return Service.none unless has_permission?(:plans) # TODO: is this permission check handled by accessible_services already?


Is it possible for an user not to have plans permission AND have accessible services?

Yes, it is possible. There are four permission types (including :plans) that make accessible_services return a non-empty array (of course, given that some - or ALL - services are enabled).

>> u.accessible_services.count => 27 >> u.has_permission?(:plans) ~> admin25 has_permission?(plans) => false => false >> u.admin_sections => #<Set: {:partners, :monitoring, :policy_registry}>

However, indeed, at the moment the page only shows service tokens to those users that have :plans permission explicitly.

josemigallas · 2026-03-19T10:41:19Z

+    @user.allowed_access_token_scopes.empty?
+  end
+
+  def service_tokens


Confusing name, I know.

josemigallas · 2026-03-19T10:43:04Z


 - content_for :javascripts do
-  = javascript_packs_with_chunks_tag 'empty_state'
+  = stylesheet_packs_chunks_tag 'empty_state'


Not related to this PR actually, but a minor typo. empty_state is a SCSS file, not TS.

* New token are generated already hashed * Old tokens are hashed first time they are used Co-Authored-By: Claude <noreply@anthropic.com>

* Remove dead code * Render on create instead of redirecting, so the user can see the token while still in memory * New template to show the generated token. It was embedded in the index template before * Add I18N, by Claude Co-authored-by: Claude <no-reply@anthropic.com>

Co-Authored-By: Claude <noreply@anthropic.com>

mayorova

It's hard to review the template changes, but the views look good to me.

mayorova · 2026-03-20T14:25:53Z

Careful though, I think this will conflict with #4236 ...

UPDATE: I don't think, I am sure 😅 huge conflict incoming...

jlledom · 2026-03-23T08:34:54Z

Careful though, I think this will conflict with #4236 ...

UPDATE: I don't think, I am sure 😅 huge conflict incoming...

Worst timing ever!

It's fine, just merge this first and I'll manage the conflicts

jlledom · 2026-03-23T09:34:23Z

-      []
-    end
+  def accessible_services_with_token
+    return Service.none unless has_permission?(:plans) # TODO: is this permission check handled by accessible_services already?


Please don't add TODOs to the code. In this case, this is not even something to do, it's a question.

This is my way of attacting reviewers' attention when I have doubts. Not a real TODO as you pointer out.

On a side note I used to hate TODOs as well, but honestly it's the only way to register small future tasks/improvements rather than opening a whole new task in JIRA. Also I hate JIRA even more than TODOs 😄.

jlledom · 2026-03-23T09:34:51Z

-    else
-      []
-    end
+  def accessible_services_with_token


Why this change? what was wrong with the previous method?

empty array was simply not proper. Other than that, the implementation looked ugly.

jlledom · 2026-03-23T09:37:38Z

+  end
+
+  def service_tokens
+    @service_tokens ||= @user.accessible_services_with_token


If accessible_services_with_token is now returning services instead of tokens, shouldn't the instance variable here be called @services instead of @service_tokens?

Yep, I left it half baked I guess. It's better now.

josemigallas · 2026-04-01T12:26:00Z

    scopes { ['stats'] }
    permission { 'rw' }
-    sequence(:name) { |n| "Alaska_#{n}" }
+    sequence(:name)  { |n| "token_#{n}" }


This was suggested by our beloved AI colleague to make it more generic.

josemigallas · 2026-04-01T12:38:06Z

@damianpm @jlledom
I fixed all the typos pointed out and rebased this branch on top of the migration to avoid problems. Worth mentioning it only generated 1 conflict but after that I have to go double check all the changes. Anyway I like it more now so it's a win-win situation.

Now the review is on top of Joan's changes.

jlledom · 2026-04-10T11:16:16Z

        dd class="pf-c-description-list__description"
          div class="pf-c-description-list__text"
-           = token.plaintext_value
+            = token.value


It should how the plain text value. This is now showing the token hash in DB, which is meaningless for the user.

Suggested change

= token.value

= token.plaintext_value

jlledom · 2026-04-10T11:17:30Z

        | Expires at | Never expires  |
      And there should be a link to "I have copied the token"


I think it should assert the plain text token is shown here. That would have caught the error in the show template.

jlledom · 2026-04-10T11:18:24Z

+  test 'show is rendered when a token is created' do
+    expires_at = 1.week.from_now.utc.iso8601
+    post :create, params: { access_token: { name: 'Le Token', scopes: ['account_management'], permission: 'ro', expires_at: } }
+
+    assert_response :success
+    assert_template :show
+  end


Beside from the cuke, this is another place where we could assert the plain token presence in the show template. Either of both is fine for me.

github-actions · 2026-05-11T04:51:14Z

This PR is stale because it has not received activity for more than 30 days. Remove stale label or comment or this will be closed in 15 days.

josemigallas requested a review from a team March 19, 2026 10:33

josemigallas self-assigned this Mar 19, 2026

josemigallas commented Mar 19, 2026

View reviewed changes

Comment thread app/controllers/provider/admin/user/access_tokens_controller.rb

josemigallas commented Mar 19, 2026

View reviewed changes

josemigallas force-pushed the THREESCALE-9869_tokens_page branch 2 times, most recently from dc78ed8 to 4c4cbc1 Compare March 19, 2026 14:59

Increase token length to 48

8052eb4

mayorova reviewed Mar 20, 2026

View reviewed changes

Comment thread app/helpers/patternfly_components_helper.rb

mayorova reviewed Mar 20, 2026

View reviewed changes

Comment thread features/provider/admin/user/access_tokens.feature Outdated

Access Tokens: Hash values to SHA-384

d0a2f1b

* New token are generated already hashed * Old tokens are hashed first time they are used Co-Authored-By: Claude <noreply@anthropic.com>

mayorova reviewed Mar 20, 2026

View reviewed changes

Comment thread config/locales/en.yml Outdated

jlledom and others added 8 commits March 20, 2026 14:56

Access Tokens API: Show token on creation

e9d66c6

Fix seeds script to hash tokens

5829348

RestoreApicastMasterTokenWorker hashes the token

c9a10cb

Add tests for new logic

864c6f2

Co-Authored-By: Claude <noreply@anthropic.com>

Fix existing tests

348fb69

Fix tests

9c9ede6

New migration to hash tokens

dfaf55e

mayorova previously approved these changes Mar 20, 2026

View reviewed changes

Set frozen_string_literal to true

44504ab

jlledom reviewed Mar 23, 2026

View reviewed changes

Rename generate_value to generate_if_missing

9e216ee

josemigallas dismissed mayorova’s stale review via ddb14dd April 1, 2026 08:27

josemigallas force-pushed the THREESCALE-9869_tokens_page branch from 4c4cbc1 to ddb14dd Compare April 1, 2026 08:27

josemigallas changed the base branch from master to THREESCALE-12408-acess-tokens-protection April 1, 2026 08:29

🦋 updates account access tokens page (also account users)

cdec879

josemigallas force-pushed the THREESCALE-9869_tokens_page branch from ddb14dd to cdec879 Compare April 1, 2026 12:11

✏️ typo!

1b8fad4

josemigallas commented Apr 1, 2026

View reviewed changes

✏️ moar typos

435e045

josemigallas requested review from jlledom and mayorova April 1, 2026 12:27

✏️ look mom, no typos!

5b462bf

josemigallas changed the title ~~🦋 Updates Tokens page~~ 🚧 🦋 Updates Tokens page Apr 1, 2026

🔥

a77546f

jlledom reviewed Apr 10, 2026

View reviewed changes

github-actions Bot added the Stale label May 11, 2026

jlledom removed the Stale label May 11, 2026

jlledom force-pushed the THREESCALE-12408-acess-tokens-protection branch 3 times, most recently from 43c3ab6 to 4865027 Compare May 13, 2026 14:32

Base automatically changed from THREESCALE-12408-acess-tokens-protection to master May 13, 2026 15:00

		if (id = flash[:token]) && (token = current_user.access_tokens.find_by(id:))
		render 'access_token_created', locals: { token: }

		\| Expires at \| Never expires \|
		And there should be a link to "I have copied the token"

Conversation

josemigallas commented Mar 19, 2026

Before:

After

User index page

Edit page action buttons

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qltysh Bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mayorova left a comment

Choose a reason for hiding this comment

Uh oh!

mayorova commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jlledom commented Mar 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josemigallas commented Apr 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

qltysh Bot commented Mar 19, 2026 •

edited

Loading

mayorova commented Mar 20, 2026 •

edited

Loading