-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: resolve authentication strategy registered via extension point #2763
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -163,3 +163,199 @@ And the abstractions for: | |
- return user | ||
- controller function: | ||
- process the injected user | ||
|
||
## Registering an authentication strategy via an extension point | ||
|
||
Authentication strategies register themselves to an authentication strategy | ||
provider using an | ||
[Extension Point and Extensions](https://loopback.io/doc/en/lb4/Extension-point-and-extensions.html) | ||
pattern. | ||
|
||
The `AuthenticationStrategyProvider` class in | ||
`src/providers/auth-strategy.provider.ts` (shown below) declares an | ||
`extension point` named | ||
`AuthenticationBindings.AUTHENTICATION_STRATEGY_EXTENSION_POINT_NAME` via the | ||
`@extensionPoint` decorator. The binding scope is set to **transient** because | ||
an authentication strategy **may** differ with each request. | ||
`AuthenticationStrategyProvider` is responsible for finding (with the aid of the | ||
`@extensions()` **getter** decorator) and returning an authentication strategy | ||
which has a specific **name** and has been registered as an **extension** of the | ||
aforementioned **extension point**. | ||
|
||
```ts | ||
@extensionPoint( | ||
AuthenticationBindings.AUTHENTICATION_STRATEGY_EXTENSION_POINT_NAME, | ||
{scope: BindingScope.TRANSIENT}, | ||
) | ||
export class AuthenticationStrategyProvider | ||
implements Provider<AuthenticationStrategy | undefined> { | ||
constructor( | ||
@extensions() | ||
private authenticationStrategies: Getter<AuthenticationStrategy[]>, | ||
@inject(AuthenticationBindings.METADATA) | ||
private metadata?: AuthenticationMetadata, | ||
) {} | ||
async value(): Promise<AuthenticationStrategy | undefined> { | ||
if (!this.metadata) { | ||
return undefined; | ||
} | ||
const name = this.metadata.strategy; | ||
const strategy = await this.findAuthenticationStrategy(name); | ||
if (!strategy) { | ||
// important not to throw a non-protocol-specific error here | ||
let error = new Error(`The strategy '${name}' is not available.`); | ||
Object.assign(error, { | ||
code: AUTHENTICATION_STRATEGY_NOT_FOUND, | ||
}); | ||
throw error; | ||
} | ||
return strategy; | ||
} | ||
|
||
async findAuthenticationStrategy(name: string) { | ||
const strategies = await this.authenticationStrategies(); | ||
const matchingAuthStrategy = strategies.find(a => a.name === name); | ||
return matchingAuthStrategy; | ||
} | ||
} | ||
``` | ||
|
||
The **name** of the strategy is specified in the `@authenticate` decorator that | ||
is added to a controller method when authentication is desired for a specific | ||
endpoint. | ||
|
||
```ts | ||
class UserController { | ||
constructor() {} | ||
@get('/whoAmI') | ||
@authenticate('basic') | ||
whoAmI() | ||
{ | ||
... | ||
} | ||
} | ||
``` | ||
|
||
An authentication strategy must implement the `AuthenticationStrategy` interface | ||
defined in `src/types.ts`. | ||
|
||
```ts | ||
export interface BasicAuthenticationStrategyCredentials { | ||
email: string; | ||
password: string; | ||
} | ||
|
||
export class BasicAuthenticationStrategy implements AuthenticationStrategy { | ||
name: string = 'basic'; | ||
|
||
constructor( | ||
@inject(BasicAuthenticationStrategyBindings.USER_SERVICE) | ||
private userService: BasicAuthenticationUserService, | ||
) {} | ||
|
||
async authenticate(request: Request): Promise<UserProfile | undefined> { | ||
const credentials: BasicAuthenticationStrategyCredentials = this.extractCredentals( | ||
request, | ||
); | ||
const user = await this.userService.verifyCredentials(credentials); | ||
const userProfile = this.userService.convertToUserProfile(user); | ||
|
||
return userProfile; | ||
} | ||
``` | ||
|
||
A custom sequence must be created to insert the | ||
`AuthenticationBindings.AUTH_ACTION` action. The `AuthenticateFn` function | ||
interface is implemented by the `value()` function of | ||
`AuthenticateActionProvider` class in `/src/providers/auth-action.provider.ts`. | ||
|
||
```ts | ||
class SequenceIncludingAuthentication implements SequenceHandler { | ||
constructor( | ||
@inject(SequenceActions.FIND_ROUTE) protected findRoute: FindRoute, | ||
@inject(SequenceActions.PARSE_PARAMS) | ||
protected parseParams: ParseParams, | ||
@inject(SequenceActions.INVOKE_METHOD) protected invoke: InvokeMethod, | ||
@inject(SequenceActions.SEND) protected send: Send, | ||
@inject(SequenceActions.REJECT) protected reject: Reject, | ||
@inject(AuthenticationBindings.AUTH_ACTION) | ||
protected authenticateRequest: AuthenticateFn, | ||
) {} | ||
|
||
async handle(context: RequestContext) { | ||
try { | ||
const {request, response} = context; | ||
const route = this.findRoute(request); | ||
|
||
// | ||
// The authentication action utilizes a strategy resolver to find | ||
// an authentication strategy by name, and then it calls | ||
// strategy.authenticate(request). | ||
// | ||
// The strategy resolver throws a non-http error if it cannot | ||
// resolve the strategy. When the strategy resolver obtains | ||
// a strategy, it calls strategy.authentication(request) which | ||
// is expected to return a user profile. If the user profile | ||
// is undefined, then it throws a non-http error. | ||
// | ||
// It is necessary to catch these errors | ||
// and rethrow them as http errors (in our REST application example) | ||
// | ||
// Errors thrown by the strategy implementations are http errors | ||
// (in our REST application example). We simply rethrow them. | ||
// | ||
try { | ||
//call authentication action | ||
await this.authenticateRequest(request); | ||
} catch (e) { | ||
// strategy not found error, or user profile undefined | ||
if ( | ||
e.code === AUTHENTICATION_STRATEGY_NOT_FOUND || | ||
e.code === USER_PROFILE_NOT_FOUND | ||
) { | ||
throw new HttpErrors.Unauthorized(e.message); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel this is adding too much friction for application developers. The current proposal is is good enough for now (incremental baby steps FTW!), but I'd like you to create a follow-up SPIKE issue to look into ways how to allow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bajtos :
@jannyHou mentioned there was a task to work on something like this. Do you mean Enable custom reject implementation to leverage the built-in "code to statusCode" mapping ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I am not sure what you mean. The code you mention is inside a Since users will be writing their own production-ready implementations for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not really. The goal of #1942 is to allow custom implementations of In this discussion, I am asking for a mechanism that will allow extension developers to contribute new mapping entries, so that
I find the custom sequence you are proposing as too complex. I would like the custom sequence to look like this: class SequenceIncludingAuthentication implements SequenceHandler {
// ...
async handle(context: RequestContext) {
try {
const {request, response} = context;
const route = this.findRoute(request);
await this.authenticateRequest(request);
const args = await this.parseParams(request, route);
const result = await this.invoke(route, args);
this.send(response, result);
} catch (error) {
this.reject(context, error);
return;
}
}
} Under the hood:
BTW the current implementation is problematic because it discards the original error stack trace pointing to the place where the error was triggered (e.g. in the authentication strategy) and replaces it with a stack trace pointing to the custom sequence. This makes troubleshooting rather difficult. Here is a better solution: try {
await this.authenticateRequest(request);
} catch (e) {
if (
e.code === AUTHENTICATION_STRATEGY_NOT_FOUND ||
e.code === USER_PROFILE_NOT_FOUND
) {
err.statusCode = 401;
}
throw err;
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point about the stack trace. I will look into it. :) |
||
} else { | ||
// strategy error | ||
throw e; | ||
} | ||
} | ||
|
||
// Authentication successful, proceed to invoke controller | ||
const args = await this.parseParams(request, route); | ||
const result = await this.invoke(route, args); | ||
this.send(response, result); | ||
} catch (error) { | ||
this.reject(context, error); | ||
return; | ||
} | ||
} | ||
} | ||
``` | ||
|
||
Then custom sequence must be bound to the application, and the authentication | ||
strategy must be added as an **extension** of the **extension point** using the | ||
`addExtension` function. | ||
|
||
```ts | ||
export class MyApplication extends BootMixin( | ||
ServiceMixin(RepositoryMixin(RestApplication)), | ||
) { | ||
constructor(options?: ApplicationConfig) { | ||
super(options); | ||
|
||
this.component(AuthenticationComponent); | ||
|
||
this.sequence(SequenceIncludingAuthentication); | ||
|
||
addExtension( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should add a sugar method such as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @raymondfeng is this something you will add? |
||
this, | ||
AuthenticationBindings.AUTHENTICATION_STRATEGY_EXTENSION_POINT_NAME, | ||
BasicAuthenticationStrategy, | ||
{ | ||
namespace: | ||
AuthenticationBindings.AUTHENTICATION_STRATEGY_EXTENSION_POINT_NAME, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Usually, the namespace is in plural form, such as |
||
}, | ||
); | ||
} | ||
} | ||
``` |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope in follow-up PRs we can move this awesome doc to the README.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@b-admike , yes. @jannyHou 's PR : #2822 overlaps with mine, and we have discussed that we will be moving all separate .md files in the authentication package into a comprehensive readme.md file. And this readme file will also be the source of the loopback.io docs that are generated by our doc build process.