Skip to content
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

Allow setting the additionalParams in .authenticate #157

Closed
Kikketer opened this issue May 26, 2016 · 9 comments · Fixed by #657
Closed

Allow setting the additionalParams in .authenticate #157

Kikketer opened this issue May 26, 2016 · 9 comments · Fixed by #657

Comments

@Kikketer
Copy link

I need to send in the RelayState when asking to login. This relay state is different for each user so I need it to be set when the user requests /login (and then .authenticate()) as opposed to when I start my server and setup passport.use(.

Basically I would like to:

  app.get('/login',
    passport.authenticate(config.passport.strategy,
      {
        additionalParams: {'RelayState': req.url},
        failureRedirect: '/loginFail'
      })
  );

Obviously even the above wouldn't work since I don't have access to the req variable. Perhaps there's a better way to simply have the user return to the page they were asking for.

@that1guy
Copy link

@ploer I'm having the same issue here. Any ideas on how to set RelayState dynamically?

Similar question here (#80 (comment))

Thanks!

@jylauril
Copy link

jylauril commented May 5, 2017

Pro tip: You can wrap the passport.auhenticate call inside your own middleware function so you can access the request and response if needed:

  app.get('/login', function(req, res, next) {
    passport.authenticate(config.passport.strategy,
      {
        additionalParams: {'RelayState': req.url},
        failureRedirect: '/loginFail'
      })(req, res, next); // <- just remember to add these
  });

@tusharbudhe0302
Copy link

tusharbudhe0302 commented Feb 23, 2018

@jylauril @Kikketer Do I need to use this additionalParams: {'RelayState': 'foo'} in my passport.use

  const passportSamlStratergy = new SamlStrategy(config.passport.saml_config,function(profile, done) {
    // console.log('user profile exist!');
    return done(null, profile); 
  });
  passport.use(passportSamlStratergy);

config used for passport :

    passport: {
      strategy: 'saml',
      saml_config: {
        callbackUrl: 'http://usdf13v0412_dev_tushar:3000/assert',
        path: '/assert',
        protocol: 'http://',
        host: 'usdf13v0412_dev_tushar:3000',
        entryPoint: 'https://ssodev.guycarp.com/oamfed/idp/samlv20',
        issuer: 'http://usdf13v0412_dev_tushar:3000/',
        cert: fs.readFileSync("./keys/saml/cert-idp.pem", 'utf-8'),
        privateCert: fs.readFileSync('./keys/saml/key-service-genrateMeataData.key', 'utf-8'),
        decryptionPvk: fs.readFileSync('./keys/saml/certificate-service-genrateMeataData.crt', 'utf-8'),
        signatureAlgorithm : 'sha256',
        identifierFormat: null,
        validateInResponseTo: false,
        disableRequestedAuthnContext: true,
        forceAuthn: true
      }
    }

Please confirm.

@maviola5
Copy link

maviola5 commented Oct 1, 2018

I was having problems with this as well, it looks like master branch and release v0.35.0 have slightly different versions of the getAdditionalParams method.

// v0.35.0
SAML.prototype.getAdditionalParams = function (req, operation) {
  var additionalParams = {};

  var RelayState = req.query && req.query.RelayState || req.body && req.body.RelayState;
  if (RelayState) {
    additionalParams.RelayState = RelayState;
  }

  var optionsAdditionalParams = this.options.additionalParams || {};
  Object.keys(optionsAdditionalParams).forEach(function(k) {
    additionalParams[k] = optionsAdditionalParams[k];
  });

  var optionsAdditionalParamsForThisOperation = {};
  if (operation == "authorize") {
    optionsAdditionalParamsForThisOperation = this.options.additionalAuthorizeParams || {};
  }
  if (operation == "logout") {
    optionsAdditionalParamsForThisOperation = this.options.additionalLogoutParams || {};
  }

  Object.keys(optionsAdditionalParamsForThisOperation).forEach(function(k) {
    additionalParams[k] = optionsAdditionalParamsForThisOperation[k];
  });

  return additionalParams;
};
// master
SAML.prototype.getAdditionalParams = function (req, operation, overrideParams) {
  var additionalParams = {};

  var RelayState = req.query && req.query.RelayState || req.body && req.body.RelayState;
  if (RelayState) {
    additionalParams.RelayState = RelayState;
  }

  var optionsAdditionalParams = this.options.additionalParams || {};
  Object.keys(optionsAdditionalParams).forEach(function(k) {
    additionalParams[k] = optionsAdditionalParams[k];
  });

  var optionsAdditionalParamsForThisOperation = {};
  if (operation == "authorize") {
    optionsAdditionalParamsForThisOperation = this.options.additionalAuthorizeParams || {};
  }
  if (operation == "logout") {
    optionsAdditionalParamsForThisOperation = this.options.additionalLogoutParams || {};
  }

  Object.keys(optionsAdditionalParamsForThisOperation).forEach(function(k) {
    additionalParams[k] = optionsAdditionalParamsForThisOperation[k];
  });

  overrideParams = overrideParams || {};
  Object.keys(overrideParams).forEach(function(k) {
    additionalParams[k] = overrideParams[k];
  });

  return additionalParams;

The docs say that the additionalParams option can be passed in through the authenticate method. Again this works in master but not v0.35.0 which is what gets installed via npm install passport-saml and leads to the confusion.

If you are just trying to pass in or override the RelayState param, you can add it to the query string, as that is supported in v0.35.0.

example:

/**
 * login route
 */
router.get('/login',
  passport.authenticate('saml', {
    failureRedirect: '/',
    failureFlash: true
  }),
  (req, res) => {
    res.redirect(req.body.RelayState || '/');
  }     
);

/**
 * protected route
*/
router.get('/profile', 
  (req, res, next) => {
    if(!req.isAuthenticated()) {
    // getAdditionalParams method will pick up RelayState param via authenticate method on the login 
    route
      return res.redirect('/login?RelayState=/profile');
    }
    next()
});

@blackpuppy
Copy link

I got the same problem, that is, not able to pass additionalParams in the second argument to passport.authenticate(). And eventually found the following solution. Note that I am using TypeScript.

// need to use AuthenticateOptions from passport-saml
import { AuthenticateOptions } from "passport-saml/lib/passport-saml/types";

// then for GET /login route
const relayState = "your-state";  // like a key of the intended URL in a cache
const options: AuthenticateOptions = {
    failureRedirect: "/",
    additionalParams: {
        RelayState: relayState,
    },
};
passport.authenticate("saml", options)(req, res);

// later in POST /login/callback
const relayState = req.body.RelayState;
const redirectUrl = MyCache.get(relayState);
res.redirect(redirectUrl);

TypeScript checks the 2nd argument against the AuthenticateOptions defined in passport, and it does not have additionalParams, so there is a type error. Using the AuthenticateOptions defined in passport-saml solves it. But for JavaScript, it does not have this type checking. So I think it should just work.

@crossan007
Copy link

To piggyback on @blackpuppy 's response here, you can also inline-typecast the <AuthenticateOptions> inside your Express route definition (assuming you added the import: import { AuthenticateOptions } from "passport-saml/lib/passport-saml/types";)

router.get(
  "/saml/login",
  passport.authenticate("saml", <AuthenticateOptions>{ session: false, failureRedirect: "/", failureFlash: true, additionalParams: { RelayState: "asdf" }}),
  function (req, res) {
    res.redirect("/");
  }
);

@cjbarth
Copy link
Collaborator

cjbarth commented Nov 17, 2021

What version of passport-saml are you using @blackpuppy , @crossan007 ?

@crossan007
Copy link

"@types/passport": "^1.0.7",
"@types/passport-saml": "^1.1.3",
"@types/passport-strategy": "^0.2.35",
"passport": "^0.5.0",
"passport-saml": "^3.2.0",

@cjbarth
Copy link
Collaborator

cjbarth commented Nov 17, 2021

I've addressed this in #657.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants