Bounty for developing Edgeryders form interface

gdpelican · October 6, 2019, 1:37am

Fine by me, I’m happy to do the standalone component; I just wanted to ensure that the “simplest” option had been considered

gdpelican · October 8, 2019, 10:08am

I’ve sunken some teeth into this over the past couple days; here’s a video update, along with a link to the WIP repo

Link to the repo:

hugi · October 8, 2019, 12:18pm

Excellent, thank you.
In this phase, it would be good to get the ux/ui designer on the platform @natalia_skoczylas.
On my end, it looks good and just as it should.

gdpelican · October 11, 2019, 6:01am

Hey @hugi @matthias,

Alright, we’ve gotten to the point with this where the frontend is just about good to go and the big piece left is to put together a post for the Discourse API.

Was wondering if I could be granted access to an active auth_key (as mentioned in the API spec) for testing purposes (as well as some instructions on how to test it without spamming a real forum), as well as some thoughts on where to store the thing once this is deployed (since holding it in the clientside app won’t be secure)

Best!

hugi · October 11, 2019, 9:26am

This is on @matthias’ table. Could you give James what he needs?

hugi · October 11, 2019, 9:33am

@matthias and I discussed this in the API documentation thread. Have a look at that and let me know if it’s still unclear.

matthias · October 11, 2019, 6:03pm

I just sent it to you by direct message here on Discourse.

That’s a bit tricky. We could create a new “Communities” forum (a whole new Discourse forum that can be accessed by the same account), but our Discourse multisite installation is on a newer Discourse version than this very forum so it’s not the most realistic testing ground …

So I think let’s split up the testing into two parts:

Account creation. This API call does not create any notification spam, so no issue doing the testing on the edgeryders.eu forum. Use some pattern for the username that makes it easy for me to identify and remove the test accounts once the testing is done … such as surveytest_1, surveytest_2 and so on.
Posting to Discourse. After the account creation step, you’ll have the API key of a new Discourse user. Using that would indeed lead to notification spam, as a new user can only post to public categories at first. So instead, for testing use the Discourse API key of your own user (which I also sent in a direct message to you), and create the topic in the access protected category Survey Tests (category_id = 343) that I just created for this purpose. It’s only accessible to admins and to you, so there will be no notification spamming in the Unread / Latest / New topic lists or in e-mails.

gdpelican · October 12, 2019, 10:41pm

Hmm, so we’ve run into a bit of a snag here.

Discourse offers 2 types of api keys:

A ApiKey (code definition) which is associated with a user and can be used to perform actions on behalf of this user (there also exists a ‘master api key’, which is associated with the Discourse system user and can do basically anything)
A UserApiKey (code definition) (usage spec) which the user can generate themselves for use with external applications (usually for mobile / desktop apps, or something without a server), and requires explicit permission from the user of scopes (read-only, write-only, one time password, etc.)

(NB that these two authentication systems are 100% distinct in Discourse the codebase)

Trouble is, as far as I can tell there’s not a way out of the box to use the first option from a clientside-only app, as the CORS settings on a Discourse instance will only accept values for User-Api-Key, which is linked to the second option. Here’s the code where the CORS settings are set, and a thread of someone having a similar issue, (in which the team’s response is 'Errr you need to be using UserApiKeys for this)

Using the second option seems possible, but would break the proposed workflow because the user needs to explicitly log in and grant permission to our application; I don’t think a hidden account creation followed by posting for them automatically is in the cards there. Also, it seems with this method the keys would come back encrypted with a public key that we provide, meaning that we’d need to store a private key (somewhere… probably not in the clientside app…) to decrypt it.

Options!

Backing the clientside app with a very simple node / ruby ( / whatever) server to perform API requests using “normal” API tokens ()
Modifying the Discourse source code to allow Api-Key creds to come in via CORS ()
Modifying the user workflow so that they explicitly
1. Complete the form
2. Are redirected to Discourse to create ( / log in to?) an account
3. Grant explicit permission to the form builder application to post for them
4. Are redirected back to the clientside app with a token we can use to post for them ()
Letting me know that I’m missing something simple and silly here ()

matthias · October 13, 2019, 3:21am

Ok this was unexpected … I guess I’m still too used to the 2017 Discourse with its one type of API access

Thanks for the detailed analysis!

So that could be just a proxy server with the only task to drop the Origin: header, so that Discourse sees the requests as not coming from a browser. It should work (and I think that’s what crossorigin.me does, but I did not check). But it’s a hack … so let’s see if we can find something better …

That would be the clean solution, right? I rarely disagree with the Discourse people on their software design decisions, but here I do disagree. The point of CORS is to protect the user from requests made by malicious sites that trigger actions on their behalf. Now the user is still protected if a Discourse site admin had to explicitly whitelist a site (that uses Admin API keys) via the “cors origins” setting because the Discourse site admin can check what that site does with the API key that is entered. And there are other legit reasons why a user might enter their own API key into a third-party website, beyond the edge case that we have here. For example a tool for API testing, or for data extraction etc…

Maybe we can persuade the Discourse devs and get this change into upstream. Otherwise we’d have to maintain it in our fork. In that case: not pretty, but clean.

Now our in-house Discourse developer is literally beyond the reach of the Internet for the next weeks, trekking around Annapurna. So, in the interest of progress: Would you want to try implementing this? For additional money of course. If so, please propose us a budget, or if that sounds a bit risky for this task, then first propose a budget for code exploration so you’d know the required effort better.

I’d install the code when ready. The deliverable would be a pull request against our Discourse fork, branch “stable”.

gdpelican · October 13, 2019, 10:13am

I’ve smacked together a little plugin that should do the trick here:

github.com

gdpelican/permit-api-cors/blob/master/plugin.rb

# frozen_string_literal: true

# name: permit-api-cors
# about: Allow CORS to accept Api-Key and Api-Username headers
# version: 0.1
# authors: James Kiesel (gdpelican)
# url: https://github.com/gdpelican/permit-api-cors

class ::Discourse::Cors
  module AllowApiKey
    HEADER_KEY = 'Access-Control-Allow-Headers'
    HEADER_VALUES = ['Api-Key', 'Api-Username']

    def apply_headers(cors_origins, env, headers)
      result = super(cors_origins, env, headers)
      result[HEADER_KEY] = result[HEADER_KEY].split(', ').concat(HEADER_VALUES).join(', ') if cors_origins.presence
      result
    end
  end
  singleton_class.prepend AllowApiKey

This file has been truncated. show original

I’ll spin it up on a local instance this week to verify, and if so we should be able to get away with sticking that plugin onto the edgeryders.eu instance.

Like you say, it would be dangerous if you have e.g. Allow-Origin: * set, but since we’ll be whitelisting foreign domains it shouldn’t be too scary a thing (as long as you trust riot.im / any other domains in the whitelist etc to not abuse it)

matthias · October 13, 2019, 11:25am

If this works, it’s great! I would not have expected that change to be possible with a plugin. Let me know when you’re done testing and I should install it.

gdpelican · October 13, 2019, 8:40pm

Alright, can confirm this will work for our use case; I do have to add a line (via this plugin) to PostsController to allow it to skip CSRF checking via the api:

skip :verify_authenticity_token, only: :create, if: :is_api?

I’ll push that up soon.

gdpelican · October 14, 2019, 6:42am

Cool, I’ve been successful in making posts from the vue app to a local discourse instance with that plugin installed; I’ve pushed that plugin up to https://github.com/gdpelican/permit-api-cors

Here’s a screenshot of an example post (open to feedback on title / formatting / etc):

One thing I wasn’t clear on was whether we are posting all of the responses in a single topic, or each response gets its own topic?

Next up: There’s CORS trouble with hitting the multisite endpoint as well (obviously 'localhost:8080 isn’t a great thing to put in your list of accepted origins), but I’m hoping I’ll be able to install that plugin locally as well and get an end-to-end test going soon.

hugi · October 14, 2019, 8:34am

@natalia_skoczylas, what’s the intention?

Nskocz · October 14, 2019, 9:03am

They get their own topics - they will be collected in this category, right now under development, but soon to be open to everyone

https://edgeryders.eu/c/rethinking-retirement-academy-of-life

gdpelican · October 14, 2019, 6:46pm

Hey @matthias, could you confirm or deny for me that users created via multisite_account.json are set to ‘active’ or not? They’ll need to be based on this line in Discourse:

github.com

discourse/discourse/blob/master/lib/auth/default_current_user_provider.rb#L115


  raise Discourse::InvalidAccess if current_user.suspended? || !current_user.active


  limiter_min.performed!
  limiter_day.performed!


  @env[USER_API_KEY_ENV] = true
end


# keep this rule here as a safeguard
# under no conditions to suspended or inactive accounts get current_user
if current_user && (current_user.suspended? || !current_user.active)
  current_user = nil
end


if current_user && should_update_last_seen?
  u = current_user
  Scheduler::Defer.later "Updating Last Seen" do
    u.update_last_seen!
    u.update_ip_address!(request.ip)
  end
end

I believe there’s an option in the discourse api to pass active or not when creating an sso user, but I’m not sure if we’re using it or not:

github.com

discourse/discourse_api/blob/master/lib/discourse_api/api/users.rb#L55


  args = API.params(args)
    .required(:level)
    .to_h
  response = put("/admin/users/#{user_id}/trust_level", args)
  response[:body]
end


def create_user(args)
  args = API.params(args)
    .required(:name, :email, :password, :username)
    .optional(:active, :staged, :user_fields)
    .to_h
  post("/users", args)
end


def log_out(id)
  post("/admin/users/#{id}/log_out")
end


def invite_admin(args = {})
  post("/admin/users/invite_admin", args)

matthias · October 14, 2019, 8:12pm

Right now it’s done this way:

So the new “master account” over at our SSO provider site communities.edgeryders.eu is not set to active, but the new edgeryders.eu account is set to active. Due to that, you should be able to use the returned edgeryders.eu API key immediately.

gdpelican · October 16, 2019, 9:07am

Alright, I think this is ready for an initial test run; I’ve gotten it posting to a local discourse instance using the multisite-accounts plugin and mimicing the setup we’ll have in production as closely as possible.

Three things will need to happen on the edgeryders.eu instance:

Set the DISCOURSE_ENABLE_CORS env to something truthy if it’s not already
Add the domain hosting the vue app to the cors_origins site setting on the admin panel
Install the permit-api-cors plugin

Steps 1 and 2 will need to be repeated on communities.edgeryders.eu as well.

Here’s the env we’ll want to set for the vue app:

VUE_APP_DISCOURSE_USER_URL=http://communities.edgeryders.eu/multisite_account.json
VUE_APP_DISCOURSE_AUTH_KEY=<auth_key>
VUE_APP_DISCOURSE_DOMAIN=edgeryders.eu
VUE_APP_DISCOURSE_TOPIC_URL=http://edgeryders.eu/posts.json
VUE_APP_DISCOURSE_CATEGORY=343 (<-- this will be changed to 339 for the actual category)

Let me know what snags you run into, and if it successfully gets going, what additional design / styling / functionality work should be done?

matthias · October 17, 2019, 12:46pm

I’ll get this done as soon as I can carve out the time. Could still be 2 days from now since we’re all squeezed right now to finish some website and communications materials for parallel projects …

gdpelican · October 24, 2019, 9:20pm

Hey @matthias what’s the ETA on this one? I’m aware of the tight deadline and want to make sure we’re not in a position where the conference is looming and things aren’t working properly