Comments (10)
You are right, path
no longer works because it was removed from node-saml
on which passport-saml
depends. The README should have its example updated. Please submit a PR. Also, note in the README how it does reference node-saml
, so that might help you see other changes that are noteworthy during migration.
from passport-saml.
This is related to node-saml/node-saml#214
While I completely agree that removing complexity is a good thing, I now miss the possibility to dynamically configure the callbackUrl
: in an environment where I do not know my public hostname, I could just use a relative path
. This is not possible anymore, and I cannot even guess my hostname using the current request, since configuration takes place outside of a request context. Or am I missing something?
from passport-saml.
So the "new" way when you can only use path
(but do not know your own host) is this:
passport.use(
new SamlStrategy(
{
callbackUrl: 'http://some-random-host/some-random-url/', // required but not used, see below
...
}
)
);
app.get(
"/login",
(req, res, next) => {
path = '/login/callback';
callbackUrl = (req.protocol || 'http').concat('://') + (req.headers?.host || 'localhost') + path;
passport.authenticate("saml", { callbackUrl })(req, res, next),
}
);
Right?
from passport-saml.
I would probably use const path = ...
, etc. I would probably also use a URL builder API instead of string concatenation like https://nodejs.org/api/url.html#url_url_format_urlobject, but yes, you have the right idea. Another approach would be to use a pattern like passport-saml
has for multi-SAML strategy.
Basically, the node-saml
library isn't the in the business of building URLs anymore. There were problems with the implementation, and to make the problems go away it was decided that we'd simplify the code and offload that work to the consumer (who could use another library). You're case does seem a little strange, and it isn't always wise to trust req.headers.host
; see node-saml/node-saml#214, https://portswigger.net/web-security/host-header, and https://cqr.company/web-vulnerabilities/http-host-header-attacks/
from passport-saml.
library isn't the in the business of building URLs anymore
I agree. However there are legitimate cases where the service does not know its hostname, e.g. when it is deployed in staging branches, etc. I cannot hard-code a callbackUrl
there, otherwise we'd risk surprises when redirecting back to the wrong host. So there should be a "best practice" guide on how to do this in case other users also need to rely on the previous path
behaviour.
I'll try to make a sensible README PR...
from passport-saml.
Our official suggestion is to pass in a constructed URL, like you're currently doing. We'd be open to a PR that would take a function for callbackUrl
, so that you can build it on invocation.
from passport-saml.
Ok I need to withdraw my solution: callbackUrl
is not used at all in the authenticate
method. So the solution needs to be a function for callbackUrl
.
from passport-saml.
Honestly, I don't know why you don't have some parameters for your different environments that you can use to build a URL specific to the environment. An environment variable could be used to switch which URL is passed in. Or, you could use the multi-SAML functionality. I'm open to the function idea still, but it seems that this might be pointing to a more endemic situation. How do you change your DB config or other such-like things per environment?
from passport-saml.
... Or, you could use the multi-SAML functionality. I'm open to the function idea still, but it seems that this might be pointing to a more endemic situation. ...
@nicokaiser as @cjbarth is trying to say: Multisaml approach provides access to request (to extract whatever request specific information) and it provides mechanism to return e.g. fixed set of other config variables and change callbackUrl config per request (which would solve this issue reporter's case).
This comment entry's actual point is: why even consider new round of fine-grained configuration stuff because (and especially in this case/for this case) there is goarse-grained approach (from passport-saml pov) already available which provides mechanism to change about everything per request.
Sidenote: host
parameters of various methods at node-saml are not used anymore https://github.com/node-saml/node-saml/blob/v5.0.0/src/saml.ts
Seems that those were missed during node-saml/node-saml#214 review. Unfortunately removing those is breaking change.
from passport-saml.
Related Issues (20)
- [BUG] Invalid document signature after upgrading from 2.2.0 to 4.0.1 HOT 12
- [BUG] Passport SAML is always chosen first by NestJS Passport, no matter the order HOT 1
- Invalid Signature While integrating passport-saml with OAM HOT 7
- [BUG] After an upgrade from passport-saml to @node-saml/passport-saml, assertion validation stopped working with HTTP-POST binding with an error: SAML.validatePostResponseAsync, Invalid document signature HOT 9
- Is there a way to test a provider configuration before call done() on MultiSamlStrategy? HOT 1
- Cannot POST /saml/consume HOT 3
- Possible to add custom relay state in SAML login requests? HOT 2
- Assertion signature is missing in SAML Response when using Google Workspace HOT 5
- [BUG] Invalid document signature HOT 7
- Functionality difference between Redhat UBI 8and UBI 9 HOT 3
- Reference id not matching with documentElement id HOT 8
- passport-saml requires session support (session auth) — Is it possible to do with JWT auth instead?
- [BUG] Bug in the implementation of MultiSamlStrategy on NestJS HOT 2
- [BUG] validateInResponseTo = true does not validate responses HOT 6
- Importing passport-saml fails on node v20.6.0 HOT 3
- [BUG] Importing @node-saml/passport-saml gives type error HOT 5
- [BUG] Error: Invalid document signature HOT 2
- [BUG] Callback URL switches from https to http HOT 2
- [BUG] TypeError: expirationTimer.unref is not a function HOT 3
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 passport-saml.