Description
Is there an existing issue for this?
- I have searched the existing issues
Current behavior
Quote from a comment in the PassportJS source code for the built-in session strategy:
[i]f login session data is not present, the request will be passed to the next middleware, rather than failing authentication - which is the behavior of most other strategies.
Not sure if "most strategies" is an accurate claim, but I will say that two rather popular strategies do not actually work like that. Both passport-jwt and passport-local reject requests if they are unauthenticated. So at the very least there is a lack of consistency in PassportJS strategies.
Here's a simplified excerpt from the built-in session strategy:
SessionStrategy.prototype.authenticate = function(req, options) {
// Obtain user object from the session somehow...
var su = {} //...
if (su) {
this._deserializeUser(su, req, function(err, user) {
if (err) { return self.error(err); }
if (user) {
// IMPORTANT! Sets the user property on the request
req.user = user;
}
// JWT / Local strategies would both call self.success() at this point, indicating that
// the request is not only authenticated, but also authorized
self.pass();
});
} else {
// This strategy does not fail if there is no user in the session (i.e. request is unauthenticated)
self.pass();
}
};
Unlike the JWT / local strategies, the session strategy does not invoke self.fail()
/ self.success()
and instead always calls self.pass()
, regardless of whether the client is authenticated or not. Its purpose is to:
- retrieve the user object from the session;
- set the
user
property on the request object; - proceed to the next middleware with or without the user object.
Now onto NestJS. The AuthGuard is a useful wrapper around PassportJS middleware, but it does not support strategies that only authenticate requests but do not authorize them (such as the session strategy). Here's the relevant excerpt from the canActivate function in the AuthGuard:
const user = await passportFn(
type || this.options.defaultStrategy,
options,
(err, user, info, status) =>
this.handleRequest(err, user, info, context, status)
);
request[options.property || defaultOptions.property] = user;
return true;
With an authenticate-only strategy, if the request is not authenticated:
passportFn
does not fail,user
is undefined,canActivate
returnstrue
, allowing an unauthenticated request to pass through.
This is fine, because it sort of respects the design of the strategy. In that case the authorization check should be delegated to a later stage. Normally, when working with PassportJS, one could use req.isAuthenticated()
in the next guard for that very purpose.
However, the problem is that the strategy, and consequently the AuthGuard, behave exactly the same if the client is authenticated:
passportFn
does not fail,user
is undefined,canActivate
returnstrue
.
Since user
is undefined, it is no longer possible to use req.isAuthentiated()
after canActivate
finishes. This causes a bit of a headache down the line, because I have to resort to a different system altogether for working with such strategies.
Minimum reproduction code
N/A
Steps to reproduce
No response
Expected behavior
I expect either of the two:
- The AuthGuard completely conforms to authenticate-only strategies thus not overwriting
req.user
with undefined values. - The AuthGuard rejects unauthenticated requests after checking
req.isAuthenticated()
.
Option #1 is probably preferable.
Package version
10.0.2
Passport version
0.7.0
NestJS version
10
Node.js version
16
In which operating systems have you tested?
- macOS
- Windows
- Linux
Other
No response