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

Add more robust support for alienated client and server instances #329

Open
signebedi opened this issue Jul 23, 2024 · 6 comments
Open

Add more robust support for alienated client and server instances #329

signebedi opened this issue Jul 23, 2024 · 6 comments

Comments

@signebedi
Copy link
Owner

signebedi commented Jul 23, 2024

A libreforms-fastapi server implements the backend logic and REST API. A llibreforms-fastapi client implements the UI routes. As it stands, a server instance can theoretically run without needing a client instance, see #18. A core assumption of our development work so far, which has worked well for our MVP, is that a client will not be cross compatible with different libreforms-fastapi servers, and as a result we have not added support for running client instances without a server instance running as part of the same core set of uvicorn processes on the same logical machine. The assumption (viz. that the server instance may be compatible with a wide array of clients; but not vice-versa) is a good one, for a number of reasons.

However, the effect of this assumption has been suboptimal; we should be able to run decoupled server and client instances and have them communicate over HTTP using a mix of some straightforward app configs, because this is consistent with most principles of enterprise software deployment. For example, let's consider a containerized deployment using something like k8s or EKS. We may want to deploy a set of client nodes behind an ALB that communicate with a set of server nodes. The ALB distributes traffic across nodes, but can be reached at a single address and given its own security rules. For example, an enterprise might want its clients to be reachable from anyone on the internet, but the servers only to be reachable from the clients' network. The application already supports (in principle, pending #35) externalized document and relational databases.

So, this architecture might look something like this:

image

To make this feature workable, we need to enable a partner configuration to UI_ENABLED that covers the REST API. It should default to True (REST API enabled), which is consistent with the project's preference for strong default behavior. If this config is set to False (that is, the REST API is disabled), however, we should require something like a SERVER_ADDR: str config that points to the location of a server instance / set of instances.

@signebedi
Copy link
Owner Author

signebedi commented Jul 26, 2024

One thing we should be mindful of is how this bifurcation will work for clients when there is an external server address for the relational and document databases vs. when the end user has elected to use the default sqlite3 and TinyDB datastores. I think this will be a non-issue but will be worth testing just in case. There is unfortunately not a perfect separation between UI routes and server logic; many UI routes, especially admin routes, assume collocation of the server and client on the same logical process.

To enable perfect bifurcation is to invite a handful of inanities, that is, additional API routes meant solely to be polled by the UI to provide it with sufficient information to implement things. For example, Should a client even really need the form config, or most of the other configs? Or, should these really reside on the server unless they are SOLELY UI-specific?

This line of questioning suggests to me that this will require more than a small refactor - and, in fact, might constitute an entire major version increment. Before we implement this, we should consider implementing #149, which will likely create a separate submodule for fastapi routes, including one for UI routes.

@signebedi
Copy link
Owner Author

signebedi commented Aug 4, 2024

Importantly, we will need to point the auth logic to a fully qualified address for the REST API auth route, instead of just a local route. This is currently implemented at:

oauth2_scheme = OAuth2PasswordBearer(tokenUrl="api/auth/login")

This seems to be possible with this logic, see implementation of this logic at:

https://github.com/fastapi/fastapi/blob/35fcb31dca6dbeac5f03bbaea3ae279da9cc0b8a/fastapi/security/oauth2.py#L391-L485

See also: https://fastapi.tiangolo.com/tutorial/security/simple-oauth2.

@signebedi
Copy link
Owner Author

signebedi commented Aug 30, 2024

Much of this will require admin API access, potentially using a service account. It's not clear whether it makes the most sense to use the service account approach because of the overhead associated with that approach. I think instead we have a special key that can be used to access special admin routes like form config, site config, etc. If we do go with the admin service account, then we will need to have a method implemented to rotate a user's own key... I think this is already implemented for admins under api/admin/toggle... so, since this service account would need admin authorization anyways, we may be good to go. That said, we will need an automated task to rotate this key periodically, and setting things up eg. using docker will be a little funky. Specifically, we will need to update the CLI to support creating service account users.

These are not called in the jinja2, but instead queried in the routes themselves.... using something like aiohttp.

@app.get("/", response_class=HTMLResponse)
async def get_data(request: Request):
    # Make an HTTP request
    async with aiohttp.ClientSession() as session:
        async with session.get("https://api.example.com/data") as response:
            data = await response.json()

    # Pass the data to the Jinja2 template
    return templates.TemplateResponse("index.html", {"request": request, "data": data})

In principle, both the client and server will run a significant amount of duplicative code... but the server will store the core configurations. The client (I would assume) will only need to store the client-specific configurations.... But we will need to think through that delineation a bit more. I don't think we would want the server to propagate client-specific configs because there could be more than one client setup, each with their own needs.

@signebedi
Copy link
Owner Author

Update CLI to support creating service account users
The CLI lags significantly behind the REST API in several ways. We could just make them the same, but I think they have meaningfully separate concerns: for example, the CLI runs even when the uvicorn server is not yet running. We specifically need to allow creating service accounts from the CLI so we can automate client/server deployments.

@signebedi
Copy link
Owner Author

A handful of the UI routes rely on a doc_db dependency injection, but this probably is a no-go in an alienated server/client environment, where we would want the server/API to be the gatekeeper of the doc_db object, as well as other objects.

@signebedi
Copy link
Owner Author

Allow different clients to access limited form_config scope
Presumably, bifurcated server/client deployments may need different clients to have access to different sets of forms. For example, form_1 and form_2 may need to be accessed by personnel employing client_1; whereas, form_3 may need to be accessed by personnel employment client_2. The principles-level question is where should this scope be manifest? Given the proposed design in #329 requires clients to access the REST API using admin-access service accounts. This means that clients already have the keys to the kingdom. Further, servers probably should not concern themselves with the clients that are accessing them, at least as it relates to the server configuration; such an approach would REQUIRE a change to the server config every time we add a client; whereas, in the current proposed designs, there need not be any change to the server config, instead admins need only create an admin-level service account, which merely concerns the relational database. Thus we arrive at the likely solution: that we specify (as a client app config) an exclusive list of forms that the client wants to make available to users. It can default to None, in which case all forms will be made available, which I think is good default behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant