-
Notifications
You must be signed in to change notification settings - Fork 672
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
IDP SAML2: added 'as:NoSession' #644
base: master
Are you sure you want to change the base?
Conversation
Allow AuthenticationSource to decide to avoid SessionIndex generation and association to current IDP session (avoid SLO)
Hi @tbenr! I'm a bit confused by this. What's the use case you have in mind? |
Hi again :-) i have some requirements where, for some Authn Requests (heaving particular AuthnContextClassRefs) i dont have to generate a session on IdP side (so no SessionIndex released to SP). But for other AuthnContextRefs the session is still required. So I still need to have a session with SP tracking (for SLO). So i developed an authentication module (the same module mentioned here #643 :-) ) which decide how to authenticate the user based on RequestedAuthnContext and also if simplesamlphp has to release a SessionIndex and track SP for SLO. |
Ok, now you definitely got my attention 😄 Why is all this needed? I mean, you are basically letting an SP control whether users will be able to log out of it or not, from what I understand in your description. What's the purpose? Also, what's the trouble with In any case, I feel this is very ad-hoc, so I'm a bit hesitant on how to approach this... |
Well.. the "pratical" reason is this: I have to be compliant to SPID regulatory requiring IdP to NOT release "Theoretically", IMHO these requirements are reasonable and have their underlying philosophy, even if awkward to implement. I still need to verify\test interaction among SpidL1 SSO session and SpidL2> "temporary auth session". In my current implementation, using the feature in this PR, i can avoid SP tracking and SessionIndex creation for SpidL2>, but it still create a real session IdP side, that could potentially lead to a successful SSO with a subsequent SpidL1. But at the and I think this can be manage within the Authentication Module. |
I somehow expected this being related to SPID. Lots of weird requirements coming from them lately 😞 Do you have a link where I can read about this policy? It's fine if it's only in Italian, I can probably make some sense out of it... In any case, I definitely think this belongs in the authentication source. We have something similar in Feide, where we use the current SimpleSAMLphp codebase plus a custom auth source. There, we evaluate if a user with an existing session needs to be authenticated with two-factor again (in the This same mechanism can be used for your use case, making sure that if SpidL2 is required, the user is reauthenticated properly and SSO doesn't happen. Regarding the |
Thanks for the useful info, I'll go in deep with I agree with you about SPID requirements oddity... Anyway here is some useful links:
|
Thanks for the pointers! I don't know Italian, but could it be that this is specifying a SHOULD instead of a MUST?
If it can be taken as a SHOULD, then you would be fine to ignore that requirement. In the end, I understand they intended this to prevent service providers from starting single logout because no session has been kept, due to the step-up level of authentication used. However, in that case, I don't see a reason why this should be enforced (as in, MUST), as that can be implicit for SPs joining SPID, and particular implementations not following that requirement might ignore logout messages anyway (like in this case). Maybe we should find some authoritative voice that can resolve our doubts before we try to implement this? |
Unfortunately in italian that means MUST (btw I'm actually italian :-D). I'm actually quite into SPID stuff (I already implemented an IDP with other solutions) and I agree with you that in this case there is no real benefit, apart of annoying implementers :) There are many little pitfalls that force implementers to drift from a standard de-facto SAML2 using opensource\vendor software to a customized solution. I heard such oddity could be blunted in the future but they are still officially in place. |
Codecov Report
@@ Coverage Diff @@
## master #644 +/- ##
============================================
- Coverage 40.39% 40.37% -0.02%
- Complexity 3454 3459 +5
============================================
Files 142 142
Lines 10401 10406 +5
============================================
Hits 4201 4201
- Misses 6200 6205 +5 |
1c686ab
to
eb20457
Compare
08ebb9c
to
64fca25
Compare
7a53fc8
to
d73ae47
Compare
e5c0e21
to
d5616df
Compare
2e6ab04
to
32f9acc
Compare
29f7b69
to
1a911ce
Compare
c7c8357
to
fdbe001
Compare
3b5f5ba
to
96357ee
Compare
7587851
to
d523b31
Compare
8c90121
to
d534e3b
Compare
bc1c5c8
to
d0a5974
Compare
ccb9b02
to
120a100
Compare
6004a77
to
58bf8db
Compare
5c9fb2c
to
0970efc
Compare
Allow AuthenticationSource to decide to avoid SessionIndex generation and association to current IDP session (avoid SLO).
I don't know it "as:" prefix is correct. If the feature is considered interesting, let's suggest the correct one.