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

Redesign of GQL api (tracking issue) #294

Closed
bodymindarts opened this issue Jul 5, 2021 · 9 comments
Closed

Redesign of GQL api (tracking issue) #294

bodymindarts opened this issue Jul 5, 2021 · 9 comments
Assignees

Comments

@bodymindarts
Copy link
Member

bodymindarts commented Jul 5, 2021

Opening an issue to have a place for discussion regarding the redesign of the public API.

Initial observations

When redesigning the API we should try to align with current best practices. This would currently include:

  • Having a use-case driven design. Queries and Mutations should do be tailored to do 1 thing well. Reuse across use-cases is not a priority. We must avoid coupling the schema to the domain model of the backend.
  • Leverage the GQL type system. Use enums for constants and prefer explicit types (even when complex) over unstructured data.
  • Adhere to the mutation pattern SomeMutationInput input type and SomeMutationPayload return (for the happy path).
  • Return application errors via a polymorphic result type. IE use a union to represent different states that an api call may return. Keep it extendable for clients by having all error-representing members of the union type implement a generic interface. That way clients can remain compatible even when the backend is extended to return more error-states.
  • Only rely on the errors field to return system level errors (eg connection timeout, auth errors... etc).
  • Adhere to the common pattern (started by the relay client) of returning collections via connections
  • Keep consistent naming

Auhentication

For the time being authentication is provided via a code sent via SMS, user data is managed and stored in the MongoDB database.
We should consider extracting the user-authentication to an OSS 3rd party component such as keycloak or ory. Maintaining our own logic for user management / authentication is a lot of work but doesn't add much business value as it is generic functionality.
The JS library passport might also be an option.

Good option for OpenID integration https://github.com/dexidp/dex.
Research / discussion required

Authorization

We are currently using graphql-shield and it seems to be working fine. An alternative might be casbin
Currently GraphQL-shield is coupled to the node-js ecosystem. If we wanted to enforce authorisation in independent services that get called via grpc or other methods we could use casbin to enforce authorisation at every service boundary.

Migration

To migrate to the new API I would suggest hosting it under a new DNS. The old API will probably be deprecated in its entirety so I wouldn't try to merge the new and old way and then partially deprecate it. Better to expose the new API on a different DNS subdomain and then turn the old one off when there are no more clients calling it.

Design workflow

Initially we can collaborate on the form the schema should take by discussing snippets of GQL schemas. To implement however I would recommend not generating code from the schema but go the other way around. Write the code and have the schema be generated from that.

Use Cases

Please help me curate an exhaustive list of use cases that are currently being served via posting links to the place in the mobile UI where the API call is being used with a hint to what the underlying use-case is. I will add them to this initial post as we find them.

@nicolasburtey
Copy link
Member

Some inputs:

Authentication: passport could be a library to consider. it is a common library used in the nodejs ecosystem. I didn't end up using it initially because there was no support for SMS auth out of the box.

Authorization: Graphql shield work well so far IMO. I don't have a lot of context on what a library like casbin would provide but I think this is fairly independant of the graphql api redesign in the sense that it's an internal system and we can refactor this one later without having breaking change to the API user. so we can move the discussion about this one later?

Migration: agree. go from something like mainnet.graphql.galoy.io to mainnet.graphql.bitcoinbeach.galoy.io

@bodymindarts
Copy link
Member Author

bodymindarts commented Jul 5, 2021

Authorization: Graphql shield work well so far IMO. I don't have a lot of context on what a library like casbin would provide but I think this is fairly independant of the graphql api redesign in the sense that it's an internal system and we can refactor this one later without having breaking change to the API user. so we can move the discussion about this one later?

Wasn't aware of GraphQL-shield. If its working well then we probably don't need to change it. I'll take a deeper look anyway to better understand the differences.
And yes its independent... but we wouldn't want to go live without it right? We do after all have an admin API and a user API so there is need for it, right?

@nicolasburtey
Copy link
Member

And yes its independent... but we wouldn't want to go live without it right? We do after all have an admin API and a user API so there is need for it, right?

agree. my point is we already have something working and I don't see a clear need to change this right now. if nothing it will make the new graphql API PR a bit bigger.

@nicolasburtey
Copy link
Member

nicolasburtey commented Jul 8, 2021

I had a look at Ory and Keycloak. On the surface, I think Ory would be my first choice.

  • its seems they rely on go which I think would be easier to have people to work with if we ever need to do some changes (unlikely). Keycloak relies on Java.
  • the codebase is probably a lot newer (their website mention they're working on it for 2y only IIRC) but given the adoption it seems it will outpace Keycloak rapidly.
  • I like the fact there is a company behind Ory (which is also our business model), because we could get support if we need to. It seems Keycloak to be more community driven (but sponsored by RedHat so there are active full time dev also I presume).

@bodymindarts
Copy link
Member Author

bodymindarts commented Jul 9, 2021

I had a look at Ory and Keycloak.

Actually we can defer the decision. The first step is to introduce a connector layer via https://github.com/dexidp/dex. Then we have an out-of-the-box solution for plugging into arbitrary OIDC providers and other auth backends. This should enable enterprises to integrate galoy into their in-house auth mechanisms.
In addition we can think about shipping a provider (like Ory) as a default solution for communities to store user-profile data in or keep on persisting that information in the app db. As long as we have the optionality that the connector-layer gives us this isn't so critical.
Often at least some profile data will need to be persisted in-app anyway because its not uniform which data the providers expose, and sometimes users may want to change things in the galoy profile vs what is in the auth providers data.

Still good to be aware of the options. If Ory is good then perhaps we can ship it as a default solution. Would certainly reduce the app-code if we can externalise all user profile stuff.

@nicolasburtey
Copy link
Member

not aware of dex, I will have a look.

It seems Ory Hydra doesn't store any of the user data but instead letting the implementation decide where to store this data, so it would still be stored in our mongodb currently.

@hsluoyz
Copy link

hsluoyz commented Jul 10, 2021

@bodymindarts @nicolasburtey hi guys, I'm from the Casbin team. Besides Casbin (https://github.com/node-casbin/graphql-authz), we also provide an open-source authentication solution called Casdoor: https://github.com/casbin/casdoor . It's similar to KeyCloak but it's developed more recently and it's written in Go. We have dogfooded Casdoor in our multiple services, like our forum: https://forum.casbin.com/

Login page: https://door.casbin.com/login/oauth/authorize?client_id=014ae4bd048734ca2dea&response_type=code&redirect_uri=https://forum.casbin.com/callback&scope=read&state=app-casbin-forum

image

New OIDC providers can be easily extended by plugging in a new middleware following our Provider interface. Compared to dex, Casdoor's one advantage is having a user-friendly admin portal UI (you can try at our demo site: https://door.casbin.com/) :

image

We also have a user profile page to change setting, SMS and Email verification are supported:

image

For DB, Casdoor uses Go's XORM to support multiple DBs like MySQL, PostgreSQL. It can reuse an existing DB by creating some tables for its use.

image

We are not company. But we have lots of active developers (Casdoor + Casbin). We are evolving fast at our side now but we can work towards your needs if needed.

Please let me know if you have any questions. Thanks!

@bodymindarts bodymindarts moved this from In progress to Review in progress in Dev Ongoing Jul 15, 2021
@bodymindarts bodymindarts moved this from Review in progress to In progress in Dev Ongoing Jul 16, 2021
@bodymindarts bodymindarts removed this from In progress in Dev Ongoing Aug 2, 2021
@nicolasburtey
Copy link
Member

closing this one now that the migration on the backend is mostly done

@nicolasburtey
Copy link
Member

there is some useful things here for the authentication/authorization server

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants