UX: Surface stripe AuthenticationError on the Admin Dashboard

[nattsw]

There's a bit of log noise now when a user has an invalid API key.

Job exception: (Status 401) Invalid API Key provided: isoa_wi******************moOs

This surfaces the issue to the admin dashboard instead of just /logs. AdminProblemChecks are done every 10.minutes, so we won't have to worry about the Problem still being there if the plugin is removed.

[martin-brennan]

I'm not sure why add_found_scheduled_check_problem was used, like I was saying yesterday the correct way of doing this is by using add_scheduled_problem_check. The example is here:

https://github.com/discourse/discourse/blob/1ed09234f98b68d13ed7b49c2588155c7d8c9947/app/models/admin_dashboard_data.rb#L133-L152

[martin-brennan]

Looking at how this plugin is set up more, I see you want a JIT kind of message for this. Usually the better solution for this is to use AdminDashboardData.add_problem_message. Sadly though this has no plugin extensibility (nor does anything in AdminDashboardData atm 😢 ), since all the messages have to be defined upfront:

https://github.com/discourse/discourse/blob/1ed09234f98b68d13ed7b49c2588155c7d8c9947/app/models/admin_dashboard_data.rb#L198-L202

So I would recommend a couple of things. First of all make a commit to core to:

1. Add a filtered register in DiscoursePluginRegistry using define_filtered_register, which could be called something like admin_problem_messages
2. Alter AdminDashboardData to also iterate through DiscoursePluginRegistry.admin_problem_messages here https://github.com/discourse/discourse/blob/1ed09234f98b68d13ed7b49c2588155c7d8c9947/app/models/admin_dashboard_data.rb#L66-L69

Then change this PR to:

1. In plugin.rb, call `DiscoursePluginRegistry.register_admin_problem_message("discourse_subscriptions.some_i18n_key", self)
2. Call AdminDashboardData.add_problem_message("discourse_subscriptions.some_i18n_key")
3. Change the clear to use AdminDashboardData.clear_problem_message("discourse_subscriptions.some_i18n_key")

In addition, I would also add a site setting validator to validate that the Stripe key is valid when setting discourse_subscriptions_secret_key.

If you need some help with this, I think @Drenmi would be the person to do it.

[martin-brennan]

I know this is more more work, but it is more correct. This is why I started up a topic too to discuss the new messaging interface for admins, because like we talked about it would be good to let plugins do this kind of thing easier :)

[nattsw]

A few points..

1. the error is only known when the API call is made. so using core's add_scheduled_problem_check's check schedule of 10.minutes would mean core is invoking the API every so often, even if the value is correct. I want to avoid that.
2. There is no way to "validate" the API key without an API call. In the gem, Stripe's recommended way is to use the web interface at https://stripe.com/api.

In defining this new interface on core, I will leave it to the staff exp team. With social auth validation coming up there will prob be more to-and-fro on this and it will probably be a better use of time. There are some complications like plugin availability and problem message removal issues that need to be better addressed if we're defining a plugin 'API' for admin problems.

[martin-brennan]

There is no way to "validate" the API key without an API call. In the gem, Stripe's recommended way is to use the web interface at https://stripe.com/api.

Yeah there will be a similar problem with the social auth checks. Any API call will do; doing a GET /v1/balance for stripe seems easiest, but people are also suggesting creating a token (but that is not very nice to have to create something).

In defining this new interface on core, I will leave it to the staff exp team. With social auth validation coming up there will prob be more to-and-fro on this and it will probably be a better use of time.

Yep sure makes sense to leave it to us 👍 There will definitely be plenty to do in this area to make things consistent.

[nattsw]

Thanks Martin!