Added don schedules!

[scholvat]

Adding a feature to see don information such as location, phone number, if they are on, and if they are on late supper. There will be a related PR in skybunk-mobile (with screenshots and such), but I haven't implemented the feature on the website version yet. Full list of feature changes are below:

-added donInfo field, which includes information of if the don is on, on late supper, location, and an automatic clock-out feature so dons don't have to remember to turn themselves off -Added phone number to info fields. This was a necessity for dons so people can text/call them, but just added the field in for everyone for easy communication outside skybunk -Added the agenda module to schedule events for the clock-out feature. Seems like a pretty cool library, and could be useful for future features. -Added copy_prod_data.js script, which copies all the data from production and puts it into a development environment. Really useful for testing new features that rely on versions of front and back end. It's a bit buggy but it works well enough. -Froze the Mongoose version, since there seemed to be an issue when running on 5.3.1. Not sure what the root cause is, but it works reliably on 5.1.1. -Replaced isAdmin with role, which is a bitmap to handle permissions for various roles. Right now I have bit 0 as don, bit 1 as admin, and the rest and unused.

[AngelaKrone]

This is amazing!! I'm sorry if this is a silly question (I'm still learning how server stuff works), but are these values protected? Or are they public on the API? Just because it's pretty personal it should probably be at least somewhat encrypted :)

[scholvat]

Good question Angela, right now nothing is encrypted and in the public eye (go to http://api.grebelife.com/users or http://api.grebelife.com/posts for example). I think some kind way to make it more private is a good idea, especially if we are adding phone numbers. That's something I wanted to look into, but I have very little experience with it.

[aopal]

There's an easier way to secure sensitive info, there's a ticket for it here: https://github.com/CGUC/skybunk-server/issues/63. One standard security measure is the use of 'tokens' sent along with every call made to a server, and that token can be used to validate that the entity requesting data from the server is an authenticated user. We already use tokens during registration, it would be relatively straightforward to require tokens be sent with every request, so that none of this info is public.

@scholvat if you want to tackle securing the endpoints, take a look at that ticket and the registration flow, and lmk if you have any questions

[scholvat]

@aopal I'll do some research and then get in contact with you when I have questions. I don't think this is necessarily a blocking issue for this PR, but lmk if you think otherwise.

[picklechips]

In regards to the privacy issues discussed above, to make an endpoint protected all you have to do is add the verifyToken parameter as used here: https://github.com/CGUC/skybunk-server/blob/master/controllers/userController.js#L56

Then when sending a request at that endpoint you just have to include the users auth token. So in order to make our public endpoints only accessible to registered users, we'd first have to update the calls on the client-side to include the users token (also trivial) as to not break everything, then we can simply add verifyToken into the parameters of the endpoint.

What this does is adds a middleware onto that endpoint, so before entering it we first execute verifyToken as defined here: https://github.com/CGUC/skybunk-server/blob/master/helpers/authorization.js which checks for the users token and sends a 403 otherwise.

[scholvat]

@picklechips https://github.com/CGUC/skybunk-mobile/pull/143 has those security changes! (server side is https://github.com/CGUC/skybunk-server/pull/71)

[picklechips]

Awesome, I'll take a look at all your PRs this week and hopefully we can get this stuff merged in finally haha

[picklechips]

I think I'm Ok with this.

I think there's a cyclic dependency right now on this PR and the Client side one, though -- since this one removes isAdmin so the client-side change needs to be shipped first, however the client-side PR needs these endpoints. It's probably fine for now since probably nobody will notice if things don't work for 5 minutes between client/server deploys (especially over the break) but we should probably be more careful about this in the future.

[scholvat]

You can update server first, then client. Since the isAdmin field is removed, the value of user.isAdmin is undefined, which is evaluated to false in conditional statements. Therefore it will not affect any older versions of the client app.

On Wed, Dec 26, 2018 at 11:49 PM Ryan Martin notifications@github.com wrote:

@picklechips commented on this pull request.

In models/User.js https://github.com/CGUC/skybunk-server/pull/70#discussion_r244082268:

@@ -26,10 +26,9 @@ const UserSchema = new Schema({ unique: true, dropDups: true },

• isAdmin: {

@AngelaKrone https://github.com/AngelaKrone not real sure how expo works but can we rely on everyone's app being updated?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/CGUC/skybunk-server/pull/70#discussion_r244082268, or mute the thread https://github.com/notifications/unsubscribe-auth/AJqudvAwZ6Pmpor6DCQRkFwpjnunoK7mks5u9FE8gaJpZM4YwGXc .

[neilbrub]

LGTM, I'd say let's get this in and sort out any user model issues if/as they arise - although from the looks of it we should be fine, maybe just wanna run a cleanup script to remove redundant fields on DB documents once everything is in place. @picklechips any outstanding requests?

[picklechips]

Sweet! Updates are live.

In the future though it would be nice to try to squash all your commits into one (or maybe a couple) before merging, just to avoid a clutter of messy commits.

I'm definitely guilty of this, when we were first starting out we sort of spammed master with little commits - but now that things are up and running it would be nice to avoid that if possible.

git fetch origin master git rebase origin master -i

[scholvat]

Thought it squashed by default, my bad... I'll make sure to do that next time