Support overriding StepUp EntityId #1279
Conversation
MKodde
force-pushed
the
feature/rollover-stepup
branch
2 times, most recently
from
34a8d52 to
d0d28cf
Compare
MKodde
force-pushed
the
feature/rollover-stepup
branch
2 times, most recently
from
89a0153 to
1895b15
Compare
MKodde
force-pushed
the
feature/rollover-stepup
branch
13 times, most recently
from
5b0b4b7 to
442c00a
Compare
MKodde
force-pushed
the
feature/rollover-stepup
branch
4 times, most recently
from
326b859 to
6dba463
Compare
MKodde
force-pushed
the
feature/rollover-stepup
branch
from
6dba463 to
57fe036
Compare
| you must provide the "stepup.sfo.override_engine_entityid" parameter.' | ||
| ); | ||
| } | ||
| $this->_logger->notice( |
There was a problem hiding this comment.
I think I prefer info or even debug. I would expect this config to last for an undetermined amount of time, does not make sense to issue notices on every stepup callout about it.
| @@ -261,6 +262,7 @@ parameters: | |||
| stepup.gateway.sfo.sso_location: 'https://gateway.stepup.vm.openconext.org/second-factor-only/single-sign-on' | |||
| ## The public key from the Stepup Gateway IdP | |||
| stepup.gateway.sfo.key_file: /etc/openconext/engineblock.crt | |||
| stepup.sfo.override_engine_entityid: 'https://engine.vm.openconext.com/new/stepup/metadata' | |||
There was a problem hiding this comment.
The docs/stepup_callout.md file refers to this section for explanation of the Engineblock config. So I think this new setting should have at least a # comment like the ones above.
MKodde
force-pushed
the
feature/rollover-stepup
branch
from
09567d3 to
94bd31e
Compare
| * `eb.stepup.sfo.override_engine_entityid`. See UPGRADING.md (6.13 -> 6.14) | ||
| * for details. | ||
| */ | ||
| private function determineRemoteSpEntityId() |
There was a problem hiding this comment.
Not sure we can dash with return type definitions here? I could of course do a @var annotation.
MKodde
force-pushed
the
feature/rollover-stepup
branch
from
94bd31e to
3e19ce2
Compare
Engineblock will verify the incoming assertion's Audience which will be set to the entityID it used on the outgoing authnrequest, so this place will also need to handle the override if present.
MKodde
force-pushed
the
feature/rollover-stepup
branch
from
3e19ce2 to
11578ad
Compare
|
|
||
| $serviceEntityId = $this->_server->getUrl('stepupMetadataService'); | ||
| if ($isEnabled && $isConfigured) { | ||
| return $entityIdOverrideValue; |
There was a problem hiding this comment.
Setting the override causes this error down the line when EB is processing the assertion (stepupAssertionConsumerService)
Response processing failed: "Invalid Assertion in SAML Response, errors: "The configured Service Provider [https://engine.vm.openconext.com/new/stepup/metadata] is not a valid audience for the assertion. Audiences: [https://engine.vm.openconext.org/authentication/stepup/metadata]"
When rollover is enabled, let the mock gateway update the assertion audience to the new entity id. That way, the engineblock is able to receive the assertion from the new entity
MKodde
force-pushed
the
feature/rollover-stepup
branch
from
1382d17 to
457260c
Compare
Currently the SAML EntityID of the engineblock SP that us used to do Stepup (SFO) authenticatons to the Stepup-Gateway is always https://<engineblock.sever.domain.name>/authentication/stepup/metadata
The key used to sign the SAML AuthnRequests from this SP is the engineblock default key.
To facilitate a rolling configuration update I want the SP entityID that is used for Stepup to be configurable so that at the same time that the engineblock default key is updated, this entityID can be changed. This then allows two entities, with two different keys, to be configured in the Stepup-Gateway.
https://www.pivotaltracker.com/story/show/186200738