-
Notifications
You must be signed in to change notification settings - Fork 62
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
[46092] adds idp_sso_service_binding config to ssoe_settings_service #10631
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ericboehs
previously approved these changes
Sep 7, 2022
joeniquette
previously approved these changes
Sep 7, 2022
bramleyjl
dismissed stale reviews from joeniquette and ericboehs
via
September 19, 2022 20:04
2beee76
bramleyjl
force-pushed
the
46092_saml_request_signing
branch
from
September 19, 2022 20:04
66e6c72
to
2beee76
Compare
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
September 19, 2022 20:05
Inactive
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
September 20, 2022 16:27
Inactive
bramleyjl
force-pushed
the
46092_saml_request_signing
branch
from
September 21, 2022 23:10
a3eecdc
to
31e2bea
Compare
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
September 21, 2022 23:11
Inactive
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
September 22, 2022 16:08
Inactive
bramleyjl
force-pushed
the
46092_saml_request_signing
branch
from
October 5, 2022 22:29
c1bf618
to
1da40db
Compare
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
October 5, 2022 22:30
Inactive
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
October 6, 2022 15:12
Inactive
bramleyjl
force-pushed
the
46092_saml_request_signing
branch
from
October 6, 2022 15:20
076cac9
to
0a0f37b
Compare
This PR has been updated to include the VA.gov-forked RubySAML library, which has been updated to include the necessary change. |
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
October 6, 2022 15:22
Inactive
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
October 6, 2022 18:06
Inactive
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
October 6, 2022 21:33
Inactive
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
October 7, 2022 16:34
Inactive
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
October 7, 2022 19:59
Inactive
bramleyjl
force-pushed
the
46092_saml_request_signing
branch
from
October 10, 2022 17:44
325b897
to
5a9d466
Compare
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
October 10, 2022 17:45
Inactive
bramleyjl
force-pushed
the
46092_saml_request_signing
branch
from
October 10, 2022 18:21
5a9d466
to
d28ecd4
Compare
va-vfs-bot
temporarily deployed
to
46092_saml_request_signing/main/main
October 10, 2022 18:22
Inactive
bramleyjl
force-pushed
the
46092_saml_request_signing
branch
from
October 10, 2022 21:15
d28ecd4
to
2a703ea
Compare
holdenhinkle
approved these changes
Oct 11, 2022
bosawt
approved these changes
Oct 11, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
joeniquette
approved these changes
Oct 11, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
This change adds an
idp_sso_service_binding
setting that changes vets-api's SSOe authentication request service binding fromArtifact
toPOST
in order to enable the request to be signed. It also changes theRubySAML
gem that is used to generate vets-api's SAML requests to a forked version in order to fix a bug with SAML signing as base gem repository is no longer under active maintainence. A devops change and a vsp infra change have already landed to populate this setting in all environments: local, dev, review, and staging instances will all have the newPOST
binding while production will keep theArtifact
binding until lower environment testing can be performed.In addition, it includes a Gemfile change to the RubySAML library to fix a bug that was improperly rendering our passed XMLto
nil
.I have also removed the deprecated
settings.security[:embed_sign]
RubySAML setting, theidp_sso_service_binding
setting that has been added performs the same function.Original issue(s)
department-of-veterans-affairs/va.gov-team#46092
Testing
This feature has been disabled by default for localhost, to enable it you will need to set the following settings in your
settings.local.yml
:true
/dsva-vagov/identity-team/dev/ssoe-sp-dev-va-gov-key
, download the private key to yourconfig/certs
directory invets-api
and link to it hereTo test, attempt to authenticate with any CSP through SSOe. You should see a substantially longer SAML payload that includes a signature: