search

farmOS / farmOS-aggregator

A microservice application for aggregating data from multiple farmOS instances.
GNU General Public License v3.0
21 stars 10 forks source link

Consolidate /authorize-farm and /register-farm to a single endpoint #109

Closed paul121 closed 3 years ago

paul121 commented 3 years ago

The simple_oauth module only supports a single redirect URI. We are currently using separate pages to support registering & re-authorizing farms. Lets consolidate this logic to a single endpoint so only one redirect URI is required.

paul121 commented 3 years ago

This shouldn't be too challenging. The main reason these endpoints were separated is because a farm profile doesn't exist when authorizing & adding a new farm to the aggregator.

But if we create a new farm profile at the beginning of the registration process we can re-use the existing re-authorization form to complete the authorization flow. The backend authorize-farm/{farm-id} endpoint supports both 1.x and 2.x #104

The downside to this is that a profile is created before being fully authorized. This opens up the possibility for an Aggregator to be spammed... but we do have an existing @todo to ping the supplied URL farm.json or /api (as part of the validate-farm-url endpoint) and ensure it responds, even if a 403 response, before continuing.

paul121 commented 3 years ago

This id done!

But if we create a new farm profile at the beginning of the registration process we can re-use the existing re-authorization form to complete the authorization flow.

I ended up taking a different approach. Being able to register new farms before they are authorized just seems weird.

I modified the /authorize-farm page to work for both the registration and re-authorization use cases. This brings the "stepper" UI to the Authorization page which is nice improvement. Consolidating this code means less to maintain too.

This refactor took out a couple features of the "public registration" use case where users could specify their farm name and tags. Now, it uses the farmOS server name & defaults to no tags. The user has a chance to "Verify" everything after they have authorized, but before the farm is added to the aggregator. This seems sensible for now, especially given that this use case isn't currently being used..

Another benefit of this is that it allows admins of existing farms to come and re-authorize their farm at any time. Previously they needed a link with an api_token to do this. After thinking about it more, though, their identity can be confirmed once they have successfully authorized their farm... so the api_token isn't always needed here.

84 comes to mind too. This could be implemented in this authorization-flow stepper as a future follow up.