Skip to content

log specific error when piv/cac second factor fails#4843

Merged
mitchellhenke merged 1 commit intomainfrom
mitchellhenke/log-piv-cac-error-type
Mar 26, 2021
Merged

log specific error when piv/cac second factor fails#4843
mitchellhenke merged 1 commit intomainfrom
mitchellhenke/log-piv-cac-error-type

Conversation

@mitchellhenke
Copy link
Contributor

so we can see what errors are happening on PIV/CACs used as a second factor, copied from what we do with the "Sign in with your Government Employee ID" event here: https://github.com/18F/identity-idp/blob/f12de9574723a4f31a2d392c8269c5b9908b62e7/app/forms/user_piv_cac_login_form.rb#L13-L14

Instead of:

{:success=>false,
 :errors=>{},
 :multi_factor_auth_method=>"piv_cac",
 :piv_cac_configuration_id=>nil,
 :key_id=>nil,
 :context=>"authentication"}

We'll have

 {:success=>false,
+ :errors=>{:type=>"certificate.invalid"},
  :multi_factor_auth_method=>"piv_cac",
  :piv_cac_configuration_id=>nil,
  :key_id=>nil,
  :context=>"authentication"}

@mitchellhenke mitchellhenke changed the title add error when failing to use piv/cac as a second factor log specific error when piv/cac second factor fails Mar 26, 2021
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/log-piv-cac-error-type branch from f12de95 to 3d27165 Compare March 26, 2021 16:36
Copy link
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 and thanks for the clear diff in the description! 🙏

@mitchellhenke mitchellhenke merged commit 66646ae into main Mar 26, 2021
@mitchellhenke mitchellhenke deleted the mitchellhenke/log-piv-cac-error-type branch March 26, 2021 16:47
Copy link
Contributor

@orenyk orenyk left a comment

Choose a reason for hiding this comment

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

Amazing, thanks!

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