Updating authenticators from latest in Tiled

[davidpcls]

Description

These changes bring in the authenticators from Tiled 0.2.12 into bluesky-httpserver along with the required changes to the app and supporting files to support them. This allows us to use the same authentication setup as from Tiled deployments (at least at the current moment). The main other change is to remove the "mode" flag and instead use the class type to determine if it is an internal or external authenticator.

Part of this work is also the updating of the bluesky-queueserver-api, for which I have created a PR. Both are required in order for the OIDC workflows to work.

Like Tiled, the authentication flow forces a login prompt on each attempt to login. I have tried to minimize any differences between similar functions/files in Tiled and bluesky-httpserver.

Motivation and Context

This solves the problem of having different authentication schemes to maintain between Tiled and HTTP server, which came from the same code around 3 years ago. Tiled has been updated but HTTP server was not. This addresses that.

Summary of Changes for Release Notes

Updated authenticators based off Tiled v0.2.12.
Made minimal changes to app.py and authentication.py to support the changes. Some new endpoints needed to be created to do this, but fairly minimal. The majority of the work was getting the unit test runners stable.
Added local parallel unit test runners to aid development.
Updated the github runners to be more stable and reliable by making the port used dynamic.

Fixed

Added

• Authenticators.py from Tiled v0.2.12
• protocols.py from Tiled v0.2.12
• new endpoints for authorization workflows in app.py
• Pending sessions in the database
• Documentation and examples on using OIDC based off Tiled

Changed

• Unit tests to support above changes
• Unit test structure to be a little more stable
• The LDAP container as the previous one was no longer supported

Removed

How Has This Been Tested?

Testing was done against MS Entra. I tested that these workflows work for both internal (localhost) and external servers. Using this the login workflow changes to just simply being:

from bluesky_queueserver_api.http import REManagerAPI
from bluesky_queueserver_api import BPlan
RM = REManagerAPI(http_server_uri="http://localhost:60610", http_auth_provider="entra/authorize")
RM.login()

While this was tested with MS Entra, the design is based on Tiled's OIDC and testing used the OIDC flow with MS Entra, so it should theoretically work with all other authenticators using OIDC.

TODO:

• Migrate in changes from tiled 0.2.10 to 0.2.12
• Update unit tests
• Update documentation for authenticators

[danielballan]

Wow, thanks @davidpcls!

[dmgav]

Thank you. I am going to try to fix the unit tests first, I may push a few commits.

[prjemian]

If either of you intend to commit more work to this PR, put it in Draft mode. Change the mode back when you are ready for final review.

[davidpcls]

@prjemian , thanks I've converted it.

@dmgav sure that sounds good.

[dmgav]

I fixed the unit tests. I also rebased the branch to main, so I had to force push the changes.

[davidpcls]

Thanks for working on that @dmgav! I just got things setup so I can test with this against MS Entra, so I'll go about verifying there are no other changes required for this to work properly. Took a little longer than expected to get the Entra stuff worked out.

[davidpcls]

I've added in these changes, I'm going to work on cleaning up failing unit tests now and then I will update the pull request description to match your style and provide information on testing. After that I will remove the WIP status.

[davidpcls]

Please also see the related PR for the API: bluesky/bluesky-queueserver-api#62

[davidpcls]

Sorry, got excited about the unit tests passing. I need to do one last integration test first, which is why I converted it back into a WIP

[davidpcls]

Comments to assist others reviewing the code

[davidpcls]

After these changes, still seems to be working for me. Neato.

[dmgav]

While testing the changes I noticed that the function app_factory() is called twice during the server startup using uvicorn. As a result, the authenticators are instantiated twice. For some authenticators (ProxiedOIDCAuthenticator and EntraAuthenticator) this results in duplicated requests to the authentication server (https://github.com/davidpcls/bluesky-httpserver/blob/90e047992fe44dfb6d8a94bf58f8aef5b3c813c0/bluesky_httpserver/authenticators.py#L289). The following change to the __getattr__() function in server.py seems to fix the issue:

def __getattr__(name):
    """
    This supports tiled.server.app.app by creating app on demand.
    """
    if name == "app":
        try:
            _app = app_factory()
            globals()["app"] = _app   # cache in module dict — prevents second call
            return _app
        except Exception as err:
            raise Exception("Failed to create app.") from err
    raise AttributeError(name)

[davidpcls]

@dmgav , interesting that it's doing that. Tiled is using the exact same code for getattr. I'm curious why it doesn't have this same problem, or if it does. Just looking into that now.

[davidpcls]

It looks like tiled starts the apps differently.

Do we even need getattr ? We can start it just fine with:

app = app_factory()

Rather than having getattr.

I think getattr just solves us importing server.start_server

[dmgav]

I don't remember why we implemented server:start_server. It is an easier way to start it in demo mode, and it allows to pass extra parameters to build_app function, but it can not be used in production with gunicorn. I don't know if anyone is using this function. We can probably temporarily fix the __getattr__ function for now, and then change the startup code so that it matches Tiled later.

[davidpcls]

I don't remember why we implemented server:start_server. It is an easier way to start it in demo mode, and it allows to pass extra parameters to build_app function, but it can not be used in production with gunicorn. I don't know if anyone is using this function. We can probably temporarily fix the __getattr__ function for now, and then change the startup code so that it matches Tiled later.

Sounds good, I've committed the changes just now

[dmgav]

@davidpcls Do you mind if I push some minor changes to this branch?