panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

Redirect Loop #195

Closed dobby5 closed 5 years ago

dobby5 commented 5 years ago

Hello,

after every successful login I will be redirected to /home as usual. However, when I try to retrieve /profile I will be redirected back to /home.

can someone please help me?

(Sorry the code is not beautifully written. ;))

const express = require('express');
const app = express();
var passport = require('passport');
const mongoose = require('mongoose');
const session = require('express-session');
const { Strategy, Issuer } = require('openid-client');

const MongoStore = require('connect-mongo')(session);

app.use(session({
    resave: true,
    saveUninitialized: true,
    key: 'Test',
    secret: 'test',
    store: new MongoStore({ mongooseConnection: mongoose.connection })
}));

app.use(passport.initialize());
app.use(passport.session());

mongoose.connect('removed');

(async () => {
    const issuer = await Issuer.discover('http://localhost:4110/oauth2/.well-known/openid-configuration')

    //after getting issuer.client add client info
    const passReqToCallback = false;
    const usePKCE = true;

    const client = new issuer.Client({
        client_id: 'KBNSQWBlktNIw4IPT3T-6',
        client_secret: 's_cvdNQ65vqwYlSMyTkPbAUL_o69wIw9UCvFbIHFSoAsJ8CK2QmfA6r5sMWYipLLMMerg_gtAcUqslPoj_ZTRA',
        redirect_uris: ['http://localhost:5984/callback'],
        response_types: ['code']
    });

    const params = {
        client_id: 'KBNSQWBlktNIw4IPT3T-6',
        response_type: 'code',
        scope: 'openid profile email',
        redirect_uri: 'http://localhost:5984/callback'
    };

    //verify function ensures the user's credientials are good
    const verify = (tokenSet, userInfo, done) => {
        console.log(tokenSet, userInfo, 'got user info here<><><><><><><>');
        return done(null, userInfo);
    };

    //set options using client obj recived from hubstaff and param
    const options = {
        client,
        params,
        passReqToCallback,
        usePKCE
    };

    passport.use('openid-client', new Strategy(options, verify));
})();

app.get('/login', passport.authenticate('openid-client'));

app.get('/callback',
    passport.authenticate('openid-client', {
        successRedirect: '/home',
        failureRedirect: '/login'
    })
);

app.get("/profile",
    passport.authenticate("openid-client", { failureRedirect: "/error" }),
    (req, res) => {
        return app.render(req, res, "/", {
            user: req.user
        });
    }
);

app.get('/user', (req, res) => {
    res.status(200).json({
        'message': 'This is not secured guest routing'
    });
});

passport.serializeUser(function (user, done) {
    done(null, user);
});

passport.deserializeUser(function (uid, done) {
    done(null, uid);
});

app.listen('5984', () => {
    console.log('Server Instance started on 5984');
});
panva commented 5 years ago

@dobby5 that's not a strategy issue, the strategy is for authenticating (and subsequent creating a session), not authorizing access to a resource. The problem is you're forcing /profile to run authentication regardless of what's in your session, which is what you'd use req.isAuthenticated for.

All of this is related to passport in general, not a specific strategy.

big-kahuna-burger commented 5 years ago

@dobby5 Try writing a middleware wrapper around passport.authenticate middleware.

const requireAuthenticated = (req, res, next) => {
  if(!req.isAuthenticated) return passport.authenticate("openid-client", { failureRedirect: "/error" })(req, res, next)
  next()
}

And you can then:

app.get("/profile",
    requireAuthenticated,
    (req, res) => {
        return app.render(req, res, "/", {
            user: req.user
        });
    }
);
dobby5 commented 5 years ago

Thank you, but now I get the error:

Error: did not find expected authorization request details in session, req.session["oidc:localhost"] is undefined

panva commented 5 years ago

Your session is not being loaded, or persisted, or both :) again, not something the strategy is responsible for.

dobby5 commented 5 years ago

So it's the client? Can I also exclude the server?

big-kahuna-burger commented 5 years ago

@dobby5 You can try this:

const strategy = new Strategy(
    {
      name: 'oidc:whatever',

And in the calls, instead of

passport.authenticate("openid-client"...

do this:

passport.authenticate(strategy.name ....

Let me know if that works.

panva commented 5 years ago

So it's the client? Can I also exclude the server?

No its your express session mechanism setup.

dobby5 commented 5 years ago

Thanks @big-kahuna-burger, but your solution didn't work.

But if it is an express session mechanism setup, why does it work when I open the server locally and the client. Then I can log in.

I just deactivated the SSL Cert on my remote server (without https) but it doesn't work either.

And thank you both.

dobby5 commented 5 years ago

Maybe it helps ;) The code

const express = require('express');
const app = express();
var passport = require('passport');
const mongoose = require('mongoose');
const session = require('express-session');
const { Strategy, Issuer } = require('openid-client');

const MongoStore = require('connect-mongo')(session);

app.use(session({
    resave: true,
    saveUninitialized: true,
    key: 'Test',
    secret: 'test',
    store: new MongoStore({ mongooseConnection: mongoose.connection })
}));

app.use(passport.initialize());
app.use(passport.session());

mongoose.connect('');

(async () => {
    const issuer = await Issuer.discover('');

    const passReqToCallback = false;
    const usePKCE = true;

    const client = new issuer.Client({
        client_id: '',
        client_secret: '',
        redirect_uris: ['http://localhost:5984/callback'],
        response_types: ['code']
    });

    const params = {
        client_id: '',
        response_type: 'code',
        scope: 'openid profile email',
        redirect_uri: 'http://localhost:5984/callback'
    };

    const verify = (tokenSet, userInfo, done) => {
        return done(null, userInfo);
    };

    const options = {
        client,
        params,
        passReqToCallback,
        usePKCE
    };

    passport.use('openid-client', new Strategy(options, verify));
})();

app.get('/login', passport.authenticate('openid-client'));

app.get('/callback',
    passport.authenticate('openid-client', {
        successRedirect: '/home',
        failureRedirect: '/login'
    })
);

const isLoggedIn = (req, res, next) => {
    if (!req.isAuthenticated) return passport.authenticate("openid-client", { failureRedirect: "/error" })(req, res, next);
    next();
};

app.get('/api/me', isLoggedIn, (req, res) => {
    return res.json({ user: req.user });
});

app.get('/safe/me', isLoggedIn, (req, res) => {
    return res.json({ user: req.user });
});

passport.serializeUser(function (user, done) {
    done(null, user);
});

passport.deserializeUser(function (uid, done) {
    done(null, uid);
});

app.listen('5984', () => {
    console.log('Server Instance started on 5984');
});
big-kahuna-burger commented 5 years ago

@dobby5 I see. So what the PassportJS's docs on serializeUser and deserializeUser say? :) I'd expect to see something like...


passport.serializeUser(function(user, done) {    
   try {
    await Users.save(user)    
    done(null, user.sub)
    } catch (err) {
      done(err)
    }
})

passport.deserializeUser(function (sub, done) {
  const user = await Users.find(sub)
  done(user ? null : new Error('could not find user with id' + sub), user)
})
dobby5 commented 5 years ago

I try this already. It doesnt work

macdoor commented 4 years ago

@panva I've got almost same problem. Do you have a plan to provide a full openid-client + passport + express example?

dobby5 commented 4 years ago

@panva I've got almost same problem. Do you have a plan to provide a full openid-client + passport + express example?

That were really nice

panva commented 4 years ago

@macdoor @dobby5 this issue tracker is not a supplement to actually learning how to use passport.

I can understand why you turn this way but maybe using passport.js isn't the way to go with it not being cared for and explained properly.

'use strict';

const util = require('util');

const express = require('express');
const app = express();

const passport = require('passport');
const { Strategy, Issuer } = require('openid-client');
var session = require('express-session');

(async () => {
  const issuer = await Issuer.discover('https://op.panva.cz');
  const client = await issuer.Client.register({
    redirect_uris: ['http://localhost:3000/auth/cb'],
  });
  console.log('registered client', client.metadata);

  passport.serializeUser(function (user, done) {
    done(null, user);
  });

  passport.deserializeUser(function (uid, done) {
    done(null, uid);
  });

  passport.use('panva.cz', new Strategy({ client }, function (tokenset, done) {
    return done(null, tokenset.claims());
  }));

  app.use(session({ secret: 'keyboard cat' }));
  app.use(passport.initialize());
  app.use(passport.session());

  app.get('/', function (req, res) {
    res.send(util.format('Hello %s!', req.isAuthenticated() ? req.user.sub : 'Anonymous. <a href="/auth">Authenticate</a>'));
  });

  app.get('/auth', passport.authenticate('panva.cz'));
  app.get('/auth/cb', passport.authenticate('panva.cz', { successRedirect: '/', failureRedirect: '/' }));

  app.listen(3000, function () {
    console.log('Example app listening on port 3000!');
  });
})().catch((err) => {
  console.error(err);
  process.exit(1);
});
dobby5 commented 4 years ago

okay thx you, i'll think it over when you say it's not the best way about passport.

But you could send me the config from your server. My server seems to be configured wrong. That would be super nice.

panva commented 4 years ago

server configuration has nothing to do with this, the example above executes a regular authorization code flow, the only thing i did there that you'll replace is that i registered a client dynamically where you'll instantiate a Client instance with your id, secret, redirect uri, etc...

dobby5 commented 4 years ago

It was my fault. It all works. Thank you :)

macdoor commented 4 years ago

@panva thank you!