Validate SAML configuration during application start#12035
Validate SAML configuration during application start#12035mitchellhenke merged 2 commits intomainfrom
Conversation
mitchellhenke
force-pushed
the
mitchellhenke/saml-metadata-500
branch
3 times, most recently
from
b94810c to
dd345c5
Compare
app/services/saml_endpoint.rb
Outdated
There was a problem hiding this comment.
[question] there's quite a lot of logic in these constants. i'm curious if we should make them methods (or even a class, although that might be overkill) rather than trying to do all this work here?
There was a problem hiding this comment.
Yeah, I was trying to figure out the best way to do that. To use methods, they have to be defined before the constants, and another class does feel like too much.
There was a problem hiding this comment.
I think the best two options are move the constants to the bottom of the file and define method for the setup (as we do here) or have long constants like it is now. Perhaps the first option is preferable?
There was a problem hiding this comment.
hm, yeah! i think that would be nicer. maybe easier to test directly too?
There was a problem hiding this comment.
That's a really good point, will update.
changelog: Internal, SAML, Validate SAML configuration during application start
mitchellhenke
force-pushed
the
mitchellhenke/saml-metadata-500
branch
from
dd345c5 to
3c7541c
Compare
2 participants
🎫 Ticket
Link to the relevant ticket:
!156
-->
🛠 Summary of changes
We had an issue discovered recently in a non-prod environment where an invalid SAML passphrase was deployed but not detected until it was live. This is similar to #11612, but requiring a slightly different approach for SAML.
This PR makes changes to eagerly read and store the SAML configuration so that instead of failing during a request, it fails the deploy.