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

Update verify hard fail screens #1640

Merged
merged 2 commits into from
Aug 31, 2017
Merged

Conversation

nickbristow
Copy link
Contributor

Why:
After a user fails to verify based on supplied info
the user is redirected to help pages if logging in
directly on the app. If coming from an SP, they
are instructed to visit the SP for more help.

@nickbristow nickbristow requested review from el-mapache and removed request for el-mapache August 24, 2017 18:54
@nickbristow
Copy link
Contributor Author

login_gov_-_we_weren_t_able_to_verify_your_identity

redesign_end_state_screens_ _issue__2236_ _18f_identity-private

@nickbristow nickbristow force-pushed the updated-identity-verify-hard-fail branch 5 times, most recently from 2a80678 to bb35d39 Compare August 29, 2017 14:06
**Why**:
After a user fails to verify based on supplied info
the user is redirected to help pages if logging in
directly on the app. If coming from an SP, they
are instructed to visit the SP for more help.
@nickbristow nickbristow force-pushed the updated-identity-verify-hard-fail branch from bb35d39 to 4543302 Compare August 29, 2017 14:19
@nickbristow nickbristow changed the title Update verify hard fail screens [WIP] Update verify hard fail screens Aug 29, 2017
.mt2.p3.col-12.center.border-box.border.border-teal.rounded-xl
p.mb3.fs-20p = t('idv.messages.hardfail4_html', sp: decorated_session.sp_name)
= link_to t('idv.buttons.continue_plain'),
decorated_session.sp_return_url,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not 100% convinced this is the right URL

  • for SAML, this URL will be the "The URL of the SP which login.gov provides to users when they wish to go directly to the SP site or cancel out of authentication."
  • for OIDC this will be the decline/cancel URL which will probably show an error

I believe our goal is to have SP register a new URL specifically as a help page, so we'll need to add a new migration, update fields in the dashboard, etc etc

I am OK with us merging this as-is but it's wrong and we'll need to plan to fix it in the short-term

Copy link
Contributor

Choose a reason for hiding this comment

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

@nickbristow let's talk about how to fix this when you get back

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zachmargolis you are correct, we don't have the SP's registering a help url atm, I think thats planned.

hr.mt2
.mt2
| <
= link_to ' Return to your login.gov profile', profile_path
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be i18n'd

Copy link
Contributor

Choose a reason for hiding this comment

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

Also we avoid hardcoding login.gov because we use APP_NAME

@zachmargolis zachmargolis self-assigned this Aug 31, 2017
@zachmargolis zachmargolis force-pushed the updated-identity-verify-hard-fail branch from ed0833e to e7f9f60 Compare August 31, 2017 15:20
**Why**: So we can translate the app
Copy link
Member

@jmhooper jmhooper left a comment

Choose a reason for hiding this comment

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

Code looks good to me. Failed the idv flow with and without an SP and both screens looked right.

👍

@zachmargolis zachmargolis merged commit ed5e456 into master Aug 31, 2017
@zachmargolis zachmargolis deleted the updated-identity-verify-hard-fail branch August 31, 2017 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants