Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change models and/or use API #61

Closed
rushsteve1 opened this issue Jan 30, 2021 · 16 comments
Closed

Change models and/or use API #61

rushsteve1 opened this issue Jan 30, 2021 · 16 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@rushsteve1
Copy link
Contributor

In order to make use of the new central RCOS database and API the models have to be updated to match that database schema.

Additionally we may want to consider entirely removing Diesel and replacing it with calls to the REST API itself. It would be a good example of dogfooding and would likely require a similar amount of work since all the models and queries are going to be rewritten anyway.

CC: @Apexal

@rushsteve1 rushsteve1 added the enhancement New feature or request label Jan 30, 2021
@Apexal
Copy link
Member

Apexal commented Jan 30, 2021

I do recommend not having any database stuff at all in Telescope or any RCOS infrastructure going forward. If you depend solely on the REST API, you you have immediate access to all data, filtering, searching, populating relationships, etc. without having to spend any time modeling that on your end. Any infrastructure becomes way easier to develop and (more importantly) maintain.

@rushsteve1
Copy link
Contributor Author

Oh, I guess #58 and #60 are tangentially related to this.

@vcfxb
Copy link
Member

vcfxb commented Jan 31, 2021

I like the motivation behind this but I'm concerned that the central database/API is not mature enough yet. The lack of independent authentication comes to mind. There are also a few other inconsistencies that we should consider. One example is that Telescope's database uses UUIDs as identifiers and the central database uses strings. We should come to a consensus on the database schema and authentication before rewriting any part of Telescope.

@Apexal
Copy link
Member

Apexal commented Feb 1, 2021

@alfriadox Interacting with Postgrest just means sending the API key with requests, which can be stored as a secret env var. Telescope can use that API key for everything. In the routes of the application it can decide whether or not to call a specific route, e.g. don't request another user's attendance unless the logged in user is an admin. Postgrest doesn't have to know on who's behalf it is receiving requests, it's either full access (authorized) or limited public access (unauthorized). For added security, we can use row level permissions and actually send to Postgrest the user who is logged in, and we can check access there. This means that authentication of users for our infrastructure can be totally separate. We don't need to wait until we have a single solution for it, and we can change it in the future. Each of our infrastructure services simply need a way to get a user's verified RCS ID.

@vcfxb
Copy link
Member

vcfxb commented Feb 1, 2021

@Apexal It's more about user authentication than database authentication. The problem is where to store the Argon2 hashes of user's passwords. Correct me if I'm wrong but I see nowhere in the rcos-data user accounts table to do that. I don't want to have a second database just for that but that does have to be stored somewhere persistently.

@rushsteve1
Copy link
Contributor Author

For added security, we can use row level permissions and actually send to Postgrest the user who is logged in, and we can check access there.

This is fairly straightforward with Postgres Policies and the variables exposed by PostgREST to queries. However we may not even need it since most of the interaction can be done server-side.

It's more about user authentication than database authentication. The problem is where to store the Argon2 hashes of user's passwords. Correct me if I'm wrong but I see nowhere in the rcos-data user accounts table to do that. I don't want to have a second database just for that but that does have to be stored somewhere persistently.

This circles back to my original point that I mentioned in #50, do we want to handle authentication ourselves, or do we want to offload to Oauth and not have to handle it ourselves.

I am a big supporter of GitHub Oauth personally. I think it will greatly simplify Telescope, and unify login systems across various projects (ie the Wiki, Telescope, GH itself, and whatever else) without needing to implement our own SSO system.

Tying into GitHub is admittedly not ideal, but I think it's pragmatic. Most of RCOS project development is on GitHub, and it opens up to interesting features like the automatic commit reports that Observatory-new has.

@Apexal
Copy link
Member

Apexal commented Feb 1, 2021

I agree that we shouldn't handle authentication. We can also use Microsoft OAuth as all RPI students have an account with their RCS ID verified.

@vcfxb
Copy link
Member

vcfxb commented Feb 1, 2021

Between the two I prefer GitHub OAuth, but I don't love being tied to any large corporation or institution. If we have consensus otherwise though, I think I am good to start moving towards GitHub Auth and removing the migrations and other database stuff from telescope. From there, should we focus on dogfooding off of the Postgrest API or should we interface directly with the central database?

@vcfxb vcfxb added this to the MVP / v1.0.0 milestone Feb 1, 2021
@Apexal
Copy link
Member

Apexal commented Feb 1, 2021

I recommend solely using Postgrest. It will make sure it meets our needs and we can work on it if we run into issues so that any future infrastructure benefits from it.

@vcfxb
Copy link
Member

vcfxb commented Feb 1, 2021

Okay Postgrest API it is.

@rushsteve1
Copy link
Contributor Author

We can also use Microsoft OAuth as all RPI students have an account with their RCS ID verified

Between the two I prefer GitHub OAuth

The nice thing about OAuth is that we can add/remove providers later without much effort.

@Apexal
Copy link
Member

Apexal commented Feb 1, 2021

We do need a way to connect connect someone to their RCS ID.

@rushsteve1
Copy link
Contributor Author

We can ask the user to login with CAS just once and tie their account with the existing user_accounts system.
The problem with that as well is that the old users from the Mongo database doesn't have RCS ids. Some users have the RPI emails which we can get RCS ids from, but some users have other emails.

@Apexal
Copy link
Member

Apexal commented Feb 1, 2021

That's frustrating :/ Perhaps we should have a way for people to claim and merge their old accounts?

@rushsteve1
Copy link
Contributor Author

Well we do have GitHub accounts for most of them. So if we have GitHub OAuth then we should be able to figure out who's who.

@vcfxb
Copy link
Member

vcfxb commented Mar 9, 2021

Telescope is now moved entirely to the RCOS GraphQL API.

@vcfxb vcfxb closed this as completed Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants