Access Tokens should be hidden by default upon loading a page

Problem

When a user visits https://gitlab.com/-/profile/personal_access_tokens, the feed token and incoming email token is displayed on screen by default. This is not good from a security perspective. If a user is screen sharing, on a webcast, or a nefarious person is overlooking their screen, the onlookers can immediately get their tokens.

Proposal

Passwords and tokens should be hidden by default and a user should have to click a button to reveal the token or password. This approach would prevent accidental disclosure.

Implementation plan

1. Move app/assets/javascripts/runner/components/registration/registration_token.vue to vue_shared directory as masked_token.vue
2. Create a mount point in app/views/profiles/personal_access_tokens/index.html.haml#L35
3. Create a new component feed_and_email_token.vue in app/assets/javascripts/access_tokens that uses masked_token.vue
4. Mount the new feed_and_email_token.vue component

added UX groupauthentication and authorization [DEPRECATED] security labels

added typemaintenance label

@hsutor Is this something for your group?

@sethgitlab Yep! You are correct.

@serenafang This reminds me of https://gitlab.com/gitlab-org/security/gitlab/-/issues/502

@rshambhuni Can you provide appsec perspective here?

@hsutor I agree this is a good security feature to have. It was already discussed on this MR and @peterhegman opened a couple of follow-up issues to address this.

• #338242
• #338243 (closed)

Perhaps these issues need to be prioritized since they have been open for a couple of months?

From what I can tell, there is not a design pattern for this yet in Pajamas. If we add this as a component into Pajamas, we could probably resolve this in a couple of places all at once. #338242

I don't think this an official pattern in Pajamas but I recently reviewed a MR that was using this pattern:

I agree it would be nice to move this to move this to GitLab UI so the pattern can be consistently used throughout GitLab

@peterhegman would this be a purely frontend change?

@hsutor yep this would be a pure frontend issue. @dmoraBerlin WDYT about the above screenshot for solving this issue?

@sethgitlab @peterhegman is there a need to see the token? We had a similar issue (I can't find it at the moment) where we added the download/copy code button for a similar use case.

@dmoraBerlin technically no need as long as you can copy the token but I think it improves the UX to be able to show/hide the token. This is a pattern I have seen in a lot of other applications.

I think you are probably thinking of #332844 (closed). In the end we didn't add the download button because of security concerns. We just hid the secret and displayed a copy button. But we created #338242 as a follow-up to add show/hide functionality

@peterhegman thanks for linking those issues, I couldn't find them but I think this parallels them perfectly. I think this proposal makes sense, especially as we will do the same in those related issues.

@dmoraBerlin sounds good

I added an implementation plan to the issue description. Going to weight this a 3 and add the workflowready for development label.

changed the description

Setting label(s) ~"Category:Authentication and Authorization" devopsmanage sectiondev based on ~"group::access".

added devopsmanage sectiondev + 1 deleted label

marked this issue as related to #338242

marked this issue as related to #338243 (closed)

added candidate14.6 label

changed milestone to %Backlog

added priority3 label

added [deprecated] Accepting merge requests label

added frontend label

added workflowready for development label

changed the description

changed milestone to %14.6

mentioned in issue gitlab-org/manage/general-discussion#17421

mentioned in issue gitlab-org/quality/triage-reports#5351 (closed)

added quad-planningcomplete-no-action label

set weight to 3

assigned to @peterhegman

added workflowin dev label and removed workflowready for development label

removed [deprecated] Accepting merge requests label

mentioned in merge request !74838 (merged)

mentioned in issue gitlab-org/manage/general-discussion#17423

Changing type label to typefeature as this is a user-facing change as well as non-vulnerability.

added typefeature label and removed typemaintenance label

mentioned in merge request !76280 (merged)

added workflowin review label and removed workflowin dev label

@hsutor an update on this one since we are getting close to the end of the milestone. This is in maintainer review, but it is also behind a feature flag so I am not 100% sure if we will be able to get it merged and default the feature flag to on before the cutoff (usually around the 17th). I will keep you updated on progress in the next couple of days.

cc @dennis

mentioned in merge request gitlab-com/www-gitlab-com!94879 (merged)

Verified on staging, enabling this on production at 17:00 UTC on December 16th

mentioned in issue #347490 (closed)

Enabled and verified on production

Awesome news! Thanks @peterhegman

Yep, still not totally sure if this will make it into the self-managed release. We generally like to enable feature flags on gitlab.com for at least a day before enabling it for the self-managed release. So, assuming there are no issues, I will open a MR tomorrow to enable it for the self-managed release. If that get's merged before the %14.6 cutoff (generally around the 17th) then it will make it into the release. I will keep you posted!

@hsutor confirmed this made it into %14.6! Closing this issue out

added workflowproduction label and removed workflowin review label

closed

mentioned in issue #350590 (closed)