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

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

Closed
paul121 opened this issue Mar 3, 2021 · 2 comments
Closed
Labels
2x farmOS 2x support

Comments

@paul121
Copy link
Member

paul121 commented Mar 3, 2021

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 paul121 added the 2x farmOS 2x support label Mar 3, 2021
@paul121
Copy link
Member Author

paul121 commented Mar 4, 2021

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
Copy link
Member Author

paul121 commented Mar 5, 2021

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.

@paul121 paul121 closed this as completed Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2x farmOS 2x support
Development

No branches or pull requests

1 participant