hippware / wocky

Server side (Erlang/Elixir) software
13 stars 3 forks source link

Discussion: HTML Bot Link #1318

Closed bengtan closed 6 years ago

bengtan commented 6 years ago

For further discussion. This is probably 4-6 weeks (or more) away.

https://github.com/hippware/rn-chat/wiki/HTML-Bot-Link-v1

bengtan commented 6 years ago

Quoting Bernard at https://hippware.slack.com/files/U0HH52JTT/F9KTU418E/HTML_Bot_Link

(Note that my cross-posting does not mean that I endorse these comments. I reserve my own opinions for later publication.)


So I just wanted to write down my thoughts on https://github.com/hippware/rn-chat/wiki/HTML-Bot-Link-v1

The "obvious" architecture (which is to say the one that I arrived at after about 20 seconds of thought, so there's probably major issues) is:

This means the website doesn't have to access the database directly (and therefore things like schema changes and permissions are still all handled in one place - wocky) and can use effectively the same server-side code as the client (or the same as the client will once it starts using GraphQL, at any rate). There's some small questions about how to handle authentication etc, but a static token seems to be a fairly simple-to-implement solution.

toland commented 6 years ago

It depends on how we want to do this. It would be trivial to add an HTML template to Wocky for rendering bots. A little LB magic and we can ensure that requests are routed to the right place.

We could do this using GraphQL as well. That would be more complicated since we are adding another layer of indirection. Not saying we shouldn't do that, but it will add some complexity. The good news is that if we go that direction, then most of what we need on the server is already in place. All we would need in addition is to tweak the GraphQL a bit to make sure we are respecting permissions and visibility rules.

bernardd commented 6 years ago

My feeling is very strongly that "rendering" should not be the job of wocky (but of course my feelings are subject to change based on someone pointing out where I'm wrong). When we decide to change the look of the website, we shouldn't have to do it in both the website repo and some little corner of wocky. Providing a GraphQL or similar interface decouples the data from the rendering and means we won't need to re-deploy wocky when we decide to change the website layout or add or remove fields we display or what have you.

bengtan commented 6 years ago

I'm tending towards decoupling the rendering code from wocky due to:

The HTML bot links should have the same branding and UI as the main website (tinyrobot.com)

If we had to make a change to wocky everytime they wanted to adjust the website/wordpress template, I think it would be too frequent and too painful.

It would be trivial to add an HTML template to Wocky for rendering bots.

Trivial to add, perhaps. Not so trivial to keep it up to date with arbitrary whims.

Hence, I'm tending towards some sort of server side API. Since we've been talking graphql recently, that seems as good a choice as any (although I'm open to other APIs ... REST?).


Re: Authentication

There's some small questions about how to handle authentication etc, but a static token seems to be a fairly simple-to-implement solution.

It's not just a case of creating a special user account for the web front-end to use. I think we need to have a new type of user account which has less privileges than regular accounts.

For example, if the web front-end used a (specially created but nonetheless a) regular account, anyone could use the web front-end's credentials to create bots, befriend people, send messages etc.

The web front-end's credentials should only allow read access, not write access. Somewhat like a linux/unix guest user account.

This new 'anonymous' account is only needed for the web (so far), so for simplicity, maybe it only needs to be implemented for graphql (or other), and not needed for XMPP.

bernardd commented 6 years ago

I think we need to have a new type of user account which has less privileges than regular accounts.

I'm not convinced.

For example, if the web front-end used a (specially created but nonetheless a) regular account, anyone could use the web front-end's credentials to create bots, befriend people, send messages etc.

By "anyone", I presume you mean "anyone with access to the credentials, which would basically be Bernard and Phil". I don't think that's a huge risk, since we can already pretty much destroy the system without needing a special extra account to do so :)

The web front-end's credentials should only allow read access, not write access. Somewhat like a linux/unix guest user account.

Yes, it would be nice and probably "best practise" to have such an account, but in the short term it's a lot more work than just tacking an extra user in.

bengtan commented 6 years ago

Uh, if Alan creates a client-side javascipt web-app that uses graphql (executed in a web browser), wouldn't the web-app need credentials to use the graphql API?

Couldn't a malicious person then inspect the web app source and extract the credentials?

bernardd commented 6 years ago

Uh, if Alan creates a client-side javascipt web-app that uses graphql (executed in a web browser), wouldn't the web-app need credentials to use the graphql API?

Yes it would, but why on earth would he do that rather than just making the request straight from the webserver and not having to send any credentials anywhere?

toland commented 6 years ago

There are two issues that have cropped up in this discussion: where to render the data and how to authenticate. These end up being interrelated. First, architecture...

My feeling is very strongly that "rendering" should not be the job of wocky (but of course my feelings are subject to change based on someone pointing out where I'm wrong).

You are wrong ;)

Sorry, couldn't resist. You are not entirely wrong, but the issue isn't quite so cut and dried.

When we decide to change the look of the website, we shouldn't have to do it in both the website repo and some little corner of wocky.

and

If we had to make a change to wocky everytime they wanted to adjust the website/wordpress template, I think it would be too frequent and too painful.

There is this incredible new technology called Cascading Style Sheets. They allow you to separate the structure of the data you are displaying from the presentation.

Seriously, though, we should be able to render the bots into HTML and use the stylesheets from the main website. Alan can change the look of the bot page by updating the CSS and JS assets and we shouldn't have to change the markup. If the presentation is driven primarily by markup, then we may be doing it wrong.

Alan will face the exact same issue since the code to render the bots will most likely be separate from any markup created to render the site. And he will solve it in the same way.

Trivial to add, perhaps. Not so trivial to keep it up to date with arbitrary whims.

Not any more difficult than keeping any other bit of code up to date with arbitrary whims. Plus, see my comments above about CSS.

Providing a GraphQL or similar interface decouples the data from the rendering and means we won't need to re-deploy wocky when we decide to change the website layout or add or remove fields we display or what have you.

That can work the other way, too: needing to redeploy the website because we changed a bot field in Wocky. We would go from having one client dependent on us (the mobile app) to two. That means potentially having to coordinate deployments across three platforms instead of two. Pick your poison.

FWIW, I suspect that we will end up splitting Wocky into multiple deployments, if not multiple repos, sooner rather than later. That was one of my motivations for moving to the Kubernetes architecture: to make it easier to deploy smaller batches of code. I would like to be able to deploy the wocky_xmpp, wocky_api, and (future) wocky_web applications independently. They would each bundle the appropriate version of wocky_core (what we today just call wocky; although maybe that becomes a set of microservices in the far future). We could do this at just about any time, so we wouldn't necessarily have to do a "whole" Wocky deployment to get updates to bot rendering.

Hence, I'm tending towards some sort of server side API. Since we've been talking graphql recently, that seems as good a choice as any (although I'm open to other APIs ... REST?).

I would really rather not add another API just for the website. If we go with GraphQL for the mobile app, then we should use GraphQL for the website. Let's not make things any more difficult than we absolutely have to.

I am not against building the bot rendering outside of Wocky, but at the same time I don't think the case for it is as strong as y'all think it is. FWIW, I had planned to build the Wocky admin site using Elm and just pushing the data down a websocket rather than using a formal API. So, it is not like I am against SPAs.

If we do go this route, at least let me work with Alan to make sure that the end product is built on a good foundation. Mainly what I don't want is a hacky one-off solution because Alan was working in isolation and without a good idea of how this fits into the larger architecture.


And now, authentication...

There's some small questions about how to handle authentication etc, but a static token seems to be a fairly simple-to-implement solution.

Please, no. I just finished overhauling authentication to be more secure. Let's give it at least a few months before we start opening new security holes. Authentication and authorization should not be an afterthought.

That having been said, I question the need for any authentication if all we are doing is exposing public data read-only.

It's not just a case of creating a special user account for the web front-end to use. I think we need to have a new type of user account which has less privileges than regular accounts.

and

The web front-end's credentials should only allow read access, not write access. Somewhat like a linux/unix guest user account.

Yes, I agree. Or just allow unauthenticated read-only access to public data. They end up being essentially the same thing in the end. Except one is easier to implement than the other.

This new 'anonymous' account is only needed for the web (so far), so for simplicity, maybe it only needs to be implemented for graphql (or other), and not needed for XMPP.

Sorry, this is a nitpick. Authentication happens at a very deep level within Wocky now. Only implementing it for GraphQL or not implementing it for XMPP doesn't really simplify much since the code to plug the underlying logic into the presentation layers is very thin. The authentication changes were huge, but they significantly simplified and consolidated authentication within the app.

However, implementing unauthenticated read access to public data via GraphQL only would be relatively simple. Creating an anonymous or guest account type would be more difficult, though probably not hard, regardless of whether or not it was implemented for XMPP.

Yes it would, but why on earth would he do that rather than just making the request straight from the webserver and not having to send any credentials anywhere?

So you are making the case to do the rendering in Wocky now? ;)

The serious answer to your question is "because that is not how web apps are built these days." Having Alan do the work in a SPA is reasonable and straightforward. Having Alan build server infrastructure to do it is not. And please, don't try doing this in Wordpress; down that path madness lay.

The reason that credentials in SPAs are not the problem that y'all seem to think it is is the same reason that credentials aren't a problem in the mobile app: they don't store credentials, they impersonate the user. If the user doesn't need to authenticate to see the data, then neither does the app.

bengtan commented 6 years ago

@bernardd:

Yes it would, but why on earth would he do that rather than just making the request straight from the webserver and not having to send any credentials anywhere?

A proportion (few? many? most?) of web apps nowadays execute all the logic in client side javascript. The server only serves static code, or static javascript source code. I'm not saying I endorse this approach, but that's what some people choose to do.


@toland:

Now I'm confused as to whether you are for 1 or 2:

  1. Render the bot in wocky and serve it from an embedded web server in (or close to) Wocky; Deploy wocky every time there are changes to the presentation,
  2. Expose bot data through an API and Alan renders it using whatever he wants; Deploy the website every time there are changes to the presentation

For the most part, I don't disagree with your micro-reasoning, but now I'm not sure what your high level conclusion is.

However ...

Hence, I'm tending towards some sort of server side API. Since we've been talking graphql recently, that seems as good a choice as any (although I'm open to other APIs ... REST?).

I would really rather not add another API just for the website.

I threw REST in there just as an example. I wasn't really serious about it.

If we do choose to expose via an API, I'm happy to go with graphql ... or anything you guys want me to consider.


It's not just a case of creating a special user account for the web front-end to use. I think we need to have a new type of user account which has less privileges than regular accounts.

and

The web front-end's credentials should only allow read access, not write access. Somewhat like a linux/unix guest user account.

Yes, I agree. Or just allow unauthenticated read-only access to public data. They end up being essentially the same thing in the end. Except one is easier to implement than the other.

For me, 'a new type of user account which has less privileges than regular accounts' is equivalent to 'unauthenticated read-only access to public data'. They are two different ways of viewing the same thing. Maybe I got too theoretical and computer-science-y.

I'm happy to change my terminology. Yes, 'unauthenticated read-only access to public data' is fine, hence I also support this statement:

However, implementing unauthenticated read access to public data via GraphQL only would be relatively simple.


The reason that credentials in SPAs are not the problem that y'all seem to think it is is the same reason that credentials aren't a problem in the mobile app: they don't store credentials, they impersonate the user. If the user doesn't need to authenticate to see the data, then neither does the app.

And I think if we are doing 'unauthenticated read-only access to public data', then the above statement becomes not applicable.


@toland, @bernardd:

One more point I want to re-iterate:

Steve wants Alan to do the (front-end) development for this so we have to find a way to accommodate his skill set.

So um, no, we are not going to ask Alan to learn Erlang or Elixir (unless he says he wants to).

toland commented 6 years ago

Now I'm confused as to whether you are for 1 or 2:

I am not really advocating for a position. I am fine with either outcome. I am just trying to correct some misunderstanding. Or maybe I am just being argumentative.

For the most part, I don't disagree with your micro-reasoning, but now I'm not sure what your high level conclusion is.

My high-level conclusion is that either works, and there aren't strong technical reasons to choose one over the other.

For me, 'a new type of user account which has less privileges than regular accounts' is equivalent to 'unauthenticated read-only access to public data'. They are two different ways of viewing the same thing.

Cool. It sounds like we are in agreement, then.

Steve wants Alan to do the (front-end) development for this so we have to find a way to accommodate his skill set.

So um, no, we are not going to ask Alan to learn Erlang or Elixir (unless he says he wants to).

Of course. I had assumed this would be the case. I think a SPA is the right way to go if we want Alan to do the work. I just want to make sure it fits in with our overall architecture, uses the backend components correctly, and is a nice foundation for whatever we want to do next. I don't think that requires Erlang or Elixir in this case.

bernardd commented 6 years ago

Righto, apparently I'm like a decade behind in my views on web..stuff. It sounds like the basic plan is:

Does that sound like a reasonable summation?

bengtan commented 6 years ago

That sounds reasonable to me. After the discussion in the preceding comments, I'm (still) tending towards a separate API, and I have no objection to using graphql for the API.

However, I'd modify this:

Use GraphQL from a SPA Allow unauthenticated access to public data via GraphQL

to just:

Allow unauthenticated access to public data via GraphQL

so he has some latitude to find a toolset that suits him. Whether @irfirl wants to use a pure SPA is up to him.

So I think that's two votes (myself and @bernardd) for this. I think @toland is not strongly against it.

So it's a go-er.

So I think this discussion is just about finished. I'll leave the ticket open for a few more days just in case.

If there are no more comments, this can be closed and server side implementation can commence [1] (pending scheduling and free time).

[1] In addition to informing @irfirl and others about this and coordinating with him/them.

toland commented 6 years ago

After the discussion in the preceding comments, I'm (still) tending towards a separate API, and I have no objection to using graphql for the API.

Due to the mechanics of authentication, I was thinking it would make sense to expose multiple GraphQL endpoints:

  1. User-centered API for the mobile app
  2. Public API for things like bot links and other public data
  3. (future) Admin API

The authentication and scope of data exposed by each endpoint would be different, but behind the scenes, they would use the same backend code. The cost of providing separate endpoints is minimal, and it would simplify auth logic while reducing the risk of accidentally overexposing data.

bernardd commented 6 years ago

Sounds good to me

bengtan commented 6 years ago

Due to the mechanics of authentication, I was thinking it would make sense to expose multiple GraphQL endpoints

Okay.

bengtan commented 6 years ago

Closing the discussion phase.

I've spun out #1356 for implementation. Also dependent on #1335 ~#1355~.

Not sure if there's anything else. If it turns out there is, I might create an Epic and include 1335 ~1355~ and 1356 in it.

Closing.