ammmir / node-oauth2-provider

A simple customizable OAuth 2.0 provider (server) for node.js.
MIT License
628 stars 161 forks source link

Fix abuse of EventEmitter for callbacks #3

Open ammmir opened 13 years ago

ammmir commented 13 years ago

Maybe subclassing or delegation would be better...

timoxley commented 12 years ago

+1

ammmir commented 12 years ago

i'm not sure how serious i was about this... please provide some inspiration for a better api.

timoxley commented 12 years ago

Events make sense if are likely going to have multiple listeners, but in this case, you're only going to provide a single listener to those events (in-fact, because you're passing a callback, multiple listeners doesn't even make sense). Just provide overridable functions.

// roughly
var provider = OAuth2Provider.create({
  access_token: function() {

  },
  lookup_grant: function(client_id, client_secret, code, next) {

  },
  create_access_token: function(user_id, client_id, next) {

  },
  save_access_token: function(user_id, client_id, atok) {

  },
  remove_grant: function() {
      // etc
  }
})
oveddan commented 12 years ago

You could also do it via dependency injection - I always prefer composition over inheritance:

The provider would act as an api, and contain a token repository. This way you have two cohesive classes, one is public facing and the other is purely responsible for saving data. You could still emit the event for backwards compatibility, but I would have that done through a contained EventEmitter rather than base one.

oveddan commented 12 years ago

Working on this https://github.com/oveddan/node-oauth2-provider