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

Remove support for ?biometric_comparison_required #10526

Merged
merged 3 commits into from
Apr 30, 2024

Conversation

matthinz
Copy link
Member

🎫 Ticket

Link to the relevant ticket:
LG-12576

🛠 Summary of changes

Removes support for passing ?biometric_comparison_required=1 in the OIDC flow in favor of using a vector of trust including Pb.

Removing this in favor of vectors of trust.

This commit inverts several tests to verify that the behavior has been removed, the next commit will remove those tests.

changelog: Internal, Identity verification, Remove biometric_comparison_required query param
@matthinz matthinz requested review from a team and dawei-nava and removed request for a team April 29, 2024 21:13
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.

end of an era!

@dawei-nava
Copy link
Contributor

@matthinz , we mainly use the test oidc controller for dev purpose, tested it and still work as expected.

Noticed that we still build the open_id_authorize_form like

def initialize(params)
@acr_values = parse_to_values(params[:acr_values], Saml::Idp::Constants::VALID_AUTHN_CONTEXTS)
@vtr = parse_vtr(params[:vtr])
SIMPLE_ATTRS.each { |key| instance_variable_set(:"@#{key}", params[key]) }
@prompt ||= 'select_account'
@scope = parse_to_values(params[:scope], scopes)
@unauthorized_scope = check_for_unauthorized_scope(params)
@biometric_comparison_required = params[:biometric_comparison_required].to_s == 'true'
if verified_within_allowed?
@duration_parser = DurationParser.new(params[:verified_within])
@verified_within = @duration_parser.parse
end

where biometric_comparison_required still needed. Not sure how vot is built, but guess this part is still needed.

…izeForm

- Use VTR for biometric_comparison_required? method
- Add unit test for biometric_comparison_required?
@matthinz matthinz merged commit 919be5e into main Apr 30, 2024
2 checks passed
@matthinz matthinz deleted the matthinz/12576-remove-biometric-comparison-required branch April 30, 2024 18:00
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.

None yet

3 participants