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

first go at an interface for auth plugins #120

Merged
merged 1 commit into from
Oct 13, 2015
Merged

Conversation

andyroyle
Copy link
Collaborator

interface for an auth plugin:

{
  validate: function(authConfig){
    // validate the config object
    // also optionally do any setup the scheme needs
    return {
      isValid: false
      message: 'blarg' // when isValid is false this should contain the error message
    };
  },
  middleware: function(authConfig){
    // should return an express middleware function
    return function(req, res, next){ next(); };
}

The plugins are indexed by type, so for an authentication type: 'foo' looks for a module called oc-auth-foo installed alongside oc

e.g.

myregistry:
  |- oc
  |- oc-auth-foo

And the authConfig will access the plugin using oc-auth-{type}.

authConfig:

{
    "type": "foo",
    "bar": "baz"
}

Basic Auth is still included by default.

@andyroyle
Copy link
Collaborator Author

relates to #106

}
else {
var cwd = process.cwd();
module.paths.push(cwd, path.join(cwd, 'node_modules'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for? Can't find any documentation about module.paths

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's equivalent to require.paths.push(). It basically adds the parent node_modules folder to the list of paths searched by require. So that you can require a module in the upper node_modules folder.

I nicked it from here: https://github.com/mochajs/mocha/blob/master/lib/mocha.js#L28

It's probably not the best way to do it. But I wasn't sure how to proceed in a world where npm3 might or might not flatten the modules folder.

Modifying the paths at runtime is bad, but I figured it's better than using ../../.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Ok, got it.

I think I'm more for a hapi-style initialisation in the way we did the plugin, so that you do the require on the initialisation and you solve this problem for free, but I'm ok with this too and we still can revisit later in case we have troubles with npm3.

matteofigus added a commit that referenced this pull request Oct 13, 2015
first go at an interface for auth plugins
@matteofigus matteofigus merged commit 7d1e441 into master Oct 13, 2015
@matteofigus matteofigus deleted the auth-plugins branch October 13, 2015 08:52
@andyroyle
Copy link
Collaborator Author

Yeah, I much prefer the hapi-style of plugin initialisation. I was attempting to keep it configuration-based for now. Especially considering that this work covers only auth and we don't yet have an idea for a "general" plugin framework.

Realistically I think converting OC to hapi would give us a much better solution (rather than having to crowbar something homebrew into express). IMHO Hapi's extension points and request life-cycle events give us a quick win.

@matteofigus
Copy link
Member

Yes!
I would be 100% hapi about that!
LOL

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.

None yet

2 participants