Skip to content

Update saml_idp with better logic for matching_cert#5220

Merged
zachmargolis merged 2 commits intomainfrom
margolis-multiple-certs-saml
Jul 21, 2021
Merged

Update saml_idp with better logic for matching_cert#5220
zachmargolis merged 2 commits intomainfrom
margolis-multiple-certs-saml

Conversation

@zachmargolis
Copy link
Contributor

nil
elsif current_service_provider&.encrypt_responses?
cert = saml_request.service_provider.matching_cert ||
current_service_provider&.ssl_certs&.first
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the .first was masking an issue where we couldn't find a matching cert, so we sent back a wrong cert... this may cause some nil errors in the specs, will see how it goes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good news, removing this didn't break the build so it didn't cause a regression that we know about

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.

Looks good to me once 18F/saml_idp#46 is merged in. Great work!

@zachmargolis zachmargolis merged commit 15b5ca9 into main Jul 21, 2021
@zachmargolis zachmargolis deleted the margolis-multiple-certs-saml branch July 21, 2021 16:57
juliasolorzano pushed a commit that referenced this pull request Jul 23, 2021
* Updating capitalization of login.gov to Login.gov.

* Updating ASCII because fun is important.

* Fixin dat YAML.

* MOAR YML!

* Update config/locales/user_mailer/es.yml

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* Update config/locales/user_mailer/es.yml

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* Update config/locales/user_mailer/es.yml

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* Update config/locales/user_mailer/fr.yml

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* Update vendor/assets/javascripts/local-time.js

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* Update vendor/assets/javascripts/local-time.js

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* Update vendor/assets/javascripts/local-time.js

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

* Update saml_idp with better logic for matching_cert (#5220)

* Bring back a default cert, to fix NoMethodErrors on nil (#5224)

* Add regression spec for NoMethodError

* Remove .erdconfig (#5226)

**Why**: We removed the ERD in #4960

* Update APP_NAME in application.rb.

* Revert Gemfile.lock.

* Update config/locales/idv/fr.yml

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Update Gemfile

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

* Run `make normalize_yaml`

* Update spec that used lowercase login to use uppercase

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov>
Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov>
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.

2 participants