Skip to content

Add feature flag and interface for MFA prototype#6076

Merged
jmdembe merged 1 commit intomainfrom
jd-create-feature-flag-and-rerouting
Mar 22, 2022
Merged

Add feature flag and interface for MFA prototype#6076
jmdembe merged 1 commit intomainfrom
jd-create-feature-flag-and-rerouting

Conversation

@jmdembe
Copy link
Copy Markdown
Contributor

@jmdembe jmdembe commented Mar 16, 2022

This is the first step in creating a prototype for multi-factor authentication. The user test will ask users to select and provide multiple authentication methods. After testing is completed, work can/will be done to implement in production.

This PR creates the feature flag and logic on whether or not users will be shown the checkbox selection vs. radio buttons. On testing on this current PR, you should only see the radio buttons as the feature flag is currently set to false.

Future considerations for creating the feature/prototype:

  • Would it be worth creating a feature branch to further develop and write tests against?

@jmdembe jmdembe force-pushed the jd-create-feature-flag-and-rerouting branch from e6ac4aa to 78ee980 Compare March 16, 2022 19:52
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I expect there might need to be some server-side changes to handle selection as an array, not a single option?

self.selection = params[:selection]

Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis Mar 16, 2022

Choose a reason for hiding this comment

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

Should we bring back self.selection = Array(params[:selection]).first?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@aduth that makes sense to change on the server side
@zachmargolis my thinking was to handle that first option in mfa_redirect_helper.rb. After seeing what helpers do, what I did may not be the best option and as it stands now. I can make your change and use it in a new controller?

@jmdembe jmdembe marked this pull request as draft March 17, 2022 14:11
Comment on lines 18 to 24
Copy link
Copy Markdown
Contributor

@aduth aduth Mar 21, 2022

Choose a reason for hiding this comment

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

We can use AlertComponent to simplify this and ensure consistency:

Suggested change
<div class="usa-alert usa-alert--info margin-bottom-4" role="status">
<div class="usa-alert__body">
<p class="usa-alert__text">
<%= t('two_factor_authentication.mfa_instructions') %>
</p>
</div>
</div>
<%= render AlertComponent.new(type: :info, class: 'margin-bottom-4') do %>
<%= t('two_factor_authentication.mfa_instructions') %>
<% end %>

@jmdembe jmdembe force-pushed the jd-create-feature-flag-and-rerouting branch from 77734a6 to 6e9b3c6 Compare March 21, 2022 19:39
@jmdembe jmdembe marked this pull request as ready for review March 21, 2022 20:09
@jmdembe jmdembe requested a review from mdiarra3 March 21, 2022 20:09
@jmdembe jmdembe force-pushed the jd-create-feature-flag-and-rerouting branch from 6e9b3c6 to 3eb5f63 Compare March 21, 2022 20:13
@jmdembe
Copy link
Copy Markdown
Contributor Author

jmdembe commented Mar 21, 2022

Breaking this PR out to solely support the feature flag and front-end ticket. LG-5986 details the work of this PR. Data storage and routing will be addressed in LG-5988

@jmdembe jmdembe force-pushed the jd-create-feature-flag-and-rerouting branch from 3eb5f63 to cf3fd61 Compare March 21, 2022 20:41
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

@jmdembe jmdembe force-pushed the jd-create-feature-flag-and-rerouting branch from cf3fd61 to 1a44e6e Compare March 22, 2022 13:19
changelog: Improvements, Authentication, change front end to support MFA feature
@jmdembe jmdembe force-pushed the jd-create-feature-flag-and-rerouting branch from 1a44e6e to 4cc4f30 Compare March 22, 2022 13:30
@jmdembe jmdembe merged commit 1135f46 into main Mar 22, 2022
@jmdembe jmdembe deleted the jd-create-feature-flag-and-rerouting branch March 22, 2022 13:44
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.

3 participants