Use lb access token - Githubissues

strongloop / strong-remoting

Communicate between objects in servers, mobile apps, and other servers.

www.strongloop.com

Other

105 stars 93 forks source link

Use lb access token #421

Closed Traksewt closed 7 years ago

Traksewt commented 7 years ago

Description

Allow the rest connector to call other loopback services by passing on the user authorization from the options where available. This is the fallback if no other authorization is specified.

This allows loopback servers to communicate securely with other loopback servers by passing on the auth token. Useful for using loopback as microservices.

Related issues

connect to strongloop/loopback-connector-rest#89
connect to strongloop/loopback-connector-remote#3

Checklist

[x] New tests added or existing tests modified to cover all changes
[x] Code conforms with the style guide

slnode commented 7 years ago

Can one of the admins verify this patch?

slnode commented 7 years ago

Can one of the admins verify this patch?

slnode commented 7 years ago

Can one of the admins verify this patch?

bajtos commented 7 years ago

@slnode ok to test

Traksewt commented 7 years ago

Thanks @bajtos, I was having trouble testing the REST in that unit test. As the fix was the setting auth for one loopback server hitting another loopback server, should I be creating an extra express/loopback (so 2 in total) in this unit test? I couldn't tell of how I could test the rest adapter setting the access token using the single server.

I also added a config to enable passing through the LB Access Token, otherwise, people may inadvertently send their loopback token to other 3rd party REST endpoints.

bajtos commented 7 years ago

I also added a config to enable passing through the LB Access Token, otherwise, people may inadvertently send their loopback token to other 3rd party REST endpoints.

👍

bajtos commented 7 years ago

As the fix was the setting auth for one loopback server hitting another loopback server, should I be creating an extra express/loopback (so 2 in total) in this unit test? I couldn't tell of how I could test the rest adapter setting the access token using the single server.

I think a single server is enough. You can create the access token via javascript API by calling something like server.models.User.create and server.models.User.login, and then pass this access token in the options argument of the client method (doing remote invocation).

Traksewt commented 7 years ago

@bajtos please check

bajtos commented 7 years ago

@Traksewt thank you for the pull request, I'll take a look next week. I see that our CI builds are failing because one of our dependencies dropped support for Node.js 0.x, I think I'll need to fix our CI config before landing this patch, to get a green build first.

bajtos commented 7 years ago

Here is a pull request that should fix our CI in the 2.x branch: https://github.com/strongloop/strong-remoting/pull/428

bajtos commented 7 years ago

@Traksewt sorry for realizing this only now: the changes you are proposing are effectively adding a new feature. Per our LTS rules, we are not adding any new features to the version lines that are in LTS or maintenance mode, and the version line 2.x is in LTS right now.

Could you please rework your patch on top of the master (version 3.x) branch?

GitHub made it recently possible to change the target branch of a pull request: click in "edit" button on the right end of pull request title, and pick master from the branch drop-down. I haven't done this myself before, I think you may need to run git rebase -i master and git push -f afterwards to fix the commit history.

Traksewt commented 7 years ago

There were too many conflicts so made a new pull #430