Comments (2)
This issue is related to #946.
from passport.
I guess that @eesdil's point is very important, because...
The method AuthGuard.getAuthenticateOptions
was implemented to resolve #322 issue on #323 PR. The goal of this method is to allow the derived AuthGuard
to customize the authenticate options that will be passed to passport.authenticate
function call. The contract of the passport.authenticate
function expects an AuthenticateOptions
object parameter that must be handled by the associated strategy when it handles the authentication request.
So, currently...
AuthGuard
's constructor receives, by injection, theAuthModuleOptions
object, that's responsible to configure the module of this library;getAuthenticateOptions
returns an object that matches the interfaceIAuthModuleOptions
;AuthGuard.canActivate
obtain the options fromgetAuthenticateOptions
and merges it with the injected options of theAuthGuard
instance;AuthGuard.canActivate
pass the merged options topassport.authenticate
.
export interface IAuthModuleOptions<T = any> {
defaultStrategy?: string | string[];
session?: boolean;
property?: string;
[key: string]: any;
}
export type IAuthGuard = CanActivate & {
// ...
getAuthenticateOptions(context: ExecutionContext): IAuthModuleOptions | undefined;
// ...
};
class MixinAuthGuard<TUser = any> implements CanActivate {
@Optional()
@Inject(AuthModuleOptions)
protected options: AuthModuleOptions = {};
constructor(@Optional() options?: AuthModuleOptions) {
this.options = options ?? this.options;
// ...
}
async canActivate(context: ExecutionContext): Promise<boolean> {
const options = { // <-- MERGED OPTIONS
...defaultOptions,
...this.options,
...(await this.getAuthenticateOptions(context))
};
const [request, response] = [
this.getRequest(context),
this.getResponse(context)
];
const passportFn = createPassportContext(request, response);
const user = await passportFn(
type || this.options.defaultStrategy,
options, // <-- PASS OPTIONS TO passport.authenticate
(err, user, info, status) =>
this.handleRequest(err, user, info, context, status)
);
request[options.property || defaultOptions.property] = user;
return true;
}
// ...
}
const createPassportContext =
(request, response) => (type, options, callback: Function) =>
new Promise<void>((resolve, reject) =>
passport.authenticate(type, options, (err, user, info, status) => { // <-- passport.authenticate CALL
try {
request.authInfo = info;
return resolve(callback(err, user, info, status));
} catch (err) {
reject(err);
}
})(request, response, (err) => (err ? reject(err) : resolve()))
);
IAuthModuleOptions
and AuthenticateOptions
are not syntactically incompatible, as both interfaces have all their properties as optional. But, semantically this is not true. The AuthenticateOptions
is a contract that must be handled by the strategy, and the IAuthModuleOptions
must be handled by the AuthGuard
's module.
I noticed this when inspecting the authorization URL generated by the openid-client
's library strategy. The openid-client
's strategy uses the incoming AuthenticateOptions
object to handle the authorization URL generation, which includes all parameters to the authorization URL. Therefore, the generated URL was wrong, containing invalid parameters that expose the AuthGuard
module options. Like this:
client_id: my-app
scope: openid profile
response_type: code
redirect_uri: http://localhost:3000/auth/oidc/callback
state: 4nCwYO3jS8tKHzbM1FN_lv5yyAOHVMlITMUW8dND494
session: true # <-- EXPOSED OPTION
property: user # <-- EXPOSED OPTION
defaultStrategy: oidc # <-- EXPOSED OPTION
code_challenge: TYrhDeMoO9j9_CXuHSLXdCGdIXukT4orrYtZ8X3783g
code_challenge_method: S256
I think IAuthModuleOptions
should be changed to have a new authenticateOptions
property that sets the AuthenticateOptions
object at the AuthGuard
's module level; and the getAuthenticateOptions
method should returns an object that matches the AuthenticateOptions
interface. In the end, the AuthGuard.canActivate
should merge these options and pass them to the passport.authenticate
function. This is the way to prevent the AuthGuard
's module options from being exposed to the strategy and segregate the interests of these options.
As proposed, the approach should be changed to:
export interface IAuthModuleOptions<T = any> {
defaultStrategy?: string | string[];
session?: boolean;
property?: string;
authenticateOptions?: AuthenticateOptions; // <-- NEW PROPERTY
[key: string]: any;
}
export type IAuthGuard = CanActivate & {
// ...
// CHANGED RETURN TYPE
getAuthenticateOptions(context: ExecutionContext): Promise<AuthenticateOptions> | AuthenticateOptions | undefined;
// ...
};
class MixinAuthGuard<TUser = any> implements CanActivate {
@Optional()
@Inject(AuthModuleOptions)
protected options: AuthModuleOptions = {};
constructor(@Optional() options?: AuthModuleOptions) {
this.options = options ?? this.options;
// ...
}
async canActivate(context: ExecutionContext): Promise<boolean> {
const options = this.options; // <-- MODULE OPTIONS
const authenticateOptions = { // <-- AUTHENTICATE OPTIONS
...defaultOptions.authenticateOptions,
...options.authenticateOptions,
...(await this.getAuthenticateOptions(context))
};
const [request, response] = [
this.getRequest(context),
this.getResponse(context)
];
const passportFn = createPassportContext(request, response);
const user = await passportFn(
type || this.options.defaultStrategy,
authenticateOptions, // <-- PASS OPTIONS TO passport.authenticate
(err, user, info, status) =>
this.handleRequest(err, user, info, context, status)
);
request[options.property || defaultOptions.property] = user;
return true;
}
// ...
}
PR #1610
from passport.
Related Issues (20)
- passport-steam Guide will be appreciated HOT 1
- How about changing the constructor parameter type of PassportStrategy like this? HOT 1
- validate(payload: any) in Implementing Passport JWT doc it's not correct HOT 2
- AuthGuard on Local Strategy return undefinded HOT 1
- overwhelmed by error logs HOT 6
- Pretty magical thing: request.authInfo = undefined and behavior is very strange HOT 3
- Unable to preserve session in passport v0.6.0 HOT 2
- v10 does not allow typing on custom `getRequest` method in class with extended AuthGuard HOT 1
- AuthGuard does not work with authentication-only strategies HOT 1
- Support for [email protected] HOT 3
- using @Inject(REQUEST) req breaks passport. HOT 4
- Guard Order Scope affects order of execution HOT 2
- Can a new release be pushed? HOT 3
- Nest can't resolve dependencies of the JwtAuthenticationGuard after upgrade 8.1.0 to 8.1.1 HOT 9
- Cannot read properties of undefined (reading 'property') HOT 1
- Using custom passport from a different package causes error HOT 1
- Dependency tree discrepancy HOT 2
- PassportStrategy interface does not contain validate() HOT 2
- The defaultStrategies cannot be set during runtime in the AuthGuard HOT 1
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.