Open hmacdope opened 3 months ago
@hmacdope I think that's a great idea to add a simple layer of authentication to bespokefit which we need, lets get this in a branch and test it out on our AWS deployment.
I'm a bit against this. We don't have an authentication system anywhere else in our org and we don't have (or plan to hire) staff who know how to maintain secure authentication. I also think this might open us to liability or endanger our funding if a pharma partner uses this and some information leaks. And if Hugo and Josh are the only people who know how to implement+maintain this and they go on to different jobs, then we either need to divert resources to hire someone who knows how to maintain an authentication system (which takes personnel-time that we don't have), or we have to deprecate the feature (which directly harms partners who had adopted this into their workflows).
I'd be happy to learn if there's some dead-simple way this can be implemented and be obviously correct and maintainable. Otherwise I might recommend that this is best implemented outside the openff github orb.
Totally understand @j-wags, don't want to create headaches (especially for liability!). Again, we can work on this in a fork if needs be, so no stress.
This would be lightweight (no passwords or users or anything) and use FastAPI's pre-existing tooling (well validated and tested)
An example
from fastapi import FastAPI, Body, Depends, HTTPException, status
from fastapi.security import OAuth2PasswordBearer
key = os.getenv("BESPOKEFIT_API_KEY", False)
oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") # use token authentication
def api_key_auth(api_key: str = Depends(oauth2_scheme)):
"""
If a key is provided as environment variable, must provide matching key in HTTP header.
"""
if key:
if api_key != key:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Forbidden"
)
else:
pass
app = FastAPI()
@app.get("/endpoint1", dependencies=[Depends(api_key_auth)])
def endpoint1():
print("Hello")
I understand confidentiality is a tricky topic, but if you want to encourage bespokefit adoption from industry partners, a way to control access to the server endpoints (from which you can see everything) is probably desirable long-term. Should also be easily CI testable and we are also able to battle test. Again, happy to discuss.
Would a way forward, be to make a PR with relevant functionality, see how it turns out, and if ya'll are not comfortable maintaning it (totally understand) we can move it to a fork. That preserves both interests and you we can merge back together again at some point if you like.
This would be lightweight (no passwords or users or anything) and use FastAPI's pre-existing tooling (well validated and tested)
Ah, ok. That auth system really does seem dead-simple (importing everything from fastapi.security
gives me a lot of confidence that they want this to be a drop-in solution and that they stand by it).
I also apologize for something I was confused on - Josh H is currently the "owner" for bespokefit and so I really shouldn't be jumping in here unless I have a very strong objection. And seeing that this is literally from fastapi.security import OAuth2PasswordBearer
alleviates a lot of my concern. Please do go ahead with a PR and @jthorton is the only approval you need :-)
@jthorton see here for example of auth
It would be fantastic if there was some auth capabilities to stop information being revealed from
server
HTTP endpoint if you hit it with a well formed request. This is important for work where the molecules of interest are sensitive and the HTTP endpoint is public (e.g) an AWS deployment. We are experimenting with running a similar setup at ASAP.The easiest way to do this IMO is to add an API token support to the HTTP header. If the server is configured with an env var or option on, e.g
BESPOKEFIT_API_TOKEN
, then requests must have the matching API token, otherwise request is rejected with401
(unauthenticated).This should be set up so that the token system is opt in and has no effect on normal operation if not required. Happy to help with this myself also tagging @jthorton