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

Creating adapter for the plugged in passport strategies #2311

Closed
3 of 4 tasks
jannyHou opened this issue Jan 30, 2019 · 5 comments
Closed
3 of 4 tasks

Creating adapter for the plugged in passport strategies #2311

jannyHou opened this issue Jan 30, 2019 · 5 comments

Comments

@jannyHou
Copy link
Contributor

jannyHou commented Jan 30, 2019

Description / Steps to reproduce / Feature proposal

We should allow users to plugin custom passport strategies like passport-jwt. I tried replacing our own implementation of jwt strategy with the community one passport-jwt in PR loopbackio/loopback4-example-shopping@3cbb0a8 to verify it works with our current @loopback/authentication module, the missing part is make the strategy registration flexible.

Acceptance Criteria

  • Allow developers to register their own passport strategy
  • A reference for the implementation would be how we register a custom body parser in the application, see this test case we have the extension point now 🎉
  • Please note that this extension point only open for passport based strategies, which means when create the binding key/resolve the strategy name, make sure it's for passport particularly.
  • The story owner could explorer different approaches other than the way we register custom body parser.

See Reporting Issues for more tips on writing good issues

@bajtos
Copy link
Member

bajtos commented Mar 5, 2019

As I commented in #2312 (comment), I think our support for Passport strategies should be built on top of a LoopBack-specific extension point described by #2312.

That means we need to implement #2312 before working on this story.

@bajtos
Copy link
Member

bajtos commented Mar 5, 2019

I think this story depends on #2466: Add abstraction for authentication strategy.

@jannyHou jannyHou changed the title Extension point for providing custom passport strategies Creating adapter for the plugged in passport strategies Mar 25, 2019
@dhmlau dhmlau mentioned this issue Apr 5, 2019
22 tasks
@dhmlau dhmlau added the p1 label Apr 10, 2019
@emonddr
Copy link
Contributor

emonddr commented Apr 18, 2019

Some of this work seems to be occurring in the PR for #2467.

@jannyHou
Copy link
Contributor Author

Some of this work seems to be occurring in the PR for #2467.

+1.

If we agree on the rewrite of adapter in the draft PR #2688, then this story can be closed along with #2467 after the formal version of #2688 get merged

@jannyHou
Copy link
Contributor Author

The new adapter is published as @loopback/[email protected], an experimental package 🎉 https://www.npmjs.com/package/@loopback/authentication-passport

We've tried 2 approaches:

  • wrapping a passport strategy module like passport-http
  • wrapping the express middleware passport

It turns out the 1st approach is more consistent with non-passport based strategies and we have need more effort to fully support wrapping an express middleware.

Closing it now.

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

No branches or pull requests

5 participants