disable requested authentication context #5
Conversation
Passport saml sets a default authentication context of 'PasswordProtectedTransport'. There is some sort of bug with passport saml when trying to use this AuthnContext from a private internal network and authenticate with an external IDP. The proposed change her allows the developer to easily disable this authentication context and let the IDP determine the method of authentication.
With this change, simply add the following line to your configuration for apostrophe-saml inside of your Apostrophe app.js:
disableRequestedAuthnContext: true
Example:
'apostrophe-saml': {
issuer: 'mysite.com',
callbackUrl: 'mysite.com/callback',
disableRequestedAuthnContext: true
}
More details can be found here: node-saml/passport-saml#226
@boutell
|
Interesting. Is there a time when you *wouldn't* want to do this?
…On Fri, Sep 28, 2018 at 2:02 PM TheIceBreaker7 ***@***.***> wrote:
Passport saml sets a default authentication context of
'PasswordProtectedTransport'. There is some sort of bug with passport saml
when trying to use this AuthnContext from a private internal network and
authenticate with an external IDP. The proposed change her allows the
developer to easily disable this authentication context and let the IDP
determine the method of authentication.
With this change, simply add the following line to your configuration for
apostrophe-saml inside of your Apostrophe app.js:
disableRequestedAuthnContext: true
Example:
'apostrophe-saml': {
issuer: 'mysite.com',
callbackUrl: 'mysite.com/callback',
disableRequestedAuthnContext: true
}
More details can be found here: node-saml/passport-saml#226
<node-saml/passport-saml#226>
@boutell <https://github.com/boutell>
------------------------------
You can view, comment on, or merge this pull request online at:
#5
Commit Summary
- disable requested authentication context
File Changes
- *M* index.js
<https://github.com/apostrophecms/apostrophe-saml/pull/5/files#diff-0>
(2)
Patch Links:
- https://github.com/apostrophecms/apostrophe-saml/pull/5.patch
- https://github.com/apostrophecms/apostrophe-saml/pull/5.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5>, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/AAB9fYj5jx3u1gjUFcNkXSGXpNklcJPwks5ufmRIgaJpZM4W_LsK>
.
--
*Thomas Boutell, Chief Software Architect*
P'unk Avenue | (215) 755-1330 | punkave.com
|
|
If your application requires a specific form of authentication context, particularly 'PasswordProtectedTransport' then you may want to keep this option set to false/null. However, if Apostrophe is not looking for this particular authentication context, then I guess you could always have it default to disabled within apostrophe-saml. If a developer has a use case where they wanted to specify a different authentication context, then they may want to keep the context enabled and then specify a different context...though the ability to specify this alternate context is not reflected here. My change above is a little bit basic in that it's either the passport-saml default 'PPT' context or let the IDP determine the context on it's own. |
|
apostrophe-saml wraps passport-saml in a pretty basic way which makes it very easy to set up with Apostrophe. However, we do lose access to a lot of configuration options with it's current form. Ideally this extension would ultimately allow you to pass any of the passport saml configuration options through. But that may be asking for too much :-P |
|
Definitely open to propping the door to changing the configuration more, in
backwards-compatible ways...
…On Fri, Sep 28, 2018 at 2:29 PM TheIceBreaker7 ***@***.***> wrote:
apostrophe-saml wraps passport-saml in a pretty basic way which makes it
very easy to set up with Apostrophe. However, we do lose access to a lot of
configuration options with it's current form.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAB9fbQWPkuzQCJcuietob7uqxYiSXtBks5ufmqDgaJpZM4W_LsK>
.
--
*Thomas Boutell, Chief Software Architect*
P'unk Avenue | (215) 755-1330 | punkave.com
|
|
Well, we could always create an 'extraOptions' object. And then copy all attributes from extra options into the options object passed into passport saml. This would allow apostrophe-saml to continue working with backwards compatible configurations |
|
`passportSamlOptions` would be more descriptive I think.
…On Fri, Sep 28, 2018 at 3:03 PM TheIceBreaker7 ***@***.***> wrote:
Well, we could always create an 'extraOptions' object. And then copy all
attributes from extra options into the options object passed into passport
saml.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAB9fXBknl_D682MAjVMgNbdKrcE8_-Zks5ufnJ6gaJpZM4W_LsK>
.
--
*Thomas Boutell, Chief Software Architect*
P'unk Avenue | (215) 755-1330 | punkave.com
|
|
Sounds good. I will put something together for you to review. |
Added the ability to use extra passport-saml options that were not previously defined in this wrapper. Previous configurations are untouched and should be backwards compatible. To pass extra options to apostrophe saml use the following syntax:
```
'apostrophe-saml': {
issuer: 'mysite.com',
callbackUrl: 'mysite.com/callback',
passportSamlOptions: {
disableRequestedAuthnContext: true,
logoutUrl: 'www.mysite.com/SLO',
forceAuthn: true
}
}
```
|
Ok take a look at my latest commit and let me know what you think. |
There was a problem hiding this comment.
We're getting close!
You should handle the overriding bit with _.defaults() or perhaps just Object.assign since all supported versions of Node support it. And follow the coding conventions of the module re: whitespace.
I think the most valuable contribution you might make here is adding a note to the README about how and why you might use this feature.
updated addPassportSamlOptions method to use Object.assign()
Readme updated to reflect new way of passing extra parameters to passport-saml through the passportSamlOptions object
|
Thanks for the tip. I didn't know about Object.assign. That's very handy. I altered the method to use that instead of my barbaric C style for-loop ;-) I also added the passportSamlOptions to the README. Let me know if you need anything else. |
|
This code is working in my own environment. Have you taken a look at the changes you requested? |
|
This has been published to npm. Thanks!
…On Mon, Oct 1, 2018 at 11:46 AM TheIceBreaker7 ***@***.***> wrote:
@boutell <https://github.com/boutell>
This code is working in my own environment. Have you taken a look at the
changes you requested?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAB9fSuinLzefJO9LKxIJxacmWl7cNyTks5ugjjDgaJpZM4W_LsK>
.
--
*Thomas Boutell, Chief Software Architect*
P'unk Avenue | (215) 755-1330 | punkave.com
|
|
No, thank you! And thanks for advocating a cleaner solution! |
|
No problem! Out of curiosity, how are you using Apostrophe?
…On Mon, Oct 1, 2018 at 12:33 PM TheIceBreaker7 ***@***.***> wrote:
No, thank you! And thanks for advocating a cleaner solution!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAB9fSfERv2hl3tuvGKrqmjwtVqh1XXjks5ugkPZgaJpZM4W_LsK>
.
--
*Thomas Boutell, Chief Software Architect*
P'unk Avenue | (215) 755-1330 | punkave.com
|
Passport saml sets a default authentication context of 'PasswordProtectedTransport'. There is some sort of bug with passport saml when trying to use this AuthnContext from a private internal network and authenticate with an external IDP. The proposed change her allows the developer to easily disable this authentication context and let the IDP determine the method of authentication.
With this change, simply add the following line to your configuration for apostrophe-saml inside of your Apostrophe app.js:
disableRequestedAuthnContext: true
Example:
'apostrophe-saml': {
issuer: 'mysite.com',
callbackUrl: 'mysite.com/callback',
disableRequestedAuthnContext: true
}
More details can be found here: https://github.com/bergie/passport-saml/issues/226
@boutell