Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions .sandstorm/sandstorm-pkgdef.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ const pkgdef :Spk.PackageDefinition = (

appTitle = (defaultText = "Hummingbird"),

appVersion = 400, # Increment this for every release.
appVersion = 410, # Increment this for every release.

appMarketingVersion = (defaultText = "0.4.0"),
appMarketingVersion = (defaultText = "0.4.1"),
# Human-readable representation of appVersion. Should match the way you
# identify versions of your app in documentation and marketing.

Expand Down Expand Up @@ -191,23 +191,34 @@ const pkgdef :Spk.PackageDefinition = (
# Name of the permission, used as an identifier for the permission in cases where string
# names are preferred. Used in sandstorm-http-bridge's X-Sandstorm-Permissions HTTP header.

title = (defaultText = "Website Tracker"),
title = (defaultText = "Website Tracking Script"),
# Display name of the permission, e.g. to display in a checklist of permissions
# that may be assigned when sharing.

description = (defaultText = "Publicly accessible tracking endpoint"),
# Prose describing what this role means, suitable for a tool tip or similar help text.
),
],
(
name = "view-data",

title = (defaultText = "View Data"),
description = (defaultText = "Ability to view hummingbird dashboard and aggregate data"),
)
],
roles = [
# Roles are logical collections of permissions. For instance, your app may have
# a "viewer" role and an "editor" role
(
title = (defaultText = "tracker"),
permissions = [true],
permissions = [true, false],
verbPhrase = (defaultText = "can submit events"),
description = (defaultText = "tracking script can submit new tracking events."),
),
(
title = (defaultText = "manager"),
permissions = [false, true],
verbPhrase = (defaultText = "can view the Hummingbird dashboard and associated data"),
),
],
),
apiPath = "/tracking_pixel/",
Expand Down
30 changes: 30 additions & 0 deletions lib/dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,29 @@ var demo = require('./demo');

var fileServer = new(staticServer.Server)("./public");

var hasViewDataPermission = function(headers) {
var permissions = headers['x-sandstorm-permissions'];
if (!permissions) {
return false;
}
var permissionList = permissions.split(',').map(function(p) {
return p.trim();
});
return permissionList.indexOf('view-data') !== -1;
};

var defaultHandler = function(request, response) {
console.log(" - " + request.url);

if (!hasViewDataPermission(request.headers)) {
console.log("Unauthorized request", { roles: request.headers['x-sandstorm-permissions'] });

response.writeHead(403, {'Content-Type': 'text/plain'});
response.end('Forbidden: invalid permissions');

return;
}

if(request.url === "/demo") {
demo.toggle();
}
Expand All @@ -23,6 +44,15 @@ if(config.origins) {

var io = new Server(server, ioOptions);

io.use(function(socket, next) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So when I tried to do this I put this in a different place, but you know your codebase better. Here's what I'd make sure of:

  1. Dashboard requires view permission (you cover that in this clause)
  2. Tracker requires tracker permission (I don't see that in this PR, and I noticed it was absent previously)
  3. Websockets, where the actual data flows, require view permission (I'm not sure if this clause covers it)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Yes, this code here protects the web sockets with view-data permissions, and the above in defaultHandler protects regular http. I should add a guard on the tracker for completeness.

var headers = socket.request.headers;
if (hasViewDataPermission(headers)) {
next();
} else {
next(new Error('Forbidden: invalid permissions'));
}
});

setInterval(function() {
var userCount = io.sockets.sockets.size;
console.log(userCount + " users connected.");
Expand Down
2 changes: 1 addition & 1 deletion public/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ <h2>Tracker Setup</h2>
var template = "<script>" + $("#tracking_template").html() + "</scr" + "ipt>\n";
template = template.replace('$API_PROTO', document.location.protocol);
window.addEventListener("message", messageListener);
window.parent.postMessage({renderTemplate: {rpcId: "0", template: template}}, "*");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I keep forgetting what forSharing actually does, but it's not what it sounds like. IIRC it makes the share show up in your "Share access" instead of your "Webkeys". For an API token I'd guess you want "Webkeys" but that's up to you.

Beyond that I don't think it affects permissions much. But I guess doublecheck my claims here.

@ocdtrekkie Is there anything else?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

forSharing : Boolean (optional) true if this token should represent the anonymous user. You can use this to detach the token from the user who created it

So I think we want it to be true here?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ah, yeah that was the other thing. But iirc you show up as anonymous when used as a share link but not as an API key, or maybe the other way around, I forget. In any case, goofy behavior. You can try it out.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yeah forSharing "detaches it from you", so IMHO it should be true for any key you intend to make public or give to someone else. I don't think it has a major security implication but it does move it from webkeys (which is sort of your API keys) to sharing (which is who you shared it with).

I think it should be true here.

window.parent.postMessage({renderTemplate: {rpcId: "0", roleAssignment: { roleId: 0 }, forSharing: true, template: template}}, "*");
});
</script>

Expand Down