Comments (3)
It looks like IDP initiated just makes the same SAMLResponse POST to /saml/acs, so this should be fairly direct. We may need to relax the requirement for RelayState to be signed (need to reason about if this is okay or not), or allow RelayState to be signed by some other key that we could provide, or not allow RelayState at all.
from saml.
Also, I couldn't find a way to do IDP-initiated with testshib, but maybe I didn't look hard enough, so we'd need some other way to do the initial interop test
from saml.
IDP-initiated logins generally somewhat work today, however there are some remaining issues that are blocking a good experience for users. This is covered briefly in the description of #183 by @praneetloke, but I've written a more detailed explanation here. Along with some proposals for how to fix the remaining issue with IDP-initiated logins at the end.
Support for IDP-initiated login was added in 72821a4, via the Options.AllowIDPInitiated
field. If set, everything works like you'd expect. The flag just allows the assertion validation code to account for an empty InReplyTo
field in the SAML response from the IDP. (Since the IDP isn't replying to an SP-initiated request, it's just empty.)
Problem Description
Like mentioned earlier, the problem remaining for IDP-initiated logins has to do with the built-in assumptions around the RelayState
query parameter. (This is also called out in the README.md
file too, here.)
For SP-initiated logins, the URL to redirect to post-login is stored via the ClientState
abstraction, and the RelayState
passed to the IDP is just a nonce. The code for this is here:
Lines 133 to 147 in ebc5f78
When the IDP returns with the SAML assertions doc, the Authorize
method will take the RelayState
returned by the IDP, use that to crack open the stored URI in the ClientState
, and complete the login.
Lines 205 to 229 in ebc5f78
While this works fine for SP-initiated login, for IDP-initiated there are two problems:
- The IDP may be configured to provide a custom
RelayState
. For example, when using GSuite a "Start URL" can be provided, which will set theRelayState
value. Thesamlsp
package assumes theRelayState
maps to a nonce value, fails with "cannot find corresponding state". - Even if the IDP does not provide a custom
RelayState
value, the default of redirecting to "/" may not be ideal. And it would be nice to allow customizing that default redirect in theOptions
struct.
Proposals
To support IDP-initiated logins, we need to change the interpretation of the RelayState
parameter for determining where to redirect to after a valid SAML assertions doc has been provided by the IDP.
(1) Just trust the IDP
The simplest to explain option is to blindly follow the RelayState
provided by the IDP. It follows that if the SAML middleware was configured to explicitly set AllowIDPInitiated = true
, it is reasonable to trust the RelayState
parameter it provided by the IDP.
The counter argument is that while the SAML assertions integrity can be verified, the request URL can not. So perhaps the request URL could be modified, and the redirect changed to something malicious.
For reference, this is generally the change involved for this option:
if relayState := r.Form.Get("RelayState"); relayState != "" {
stateValue := m.ClientState.GetState(r, relayState)
if stateValue != "" {
... use stored redirect URI from ClientState
} else if m.AllowIDPInitiated {
... trust the IDP RelayState
redirectURl = relayState
} else {
... error. We don't know what to make of the relay state
}
...
(2) Only honor specific RelayState values
Instead of blindly trusting any RelayState
value from the IDP, perhaps that could be pared down to some set of whitelisted values in the middleware's configuration?
OAuth has a similar mechanism. The OAuth provider may be passed a redirect_uri
from a client, and after the OAuth provider confirms the identity of the request, they redirect to that URI. The restriction is that the OAuth client provides that redirect_uri
to the OAuth provider as part of the application configuration.
e.g. when creating a GitHub OAuth app, you specify a "callback URL" that the GitHub will redirect to after any OAuth authorization request completes.
So if the SAML library were to use an approach like this, the code could be something like:
... := samlsp.New(samlsp.Options{
...
AllowIDPInitiated: true,
// Only honor IDP-supplied RelayStates if prefixed with one of these strings.
AllowedIDPRedirectPrefixes: []string {
"https://example.com/",
"https://beta.example.com/",
},
})
This approach seems reasonable, in that the developer essentially specifies a domain or URL route restriction for any IDP RelayState
values. That avoids the most common sort of malicious URL redirect type attacks.
The drawback here is that the RelayState
is assumed to be a redirect URL. When it (to my very poor understanding of SAML) is intended to be a more general way to pass state between parties.
(3) Ignore the RelayState if AllowIDPInitiated is set
This boils down to "don't trust the IDP" and ignore its RelayState
. This means you cannot customize things via the IDP-side configuration, but at least doesn't lead to a 403.
Something like:
if relayState := r.Form.Get("RelayState"); relayState != "" {
stateValue := m.ClientState.GetState(r, relayState)
if stateValue != "" {
... use stored redirect URI from ClientState
} else if m.AllowIDPInitiated {
// Don't treat this as an error. If we have a valid set of SAML assertions just
// ignore the RelayState and carry on.
redirectURl = m.DefaultRelayState
} else {
... error. We don't know what to make of the relay state
}
...
@crewjam , I'm happy to send out a PR for any of these. Just let me know what approach (or something I haven't mentioned) seems the most reasonable. (3) seems like the best option given the constraint of not making a breaking change to the existing library surface area.
Testing
On a somewhat related note, I looked into adding some unit tests for IDP-initiated login flows. I was able to write some tests (gist of the WIP).
However, I wasn't able to complete the tests because the test.SamlResponse
includes a saml2:EncryptedAssertion
element which needs to be modified to simulate the IDP-initiated login scenario. (Specifically, the SubjectConfirmation
embedded in that assertions doc has the InReplyTo
ID of the SP-initiated login; whereas it will be "" for the IDP-initated case.)
The reason I mention this, is that it isn't clear how to write new tests for this scenario. I assume the checked-in SamlResponse
value is from some shibboleth.net response from several years ago? (i.e. this comment.)
Would it be possible for you to log into that same shibboleth configuration/application and see if you can generate an IDP-initiated SAML response to support a new set of unit tests? Or would it be better to just start fresh with a new test configuration/pre-canned response, etc.?
from saml.
Related Issues (20)
- Azure SAML2.0 not surport
- [question] idp example HOT 1
- Is this project still maintained? HOT 1
- Path property in CookieSessioProvider struct is missing
- ADFS HTTP-Artifact Signature
- Upgrade to use golang-jwt v5
- Why is the default SP signing algorithm SHA1
- bug: make logout request need add signature logic if `sp.SignatureMethod` is not empty. HOT 7
- Feature: IDP metadata URL should not be mandatory
- Should I fork a repo to receive the contribution [Or a community]
- Fail to authenticate on AzureAD when creating signed MakeAuthenticationRequest
- Externalize Login form
- How to log in by clicking on my app via Okta app?
- How to support multiple idp? HOT 1
- [Update Request] Update Project EOL component github.com/beevik/etree:v1.1.0
- can't figure out how to implement IDP initiated from IDP (for writing tests) HOT 2
- How to create a SAML response with Response and Assertion signature
- How to handle IdP-initiated Single Logout (SLO) in the SP ? HOT 1
- When encrypting SAML assertion only SHA1 option can be used
- [Question] Why the Redirect function doesn't handle the error when parsing an URL
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from saml.