Support connecting to external HTTP APIs via the Powerbox #2870

kentonv · 2017-02-16T12:02:26Z

All requests for an ApiSession will offer the option to specify an HTTP URL manually. If the user specifies a URL containing a username/password or a webkey, the authorization mechanism is understood and used (without passing details to the app).

Additionally, requests with an HTTP canonicalUrl in their tag will offer a one-click option to use that URL. If the canonicalUrl refers to a well-known OAuth API service (currently recognizes Github and some Google API hosts), an OAuth handshake will occur.

dwrensha

This is pretty exciting!

dwrensha · 2017-02-16T14:15:14Z

shell/server/drivers/external-ui-view.js

+function registerHttpApiFrontendRef(registry) {
+  registry.register({
+    frontendRefField: "http",
+    typeId: "14445827391922490823",


Why write this out rather than ApiSession.typeId?

dwrensha · 2017-02-16T14:25:53Z

shell/server/drivers/external-ui-view.js

+          if ("none" in request.auth) {
+            request.auth = { basic: { username: parts[0], password: parts[1] } };
+          } else {
+            throw new Meteor.Error(400, "Can't supprot multiple authentication mechanisms at once");


s/supprot/support/

dwrensha · 2017-02-16T14:48:49Z

shell/server/drivers/external-ui-view.js

+          });
+        }
+      }
+


Is this http-arbitrary option intended to be in an else block? If not, then it probably deserves a comment about why it is always returned.

dwrensha · 2017-02-16T14:54:07Z

src/sandstorm/api-session-impl.capnp

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This is used specifically with hack-session.capnp.


Is this comment a copy-pasto?

dwrensha · 2017-02-16T15:00:37Z

shell/packages/sandstorm-db/db.js

+//         auth :union {
+//           none :Void;
+//           bearer :Text;
+//           basic :group { username :Text; password :Text; }


I wish we could encrypt these fields somehow. I don't see a good way to do that, though.

We could encrypt them using the ApiToken as a key, since we only store the hash of the API token in the database. Child ApiTokens would store an encrypted copy of the parent token (in addition to the parent token ID as they do today). But a complete solution would require that app storage be encrypted as well, since that's where the tokens are ultimately stored. That is, of course, a big project...

dwrensha · 2017-02-16T15:03:33Z

shell/server/drivers/external-ui-view.js

+          + "&grant_type=refresh_token",
+  });
+
+  console.log(response.data);


Stray debug print?

dwrensha · 2017-02-16T15:07:03Z

shell/packages/sandstorm-db/db.js

@@ -426,6 +426,14 @@ ApiTokens = new Mongo.Collection("apiTokens", collectionOptions);
 //       verifiedEmail: An VerifiedEmail capability that is implemented by the frontend.
 //                      An object containing `verifierId`, `tabId`, and `address`.
 //       identity: An Identity capability. The field is the identity ID.
+//       http: An ApiSession capability pointing to an external HTTP service. Object containing:


Where in the UI will the user be able to audit such frontendRefs?

Good question. I don't plan to implement that in this PR, but it's important.

kentonv · 2017-02-17T04:19:15Z

OK, I added a bunch of commits to implement encryption of these HTTP credentials!

Caveat: The encryption key is the token given to the grain. So, if the grain stores that token unencrypted, and if an attacker gets a copy of the grain's storage and a copy of Sandstorm's Mongo database, then the attacker can decrypt the credentials.

However, an attacker who gets only the Mongo database cannot decrypt any credentials within it. I suspect that getting the database and grain storage is in many cases significantly harder than getting just the database. For example, it's easy to imagine a Mongo injection bug in the Sandstorm shell which allows an attacker to get access to all ApiTokens, but it's harder to imagine a bug in which Sandstorm delivers raw grain storage data to attackers.

Some day, we will implement transparent encrypted grain storage in Blackrock, which will take things a step further.

dwrensha · 2017-02-17T13:12:03Z

shell/packages/sandstorm-db/db.js

+// strings, hence can be used directly as the key. No MAC is applied, because this scheme is not
+// intended to protect against attackers who have write access to the database -- such an attacker
+// could almost certainly do more damage by modifying the non-encrypted fields anyway. (Put another
+// way, if we wanted to MAC something, we'd needto MAC the entire ApiToken structure, not just


s/needto/need to/

ocdtrekkie · 2017-02-17T19:55:58Z

shell/packages/sandstorm-db/db.js

@@ -426,13 +426,33 @@ ApiTokens = new Mongo.Collection("apiTokens", collectionOptions);
 //       verifiedEmail: An VerifiedEmail capability that is implemented by the frontend.
 //                      An object containing `verifierId`, `tabId`, and `address`.
 //       identity: An Identity capability. The field is the identity ID.
+//       http: An ApiSession capability pointing to an external HTTP service. Object containing:
+//           url: Base URL of the external service.
+//           auth: Authentitation mechanism. Object containing one of:


"Authentication"

ocdtrekkie · 2017-02-17T19:59:12Z

shell/server/drivers/external-ui-view.js

+        if ("none" in request.auth) {
+          request.auth = { bearer: parsedUrl.hash.slice(1) };
+        } else {
+          throw new Meteor.Error(400, "Can't supprot multiple authentication mechanisms at once");


So far, this includes: * Offering the canonicalUrl as a one-click option, but with no authentication. * Offering an option to enter a URL manually. * If the URL contains username/password or is a webkey, the session will be authenticated as specified.

…on in admin-server.js. This dependency is now explicit, so this hack isn't needed anymore. (Why it was ever used, I'm not sure.)

…redentials in ApiToken.

…ic.password.

Main problem is that various methods that previously accepted either a String or a Buffer as a SturdyRef representation are now more strict. I wish we had a type system.

kentonv · 2017-03-01T01:33:29Z

Garply, retest this please.

kentonv requested a review from dwrensha February 16, 2017 12:02

dwrensha reviewed Feb 16, 2017

View reviewed changes

dwrensha reviewed Feb 17, 2017

View reviewed changes

ocdtrekkie reviewed Feb 17, 2017

View reviewed changes

kentonv added 22 commits February 27, 2017 15:41

Remove unused grainId parameter from ExternalWebSession.

a2a7ec9

Catch exceptions properly in ExternalWebSession.

23e7fb5

cleanup: Move builtin powerbox card templates to their own files.

b406da6

Add some TODOs to ApiSession.PowerboxTag.

3a6617f

Add api-session-impl.capnp to define PersistentApiSession.

ad7c1e7

Define external HTTP frontendRef schema.

7c1993d

Implement OAuth-authenticated HTTP powerbox options.

2c952c0

Fix broken path handling in ExternalWebSession.

bc7525f

Add some TODOs to ExternalWebSession.

6992b5b

cleanup: Fix weird use of undeclared dependency on ServiceConfigurati…

9df7488

…on in admin-server.js. This dependency is now explicit, so this hack isn't needed anymore. (Why it was ever used, I'm not sure.)

jscs --fix

c80c7c3

Address @dwrensha's review comments.

2a7458d

Add comment about revoking OAuth tokens when deleting ApiToken records.

aa25051

Document how we encrypt credentials in ApiTokens.

3b66bc8

Add helpers to read and write ApiTokens with encryption applied.

0537541

Use new fetchApiToken/insertApiToken helpers to implement encrypted c…

69f9f8c

…redentials in ApiToken.

Pad short secrets to 32 bytes, per the documentation of http.auth.bas…

33cdbb9

…ic.password.

jscs --fix

9b4df64

Fix bugs caught by tests.

68d83da

Main problem is that various methods that previously accepted either a String or a Buffer as a SturdyRef representation are now more strict. I wish we had a type system.

typo

7658292

Typos spotted by @ocdtrekkie

60d3b87

kentonv force-pushed the powerbox-external-http-api branch from 90cc456 to 60d3b87 Compare February 27, 2017 23:41

kentonv merged commit d534fcb into master Mar 1, 2017

kentonv deleted the powerbox-external-http-api branch March 1, 2017 22:18

dwrensha mentioned this pull request Apr 28, 2017

cannot tweet from grains #2925

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support connecting to external HTTP APIs via the Powerbox #2870

Support connecting to external HTTP APIs via the Powerbox #2870

kentonv commented Feb 16, 2017

dwrensha left a comment

dwrensha Feb 16, 2017

dwrensha Feb 16, 2017

dwrensha Feb 16, 2017

dwrensha Feb 16, 2017

dwrensha Feb 16, 2017

kentonv Feb 16, 2017

dwrensha Feb 16, 2017

dwrensha Feb 16, 2017

kentonv Feb 16, 2017

kentonv commented Feb 17, 2017

dwrensha Feb 17, 2017

ocdtrekkie Feb 17, 2017

ocdtrekkie Feb 17, 2017

kentonv commented Mar 1, 2017

Support connecting to external HTTP APIs via the Powerbox #2870

Support connecting to external HTTP APIs via the Powerbox #2870

Conversation

kentonv commented Feb 16, 2017

dwrensha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kentonv commented Feb 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kentonv commented Mar 1, 2017