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

refactor auth mechanism into plugin structure #251

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jfemia
Copy link

@jfemia jfemia commented Apr 1, 2018

Hello,

I am performing some integration of OpenKB into our internal authentication systems. As part of this work I needed to refactor some of the login procedures so that I could insert my own login code.

After seeing #72, #144, and #248, I decided to implement my integration in such a way that might be useful for you, and am submitting the change as a PR, with a brief description. This is partly selfish, as my design choice was made so that assuming a PR was accepted, implementation of each auth provider would have no impact on the rest of the codebase, therefore making future version upgrades simpler.

This change adds a plugins directory, with an auth subdirectory. (Who knows what else could be made pluggable.) Inside this directory should be self contained modules, including their own package.json/installed node_modules. I did this so the plugins themselves are not dependant on OpenKB's ownpackage.json, nor do they need to add their dependencies to it.

I then moved a number of database-login related routes into a new db plugin in this directory, and added a setting in the config file, auth_provider to use this by default. The resulting behaviour in a default installation is the same as it was before, however, additional plugins can now be inserted into this directory and the corresponding setting changed to use a different one.

Plugins themselves are not completely decoupled - they assume the role of express middleware. For auth provider plugins (currently the only type of plugin...), there are three functions that the module is expected to export:

  • route(router) - called just before the rest of the index.js routes are set up, allowing the plugin to insert its own routes to handle things like posting back to whatever login page it provides
  • login(req, res) - called by the /login route in index.js. I guess this could have been provided by the plugin but I wanted to retain some consistency - (also because there was a lot of stuff referring to /login that didn't need refactoring for what I needed to do)
  • logout(req, res) - called by the /logout route in index.js after it's cleared all the session data. it should redirect or do whatever else is appropriate

I used this to auto-login users based on some data that my authentication service provided, but I can see this also being useful to integrate LDAP, OAuth2, and even things like HTTP basic auth by the proxy webserver (e.g. Apache)

I welcome any thoughts / comments

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

Successfully merging this pull request may close these issues.

1 participant