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

HttpInvocation.invoke() should support authentication #105

Closed
coodoo opened this issue Sep 18, 2014 · 26 comments · Fixed by #181
Closed

HttpInvocation.invoke() should support authentication #105

coodoo opened this issue Sep 18, 2014 · 26 comments · Fixed by #181
Assignees

Comments

@coodoo
Copy link

coodoo commented Sep 18, 2014

When replicating local model with remote models, it will trigger REST end points on remote server, sometimes it would be necessary to pass TOKEN in the request header for authorization, it's not possible with current implementation.

Ideally, TOKEN should be passed down all the way from persisted-model when invoking, or better yet, token can be stored somewhere (probably app.settings{} or app.configs{}) for easier access.

Here's the pseudo code of how it works if TOKEN were passed down:

In http-invocation.js:

HttpInvocation.prototype.invoke = function (callback) {
  var req = this.createRequest();

  // append TOKEN to req.headers, in this case, I storing it in window{} earlier, which of course is a hack
  req.headers = {'Authorization': window.zoken};

  request(req, function(err, res, body) {
    if(err instanceof SyntaxError) {
      if(res.status === 204) err = null;
    }
    if(err) return callback(err);
    this.transformResponse(res, body, callback);
  }.bind(this));
}

PS. just reviewed the whole replication/diff/merge implementation, really great work, two thumbs up! Can't wait to use it in my next project.

@bajtos
Copy link
Member

bajtos commented Sep 18, 2014

In other words, we need to implement authentication support in the remoting connector.

@bajtos bajtos changed the title HttpInvocation.invoke() should take in TOKEN as one of it's request parameters HttpInvocation.invoke() should support authentication Sep 18, 2014
@coodoo
Copy link
Author

coodoo commented Sep 18, 2014

Exactly that! 💯

@JonnyBGod
Copy link

+1

@demisx
Copy link

demisx commented Nov 1, 2014

👍 Will this be implemented soon? Currently evaluating different frameworks and this is a must for us.

@jsvnm
Copy link

jsvnm commented Nov 8, 2014

wonderful, instead of trying to figure out how to do that, should've just done the req.headers = {'Authorization': window.zoken}; hack instead.

@activestylus
Copy link

+1 - this one's a dealbreaker for me too

@brianfeister
Copy link

👍 Same question as the others, need this in order to commit to Loopback as a framework for a project

@bajtos
Copy link
Member

bajtos commented Nov 26, 2014

There are two parts to this:

  1. Extend loopback-remoting-connector API and add a method for specifying AccessToken (or more generally any headers) to send in all requests. This is fairly easy to implement and it will enable you to implement auth support in your loopback app.
  2. Extend either LoopBack or loopback-remoting-connector to intercept login & logout requests, extract the AccessToken, store it in browser's local-storage/session-storage (when running in the browser) and restore it on start. This requires little bit more effort in order to get all use cases and interactions correct, but it's still reasonably easy.

I am afraid this is not a high priority for us at the moment. However, since LoopBack is an open-source project, would you mind to implement this change yourself and submit a pull request?

@brianfeister
Copy link

Thanks for being open to a pull request. No problem about low priority, I will consider it "part of the task" to get something like this in place if we decide to move this direction. We're definitely leaning toward Strongloop's stack (in general), I'll let you know. Thanks for the response!

@bajtos
Copy link
Member

bajtos commented Nov 26, 2014

@brianfeister That's great to hear. I am happy to help you with the patch and guide you along the way to get the pull request into the state when it can be accepted. Feel free to open a pull request as soon as you have something working, even if it's not complete.

@activestylus
Copy link

Thanks for the prompt response and concise direction for pull requests. Responsiveness of core devs is definitely a contributing factor for people choosing a framework

@BerkeleyTrue
Copy link
Contributor

Is there anyone working on this? If so I would like to help move it along, if not, I will be forming a PR. @bajtos If there is no one working on this I will need some guidance on where to get started.

@brianfeister
Copy link

I'm not currently working on this because I'm not using StrongLoop for this yet. Have at it @BerkeleyTrue

@BerkeleyTrue
Copy link
Contributor

Will do!

@bajtos
Copy link
Member

bajtos commented Dec 10, 2014

@BerkeleyTrue cool! let me know when you need help.

@BerkeleyTrue
Copy link
Contributor

@bajtos Looks this I will start working on this tomorrow.

When you say loopback-remote-connector do you mean the loopback-connector-remote package or the connectors specific to strong-remoting?

@bajtos
Copy link
Member

bajtos commented Dec 15, 2014

When you say loopback-remote-connector do you mean the loopback-connector-remote package or the connectors specific to strong-remoting?

The former, see

https://github.com/strongloop/loopback-connector-remote

@BerkeleyTrue
Copy link
Contributor

@bajtos I have adding headers working through loopback-remote-connector and auth working similar to how angular sdk does auth. My problem at the moment is I cannot get the custom user methods to work with strong-remoting, like login/logout. It seems like User only picks up the basic PersistedModel methods but not the custom ones.

Can I get some guidance as to how to create the remote methods for those?

@BerkeleyTrue
Copy link
Contributor

Seems the issue is the fact that currently the built in user model uses loopback.remoteMethod instead of User.remoteMethod.

see strongloop/loopback#943

raymondfeng pushed a commit that referenced this issue Jan 15, 2015
2.11.1

 * Optimize the code to remove closure and bind (Raymond Feng)

 * Fix parsing JSON arrays with arrayItemDelimiters on. (Samuel Reed)

 * Add remote hooks to RestAdapter.invoke method Hooks on invoke are needed to use methods like User.login remotely from loopback-connector-remote. see #150 and #105 and strongloop/loopback#943 (Berkeley Martinez)

 * Ensure Remote-Connector Converts Types in Array Fix bug strongloop/loopback#886 (Berkeley Martinez)
@bajtos
Copy link
Member

bajtos commented Jan 27, 2015

@ritch
Copy link
Member

ritch commented Feb 19, 2015

remotes.auth released as 2.13.0

@bajtos bajtos reopened this Apr 2, 2015
@bajtos bajtos closed this as completed Apr 2, 2015
@bajtos
Copy link
Member

bajtos commented Apr 2, 2015

Here is a code snippet showing how to set the access token in a loopback application, assuming that the data-source backed by the remote connector is called "remote":

app.dataSources.remote.connector.remotes.auth = {
  bearer: new Buffer(token).toString('base64'),
  sendImmediately: true
};

@ritch I find this rather difficult to use, it took me quite a lot of debugging to find out where exactly the token should be set. I think we should add some sugar API to make this easier.

@crandmck Is there any documentation explaining how to configure the remoting connector? I'd like to include this information there.

@bajtos
Copy link
Member

bajtos commented Apr 2, 2015

Let's follow up here: strongloop/loopback-connector-remote#3

@crandmck
Copy link
Contributor

@bajtos Here is the doc for the remote connector: http://docs.strongloop.com/display/LB/Remote+connector Feel free to add something there until strongloop/loopback-connector-remote#3 is resolved.

@bajtos
Copy link
Member

bajtos commented Apr 20, 2015

@crandmck done, I have added a new section named "Configuring authentication". Feel free to improve the content as you wish.

@crandmck
Copy link
Contributor

Thanks. LGTM.

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 a pull request may close this issue.