Review security of app

[statevoicesnational]

Review security of app

[sreynen]

Currently /admin/1/incoming gets /allmessages, which returns every incoming message sent to the Twilio account, regardless of which org the message was part of. Also currently an admin on any org can go to /admin/1/incoming, even if they're not an admin on org ID 1. While client-side org check would be nice, I think the important check for security is on the server (since clients can lie), so my plan is to change /allmessages to /allmessages/{orgID}, confirm the current user is admin on {orgID}, and only return messages from the local database that are attached to {orgID}.

[sreynen]

I went through all the resolvers and mutations to see how they limit access, and here's what I found, with things that I think need tighter security in bold.

Mutations:

• sendReply: only works in development mode, no security concern
• exportCampaign: requires user is org admin
• editOrganizationRoles: requires user is org owner or admin
• joinOrganization: currently anyone can become a texter in any org p1 -- see https://github.com/MoveOnOrg/Spoke/pull/123
• updateTextingHours: requires user is org owner
• updateTextingHoursEnforcement: requires user is org owner
• createInvite: currently anyone can create an invite p2
• createCampaign: requires user is org admin
• unarchiveCampaign: requires user is org admin
• archiveCampaign: requires user is org admin
• startCampaign: requires user is org admin
• editCampaign: requires user is org admin
• createCannedResponse: currently anyone can create a canned response for any user p1 @shakalee14
• createOrganization: requires valid inviteId
• editCampaignContactMessageStatus: currently anyone can edit any message status p1 -- needs assignment security check -- see https://github.com/MoveOnOrg/Spoke/pull/131
• createOptOut: currently anyone can create an opt-out for any contact p1 -- needs assignment security check @shakalee14
• sendMessage: currently anyone can send a message p2
• deleteQuestionResponses: currently anyone can delete responses p1 -- needs assignment security check @shakalee14
• updateQuestionResponses: currently anyone can update responses p1 -- needs assignment security check @shakalee14

Resolvers:

• campaign: requires user is org admin
• assignment: currently requires user is org texter, but not necessarily on loaded assignment p1 -- needs assignment security check -- @sandramchung PR
• organization: currently anyone can load any org p2
• invite: currently requires only logged in user p1 -- see https://github.com/MoveOnOrg/Spoke/pull/125
• inviteByHash: requires logged in user and valid hash
• currentUser: users can only load self
• contact: currently anyone can load any contact p1 -- @sandramchung PR
• organizations: currently only super-admin can load all orgs

[shakalee14]

twilio security one-liner
https://github.com/MoveOnOrg/Spoke/pull/112

[shakalee14]

createInvite: currently anyone can create an invite

• Only admins should be able to create a campaign. Previously you had to email an admin for a link. Should we revert to that behavior?

[sreynen]

Limiting invite creation to admins sounds right to me.

[sandramchung]

For the item contact: currently anyone can load any contact p1:

• texters should be able to load only the contacts assigned to them
• campaign owners can load any contact in the campaigns they own
• admins can load any contact in their organization

[shakalee14]

Nexmo message checking requires us to email [email protected] and request one of the following options:

• Outbound messages can be signed.
• Outbound messages must be signed.
• Inbound messages and DLRs sent to your webhook endpoint are signed.

And then implement a signature workflow. Since we are not working with nexmo and its not a one-liner, I'm going to remove this as a checkbox.

[shakalee14]

Circling back on this, I think createInvite is a p1 issue. We don't want anyone to be able to create an invite to be an admin.

[sreynen]

I marked createInvite as p2 and the invite resolver as p1 thinking we don't want anyone to be able to use an invite, but there's not much harm in creating an invite one can't actually use.

[shakalee14]

https://github.com/MoveOnOrg/Spoke/pull/134 removes create invite org handler from login link for now

[sandramchung]

Per flag in standup, we should set aside a chunk of time to test and document what we've done here.

[sandramchung]

Today I realized that 1) we should document what the permissions should be before we encode them, and 2) I don't understand what the roles (superadmin, owner, admin, texter) mean / what their experience and access should look like per org or campaign. Right now all roles are set per organization.

[shakalee14]

Current roles - owner, admin, texter